From fe5b05470b734a74e6924f200fcabd754853c357 Mon Sep 17 00:00:00 2001 From: Diego Romero Date: Mon, 24 Apr 2023 23:45:51 -0300 Subject: [PATCH] feat(sync): Validate bad blocks (#3220) --- dot/services.go | 6 ++++++ dot/sync/chain_processor.go | 1 + dot/sync/chain_sync.go | 10 ++++++++++ dot/sync/chain_sync_test.go | 30 ++++++++++++++++++++++++++++++ dot/sync/errors.go | 1 + dot/sync/syncer.go | 2 ++ 6 files changed, 50 insertions(+) diff --git a/dot/services.go b/dot/services.go index c7b5f2f389e..039d10ca637 100644 --- a/dot/services.go +++ b/dot/services.go @@ -441,6 +441,11 @@ func (nodeBuilder) newSyncService(cfg *Config, st *state.Service, fg BlockJustif return nil, err } + genesisData, err := st.Base.LoadGenesisData() + if err != nil { + return nil, err + } + syncCfg := &sync.Config{ LogLvl: cfg.Log.SyncLvl, Network: net, @@ -454,6 +459,7 @@ func (nodeBuilder) newSyncService(cfg *Config, st *state.Service, fg BlockJustif MaxPeers: cfg.Network.MaxPeers, SlotDuration: slotDuration, Telemetry: telemetryMailer, + BadBlocks: genesisData.BadBlocks, } return sync.NewService(syncCfg) diff --git a/dot/sync/chain_processor.go b/dot/sync/chain_processor.go index 4ba27803f0c..ef060b6a2bb 100644 --- a/dot/sync/chain_processor.go +++ b/dot/sync/chain_processor.go @@ -55,6 +55,7 @@ type chainProcessorConfig struct { finalityGadget FinalityGadget blockImportHandler BlockImportHandler telemetry Telemetry + badBlocks []string } func newChainProcessor(cfg chainProcessorConfig) *chainProcessor { diff --git a/dot/sync/chain_sync.go b/dot/sync/chain_sync.go index 5f45beb7250..0629cb9400e 100644 --- a/dot/sync/chain_sync.go +++ b/dot/sync/chain_sync.go @@ -17,6 +17,7 @@ import ( "github.com/libp2p/go-libp2p/core/peer" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" + "golang.org/x/exp/slices" "github.com/ChainSafe/gossamer/dot/network" "github.com/ChainSafe/gossamer/dot/peerset" @@ -165,6 +166,7 @@ type chainSync struct { logSyncTickerC <-chan time.Time // channel as field for unit testing logSyncStarted bool logSyncDone chan struct{} + badBlocks []string } type chainSyncConfig struct { @@ -174,6 +176,7 @@ type chainSyncConfig struct { pendingBlocks DisjointBlockSet minPeers, maxPeers int slotDuration time.Duration + badBlocks []string } func newChainSync(cfg chainSyncConfig) *chainSync { @@ -204,6 +207,7 @@ func newChainSync(cfg chainSyncConfig) *chainSync { logSyncTicker: logSyncTicker, logSyncTickerC: logSyncTicker.C, logSyncDone: make(chan struct{}), + badBlocks: cfg.badBlocks, } } @@ -829,6 +833,7 @@ func (cs *chainSync) determineSyncPeers(req *network.BlockRequestMessage, peersT // It checks the following: // - the response is not empty // - the response contains all the expected fields +// - the block is not contained in the bad block list // - each block has the correct parent, ie. the response constitutes a valid chain func (cs *chainSync) validateResponse(req *network.BlockRequestMessage, resp *network.BlockResponseMessage, p peer.ID) error { @@ -929,6 +934,11 @@ func (cs *chainSync) validateBlockData(req *network.BlockRequestMessage, bd *typ requestedData := req.RequestedData + if slices.Contains(cs.badBlocks, bd.Hash.String()) { + logger.Errorf("Rejecting known bad block Number: %d Hash: %s", bd.Number(), bd.Hash) + return errBadBlock + } + if (requestedData&network.RequestedDataHeader) == 1 && bd.Header == nil { cs.network.ReportPeer(peerset.ReputationChange{ Value: peerset.IncompleteHeaderValue, diff --git a/dot/sync/chain_sync_test.go b/dot/sync/chain_sync_test.go index dad13e5a03e..1633371d4c6 100644 --- a/dot/sync/chain_sync_test.go +++ b/dot/sync/chain_sync_test.go @@ -706,6 +706,8 @@ func TestWorkerToRequests(t *testing.T) { func TestChainSync_validateResponse(t *testing.T) { t.Parallel() + badBlockHash := common.NewHash([]byte("badblockhash")) + tests := map[string]struct { blockStateBuilder func(ctrl *gomock.Controller) BlockState networkBuilder func(ctrl *gomock.Controller) Network @@ -813,6 +815,31 @@ func TestChainSync_validateResponse(t *testing.T) { }, expectedError: errUnknownParent, }, + "handle_error_bad_block": { + blockStateBuilder: func(ctrl *gomock.Controller) BlockState { + mockBlockState := NewMockBlockState(ctrl) + mockBlockState.EXPECT().GetFinalisedNotifierChannel().Return(make(chan *types.FinalisationInfo)) + return mockBlockState + }, + networkBuilder: func(ctrl *gomock.Controller) Network { + return NewMockNetwork(ctrl) + }, + req: &network.BlockRequestMessage{ + RequestedData: network.RequestedDataHeader, + }, + resp: &network.BlockResponseMessage{ + BlockData: []*types.BlockData{ + { + Hash: badBlockHash, + Header: &types.Header{ + Number: 2, + }, + Body: &types.Body{}, + }, + }, + }, + expectedError: errBadBlock, + }, "no_error": { blockStateBuilder: func(ctrl *gomock.Controller) BlockState { mockBlockState := NewMockBlockState(ctrl) @@ -858,6 +885,9 @@ func TestChainSync_validateResponse(t *testing.T) { pendingBlocks: newDisjointBlockSet(pendingBlocksLimit), readyBlocks: newBlockQueue(maxResponseSize), net: tt.networkBuilder(ctrl), + badBlocks: []string{ + badBlockHash.String(), + }, } cs := newChainSync(cfg) diff --git a/dot/sync/errors.go b/dot/sync/errors.go index 51334ab66f6..1a90802d685 100644 --- a/dot/sync/errors.go +++ b/dot/sync/errors.go @@ -34,4 +34,5 @@ var ( errFailedToGetParent = errors.New("failed to get parent header") errStartAndEndMismatch = errors.New("request start and end hash are not on the same chain") errFailedToGetDescendant = errors.New("failed to find descendant block") + errBadBlock = errors.New("known bad block") ) diff --git a/dot/sync/syncer.go b/dot/sync/syncer.go index 32c9d74fa52..cc268beb79e 100644 --- a/dot/sync/syncer.go +++ b/dot/sync/syncer.go @@ -36,6 +36,7 @@ type Config struct { MinPeers, MaxPeers int SlotDuration time.Duration Telemetry Telemetry + BadBlocks []string } // NewService returns a new *sync.Service @@ -67,6 +68,7 @@ func NewService(cfg *Config) (*Service, error) { finalityGadget: cfg.FinalityGadget, blockImportHandler: cfg.BlockImportHandler, telemetry: cfg.Telemetry, + badBlocks: cfg.BadBlocks, } chainProcessor := newChainProcessor(cpCfg)