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

feat: block production misbehavior detection #1071

Merged
merged 53 commits into from
Oct 22, 2024
Merged
Show file tree
Hide file tree
Changes from 50 commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
b42b2a8
include fraud checking
Sep 11, 2024
bf0e7eb
update test
Sep 11, 2024
c7939c7
first check on mockFraud Handler
Sep 12, 2024
7fb5faa
fraud check done
Sep 15, 2024
bb1ebfb
add fraud check on p2p layer
Sep 16, 2024
1bb5ffe
even received check
Sep 16, 2024
11c6703
fix tests
Sep 16, 2024
242863e
add validation for block timestamp fraud
Sep 16, 2024
07277e1
include test for block validation and fraud
Sep 17, 2024
2c147dd
include fraud test errors
Sep 17, 2024
a45ef32
fix tests
Sep 17, 2024
c58d16d
fix units
Sep 17, 2024
427abd7
remove generic handler
Sep 17, 2024
32fb103
fix linter
Sep 17, 2024
58cf2a8
reordering imports by hand
Sep 17, 2024
a9ca873
pr comments 1
Sep 19, 2024
4963924
fix pr suggestions
Sep 19, 2024
94923c0
Merge remote-tracking branch 'origin/main' into feat/adr-fraud-handling
Sep 19, 2024
b913735
move some packages
Sep 19, 2024
338ad82
Merge remote-tracking branch 'origin/main' into feat/adr-fraud-handling
Sep 19, 2024
cd804f3
remove unused file
Sep 19, 2024
835dcae
Merge branch 'feat/adr-fraud-handling' of github.com-faulty:dymension…
Sep 20, 2024
f88b299
export methods
Sep 20, 2024
48c862f
update comments
Sep 20, 2024
d54bdb3
Merge remote-tracking branch 'origin/main' into feat/adr-fraud-handling
Sep 27, 2024
e3ffab8
fix test
Sep 30, 2024
7ba65a8
fix linter
Sep 30, 2024
921ff1e
block height invalid, fraud
Oct 2, 2024
d5f89ef
update last block height comparison
Oct 2, 2024
a1a8d01
add last header block hash
Oct 2, 2024
c623b40
add validations
Oct 7, 2024
3378be8
add sequencer hash
Oct 7, 2024
465ef3f
Merge remote-tracking branch 'origin/main' into feat/adr-fraud-handling
Oct 7, 2024
99abc4e
include next sequencer hash
Oct 8, 2024
6408d12
include tmsignature validate
Oct 8, 2024
5191dda
linter
Oct 8, 2024
ec1a825
add missing lib
Oct 16, 2024
e5a767d
Merge remote-tracking branch 'origin/main' into feat/adr-fraud-handling
Oct 16, 2024
dec030c
add to state serialization
Oct 16, 2024
a978587
include param to last header hash to UpdateStateFromApp
Oct 16, 2024
1ae1c11
Add fraud handler to manager.
Oct 16, 2024
fc85b29
Fix units
Oct 17, 2024
806fc8c
solve some issues based on the PR comments
Oct 17, 2024
710f523
comment applyBlockWithFraudHandling
Oct 17, 2024
40e5f8b
remove comment
Oct 17, 2024
684c515
simplify apply block with block fraud handling
Oct 17, 2024
decd83e
cleanup the code and duplications on validate
Oct 17, 2024
e740570
update comments
Oct 17, 2024
ad473d1
expected heights
Oct 17, 2024
2e27ce7
updated fraud info
Oct 17, 2024
aa74bec
Merge remote-tracking branch 'origin/main' into feat/adr-fraud-handling
Oct 22, 2024
166ef41
update branch with validator set
Oct 22, 2024
adffd84
move err invalid signature fraud
Oct 22, 2024
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: 4 additions & 4 deletions .mockery.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ packages:
github.com/dymensionxyz/dymint/p2p:
interfaces:
GetProposerI:




github.com/dymensionxyz/dymint/block:
interfaces:
ExecutorI:
FraudHandler:
49 changes: 40 additions & 9 deletions block/block.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,50 @@
package block

import (
"context"
"errors"
"fmt"

"github.com/dymensionxyz/gerr-cosmos/gerrc"

errorsmod "cosmossdk.io/errors"

"github.com/dymensionxyz/dymint/types"
)

// applyBlockWithFraudHandling calls applyBlock and validateBlockBeforeApply with fraud handling.
func (m *Manager) applyBlockWithFraudHandling(block *types.Block, commit *types.Commit, blockMetaData types.BlockMetaData) error {
validateWithFraud := func() error {
if err := m.validateBlockBeforeApply(block, commit); err != nil {
if err != nil {
m.blockCache.Delete(block.Header.Height)
// TODO: can we take an action here such as dropping the peer / reducing their reputation?
srene marked this conversation as resolved.
Show resolved Hide resolved

return fmt.Errorf("block not valid at height %d, dropping it: err:%w", block.Header.Height, err)
}
}

if err := m.applyBlock(block, commit, blockMetaData); err != nil {
return fmt.Errorf("apply block: %w", err)
}

return nil
}

err := validateWithFraud()
if errors.Is(err, gerrc.ErrFault) {
// Here we handle the fault by calling the fraud handler.
// FraudHandler is an interface that defines a method to handle faults. Implement this interface to handle faults
// in specific ways. For example, once a fault is detected, it publishes a DataHealthStatus event to the
// pubsub which sets the node in a frozen state.
m.FraudHandler.HandleFault(context.Background(), err)

return err
}

return nil
}

// applyBlock applies the block to the store and the abci app.
// Contract: block and commit must be validated before calling this function!
// steps: save block -> execute block with app -> update state -> commit block to app -> update state's height and commit result.
Expand Down Expand Up @@ -35,7 +72,7 @@ func (m *Manager) applyBlock(block *types.Block, commit *types.Commit, blockMeta
// In case the following true, it means we crashed after the app commit but before updating the state
// In that case we'll want to align the state with the app commit result, as if the block was applied.
if isBlockAlreadyApplied {
err := m.UpdateStateFromApp()
err := m.UpdateStateFromApp(block.Header.Hash())
if err != nil {
return fmt.Errorf("update state from app: %w", err)
}
Expand Down Expand Up @@ -83,7 +120,7 @@ func (m *Manager) applyBlock(block *types.Block, commit *types.Commit, blockMeta

// Update the state with the new app hash, and store height from the commit.
// Every one of those, if happens before commit, prevents us from re-executing the block in case failed during commit.
m.Executor.UpdateStateAfterCommit(m.State, responses, appHash, block.Header.Height)
m.Executor.UpdateStateAfterCommit(m.State, responses, appHash, block.Header.Height, block.Header.Hash())
}

// check if the proposer needs to be changed
Expand Down Expand Up @@ -150,18 +187,12 @@ func (m *Manager) attemptApplyCachedBlocks() error {
if !blockExists {
break
}
if err := m.validateBlockBeforeApply(cachedBlock.Block, cachedBlock.Commit); err != nil {
m.blockCache.Delete(cachedBlock.Block.Header.Height)
// TODO: can we take an action here such as dropping the peer / reducing their reputation?
return fmt.Errorf("block not valid at height %d, dropping it: err:%w", cachedBlock.Block.Header.Height, err)
}

err := m.applyBlock(cachedBlock.Block, cachedBlock.Commit, types.BlockMetaData{Source: cachedBlock.Source})
err := m.applyBlockWithFraudHandling(cachedBlock.Block, cachedBlock.Commit, types.BlockMetaData{Source: cachedBlock.Source})
if err != nil {
return fmt.Errorf("apply cached block: expected height: %d: %w", expectedHeight, err)
}
m.logger.Info("Block applied", "height", expectedHeight)

}

return nil
Expand Down
14 changes: 13 additions & 1 deletion block/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,18 @@ import (
// default minimum block max size allowed. not specific reason to set it to 10K, but we need to avoid no transactions can be included in a block.
const minBlockMaxBytes = 10000

type ExecutorI interface {
Copy link
Contributor

Choose a reason for hiding this comment

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

dont need I suffix in go

Copy link
Contributor

@danwt danwt Sep 19, 2024

Choose a reason for hiding this comment

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

and interfaces should be defined in consumer package

Copy link
Contributor

Choose a reason for hiding this comment

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

nvm it is already

InitChain(genesis *tmtypes.GenesisDoc, valset []*tmtypes.Validator) (*abci.ResponseInitChain, error)
CreateBlock(height uint64, lastCommit *types.Commit, lastHeaderHash, nextSeqHash [32]byte, state *types.State, maxBlockDataSizeBytes uint64) *types.Block
Commit(state *types.State, block *types.Block, resp *tmstate.ABCIResponses) ([]byte, int64, error)
GetAppInfo() (*abci.ResponseInfo, error)
ExecuteBlock(state *types.State, block *types.Block) (*tmstate.ABCIResponses, error)
UpdateStateAfterInitChain(s *types.State, res *abci.ResponseInitChain)
UpdateMempoolAfterInitChain(s *types.State)
UpdateStateAfterCommit(s *types.State, resp *tmstate.ABCIResponses, appHash []byte, height uint64, lastHeaderHash [32]byte)
UpdateProposerFromBlock(s *types.State, block *types.Block) bool
}

// Executor creates and applies blocks and maintains state.
type Executor struct {
localAddress []byte
Expand All @@ -46,7 +58,7 @@ func NewExecutor(
eventBus *tmtypes.EventBus,
consensusMessagesStream ConsensusMessagesStream,
logger types.Logger,
) (*Executor, error) {
) (ExecutorI, error) {
be := Executor{
localAddress: localAddress,
chainID: chainID,
Expand Down
26 changes: 14 additions & 12 deletions block/executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,11 @@ func TestApplyBlock(t *testing.T) {

logger := log.TestingLogger()

// Create a valid proposer for the block
proposerKey := ed25519.GenPrivKey()
tmPubKey, err := cryptocodec.ToTmPubKeyInterface(proposerKey.PubKey())
require.NoError(err)

// Mock ABCI app
app := &tmmocks.MockApplication{}
app.On("CheckTx", mock.Anything).Return(abci.ResponseCheckTx{})
Expand All @@ -201,7 +206,7 @@ func TestApplyBlock(t *testing.T) {
},
})
var mockAppHash [32]byte
_, err := rand.Read(mockAppHash[:])
_, err = rand.Read(mockAppHash[:])
require.NoError(err)
app.On("Commit", mock.Anything).Return(abci.ResponseCommit{
Data: mockAppHash[:],
Expand Down Expand Up @@ -229,7 +234,7 @@ func TestApplyBlock(t *testing.T) {
appConns := &tmmocksproxy.MockAppConns{}
appConns.On("Consensus").Return(abciClient)
appConns.On("Query").Return(abciClient)
executor, err := block.NewExecutor([]byte("test address"), chainID, mpool, appConns, eventBus, nil, logger)
executor, err := block.NewExecutor(proposerKey.PubKey().Address(), chainID, mpool, appConns, eventBus, nil, logger)
assert.NoError(err)

// Subscribe to tx events
Expand All @@ -246,25 +251,22 @@ func TestApplyBlock(t *testing.T) {
require.NoError(err)
require.NotNil(headerSub)

// Create a valid proposer for the block
proposerKey := ed25519.GenPrivKey()
tmPubKey, err := cryptocodec.ToTmPubKeyInterface(proposerKey.PubKey())
require.NoError(err)

// Init state
state := &types.State{}
state.Sequencers.SetProposer(types.NewSequencerFromValidator(*tmtypes.NewValidator(tmPubKey, 1)))
state.InitialHeight = 1
state.ChainID = chainID
state.SetHeight(0)
maxBytes := uint64(10000)
state.ConsensusParams.Block.MaxBytes = int64(maxBytes)
state.ConsensusParams.Block.MaxGas = 100000
state.RollappParams.Da = "mock"
state.LastHeaderHash = [32]byte{0x01}

// Create first block with one Tx from mempool
_ = mpool.CheckTx([]byte{1, 2, 3, 4}, func(r *abci.Response) {}, mempool.TxInfo{})
require.NoError(err)
block := executor.CreateBlock(1, &types.Commit{Height: 0}, [32]byte{}, [32]byte(state.Sequencers.ProposerHash()), state, maxBytes)
block := executor.CreateBlock(1, &types.Commit{Height: 0}, [32]byte{0x01}, [32]byte(state.Sequencers.ProposerHash()), state, maxBytes)
require.NotNil(block)
assert.Equal(uint64(1), block.Header.Height)
assert.Len(block.Data.Txs, 1)
Expand All @@ -290,7 +292,7 @@ func TestApplyBlock(t *testing.T) {
require.NotNil(resp)
appHash, _, err := executor.Commit(state, block, resp)
require.NoError(err)
executor.UpdateStateAfterCommit(state, resp, appHash, block.Header.Height)
executor.UpdateStateAfterCommit(state, resp, appHash, block.Header.Height, block.Header.Hash())
assert.Equal(uint64(1), state.Height())
assert.Equal(mockAppHash, state.AppHash)

Expand All @@ -299,7 +301,7 @@ func TestApplyBlock(t *testing.T) {
require.NoError(mpool.CheckTx([]byte{5, 6, 7, 8, 9}, func(r *abci.Response) {}, mempool.TxInfo{}))
require.NoError(mpool.CheckTx([]byte{1, 2, 3, 4, 5}, func(r *abci.Response) {}, mempool.TxInfo{}))
require.NoError(mpool.CheckTx(make([]byte, 9990), func(r *abci.Response) {}, mempool.TxInfo{}))
block = executor.CreateBlock(2, commit, [32]byte{}, [32]byte(state.Sequencers.ProposerHash()), state, maxBytes)
block = executor.CreateBlock(2, commit, block.Header.Hash(), [32]byte(state.Sequencers.ProposerHash()), state, maxBytes)
require.NotNil(block)
assert.Equal(uint64(2), block.Header.Height)
assert.Len(block.Data.Txs, 3)
Expand All @@ -321,7 +323,7 @@ func TestApplyBlock(t *testing.T) {

// Apply the block with an invalid commit
err = types.ValidateProposedTransition(state, block, invalidCommit, state.Sequencers.GetProposerPubKey())
require.ErrorIs(err, types.ErrInvalidSignature)
require.ErrorContains(err, types.ErrInvalidSignature.Error())

// Create a valid commit for the block
signature, err = proposerKey.Sign(abciHeaderBytes)
Expand All @@ -340,7 +342,7 @@ func TestApplyBlock(t *testing.T) {
require.NotNil(resp)
_, _, err = executor.Commit(state, block, resp)
require.NoError(err)
executor.UpdateStateAfterCommit(state, resp, appHash, block.Header.Height)
executor.UpdateStateAfterCommit(state, resp, appHash, block.Header.Height, block.Header.Hash())
assert.Equal(uint64(2), state.Height())

// check rollapp params update
Expand Down
32 changes: 32 additions & 0 deletions block/fraud.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package block

import (
"context"

"github.com/dymensionxyz/dymint/node/events"
uevent "github.com/dymensionxyz/dymint/utils/event"
)

// FraudHandler is an interface that defines a method to handle faults.
// Contract: should not be blocking.
type FraudHandler interface {
// HandleFault handles a fault that occurred in the system.
// The fault is passed as an error type.
HandleFault(ctx context.Context, fault error)
}

// FreezeHandler is used to handle faults coming from executing and validating blocks.
// once a fault is detected, it publishes a DataHealthStatus event to the pubsub which sets the node in a frozen state.
type FreezeHandler struct {
m *Manager
}
danwt marked this conversation as resolved.
Show resolved Hide resolved

func (f FreezeHandler) HandleFault(ctx context.Context, fault error) {
uevent.MustPublish(ctx, f.m.Pubsub, &events.DataHealthStatus{Error: fault}, events.HealthStatusList)
}
Comment on lines +24 to +26
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the intended behaviour?
This will just log the error string? what about dumping to file etc, is that coming in another pr?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I never heard about those features

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in theory this is what was commented, to set the node freeze by sending the event.

Copy link
Contributor

Choose a reason for hiding this comment

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

how does sending this event 'freeze' anything? what does freeze even mean?


func NewFreezeHandler(manager *Manager) *FreezeHandler {
return &FreezeHandler{
m: manager,
}
}
35 changes: 35 additions & 0 deletions block/fraud_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package block_test

import (
"errors"
"testing"

"github.com/dymensionxyz/gerr-cosmos/gerrc"
)

type mockError struct {
name string
data string
}

func (m mockError) Error() string {
return "some string"
}

func (mockError) Unwrap() error {
return gerrc.ErrFault
}

func TestErrorIsErrFault(t *testing.T) {
err := mockError{name: "test", data: "test"}

if !errors.Is(err, gerrc.ErrFault) {
t.Error("Expected Is to return true")
}

anotherErr := errors.New("some error")

if errors.Is(anotherErr, gerrc.ErrFault) {
t.Error("Expected Is to return false")
}
}
21 changes: 14 additions & 7 deletions block/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,13 @@
tmcrypto "github.com/tendermint/tendermint/crypto"
"github.com/tendermint/tendermint/libs/log"
"github.com/tendermint/tendermint/libs/pubsub"
tmtypes "github.com/tendermint/tendermint/types"

"github.com/dymensionxyz/dymint/p2p"

"github.com/tendermint/tendermint/proxy"
tmtypes "github.com/tendermint/tendermint/types"

"github.com/dymensionxyz/dymint/config"
"github.com/dymensionxyz/dymint/da"
"github.com/dymensionxyz/dymint/mempool"
"github.com/dymensionxyz/dymint/p2p"
"github.com/dymensionxyz/dymint/settlement"
"github.com/dymensionxyz/dymint/types"
)
Expand All @@ -46,7 +44,7 @@
// Store and execution
Store store.Store
State *types.State
Executor *Executor
Executor ExecutorI

// Clients and servers
Pubsub *pubsub.Server
Expand Down Expand Up @@ -79,6 +77,9 @@
// TargetHeight holds the value of the current highest block seen from either p2p (probably higher) or the DA
TargetHeight atomic.Uint64

// Fraud handler
FraudHandler FraudHandler

// channel used to send the retain height to the pruning background loop
pruningC chan int64

Expand Down Expand Up @@ -135,6 +136,7 @@
},
pruningC: make(chan int64, 10), // use of buffered channel to avoid blocking applyBlock thread. In case channel is full, pruning will be skipped, but the retain height can be pruned in the next iteration.
}
m.setFraudHandler(NewFreezeHandler(m))

err = m.LoadStateOnInit(store, genesis, logger)
if err != nil {
Expand Down Expand Up @@ -197,8 +199,8 @@
}()

// P2P Sync. Subscribe to P2P received blocks events
go uevent.MustSubscribe(ctx, m.Pubsub, "applyGossipedBlocksLoop", p2p.EventQueryNewGossipedBlock, m.onReceivedBlock, m.logger)
go uevent.MustSubscribe(ctx, m.Pubsub, "applyBlockSyncBlocksLoop", p2p.EventQueryNewBlockSyncBlock, m.onReceivedBlock, m.logger)
go uevent.MustSubscribe(ctx, m.Pubsub, "applyGossipedBlocksLoop", p2p.EventQueryNewGossipedBlock, m.OnReceivedBlock, m.logger)

Check notice

Code scanning / CodeQL

Spawning a Go routine Note

Spawning a Go routine may be a possible source of non-determinism
go uevent.MustSubscribe(ctx, m.Pubsub, "applyBlockSyncBlocksLoop", p2p.EventQueryNewBlockSyncBlock, m.OnReceivedBlock, m.logger)

Check notice

Code scanning / CodeQL

Spawning a Go routine Note

Spawning a Go routine may be a possible source of non-determinism
return nil
}

Expand Down Expand Up @@ -352,3 +354,8 @@
m.Retriever = retriever
return nil
}

// setFraudHandler sets the fraud handler for the block manager.
func (m *Manager) setFraudHandler(handler *FreezeHandler) {
m.FraudHandler = handler
}
Loading
Loading