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 22 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
8 changes: 1 addition & 7 deletions dot/core/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (
"github.com/ChainSafe/gossamer/lib/transaction"
)

//go:generate mockery --name BlockState --structname BlockState --case underscore --keeptree
//go:generate mockgen -destination=mock_core_test.go -package $GOPACKAGE . BlockState,StorageState,TransactionState,Network,EpochState,CodeSubstitutedState,DigestHandler

// BlockState interface for block state methods
type BlockState interface {
Expand Down Expand Up @@ -47,8 +47,6 @@ type BlockState interface {
StoreRuntime(common.Hash, runtime.Instance)
}

//go:generate mockery --name StorageState --structname StorageState --case underscore --keeptree

// StorageState interface for storage state methods
type StorageState interface {
LoadCode(root *common.Hash) ([]byte, error)
Expand All @@ -70,8 +68,6 @@ type TransactionState interface {
PendingInPool() []*transaction.ValidTransaction
}

//go:generate mockery --name Network --structname Network --case underscore --keeptree

// Network is the interface for the network service
type Network interface {
GossipMessage(network.NotificationsMessage)
Expand All @@ -92,8 +88,6 @@ type CodeSubstitutedState interface {
StoreCodeSubstitutedBlockHash(hash common.Hash) error
}

//go:generate mockery --name DigestHandler --structname DigestHandler --case underscore --keeptree

// DigestHandler is the interface for the consensus digest handler
type DigestHandler interface {
HandleDigests(header *types.Header)
Expand Down
98 changes: 54 additions & 44 deletions dot/core/messages.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,53 @@ package core

import (
"errors"

"github.com/libp2p/go-libp2p-core/peer"
"fmt"

"github.com/ChainSafe/gossamer/dot/network"
"github.com/ChainSafe/gossamer/dot/peerset"
"github.com/ChainSafe/gossamer/dot/types"
"github.com/ChainSafe/gossamer/lib/runtime"
"github.com/ChainSafe/gossamer/lib/transaction"

"github.com/libp2p/go-libp2p-core/peer"
)

func (s *Service) validateTransaction(peerID peer.ID, head *types.Header, rt runtime.Instance,
tx types.Extrinsic) (validity *transaction.Validity, valid bool, err error) {
s.storageState.Lock()
defer s.storageState.Unlock()

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

rt.SetContextStorage(ts)

// validate each transaction
externalExt := types.Extrinsic(append([]byte{byte(types.TxnExternal)}, tx...))
val, 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
}

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)

return val, true, nil
}

// HandleTransactionMessage validates each transaction in the message and
// adds valid transactions to the transaction queue of the BABE session
// returns boolean for transaction propagation, true - transactions should be propagated
Expand All @@ -41,56 +78,29 @@ func (s *Service) HandleTransactionMessage(peerID peer.ID, msg *network.Transact
return false, err
}

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 {
err = func() error {
s.storageState.Lock()
defer s.storageState.Unlock()

ts, err := s.storageState.TrieState(&head.StateRoot)
if err != nil {
return err
}

rt.SetContextStorage(ts)

// validate each transaction
externalExt := types.Extrinsic(append([]byte{byte(types.TxnExternal)}, tx...))
val, 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
}

// 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)
validity, isValidTxn, err := s.validateTransaction(peerID, head, rt, tx)
if err != nil {
return false, fmt.Errorf("failed validating transaction for peerID %s: %w", peerID, err)
}

if !isValidTxn {
isValid = false
} else {
// find tx(s) that should propagate
if val.Propagate {
if validity.Propagate {
toPropagate = append(toPropagate, tx)
}

return nil
}()

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

s.net.ReportPeer(peerset.ReputationChange{
Value: peerset.GoodTransactionValue,
Reason: peerset.GoodTransactionReason,
}, peerID)
if isValid {
s.net.ReportPeer(peerset.ReputationChange{
Value: peerset.GoodTransactionValue,
Reason: peerset.GoodTransactionReason,
}, peerID)
}

msg.Extrinsics = toPropagate
return len(msg.Extrinsics) > 0, nil
Expand Down
7 changes: 6 additions & 1 deletion dot/core/messages_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (

"github.com/centrifuge/go-substrate-rpc-client/v3/signature"
ctypes "github.com/centrifuge/go-substrate-rpc-client/v3/types"
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/require"

"github.com/ChainSafe/gossamer/dot/core/mocks"
Expand Down Expand Up @@ -127,9 +128,13 @@ func TestService_HandleTransactionMessage(t *testing.T) {
ks := keystore.NewGlobalKeystore()
ks.Acco.Insert(kp)

ctrl := gomock.NewController(t)
telemetryMock := NewMockClient(ctrl)
telemetryMock.EXPECT().SendMessage(gomock.Any()).AnyTimes()

cfg := &Config{
Keystore: ks,
TransactionState: state.NewTransactionState(),
TransactionState: state.NewTransactionState(telemetryMock),
}

s := NewTestService(t, cfg)
Expand Down
Loading