diff --git a/consensus/polybft/checkpoint_manager.go b/consensus/polybft/checkpoint_manager.go index 806f705265..8d2f7aa2ab 100644 --- a/consensus/polybft/checkpoint_manager.go +++ b/consensus/polybft/checkpoint_manager.go @@ -126,6 +126,16 @@ func (c *checkpointManager) submitCheckpoint(latestHeader *types.Header, isEndOf return err } + if lastCheckpointBlockNumber > latestHeader.Number { + // this node is syncing after its data was cleaned up + // what happened is that, node was the proposer on this block, + // and it already sent this checkpoint, but since its data got cleaned + // it started syncing from scratch, even its own minted blocks + // so there is no need to send a checkpoint, since it is already sent + // CheckpointManager will fail the transaction, so no need to spend tokens on it + return nil + } + c.logger.Debug("submitCheckpoint invoked...", "latest checkpoint block", lastCheckpointBlockNumber, "checkpoint block", latestHeader.Number) diff --git a/consensus/polybft/checkpoint_manager_test.go b/consensus/polybft/checkpoint_manager_test.go index faf0666417..c0032c6bbc 100644 --- a/consensus/polybft/checkpoint_manager_test.go +++ b/consensus/polybft/checkpoint_manager_test.go @@ -39,83 +39,109 @@ func TestCheckpointManager_SubmitCheckpoint(t *testing.T) { validators := validator.NewTestValidatorsWithAliases(t, aliases) validatorsMetadata := validators.GetPublicIdentities() - txRelayerMock := newDummyTxRelayer(t) - txRelayerMock.On("Call", mock.Anything, mock.Anything, mock.Anything). - Return("2", error(nil)). - Once() - txRelayerMock.On("SendTransaction", mock.Anything, mock.Anything). - Return(ðgo.Receipt{Status: uint64(types.ReceiptSuccess)}, error(nil)). - Times(4) // send transactions for checkpoint blocks: 4, 6, 8 (pending checkpoint blocks) and 10 (latest checkpoint block) - backendMock := new(polybftBackendMock) - backendMock.On("GetValidators", mock.Anything, mock.Anything).Return(validatorsMetadata) - - var ( - headersMap = &testHeadersMap{} - epochNumber = uint64(1) - dummyMsg = []byte("checkpoint") - idx = uint64(0) - header *types.Header - bitmap bitmap.Bitmap - signatures bls.Signatures - ) + t.Run("submit checkpoint happy path", func(t *testing.T) { + t.Parallel() - validators.IterAcct(aliases, func(t *validator.TestValidator) { - bitmap.Set(idx) - signatures = append(signatures, t.MustSign(dummyMsg, bls.DomainCheckpointManager)) - idx++ - }) + txRelayerMock := newDummyTxRelayer(t) + txRelayerMock.On("Call", mock.Anything, mock.Anything, mock.Anything). + Return("2", error(nil)). + Once() + txRelayerMock.On("SendTransaction", mock.Anything, mock.Anything). + Return(ðgo.Receipt{Status: uint64(types.ReceiptSuccess)}, error(nil)). + Times(4) // send transactions for checkpoint blocks: 4, 6, 8 (pending checkpoint blocks) and 10 (latest checkpoint block) + + backendMock := new(polybftBackendMock) + backendMock.On("GetValidators", mock.Anything, mock.Anything).Return(validatorsMetadata) + + var ( + headersMap = &testHeadersMap{} + epochNumber = uint64(1) + dummyMsg = []byte("checkpoint") + idx = uint64(0) + header *types.Header + bitmap bitmap.Bitmap + signatures bls.Signatures + ) + + validators.IterAcct(aliases, func(t *validator.TestValidator) { + bitmap.Set(idx) + signatures = append(signatures, t.MustSign(dummyMsg, bls.DomainCheckpointManager)) + idx++ + }) - signature, err := signatures.Aggregate().Marshal() - require.NoError(t, err) + signature, err := signatures.Aggregate().Marshal() + require.NoError(t, err) - for i := uint64(1); i <= blocksCount; i++ { - if i%epochSize == 1 { - // epoch-beginning block - checkpoint := &CheckpointData{ - BlockRound: 0, - EpochNumber: epochNumber, - EventRoot: types.BytesToHash(generateRandomBytes(t)), - } - extra := createTestExtraObject(validatorsMetadata, validatorsMetadata, 3, 3, 3) - extra.Checkpoint = checkpoint - extra.Committed = &Signature{Bitmap: bitmap, AggregatedSignature: signature} - header = &types.Header{ - ExtraData: extra.MarshalRLPTo(nil), + for i := uint64(1); i <= blocksCount; i++ { + if i%epochSize == 1 { + // epoch-beginning block + checkpoint := &CheckpointData{ + BlockRound: 0, + EpochNumber: epochNumber, + EventRoot: types.BytesToHash(generateRandomBytes(t)), + } + extra := createTestExtraObject(validatorsMetadata, validatorsMetadata, 3, 3, 3) + extra.Checkpoint = checkpoint + extra.Committed = &Signature{Bitmap: bitmap, AggregatedSignature: signature} + header = &types.Header{ + ExtraData: extra.MarshalRLPTo(nil), + } + epochNumber++ + } else { + header = header.Copy() } - epochNumber++ - } else { - header = header.Copy() + + header.Number = i + header.ComputeHash() + headersMap.addHeader(header) } - header.Number = i - header.ComputeHash() - headersMap.addHeader(header) - } + // mock blockchain + blockchainMock := new(blockchainMock) + blockchainMock.On("GetHeaderByNumber", mock.Anything).Return(headersMap.getHeader) + + validatorAcc := validators.GetValidator("A") + c := &checkpointManager{ + key: wallet.NewEcdsaSigner(validatorAcc.Key()), + rootChainRelayer: txRelayerMock, + consensusBackend: backendMock, + blockchain: blockchainMock, + logger: hclog.NewNullLogger(), + } - // mock blockchain - blockchainMock := new(blockchainMock) - blockchainMock.On("GetHeaderByNumber", mock.Anything).Return(headersMap.getHeader) + err = c.submitCheckpoint(headersMap.getHeader(blocksCount), false) + require.NoError(t, err) + txRelayerMock.AssertExpectations(t) - validatorAcc := validators.GetValidator("A") - c := &checkpointManager{ - key: wallet.NewEcdsaSigner(validatorAcc.Key()), - rootChainRelayer: txRelayerMock, - consensusBackend: backendMock, - blockchain: blockchainMock, - logger: hclog.NewNullLogger(), - } + // make sure that expected blocks are checkpointed (epoch-ending ones) + for _, checkpointBlock := range txRelayerMock.checkpointBlocks { + header := headersMap.getHeader(checkpointBlock) + require.NotNil(t, header) + require.True(t, isEndOfPeriod(header.Number, epochSize)) + } + }) - err = c.submitCheckpoint(headersMap.getHeader(blocksCount), false) - require.NoError(t, err) - txRelayerMock.AssertExpectations(t) + t.Run("checkpoint not submitted when node is syncing", func(t *testing.T) { + t.Parallel() - // make sure that expected blocks are checkpointed (epoch-ending ones) - for _, checkpointBlock := range txRelayerMock.checkpointBlocks { - header := headersMap.getHeader(checkpointBlock) - require.NotNil(t, header) - require.True(t, isEndOfPeriod(header.Number, epochSize)) - } + txRelayerMock := newDummyTxRelayer(t) + txRelayerMock.On("Call", mock.Anything, mock.Anything, mock.Anything). + Return("20", error(nil)). + Once() + + c := &checkpointManager{ + key: wallet.NewEcdsaSigner(validators.GetValidator("A").Key()), + rootChainRelayer: txRelayerMock, + logger: hclog.NewNullLogger(), + } + + err := c.submitCheckpoint(&types.Header{Number: 19 /*lower than what Call returns*/}, false) + require.NoError(t, err) + // since transaction will not be sent to CheckpointManager, we only expect that Call + // will called on tx relayer, and nothing else in submitCheckpoint will be executed + txRelayerMock.AssertExpectations(t) + }) } func TestCheckpointManager_abiEncodeCheckpointBlock(t *testing.T) {