Skip to content

Commit

Permalink
Fix engine_forkchoiceUpdated bug (#2274)
Browse files Browse the repository at this point in the history
* fCU Ignore update to old head

* Only enable terminal PoW block conditions for fCUV1

* Typo fix: TDD -> TTD

* Add link to Shanghai fCUV2 specification

* Add link to Paris fCUV1 specification
  • Loading branch information
jangko committed Jun 1, 2024
1 parent 0f0ac0b commit 1565c57
Showing 1 changed file with 34 additions and 27 deletions.
61 changes: 34 additions & 27 deletions nimbus/beacon/api_handler/api_forkchoice.nim
Original file line number Diff line number Diff line change
Expand Up @@ -102,38 +102,45 @@ proc forkchoiceUpdated*(ben: BeaconEngineRef,

# Block is known locally, just sanity check that the beacon client does not
# attempt to push us back to before the merge.
let blockNumber = header.blockNumber.truncate(uint64)
if header.difficulty > 0.u256 or blockNumber == 0'u64:
var
td, ptd: DifficultyInt
ttd = com.ttd.get(high(common.BlockNumber))

if not db.getTd(blockHash, td) or (blockNumber > 0'u64 and not db.getTd(header.parentHash, ptd)):
error "TDs unavailable for TTD check",
number = blockNumber,
hash = blockHash.short,
td = td,
parent = header.parentHash.short,
ptd = ptd
return simpleFCU(PayloadExecutionStatus.invalid, "TDs unavailable for TDD check")

if td < ttd or (blockNumber > 0'u64 and ptd > ttd):
notice "Refusing beacon update to pre-merge",
number = blockNumber,
hash = blockHash.short,
diff = header.difficulty,
ptd = ptd,
ttd = ttd

return invalidFCU("Refusing beacon update to pre-merge")
#
# Disable terminal PoW block conditions validation for fCUV2 and later.
# https://github.com/ethereum/execution-apis/blob/v1.0.0-beta.4/src/engine/shanghai.md#specification-1
if apiVersion == Version.V1:
let blockNumber = header.blockNumber.truncate(uint64)
if header.difficulty > 0.u256 or blockNumber == 0'u64:
var
td, ptd: DifficultyInt
ttd = com.ttd.get(high(common.BlockNumber))

if not db.getTd(blockHash, td) or (blockNumber > 0'u64 and not db.getTd(header.parentHash, ptd)):
error "TDs unavailable for TTD check",
number = blockNumber,
hash = blockHash.short,
td = td,
parent = header.parentHash.short,
ptd = ptd
return simpleFCU(PayloadExecutionStatus.invalid, "TDs unavailable for TTD check")

if td < ttd or (blockNumber > 0'u64 and ptd > ttd):
notice "Refusing beacon update to pre-merge",
number = blockNumber,
hash = blockHash.short,
diff = header.difficulty,
ptd = ptd,
ttd = ttd

return invalidFCU("Refusing beacon update to pre-merge")

# If the head block is already in our canonical chain, the beacon client is
# probably resyncing. Ignore the update.
# See point 2 of fCUV1 specification
# https://github.com/ethereum/execution-apis/blob/v1.0.0-beta.4/src/engine/paris.md#specification-1
var canonHash: common.Hash256
if db.getBlockHash(header.blockNumber, canonHash) and canonHash == blockHash:
# TODO should this be possible?
# If we allow these types of reorgs, we will do lots and lots of reorgs during sync
notice "Reorg to previous block", blockHash
notice "Ignoring beacon update to old head",
blockHash=blockHash.short,
blockNumber=header.blockNumber
return validFCU(none(PayloadID), blockHash)

chain.setCanonical(header).isOkOr:
return invalidFCU(error, com, header)
Expand Down

0 comments on commit 1565c57

Please sign in to comment.