Skip to content

Commit

Permalink
fix(dot/sync): verify justification before importing blocks (#3576)
Browse files Browse the repository at this point in the history
  • Loading branch information
EclesioMeloJunior authored Nov 29, 2023
1 parent dd12623 commit 2954fc0
Show file tree
Hide file tree
Showing 10 changed files with 204 additions and 153 deletions.
2 changes: 1 addition & 1 deletion dot/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ type ServiceRegisterer interface {

// BlockJustificationVerifier has a verification method for block justifications.
type BlockJustificationVerifier interface {
VerifyBlockJustification(common.Hash, []byte) error
VerifyBlockJustification(common.Hash, []byte) (round uint64, setID uint64, err error)
}

// Telemetry is the telemetry client to send telemetry messages.
Expand Down
2 changes: 1 addition & 1 deletion dot/network/message.go
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ func NewAscendingBlockRequests(startNumber, targetNumber uint, requestedData byt
numRequests := diff / MaxBlocksInResponse
// we should check if the diff is in the maxResponseSize bounds
// otherwise we should increase the numRequests by one, take this
// example, we want to sync from 0 to 259, the diff is 259
// example, we want to sync from 1 to 259, the diff is 259
// then the num of requests is 2 (uint(259)/uint(128)) however two requests will
// retrieve only 256 blocks (each request can retrieve a max of 128 blocks), so we should
// create one more request to retrieve those missing blocks, 3 in this example.
Expand Down
114 changes: 38 additions & 76 deletions dot/sync/chain_sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"github.com/ChainSafe/gossamer/dot/telemetry"
"github.com/ChainSafe/gossamer/dot/types"
"github.com/ChainSafe/gossamer/internal/database"
"github.com/ChainSafe/gossamer/lib/blocktree"
"github.com/ChainSafe/gossamer/lib/common"
"github.com/ChainSafe/gossamer/lib/common/variadic"
"github.com/ChainSafe/gossamer/lib/trie"
Expand Down Expand Up @@ -795,94 +794,51 @@ func (cs *chainSync) handleReadyBlock(bd *types.BlockData) error {
// returns the index of the last BlockData it handled on success,
// or the index of the block data that errored on failure.
func (cs *chainSync) processBlockData(blockData types.BlockData) error {
headerInState, err := cs.blockState.HasHeader(blockData.Hash)
if err != nil {
return fmt.Errorf("checking if block state has header: %w", err)
}

bodyInState, err := cs.blockState.HasBlockBody(blockData.Hash)
if err != nil {
return fmt.Errorf("checking if block state has body: %w", err)
}

// while in bootstrap mode we don't need to broadcast block announcements
announceImportedBlock := cs.getSyncMode() == tip
if headerInState && bodyInState {
err = cs.processBlockDataWithStateHeaderAndBody(blockData, announceImportedBlock)
if err != nil {
return fmt.Errorf("processing block data with header and "+
"body in block state: %w", err)
}
return nil
var blockDataJustification []byte
if blockData.Justification != nil {
blockDataJustification = *blockData.Justification
}

if blockData.Header != nil {
round, setID, err := cs.verifyJustification(blockData.Header.Hash(), blockDataJustification)
if err != nil {
return err
}

if blockData.Body != nil {
err = cs.processBlockDataWithHeaderAndBody(blockData, announceImportedBlock)
if err != nil {
return fmt.Errorf("processing block data with header and body: %w", err)
}
}

if blockData.Justification != nil && len(*blockData.Justification) > 0 {
logger.Infof("handling justification for block %s (#%d)", blockData.Hash.Short(), blockData.Number())
err = cs.handleJustification(blockData.Header, *blockData.Justification)
if err != nil {
return fmt.Errorf("handling justification: %w", err)
}
err = cs.finalizeAndSetJustification(
blockData.Header,
round, setID,
blockDataJustification)
if err != nil {
return fmt.Errorf("while setting justification: %w", err)
}
}

err = cs.blockState.CompareAndSetBlockData(&blockData)
err := cs.blockState.CompareAndSetBlockData(&blockData)
if err != nil {
return fmt.Errorf("comparing and setting block data: %w", err)
}

return nil
}

func (cs *chainSync) processBlockDataWithStateHeaderAndBody(blockData types.BlockData,
announceImportedBlock bool) (err error) {
// TODO: fix this; sometimes when the node shuts down the "best block" isn't stored properly,
// so when the node restarts it has blocks higher than what it thinks is the best, causing it not to sync
// if we update the node to only store finalised blocks in the database, this should be fixed and the entire
// code block can be removed (#1784)
block, err := cs.blockState.GetBlockByHash(blockData.Hash)
if err != nil {
return fmt.Errorf("getting block by hash: %w", err)
}

err = cs.blockState.AddBlockToBlockTree(block)
if errors.Is(err, blocktree.ErrBlockExists) {
logger.Debugf(
"block number %d with hash %s already exists in block tree, skipping it.",
block.Header.Number, blockData.Hash)
return nil
} else if err != nil {
return fmt.Errorf("adding block to blocktree: %w", err)
}

if blockData.Justification != nil && len(*blockData.Justification) > 0 {
err = cs.handleJustification(&block.Header, *blockData.Justification)
if err != nil {
return fmt.Errorf("handling justification: %w", err)
}
}

// TODO: this is probably unnecessary, since the state is already in the database
// however, this case shouldn't be hit often, since it's only hit if the node state
// is rewinded or if the node shuts down unexpectedly (#1784)
state, err := cs.storageState.TrieState(&block.Header.StateRoot)
if err != nil {
return fmt.Errorf("loading trie state: %w", err)
}

err = cs.blockImportHandler.HandleBlockImport(block, state, announceImportedBlock)
if err != nil {
return fmt.Errorf("handling block import: %w", err)
func (cs *chainSync) verifyJustification(headerHash common.Hash, justification []byte) (
round uint64, setID uint64, err error) {
if len(justification) > 0 {
round, setID, err = cs.finalityGadget.VerifyBlockJustification(headerHash, justification)
return round, setID, err
}

return nil
return 0, 0, nil
}

func (cs *chainSync) processBlockDataWithHeaderAndBody(blockData types.BlockData,
Expand Down Expand Up @@ -918,21 +874,27 @@ func (cs *chainSync) handleBody(body *types.Body) {
blockSizeGauge.Set(float64(acc))
}

func (cs *chainSync) handleJustification(header *types.Header, justification []byte) (err error) {
logger.Debugf("handling justification for block %d...", header.Number)
func (cs *chainSync) finalizeAndSetJustification(header *types.Header,
round, setID uint64, justification []byte) (err error) {
if len(justification) > 0 {
err = cs.blockState.SetFinalisedHash(header.Hash(), round, setID)
if err != nil {
return fmt.Errorf("setting finalised hash: %w", err)
}

headerHash := header.Hash()
err = cs.finalityGadget.VerifyBlockJustification(headerHash, justification)
if err != nil {
return fmt.Errorf("verifying block number %d justification: %w", header.Number, err)
}
logger.Debugf(
"finalised block with hash #%d (%s), round %d and set id %d",
header.Number, header.Hash(), round, setID)

err = cs.blockState.SetJustification(headerHash, justification)
if err != nil {
return fmt.Errorf("setting justification for block number %d: %w", header.Number, err)
err = cs.blockState.SetJustification(header.Hash(), justification)
if err != nil {
return fmt.Errorf("setting justification for block number %d: %w",
header.Number, err)
}

logger.Infof("🔨 finalised block number #%d (%s)", header.Number, header.Hash())
}

logger.Infof("🔨 finalised block number %d with hash %s", header.Number, headerHash)
return nil
}

Expand Down
92 changes: 90 additions & 2 deletions dot/sync/chain_sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1288,8 +1288,6 @@ func ensureSuccessfulBlockImportFlow(t *testing.T, parentHeader *types.Header,
t.Helper()

for idx, blockData := range blocksReceived {
mockBlockState.EXPECT().HasHeader(blockData.Header.Hash()).Return(false, nil)
mockBlockState.EXPECT().HasBlockBody(blockData.Header.Hash()).Return(false, nil)
mockBabeVerifier.EXPECT().VerifyBlock(blockData.Header).Return(nil)

var previousHeader *types.Header
Expand Down Expand Up @@ -1676,3 +1674,93 @@ func TestChainSync_getHighestBlock(t *testing.T) {
})
}
}

func TestChainSync_BootstrapSync_SuccessfulSync_WithInvalidJusticationBlock(t *testing.T) {
t.Parallel()

ctrl := gomock.NewController(t)
mockBlockState := NewMockBlockState(ctrl)
mockBlockState.EXPECT().GetFinalisedNotifierChannel().Return(make(chan *types.FinalisationInfo))
mockedGenesisHeader := types.NewHeader(common.NewHash([]byte{0}), trie.EmptyHash,
trie.EmptyHash, 0, types.NewDigest())

mockNetwork := NewMockNetwork(ctrl)
mockRequestMaker := NewMockRequestMaker(ctrl)

mockBabeVerifier := NewMockBabeVerifier(ctrl)
mockStorageState := NewMockStorageState(ctrl)
mockImportHandler := NewMockBlockImportHandler(ctrl)
mockTelemetry := NewMockTelemetry(ctrl)
mockFinalityGadget := NewMockFinalityGadget(ctrl)

// this test expects two workers responding each request with 128 blocks which means
// we should import 256 blocks in total
blockResponse := createSuccesfullBlockResponse(t, mockedGenesisHeader.Hash(), 1, 129)
const announceBlock = false

invalidJustificationBlock := blockResponse.BlockData[90]
invalidJustification := &[]byte{0x01, 0x01, 0x01, 0x02}
invalidJustificationBlock.Justification = invalidJustification

// here we split the whole set in two parts each one will be the "response" for each peer
worker1Response := &network.BlockResponseMessage{
BlockData: blockResponse.BlockData[:128],
}

// the first peer will respond the from the block 1 to 128 so the ensureBlockImportFlow
// will setup the expectations starting from the genesis header until block 128
ensureSuccessfulBlockImportFlow(t, mockedGenesisHeader, worker1Response.BlockData[:90], mockBlockState,
mockBabeVerifier, mockStorageState, mockImportHandler, mockTelemetry, announceBlock)

errVerifyBlockJustification := errors.New("VerifyBlockJustification mock error")
mockFinalityGadget.EXPECT().
VerifyBlockJustification(
invalidJustificationBlock.Header.Hash(),
*invalidJustification).
Return(uint64(0), uint64(0), errVerifyBlockJustification)

// we use gomock.Any since I cannot guarantee which peer picks which request
// but the first call to DoBlockRequest will return the first set and the second
// call will return the second set
mockRequestMaker.EXPECT().
Do(gomock.Any(), gomock.Any(), &network.BlockResponseMessage{}).
DoAndReturn(func(peerID, _, response any) any {
responsePtr := response.(*network.BlockResponseMessage)
*responsePtr = *worker1Response

fmt.Println("mocked request maker")
return nil
})

// setup a chain sync which holds in its peer view map
// 3 peers, each one announce block 129 as its best block number.
// We start this test with genesis block being our best block, so
// we're far behind by 128 blocks, we should execute a bootstrap
// sync request those blocks
const blocksAhead = 128
cs := setupChainSyncToBootstrapMode(t, blocksAhead,
mockBlockState, mockNetwork, mockRequestMaker, mockBabeVerifier,
mockStorageState, mockImportHandler, mockTelemetry)

cs.finalityGadget = mockFinalityGadget

target, err := cs.getTarget()
require.NoError(t, err)
require.Equal(t, uint(blocksAhead), target)

// include a new worker in the worker pool set, this worker
// should be an available peer that will receive a block request
// the worker pool executes the workers management
cs.workerPool.fromBlockAnnounce(peer.ID("alice"))
//cs.workerPool.fromBlockAnnounce(peer.ID("bob"))

err = cs.requestMaxBlocksFrom(mockedGenesisHeader)
require.ErrorIs(t, err, errVerifyBlockJustification)

err = cs.workerPool.stop()
require.NoError(t, err)

// peer should be not in the worker pool
// peer should be in the ignore list
require.Len(t, cs.workerPool.workers, 1)
}
4 changes: 2 additions & 2 deletions dot/sync/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ type BlockState interface {
BestBlockHeader() (*types.Header, error)
BestBlockNumber() (number uint, err error)
CompareAndSetBlockData(bd *types.BlockData) error
HasBlockBody(hash common.Hash) (bool, error)
GetBlockBody(common.Hash) (*types.Body, error)
GetHeader(common.Hash) (*types.Header, error)
HasHeader(hash common.Hash) (bool, error)
Expand All @@ -40,6 +39,7 @@ type BlockState interface {
GetHeaderByNumber(num uint) (*types.Header, error)
GetAllBlocksAtNumber(num uint) ([]common.Hash, error)
IsDescendantOf(parent, child common.Hash) (bool, error)
SetFinalisedHash(common.Hash, uint64, uint64) error
}

// StorageState is the interface for the storage state
Expand All @@ -60,7 +60,7 @@ type BabeVerifier interface {

// FinalityGadget implements justification verification functionality
type FinalityGadget interface {
VerifyBlockJustification(common.Hash, []byte) error
VerifyBlockJustification(common.Hash, []byte) (round uint64, setID uint64, err error)
}

// BlockImportHandler is the interface for the handler of newly imported blocks
Expand Down
37 changes: 19 additions & 18 deletions dot/sync/mocks_test.go

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

7 changes: 4 additions & 3 deletions dot/sync/syncer_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,10 @@ func newTestSyncer(t *testing.T) *Service {
cfg.LogLvl = log.Trace
mockFinalityGadget := NewMockFinalityGadget(ctrl)
mockFinalityGadget.EXPECT().VerifyBlockJustification(gomock.AssignableToTypeOf(common.Hash{}),
gomock.AssignableToTypeOf([]byte{})).DoAndReturn(func(hash common.Hash, justification []byte) error {
return nil
}).AnyTimes()
gomock.AssignableToTypeOf([]byte{})).DoAndReturn(
func(hash common.Hash, justification []byte) (uint64, uint64, error) {
return 1, 1, nil
}).AnyTimes()

cfg.FinalityGadget = mockFinalityGadget
cfg.Network = NewMockNetwork(ctrl)
Expand Down
Loading

0 comments on commit 2954fc0

Please sign in to comment.