Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

testing (dot/core): rewrite message.go unit tests #2197

Merged
merged 23 commits into from
Jan 21, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 15 additions & 15 deletions dot/core/messages.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package core

import (
"errors"
"fmt"

"github.com/ChainSafe/gossamer/dot/network"
"github.com/ChainSafe/gossamer/dot/peerset"
Expand All @@ -16,13 +17,13 @@ import (
)

func (s *Service) validateTransaction(peerID peer.ID, head *types.Header, rt runtime.Instance,
tx types.Extrinsic, toPropagate *[]types.Extrinsic) (bool, error) {
tx types.Extrinsic) (validity *transaction.Validity, failed bool, err error) {
qdm12 marked this conversation as resolved.
Show resolved Hide resolved
s.storageState.Lock()
defer s.storageState.Unlock()

ts, err := s.storageState.TrieState(&head.StateRoot)
if err != nil {
return true, err
return nil, false, fmt.Errorf("cannot get trie state from storage for root %s: %w", head.StateRoot, err)
}

rt.SetContextStorage(ts)
Expand All @@ -39,22 +40,16 @@ func (s *Service) validateTransaction(peerID peer.ID, head *types.Header, rt run
}

logger.Debugf("failed to validate transaction: %s", err)
return true, nil
return nil, false, nil
}

// create new valid transaction
vtx := transaction.NewValidTransaction(tx, val)

// push to the transaction queue of BABE session
hash := s.transactionState.AddToPool(vtx)
logger.Tracef("added transaction with hash %s to pool", hash)

// find tx(s) that should propagate
if val.Propagate {
*toPropagate = append(*toPropagate, tx)
}

return false, nil
return val, true, nil
}

// HandleTransactionMessage validates each transaction in the message and
Expand Down Expand Up @@ -83,19 +78,24 @@ func (s *Service) HandleTransactionMessage(peerID peer.ID, msg *network.Transact
return false, err
}

hasInvalidTxn := false
isValid := true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit perhaps rename to allTxsAreValid? 🤔

for _, tx := range txs {
isInvalid, err := s.validateTransaction(peerID, head, rt, tx, &toPropagate)
if isInvalid {
hasInvalidTxn = true
validity, isValidTxn, err := s.validateTransaction(peerID, head, rt, tx)
if !isValidTxn {
qdm12 marked this conversation as resolved.
Show resolved Hide resolved
isValid = false
} else {
// find tx(s) that should propagate
if validity.Propagate {
toPropagate = append(toPropagate, tx)
}
}

if err != nil {
return false, err
}
}

if !hasInvalidTxn {
if isValid {
s.net.ReportPeer(peerset.ReputationChange{
Value: peerset.GoodTransactionValue,
Reason: peerset.GoodTransactionReason,
Expand Down
4 changes: 2 additions & 2 deletions dot/core/messages_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,8 @@ func TestServiceHandleTransactionMessage(t *testing.T) {
Extrinsics: []types.Extrinsic{{1, 2, 3}, {7, 8, 9, 0}, {0xa, 0xb}},
},
},
expErr: errDummyErr,
expErrMsg: errDummyErr.Error(),
expErr: errDummyErr,
expErrMsg: "cannot get trie state from storage for root 0x0000000000000000000000000000000000000000000000000000000000000000: dummy error for testing",
},
{
name: "runtime.ErrInvalidTransaction",
Expand Down