[NOD-1259] Do not panic on non-protocol errors from RPC (#863)

* [NOD-1259] All rule-errors should be protocol-errors

* [NOD-1259] Handle submitting of coinbase transactions properly

* Revert "[NOD-1259] All rule-errors should be protocol-errors"

This reverts commit 2fd30c185640fcee62030f72ed14654570a1d7c7.

* [NOD-1259] Don't panic on non-protocol errors in ProtocolManager.AddTransaction/AddBlock

* [NOD-1259] Implement subnetworkid.IsBuiltInOrNative and use where appropriate

blockdag/dag.go

@@ -881,7 +881,7 @@ func (dag *BlockDAG) validateGasLimit(block *util.Block) error {
 
 		// In native and Built-In subnetworks all txs must have Gas = 0, and that was already validated in checkTransactionSanity
 		// Therefore - no need to check them here.
-		if msgTx.SubnetworkID.IsEqual(subnetworkid.SubnetworkIDNative) || msgTx.SubnetworkID.IsBuiltIn() {
+		if msgTx.SubnetworkID.IsBuiltInOrNative() {
 			continue
 		}
 

blockdag/validate.go

@@ -6,11 +6,12 @@ package blockdag
 
 import (
 	"fmt"
-	"github.com/kaspanet/kaspad/util/mstime"
 	"math"
 	"sort"
 	"time"
 
+	"github.com/kaspanet/kaspad/util/mstime"
+
 	"github.com/pkg/errors"
 
 	"github.com/kaspanet/kaspad/dagconfig"
@@ -212,10 +213,7 @@ func CheckTransactionSanity(tx *util.Tx, subnetworkID *subnetworkid.SubnetworkID
 	}
 
 	// Transactions in native, registry and coinbase subnetworks must have Gas = 0
-	if (msgTx.SubnetworkID.IsEqual(subnetworkid.SubnetworkIDNative) ||
-		msgTx.SubnetworkID.IsBuiltIn()) &&
-		msgTx.Gas > 0 {
-
+	if msgTx.SubnetworkID.IsBuiltInOrNative() && msgTx.Gas > 0 {
 		return ruleError(ErrInvalidGas, "transaction in the native or "+
 			"registry subnetworks has gas > 0 ")
 	}

mempool/mempool.go

@@ -851,7 +851,7 @@ func (mp *TxPool) maybeAcceptTransaction(tx *util.Tx, rejectDupOrphans bool) ([]
 
 	// Check that transaction does not overuse GAS
 	msgTx := tx.MsgTx()
-	if !msgTx.SubnetworkID.IsEqual(subnetworkid.SubnetworkIDNative) && !msgTx.SubnetworkID.IsEqual(subnetworkid.SubnetworkIDRegistry) {
+	if !msgTx.SubnetworkID.IsBuiltInOrNative() {
 		gasLimit, err := mp.cfg.DAG.GasLimit(&msgTx.SubnetworkID)
 		if err != nil {
 			return nil, nil, err

mining/txselection.go

@@ -156,7 +156,7 @@ func (g *BlkTmplGenerator) collectCandidatesTxs(sourceTxs []*TxDesc) []*candidat
 		}
 
 		gasLimit := uint64(0)
-		if !tx.MsgTx().SubnetworkID.IsEqual(subnetworkid.SubnetworkIDNative) && !tx.MsgTx().SubnetworkID.IsBuiltIn() {
+		if !tx.MsgTx().SubnetworkID.IsBuiltInOrNative() {
 			subnetworkID := tx.MsgTx().SubnetworkID
 			gasLimit, err = g.dag.GasLimit(&subnetworkID)
 			if err != nil {
@@ -202,8 +202,7 @@ func (g *BlkTmplGenerator) calcTxValue(tx *util.Tx, fee uint64) (float64, error)
 	massLimit := g.policy.BlockMaxMass
 
 	msgTx := tx.MsgTx()
-	if msgTx.SubnetworkID.IsEqual(subnetworkid.SubnetworkIDNative) ||
-		msgTx.SubnetworkID.IsBuiltIn() {
+	if msgTx.SubnetworkID.IsBuiltInOrNative() {
 		return float64(fee) / (float64(mass) / float64(massLimit)), nil
 	}
 
@@ -266,7 +265,7 @@ func (g *BlkTmplGenerator) populateTemplateFromCandidates(candidateTxs []*candid
 
 		// Enforce maximum gas per subnetwork per block. Also check
 		// for overflow.
-		if !tx.MsgTx().SubnetworkID.IsEqual(subnetworkid.SubnetworkIDNative) && !tx.MsgTx().SubnetworkID.IsBuiltIn() {
+		if !tx.MsgTx().SubnetworkID.IsBuiltInOrNative() {
 			subnetworkID := tx.MsgTx().SubnetworkID
 			gasUsage, ok := gasUsageMap[subnetworkID]
 			if !ok {

protocol/manager.go

@@ -11,9 +11,7 @@ import (
 	"github.com/kaspanet/kaspad/netadapter"
 	"github.com/kaspanet/kaspad/protocol/flowcontext"
 	peerpkg "github.com/kaspanet/kaspad/protocol/peer"
-	"github.com/kaspanet/kaspad/protocol/protocolerrors"
 	"github.com/kaspanet/kaspad/util"
-	"github.com/pkg/errors"
 )
 
 // Manager manages the p2p protocol
@@ -56,30 +54,12 @@ func (m *Manager) IBDPeer() *peerpkg.Peer {
 
 // AddTransaction adds transaction to the mempool and propagates it.
 func (m *Manager) AddTransaction(tx *util.Tx) error {
-	err := m.context.AddTransaction(tx)
-	if err != nil {
-		if protocolErr := &(protocolerrors.ProtocolError{}); errors.As(err, &protocolErr) {
-			return err
-		}
-
-		// TODO(libp2p): Remove panic once RPC is integrated into protocol architecture
-		panic(err)
-	}
-	return nil
+	return m.context.AddTransaction(tx)
 }
 
 // AddBlock adds the given block to the DAG and propagates it.
 func (m *Manager) AddBlock(block *util.Block, flags blockdag.BehaviorFlags) error {
-	err := m.context.AddBlock(block, flags)
-	if err != nil {
-		if protocolErr := &(protocolerrors.ProtocolError{}); errors.As(err, &protocolErr) {
-			return err
-		}
-
-		// TODO(libp2p): Remove panic once RPC is integrated into protocol architecture
-		panic(err)
-	}
-	return nil
+	return m.context.AddBlock(block, flags)
 }
 
 func (m *Manager) runFlows(flows []*flow, peer *peerpkg.Peer, errChan <-chan error) error {

rpc/handle_get_subnetwork.go

@@ -15,8 +15,7 @@ func handleGetSubnetwork(s *Server, cmd interface{}, closeChan <-chan struct{})
 	}
 
 	var gasLimit *uint64
-	if !subnetworkID.IsEqual(subnetworkid.SubnetworkIDNative) &&
-		!subnetworkID.IsBuiltIn() {
+	if !subnetworkID.IsBuiltInOrNative() {
 		limit, err := s.dag.GasLimit(subnetworkID)
 		if err != nil {
 			return nil, &model.RPCError{

util/subnetworkid/subnetworkid.go

@@ -7,8 +7,9 @@ package subnetworkid
 
 import (
 	"encoding/hex"
-	"github.com/pkg/errors"
 	"math/big"
+
+	"github.com/pkg/errors"
 )
 
 // IDLength of array used to store the subnetwork ID. See SubnetworkID.
@@ -196,6 +197,12 @@ func (id *SubnetworkID) IsBuiltIn() bool {
 	return id.IsEqual(SubnetworkIDCoinbase) || id.IsEqual(SubnetworkIDRegistry)
 }
 
+// IsBuiltInOrNative returns true if the subnetwork is the native or a built in subnetwork,
+// see IsBuiltIn for further details
+func (id *SubnetworkID) IsBuiltInOrNative() bool {
+	return id.IsEqual(SubnetworkIDNative) || id.IsBuiltIn()
+}
+
 // Less returns true iff id a is less than id b
 func Less(a *SubnetworkID, b *SubnetworkID) bool {
 	return a.Cmp(b) < 0