Merge branch 'development' into kishan/fix/slot-number-must-increase

dot/core/messages.go

@@ -4,7 +4,6 @@
 package core
 
 import (
-	"errors"
 	"fmt"
 
 	"github.com/ChainSafe/gossamer/dot/network"
@@ -16,14 +15,14 @@ import (
 	"github.com/libp2p/go-libp2p-core/peer"
 )
 
-func (s *Service) validateTransaction(peerID peer.ID, head *types.Header, rt RuntimeInstance,
-	tx types.Extrinsic) (validity *transaction.Validity, valid bool, err error) {
+func (s *Service) validateTransaction(head *types.Header, rt RuntimeInstance,
+	tx types.Extrinsic) (validity *transaction.Validity, err error) {
 	s.storageState.Lock()
 
 	ts, err := s.storageState.TrieState(&head.StateRoot)
 	s.storageState.Unlock()
 	if err != nil {
-		return nil, false, fmt.Errorf("cannot get trie state from storage for root %s: %w", head.StateRoot, err)
+		return nil, fmt.Errorf("cannot get trie state from storage for root %s: %w", head.StateRoot, err)
 	}
 
 	rt.SetContextStorage(ts)
@@ -32,15 +31,8 @@ func (s *Service) validateTransaction(peerID peer.ID, head *types.Header, rt Run
 	externalExt := types.Extrinsic(append([]byte{byte(types.TxnExternal)}, tx...))
 	validity, err = rt.ValidateTransaction(externalExt)
 	if err != nil {
-		if errors.Is(err, runtime.ErrInvalidTransaction) {
-			s.net.ReportPeer(peerset.ReputationChange{
-				Value:  peerset.BadTransactionValue,
-				Reason: peerset.BadTransactionReason,
-			}, peerID)
-		}
-
 		logger.Debugf("failed to validate transaction: %s", err)
-		return nil, false, nil
+		return nil, err
 	}
 
 	vtx := transaction.NewValidTransaction(tx, validity)
@@ -49,7 +41,7 @@ func (s *Service) validateTransaction(peerID peer.ID, head *types.Header, rt Run
 	hash := s.transactionState.AddToPool(vtx)
 	logger.Tracef("added transaction with hash %s to pool", hash)
 
-	return validity, true, nil
+	return validity, nil
 }
 
 // HandleTransactionMessage validates each transaction in the message and
@@ -78,22 +70,32 @@ func (s *Service) HandleTransactionMessage(peerID peer.ID, msg *network.Transact
 		return false, err
 	}
 
-	allTxsAreValid := true
+	allTxnsAreValid := true
 	for _, tx := range txs {
-		validity, isValidTxn, err := s.validateTransaction(peerID, head, rt, tx)
+		txnIsValid := true
+		validity, err := s.validateTransaction(head, rt, tx)
 		if err != nil {
-			return false, fmt.Errorf("failed validating transaction for peerID %s: %w", peerID, err)
+			txnIsValid = false
+			allTxnsAreValid = false
+			switch err := err.(type) {
+			case runtime.InvalidTransaction:
+				s.net.ReportPeer(peerset.ReputationChange{
+					Value:  peerset.BadTransactionValue,
+					Reason: peerset.BadTransactionReason,
+				}, peerID)
+			case runtime.UnknownTransaction:
+			default:
+				return false, fmt.Errorf("validating transaction from peerID %s: %w", peerID, err)
+			}
 		}
 
-		if !isValidTxn {
-			allTxsAreValid = false
-		} else if validity.Propagate {
+		if txnIsValid && validity.Propagate {
 			// find tx(s) that should propagate
 			toPropagate = append(toPropagate, tx)
 		}
 	}
 
-	if allTxsAreValid {
+	if allTxnsAreValid {
 		s.net.ReportPeer(peerset.ReputationChange{
 			Value:  peerset.GoodTransactionValue,
 			Reason: peerset.GoodTransactionReason,

dot/core/messages_integration_test.go

@@ -172,9 +172,9 @@ func TestService_HandleTransactionMessage(t *testing.T) {
 	require.NotEmpty(t, pending)
 	require.Equal(t, extBytes, pending[0].Extrinsic)
 
-	extBytes = []byte(`bogus extrinsic`)
-	msg = &network.TransactionMessage{Extrinsics: []types.Extrinsic{extBytes}}
+	invalidExtBytes := types.Extrinsic{byte(1)}
+	msg = &network.TransactionMessage{Extrinsics: []types.Extrinsic{invalidExtBytes}}
 	shouldPropagate, err = s.HandleTransactionMessage(peer1, msg)
-	require.NoError(t, err)
+	require.Error(t, err)
 	require.False(t, shouldPropagate)
 }

dot/core/messages_test.go

@@ -12,13 +12,13 @@ import (
 	"github.com/ChainSafe/gossamer/dot/types"
 	"github.com/ChainSafe/gossamer/lib/common"
 	"github.com/ChainSafe/gossamer/lib/runtime"
-	mocksruntime "github.com/ChainSafe/gossamer/lib/runtime/mocks"
 	"github.com/ChainSafe/gossamer/lib/runtime/storage"
 	"github.com/ChainSafe/gossamer/lib/transaction"
 
 	"github.com/golang/mock/gomock"
 	"github.com/libp2p/go-libp2p-core/peer"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 var errDummyErr = errors.New("dummy error for testing")
@@ -70,7 +70,7 @@ type mockValidateTxn struct {
 }
 
 type mockRuntime struct {
-	runtime           *mocksruntime.Instance
+	runtime           *MockRuntimeInstance
 	setContextStorage *mockSetContextStorage
 	validateTxn       *mockValidateTxn
 }
@@ -114,9 +114,14 @@ func TestServiceHandleTransactionMessage(t *testing.T) {
 	testEmptyHeader := types.NewEmptyHeader()
 	testExtrinsic := []types.Extrinsic{{1, 2, 3}}
 
-	runtimeMock := mocksruntime.NewInstance(t)
-	runtimeMock2 := mocksruntime.NewInstance(t)
-	runtimeMock3 := mocksruntime.NewInstance(t)
+	ctrl := gomock.NewController(t)
+	runtimeMock := NewMockRuntimeInstance(ctrl)
+	runtimeMock2 := NewMockRuntimeInstance(ctrl)
+	runtimeMock3 := NewMockRuntimeInstance(ctrl)
+
+	invalidTransaction := runtime.NewInvalidTransaction()
+	err := invalidTransaction.Set(runtime.Future{})
+	require.NoError(t, err)
 
 	type args struct {
 		peerID peer.ID
@@ -225,7 +230,7 @@ func TestServiceHandleTransactionMessage(t *testing.T) {
 				},
 			},
 			expErr: errDummyErr,
-			expErrMsg: "failed validating transaction for peerID D1KeRhQ: cannot get trie state from storage" +
+			expErrMsg: "validating transaction from peerID D1KeRhQ: cannot get trie state from storage" +
 				" for root 0x0000000000000000000000000000000000000000000000000000000000000000: dummy error for testing",
 		},
 		{
@@ -257,7 +262,7 @@ func TestServiceHandleTransactionMessage(t *testing.T) {
 				setContextStorage: &mockSetContextStorage{trieState: &storage.TrieState{}},
 				validateTxn: &mockValidateTxn{
 					input: types.Extrinsic(append([]byte{byte(types.TxnExternal)}, testExtrinsic[0]...)),
-					err:   runtime.ErrInvalidTransaction,
+					err:   invalidTransaction,
 				},
 			},
 			args: args{
@@ -357,8 +362,8 @@ func TestServiceHandleTransactionMessage(t *testing.T) {
 			}
 			if tt.mockRuntime != nil {
 				rt := tt.mockRuntime.runtime
-				rt.On("SetContextStorage", tt.mockRuntime.setContextStorage.trieState)
-				rt.On("ValidateTransaction", tt.mockRuntime.validateTxn.input).
+				rt.EXPECT().SetContextStorage(tt.mockRuntime.setContextStorage.trieState)
+				rt.EXPECT().ValidateTransaction(tt.mockRuntime.validateTxn.input).
 					Return(tt.mockRuntime.validateTxn.validity, tt.mockRuntime.validateTxn.err)
 			}