Skip to content

Commit

Permalink
fix(validation): fix nil deref , and use validation result (#1325)
Browse files Browse the repository at this point in the history
  • Loading branch information
danwt authored Jan 13, 2025
1 parent ec78144 commit ace06fc
Show file tree
Hide file tree
Showing 9 changed files with 122 additions and 81 deletions.
4 changes: 4 additions & 0 deletions block/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,10 @@ func (m *Manager) GetProposerPubKey() tmcrypto.PubKey {
return m.State.GetProposerPubKey()
}

func (m *Manager) SafeProposerPubKey() (tmcrypto.PubKey, error) {
return m.State.SafeProposerPubKey()
}

func (m *Manager) GetRevision() uint64 {
return m.State.GetRevision()
}
Expand Down
74 changes: 42 additions & 32 deletions mocks/github.com/dymensionxyz/dymint/p2p/mock_StateGetter.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions p2p/block_sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/dymensionxyz/dymint/testutil"
"github.com/dymensionxyz/dymint/version"
"github.com/ipfs/go-datastore"
pubsub "github.com/libp2p/go-libp2p-pubsub"
"github.com/stretchr/testify/require"
"github.com/tendermint/tendermint/libs/log"
)
Expand All @@ -23,8 +24,8 @@ func TestBlockSync(t *testing.T) {
require.NotNil(t, manager)

// required for tx validator
assertRecv := func(tx *p2p.GossipMessage) bool {
return true
assertRecv := func(tx *p2p.GossipMessage) pubsub.ValidationResult {
return pubsub.ValidationAccept
}

// Create a block for height 1
Expand Down
11 changes: 8 additions & 3 deletions p2p/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -523,8 +523,8 @@ func (c *Client) getBlockTopic() string {
// NewTxValidator creates a pubsub validator that uses the node's mempool to check the
// transaction. If the transaction is valid, then it is added to the mempool
func (c *Client) NewTxValidator() GossipValidator {
return func(g *GossipMessage) bool {
return true
return func(g *GossipMessage) pubsub.ValidationResult {
return pubsub.ValidationAccept
}
}

Expand Down Expand Up @@ -640,7 +640,12 @@ func (c *Client) retrieveBlockSyncLoop(ctx context.Context, msgHandler BlockSync
if err != nil {
return
}
if err := block.Validate(state.GetProposerPubKey()); err != nil {
propKey, err := state.SafeProposerPubKey()
if err != nil {
c.logger.Error("Get proposer public key.", "err", err)
continue
}
if err := block.Validate(propKey); err != nil {
c.logger.Error("Failed to validate blocksync block.", "height", block.Block.Header.Height)
continue
}
Expand Down
17 changes: 9 additions & 8 deletions p2p/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/dymensionxyz/dymint/config"
"github.com/dymensionxyz/dymint/p2p"
"github.com/dymensionxyz/dymint/testutil"
p2ppubsub "github.com/libp2p/go-libp2p-pubsub"
)

func TestClientStartup(t *testing.T) {
Expand Down Expand Up @@ -104,17 +105,17 @@ func TestGossiping(t *testing.T) {
wg.Add(2)

// ensure that Tx is delivered to client
assertRecv := func(tx *p2p.GossipMessage) bool {
assertRecv := func(tx *p2p.GossipMessage) p2ppubsub.ValidationResult {
logger.Debug("received tx", "body", string(tx.Data), "from", tx.From)
assert.Equal(expectedMsg, tx.Data)
wg.Done()
return true
return p2ppubsub.ValidationAccept
}

// ensure that Tx is not delivered to client
assertNotRecv := func(*p2p.GossipMessage) bool {
assertNotRecv := func(*p2p.GossipMessage) p2ppubsub.ValidationResult {
t.Fatal("unexpected Tx received")
return false
return p2ppubsub.ValidationReject
}

validators := []p2p.GossipValidator{assertRecv, assertNotRecv, assertNotRecv, assertRecv, assertNotRecv}
Expand Down Expand Up @@ -151,8 +152,8 @@ func TestAdvertiseBlock(t *testing.T) {
ctx := context.Background()

// required for tx validator
assertRecv := func(tx *p2p.GossipMessage) bool {
return true
assertRecv := func(tx *p2p.GossipMessage) p2ppubsub.ValidationResult {
return p2ppubsub.ValidationAccept
}

// Create a cid manually by specifying the 'prefix' parameters
Expand Down Expand Up @@ -201,8 +202,8 @@ func TestAdvertiseWrongCid(t *testing.T) {
ctx := context.Background()

// required for tx validator
assertRecv := func(tx *p2p.GossipMessage) bool {
return true
assertRecv := func(tx *p2p.GossipMessage) p2ppubsub.ValidationResult {
return p2ppubsub.ValidationAccept
}

validators := []p2p.GossipValidator{assertRecv, assertRecv, assertRecv, assertRecv, assertRecv}
Expand Down
6 changes: 3 additions & 3 deletions p2p/gossip.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,12 +108,12 @@ func (g *Gossiper) ProcessMessages(ctx context.Context) {
}
}

func wrapValidator(gossiper *Gossiper, validator GossipValidator) pubsub.Validator {
return func(_ context.Context, _ peer.ID, msg *pubsub.Message) bool {
func wrapValidator(gossiper *Gossiper, validator GossipValidator) pubsub.ValidatorEx {
return func(_ context.Context, _ peer.ID, msg *pubsub.Message) pubsub.ValidationResult {
// Make sure we don't process our own messages.
// In this case we'll want to return true but not to actually handle the message.
if msg.GetFrom() == gossiper.ownID {
return true
return pubsub.ValidationAccept
}
return validator(&GossipMessage{
Data: msg.Data,
Expand Down
41 changes: 25 additions & 16 deletions p2p/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,24 @@ import (
"github.com/dymensionxyz/dymint/mempool"
nodemempool "github.com/dymensionxyz/dymint/node/mempool"
"github.com/dymensionxyz/dymint/types"
pubsub "github.com/libp2p/go-libp2p-pubsub"
abci "github.com/tendermint/tendermint/abci/types"
tmcrypto "github.com/tendermint/tendermint/crypto"
corep2p "github.com/tendermint/tendermint/p2p"
)

type StateGetter interface {
GetProposerPubKey() tmcrypto.PubKey
SafeProposerPubKey() (tmcrypto.PubKey, error)
GetRevision() uint64
}

// GossipValidator is a callback function type.
type GossipValidator func(*GossipMessage) bool
type GossipValidator func(*GossipMessage) pubsub.ValidationResult

// IValidator is an interface for implementing validators of messages gossiped in the p2p network.
type IValidator interface {
// TxValidator creates a pubsub validator that uses the node's mempool to check the
// transaction. If the transaction is valid, then it is added to the mempool
// transaction. If the transaction is want, then it is added to the mempool
TxValidator(mp mempool.Mempool, mpoolIDS *nodemempool.MempoolIDs) GossipValidator
}

Expand All @@ -46,7 +47,7 @@ func NewValidator(logger types.Logger, blockmanager StateGetter) *Validator {
// transaction.
// False means the TX is considered invalid and should not be gossiped.
func (v *Validator) TxValidator(mp mempool.Mempool, mpoolIDS *nodemempool.MempoolIDs) GossipValidator {
return func(txMessage *GossipMessage) bool {
return func(txMessage *GossipMessage) pubsub.ValidationResult {
v.logger.Debug("Transaction received.", "bytes", len(txMessage.Data))
var res *abci.Response
err := mp.CheckTx(txMessage.Data, func(resp *abci.Response) {
Expand All @@ -57,38 +58,46 @@ func (v *Validator) TxValidator(mp mempool.Mempool, mpoolIDS *nodemempool.Mempoo
})
switch {
case errors.Is(err, mempool.ErrTxInCache):
return true
return pubsub.ValidationAccept
case errors.Is(err, mempool.ErrMempoolIsFull{}):
return true // we have no reason to believe that we should throw away the message
return pubsub.ValidationAccept // we have no reason to believe that we should throw away the message
case errors.Is(err, mempool.ErrTxTooLarge{}):
return false
return pubsub.ValidationReject
case errors.Is(err, mempool.ErrPreCheck{}):
return false
return pubsub.ValidationReject
case err != nil:
v.logger.Error("Check tx.", "error", err)
return false
return pubsub.ValidationReject
}

return res.GetCheckTx().Code == abci.CodeTypeOK
if res.GetCheckTx().Code == abci.CodeTypeOK {
return pubsub.ValidationAccept
}
return pubsub.ValidationReject
}
}

// BlockValidator runs basic checks on the gossiped block
func (v *Validator) BlockValidator() GossipValidator {
return func(blockMsg *GossipMessage) bool {
return func(blockMsg *GossipMessage) pubsub.ValidationResult {
var gossipedBlock BlockData
if err := gossipedBlock.UnmarshalBinary(blockMsg.Data); err != nil {
v.logger.Error("Deserialize gossiped block.", "error", err)
return false
return pubsub.ValidationReject
}
if v.stateGetter.GetRevision() != gossipedBlock.Block.GetRevision() {
return false
return pubsub.ValidationReject
}
propKey, err := v.stateGetter.SafeProposerPubKey()
if err != nil {
v.logger.Error("Get proposer pubkey.", "error", err)
return pubsub.ValidationIgnore
}
if err := gossipedBlock.Validate(v.stateGetter.GetProposerPubKey()); err != nil {
if err := gossipedBlock.Validate(propKey); err != nil {
v.logger.Error("P2P block validation.", "height", gossipedBlock.Block.Header.Height, "err", err)
return false
return pubsub.ValidationReject
}

return true
return pubsub.ValidationAccept
}
}
Loading

0 comments on commit ace06fc

Please sign in to comment.