diff --git a/eth/backend.go b/eth/backend.go index c159df7725..cc65d9837f 100644 --- a/eth/backend.go +++ b/eth/backend.go @@ -695,7 +695,9 @@ func (s *Ethereum) handleWhitelistCheckpoint(ctx context.Context, first bool) er return ErrBorConsensusWithoutHeimdall } - blockNums, blockHashes, err := ethHandler.fetchWhitelistCheckpoints(ctx, bor, first) + // Create a new checkpoint verifier + verifier := newCheckpointVerifier(nil) + blockNums, blockHashes, err := ethHandler.fetchWhitelistCheckpoints(ctx, bor, verifier, first) // If the array is empty, we're bound to receive an error. Non-nill error and non-empty array // means that array has partial elements and it failed for some block. We'll add those partial // elements anyway. diff --git a/eth/bor_checkpoint_verifier.go b/eth/bor_checkpoint_verifier.go new file mode 100644 index 0000000000..61e8c382e1 --- /dev/null +++ b/eth/bor_checkpoint_verifier.go @@ -0,0 +1,60 @@ +package eth + +import ( + "context" + "fmt" + + "github.com/ethereum/go-ethereum/common/hexutil" + "github.com/ethereum/go-ethereum/consensus/bor/heimdall/checkpoint" + "github.com/ethereum/go-ethereum/log" + "github.com/ethereum/go-ethereum/rpc" +) + +type checkpointVerifier struct { + verify func(ctx context.Context, handler *ethHandler, checkpoint *checkpoint.Checkpoint) (string, error) +} + +func newCheckpointVerifier(verifyFn func(ctx context.Context, handler *ethHandler, checkpoint *checkpoint.Checkpoint) (string, error)) *checkpointVerifier { + if verifyFn != nil { + return &checkpointVerifier{verifyFn} + } + + verifyFn = func(ctx context.Context, handler *ethHandler, checkpoint *checkpoint.Checkpoint) (string, error) { + var ( + startBlock = checkpoint.StartBlock.Uint64() + endBlock = checkpoint.EndBlock.Uint64() + ) + + // check if we have the checkpoint blocks + head := handler.ethAPI.BlockNumber() + if head < hexutil.Uint64(endBlock) { + log.Debug("Head block behind checkpoint block", "head", head, "checkpoint end block", endBlock) + return "", errMissingCheckpoint + } + + // verify the root hash of checkpoint + roothash, err := handler.ethAPI.GetRootHash(ctx, startBlock, endBlock) + if err != nil { + log.Debug("Failed to get root hash of checkpoint while whitelisting", "err", err) + return "", errRootHash + } + + if roothash != checkpoint.RootHash.String()[2:] { + log.Warn("Checkpoint root hash mismatch while whitelisting", "expected", checkpoint.RootHash.String()[2:], "got", roothash) + return "", errCheckpointRootHashMismatch + } + + // fetch the end checkpoint block hash + block, err := handler.ethAPI.GetBlockByNumber(ctx, rpc.BlockNumber(endBlock), false) + if err != nil { + log.Debug("Failed to get end block hash of checkpoint while whitelisting", "err", err) + return "", errEndBlock + } + + hash := fmt.Sprintf("%v", block["hash"]) + + return hash, nil + } + + return &checkpointVerifier{verifyFn} +} diff --git a/eth/handler_bor.go b/eth/handler_bor.go index 35d6e00bfc..604deef282 100644 --- a/eth/handler_bor.go +++ b/eth/handler_bor.go @@ -3,13 +3,10 @@ package eth import ( "context" "errors" - "fmt" "github.com/ethereum/go-ethereum/common" - "github.com/ethereum/go-ethereum/common/hexutil" "github.com/ethereum/go-ethereum/consensus/bor" "github.com/ethereum/go-ethereum/log" - "github.com/ethereum/go-ethereum/rpc" ) var ( @@ -43,7 +40,7 @@ var ( // fetchWhitelistCheckpoints fetches the latest checkpoint/s from it's local heimdall // and verifies the data against bor data. -func (h *ethHandler) fetchWhitelistCheckpoints(ctx context.Context, bor *bor.Bor, first bool) ([]uint64, []common.Hash, error) { +func (h *ethHandler) fetchWhitelistCheckpoints(ctx context.Context, bor *bor.Bor, checkpointVerifier *checkpointVerifier, first bool) ([]uint64, []common.Hash, error) { // Create an array for block number and block hashes //nolint:prealloc var ( @@ -62,56 +59,44 @@ func (h *ethHandler) fetchWhitelistCheckpoints(ctx context.Context, bor *bor.Bor return blockNums, blockHashes, errNoCheckpoint } - // If we're in the first iteration, we'll fetch last 10 checkpoints, else only the latest one - iterations := 1 - if first { - iterations = 10 + var ( + start int64 + end int64 + ) + + // Prepare the checkpoint range to fetch + if count <= 10 { + start = 1 + } else { + start = count - 10 + 1 // 10 is the max number of checkpoints to fetch } - for i := 0; i < iterations; i++ { - // If we don't have any checkpoints in heimdall, break - if count == 0 { - break - } + end = count - // fetch `count` indexed checkpoint from heimdall - checkpoint, err := bor.HeimdallClient.FetchCheckpoint(ctx, count) + // If we're in not in the first iteration, only fetch the latest checkpoint + if !first { + start = count + } + + for i := start; i <= end; i++ { + // fetch `i` indexed checkpoint from heimdall + checkpoint, err := bor.HeimdallClient.FetchCheckpoint(ctx, i) if err != nil { log.Debug("Failed to fetch latest checkpoint for whitelisting", "err", err) return blockNums, blockHashes, errCheckpoint } - // check if we have the checkpoint blocks - head := h.ethAPI.BlockNumber() - if head < hexutil.Uint64(checkpoint.EndBlock.Uint64()) { - log.Debug("Head block behind checkpoint block", "head", head, "checkpoint end block", checkpoint.EndBlock) - return blockNums, blockHashes, errMissingCheckpoint - } + // Verify if the checkpoint fetched can be added to the local whitelist entry or not + // If verified, it returns the hash of the end block of the checkpoint. If not, + // it will return appropriate error. - // verify the root hash of checkpoint - roothash, err := h.ethAPI.GetRootHash(ctx, checkpoint.StartBlock.Uint64(), checkpoint.EndBlock.Uint64()) + hash, err := checkpointVerifier.verify(ctx, h, checkpoint) if err != nil { - log.Debug("Failed to get root hash of checkpoint while whitelisting", "err", err) - return blockNums, blockHashes, errRootHash + return blockNums, blockHashes, err } - if roothash != checkpoint.RootHash.String()[2:] { - log.Warn("Checkpoint root hash mismatch while whitelisting", "expected", checkpoint.RootHash.String()[2:], "got", roothash) - return blockNums, blockHashes, errCheckpointRootHashMismatch - } - - // fetch the end checkpoint block hash - block, err := h.ethAPI.GetBlockByNumber(ctx, rpc.BlockNumber(checkpoint.EndBlock.Uint64()), false) - if err != nil { - log.Debug("Failed to get end block hash of checkpoint while whitelisting", "err", err) - return blockNums, blockHashes, errEndBlock - } - - hash := fmt.Sprintf("%v", block["hash"]) - blockNums = append(blockNums, checkpoint.EndBlock.Uint64()) blockHashes = append(blockHashes, common.HexToHash(hash)) - count-- } return blockNums, blockHashes, nil diff --git a/eth/handler_bor_test.go b/eth/handler_bor_test.go new file mode 100644 index 0000000000..857db70e95 --- /dev/null +++ b/eth/handler_bor_test.go @@ -0,0 +1,134 @@ +package eth + +import ( + "context" + "math/big" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/consensus/bor" + "github.com/ethereum/go-ethereum/consensus/bor/clerk" + "github.com/ethereum/go-ethereum/consensus/bor/heimdall/checkpoint" + "github.com/ethereum/go-ethereum/consensus/bor/heimdall/span" +) + +type mockHeimdall struct { + fetchCheckpoint func(ctx context.Context, number int64) (*checkpoint.Checkpoint, error) + fetchCheckpointCount func(ctx context.Context) (int64, error) +} + +func (m *mockHeimdall) StateSyncEvents(ctx context.Context, fromID uint64, to int64) ([]*clerk.EventRecordWithTime, error) { + return nil, nil +} +func (m *mockHeimdall) Span(ctx context.Context, spanID uint64) (*span.HeimdallSpan, error) { + //nolint:nilnil + return nil, nil +} +func (m *mockHeimdall) FetchCheckpoint(ctx context.Context, number int64) (*checkpoint.Checkpoint, error) { + return m.fetchCheckpoint(ctx, number) +} +func (m *mockHeimdall) FetchCheckpointCount(ctx context.Context) (int64, error) { + return m.fetchCheckpointCount(ctx) +} +func (m *mockHeimdall) Close() {} + +func TestFetchWhitelistCheckpoints(t *testing.T) { + t.Parallel() + + // create an empty ethHandler + handler := ðHandler{} + + // create a mock checkpoint verification function and use it to create a verifier + verify := func(ctx context.Context, handler *ethHandler, checkpoint *checkpoint.Checkpoint) (string, error) { + return "", nil + } + + verifier := newCheckpointVerifier(verify) + + // Create a mock heimdall instance and use it for creating a bor instance + var heimdall mockHeimdall + + bor := &bor.Bor{HeimdallClient: &heimdall} + + // create 20 mock checkpoints + checkpoints := createMockCheckpoints(20) + + // create a mock fetch checkpoint function + heimdall.fetchCheckpoint = func(_ context.Context, number int64) (*checkpoint.Checkpoint, error) { + return checkpoints[number-1], nil // we're sure that number won't exceed 20 + } + + // create a background context + ctx := context.Background() + + testCases := []struct { + name string + first bool + count int64 + length int + start uint64 + end uint64 + fetchErr error + expectedErr error + }{ + {"fail to fetch checkpoint count", false, 0, 0, 0, 0, errCheckpointCount, errCheckpointCount}, + {"no checkpoints available", false, 0, 0, 0, 0, nil, errNoCheckpoint}, + {"fetch multiple checkpoints (count < 10)", true, 6, 6, 0, 6, nil, nil}, + {"fetch multiple checkpoints (count = 10)", true, 10, 10, 0, 10, nil, nil}, + {"fetch multiple checkpoints (count > 10)", true, 16, 10, 6, 16, nil, nil}, + {"fetch single checkpoint", false, 18, 1, 17, 18, nil, nil}, + } + + for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + heimdall.fetchCheckpointCount = getMockFetchCheckpointFn(tc.count, tc.fetchErr) + blockNums, blockHashes, err := handler.fetchWhitelistCheckpoints(ctx, bor, verifier, tc.first) + + // Check if we have expected result + require.Equal(t, tc.expectedErr, err) + require.Equal(t, tc.length, len(blockNums)) + require.Equal(t, tc.length, len(blockHashes)) + validateBlockNumber(t, blockNums, checkpoints[tc.start:tc.end]) + }) + } +} + +func validateBlockNumber(t *testing.T, blockNums []uint64, checkpoints []*checkpoint.Checkpoint) { + t.Helper() + + for i, blockNum := range blockNums { + require.Equal(t, blockNum, checkpoints[i].EndBlock.Uint64(), "expect block number in array to match with checkpoint") + } +} + +func getMockFetchCheckpointFn(number int64, err error) func(ctx context.Context) (int64, error) { + return func(_ context.Context) (int64, error) { + return number, err + } +} + +func createMockCheckpoints(count int) []*checkpoint.Checkpoint { + var ( + checkpoints []*checkpoint.Checkpoint = make([]*checkpoint.Checkpoint, count) + startBlock int64 = 257 // any number can be used + ) + + for i := 0; i < count; i++ { + checkpoints[i] = &checkpoint.Checkpoint{ + Proposer: common.Address{}, + StartBlock: big.NewInt(startBlock), + EndBlock: big.NewInt(startBlock + 255), + RootHash: common.Hash{}, + BorChainID: "137", + Timestamp: uint64(time.Now().Unix()), + } + startBlock += 256 + } + + return checkpoints +}