From 916f88a3731490d1deed848d5a43583a4c6dcea0 Mon Sep 17 00:00:00 2001 From: andri lim Date: Wed, 17 Jul 2024 17:05:53 +0700 Subject: [PATCH] Use block number or timestamp to determine fork rules (#2496) * Use block number or timestamp to determine fork rules Avoid confusion raised by `forkGTE` usage where block informations are present. * Get rid of forkGTE --- nimbus/beacon/api_handler/api_newpayload.nim | 11 +++-- nimbus/common/common.nim | 46 ++++++-------------- nimbus/common/hardforks.nim | 34 --------------- nimbus/core/eip4844.nim | 2 +- nimbus/core/executor/calculate_reward.nim | 32 +++++++++++++- nimbus/core/executor/executor_helpers.nim | 2 +- nimbus/core/tx_pool.nim | 6 +-- nimbus/core/tx_pool/tx_chain.nim | 4 +- nimbus/core/tx_pool/tx_tasks/tx_packer.nim | 2 +- nimbus/core/withdrawals.nim | 2 +- nimbus/evm/state.nim | 10 ++--- nimbus/sync/handlers/eth.nim | 44 ++----------------- tools/common/helpers.nim | 2 + tools/t8n/transition.nim | 7 +-- 14 files changed, 77 insertions(+), 127 deletions(-) diff --git a/nimbus/beacon/api_handler/api_newpayload.nim b/nimbus/beacon/api_handler/api_newpayload.nim index 1b7f18fcc..408d2ea3a 100644 --- a/nimbus/beacon/api_handler/api_newpayload.nim +++ b/nimbus/beacon/api_handler/api_newpayload.nim @@ -159,11 +159,16 @@ proc newPayload*(ben: BeaconEngineRef, let ttd = com.ttd.get(high(UInt256)) if version == Version.V1: - let td = db.getScore(header.parentHash).valueOr: + let ptd = db.getScore(header.parentHash).valueOr: 0.u256 - if (not com.forkGTE(MergeFork)) and td < ttd: + let gptd = db.getScore(parent.parentHash) + if ptd < ttd: warn "Ignoring pre-merge payload", - number = header.number, hash = blockHash, td, ttd + number = header.number, hash = blockHash.short, ptd, ttd + return invalidStatus() + if parent.difficulty > 0.u256 and gptd.isSome and gptd.value >= ttd: + warn "Ignoring pre-merge parent block", + number = header.number, hash = blockHash.short, ptd, ttd return invalidStatus() if header.timestamp <= parent.timestamp: diff --git a/nimbus/common/common.nim b/nimbus/common/common.nim index 8e5594870..ba7edf442 100644 --- a/nimbus/common/common.nim +++ b/nimbus/common/common.nim @@ -62,9 +62,6 @@ type # synchronizer need this syncProgress: SyncProgress - # current hard fork, updated after calling `hardForkTransition` - currentFork: HardFork - # one of POW/POS, updated after calling `hardForkTransition` consensusType: ConsensusType @@ -140,7 +137,7 @@ proc init(com : CommonRef, com.pruneHistory= pruneHistory com.pos = CasperRef.new - # com.currentFork and com.consensusType + # com.consensusType # is set by hardForkTransition. # set it before creating genesis block # TD need to be some(0.u256) because it can be the genesis @@ -150,16 +147,20 @@ proc init(com : CommonRef, # com.forkIdCalculator and com.genesisHash are set # by setForkId if genesis.isNil.not: - com.hardForkTransition(ForkDeterminationInfo( - number: 0.BlockNumber, - td: Opt.some(0.u256), - time: Opt.some(genesis.timestamp) - )) + let + forkDeterminer = ForkDeterminationInfo( + number: 0.BlockNumber, + td: Opt.some(0.u256), + time: Opt.some(genesis.timestamp) + ) + fork = toHardFork(com.forkTransitionTable, forkDeterminer) + + com.consensusTransition(fork) # Must not overwrite the global state on the single state DB if not db.getBlockHeader(0.BlockNumber, com.genesisHeader): com.genesisHeader = toGenesisHeader(genesis, - com.currentFork, com.db) + fork, com.db) com.setForkId(com.genesisHeader) com.pos.timestamp = genesis.timestamp @@ -245,7 +246,6 @@ func clone*(com: CommonRef, db: CoreDbRef): CommonRef = genesisHeader: com.genesisHeader, syncProgress : com.syncProgress, networkId : com.networkId, - currentFork : com.currentFork, consensusType: com.consensusType, pos : com.pos, pruneHistory : com.pruneHistory) @@ -270,7 +270,6 @@ func hardForkTransition( ## Same thing happen before London block, TD can be ignored. let fork = com.toHardFork(forkDeterminer) - com.currentFork = fork com.consensusTransition(fork) func hardForkTransition*( @@ -299,24 +298,16 @@ func toEVMFork*(com: CommonRef, forkDeterminer: ForkDeterminationInfo): EVMFork let fork = com.toHardFork(forkDeterminer) ToEVMFork[fork] -func toEVMFork*(com: CommonRef): EVMFork = - ToEVMFork[com.currentFork] - func isSpuriousOrLater*(com: CommonRef, number: BlockNumber): bool = com.toHardFork(number.forkDeterminationInfo) >= Spurious +func isByzantiumOrLater*(com: CommonRef, number: BlockNumber): bool = + com.toHardFork(number.forkDeterminationInfo) >= Byzantium + func isLondonOrLater*(com: CommonRef, number: BlockNumber): bool = # TODO: Fixme, use only London comparator com.toHardFork(number.forkDeterminationInfo) >= London -func forkGTE*(com: CommonRef, fork: HardFork): bool = - com.currentFork >= fork - -# TODO: move this consensus code to where it belongs -func minerAddress*(com: CommonRef; header: BlockHeader): EthAddress = - # POW and POS return header.coinbase - return header.coinbase - func forkId*(com: CommonRef, head, time: uint64): ForkID {.gcsafe.} = ## EIP 2364/2124 com.forkIdCalculator.newID(head, time) @@ -432,9 +423,6 @@ func chainId*(com: CommonRef): ChainId = func networkId*(com: CommonRef): NetworkId = com.networkId -func blockReward*(com: CommonRef): UInt256 = - BlockRewards[com.currentFork] - func genesisHash*(com: CommonRef): Hash256 = ## Getter com.genesisHash @@ -475,12 +463,6 @@ func setTTD*(com: CommonRef, ttd: Opt[DifficultyInt]) = # rebuild the MergeFork piece of the forkTransitionTable com.forkTransitionTable.mergeForkTransitionThreshold = com.config.mergeForkTransitionThreshold -func setFork*(com: CommonRef, fork: HardFork): HardFork = - ## useful for testing - result = com.currentFork - com.currentFork = fork - com.consensusTransition(fork) - func `syncReqNewHead=`*(com: CommonRef; cb: SyncReqNewHeadCB) = ## Activate or reset a call back handler for syncing. com.syncReqNewHead = cb diff --git a/nimbus/common/hardforks.nim b/nimbus/common/hardforks.nim index 126dda6a7..3b2f03bc2 100644 --- a/nimbus/common/hardforks.nim +++ b/nimbus/common/hardforks.nim @@ -318,40 +318,6 @@ const FkPrague, # Prague ] -# ------------------------------------------------------------------------------ -# Block reward helpers -# ------------------------------------------------------------------------------ - -func eth(n: int): UInt256 {.compileTime.} = - n.u256 * pow(10.u256, 18) - -const - eth5 = 5.eth - eth3 = 3.eth - eth2 = 2.eth - eth0 = 0.u256 - - BlockRewards*: array[HardFork, UInt256] = [ - eth5, # Frontier - eth5, # Homestead - eth5, # DAOFork - eth5, # Tangerine - eth5, # Spurious - eth3, # Byzantium - eth2, # Constantinople - eth2, # Petersburg - eth2, # Istanbul - eth2, # MuirGlacier - eth2, # Berlin - eth2, # London - eth2, # ArrowGlacier - eth2, # GrayGlacier - eth0, # MergeFork - eth0, # Shanghai - eth0, # Cancun - eth0, # Prague - ] - # ------------------------------------------------------------------------------ # Fork ID helpers # ------------------------------------------------------------------------------ diff --git a/nimbus/core/eip4844.nim b/nimbus/core/eip4844.nim index b7a0c64bd..9d00b9431 100644 --- a/nimbus/core/eip4844.nim +++ b/nimbus/core/eip4844.nim @@ -134,7 +134,7 @@ func validateEip4844Header*( com: CommonRef, header, parentHeader: BlockHeader, txs: openArray[Transaction]): Result[void, string] {.raises: [].} = - if not com.forkGTE(Cancun): + if not com.isCancunOrLater(header.timestamp): if header.blobGasUsed.isSome: return err("unexpected EIP-4844 blobGasUsed in block header") diff --git a/nimbus/core/executor/calculate_reward.nim b/nimbus/core/executor/calculate_reward.nim index f70d9a2bf..950289ce2 100644 --- a/nimbus/core/executor/calculate_reward.nim +++ b/nimbus/core/executor/calculate_reward.nim @@ -16,9 +16,39 @@ import {.push raises: [].} +# ------------------------------------------------------------------------------ +# Block reward helpers +# ------------------------------------------------------------------------------ + +func eth(n: int): UInt256 {.compileTime.} = + n.u256 * pow(10.u256, 18) + +const + eth5 = 5.eth + eth3 = 3.eth + eth2 = 2.eth + eth0 = 0.u256 + + BlockRewards: array[EVMFork, UInt256] = [ + eth5, # Frontier + eth5, # Homestead + eth5, # Tangerine + eth5, # Spurious + eth3, # Byzantium + eth2, # Constantinople + eth2, # Petersburg + eth2, # Istanbul + eth2, # Berlin + eth2, # London + eth0, # Paris + eth0, # Shanghai + eth0, # Cancun + eth0, # Prague + ] + proc calculateReward*(vmState: BaseVMState; account: EthAddress; number: BlockNumber; uncles: openArray[BlockHeader]) = - let blockReward = vmState.com.blockReward() + let blockReward = BlockRewards[vmState.fork] var mainReward = blockReward for uncle in uncles: diff --git a/nimbus/core/executor/executor_helpers.nim b/nimbus/core/executor/executor_helpers.nim index fadf8af74..1f4aac0ae 100644 --- a/nimbus/core/executor/executor_helpers.nim +++ b/nimbus/core/executor/executor_helpers.nim @@ -49,7 +49,7 @@ func createBloom*(receipts: openArray[Receipt]): Bloom = proc makeReceipt*(vmState: BaseVMState; txType: TxType): Receipt = var rec: Receipt - if vmState.com.forkGTE(Byzantium): + if vmState.com.isByzantiumOrLater(vmState.blockNumber): rec.isHash = false rec.status = vmState.status else: diff --git a/nimbus/core/tx_pool.nim b/nimbus/core/tx_pool.nim index 6d41a4ad5..05b2f20fc 100644 --- a/nimbus/core/tx_pool.nim +++ b/nimbus/core/tx_pool.nim @@ -648,13 +648,13 @@ proc assembleBlock*( blobsBundle.blobs.add blob let com = xp.chain.com - if com.forkGTE(Shanghai): + if com.isShanghaiOrLater(blk.header.timestamp): blk.withdrawals = Opt.some(com.pos.withdrawals) - if not com.forkGTE(Cancun) and blobsBundle.commitments.len > 0: + if not com.isCancunOrLater(blk.header.timestamp) and blobsBundle.commitments.len > 0: return err("PooledTransaction contains blobs prior to Cancun") let blobsBundleOpt = - if com.forkGTE(Cancun): + if com.isCancunOrLater(blk.header.timestamp): doAssert blobsBundle.commitments.len == blobsBundle.blobs.len doAssert blobsBundle.proofs.len == blobsBundle.blobs.len Opt.some blobsBundle diff --git a/nimbus/core/tx_pool/tx_chain.nim b/nimbus/core/tx_pool/tx_chain.nim index 299a84a90..2e8de49f9 100644 --- a/nimbus/core/tx_pool/tx_chain.nim +++ b/nimbus/core/tx_pool/tx_chain.nim @@ -194,10 +194,10 @@ proc getHeader*(dh: TxChainRef): BlockHeader blobGasUsed: dh.txEnv.blobGasUsed, excessBlobGas: dh.txEnv.excessBlobGas) - if dh.com.forkGTE(Shanghai): + if dh.com.isShanghaiOrLater(result.timestamp): result.withdrawalsRoot = Opt.some(calcWithdrawalsRoot(dh.com.pos.withdrawals)) - if dh.com.forkGTE(Cancun): + if dh.com.isCancunOrLater(result.timestamp): result.parentBeaconBlockRoot = Opt.some(dh.com.pos.parentBeaconBlockRoot) dh.prepareForSeal(result) diff --git a/nimbus/core/tx_pool/tx_tasks/tx_packer.nim b/nimbus/core/tx_pool/tx_tasks/tx_packer.nim index f3188488e..c736e21f8 100644 --- a/nimbus/core/tx_pool/tx_tasks/tx_packer.nim +++ b/nimbus/core/tx_pool/tx_tasks/tx_packer.nim @@ -253,7 +253,7 @@ proc vmExecCommit(pst: TxPackerStateRef) raiseAssert "vmExecCommit(): state() failed " & $$error xp.chain.stateRoot = vmState.stateDB.rootHash - if vmState.com.forkGTE(Cancun): + if xp.chain.nextFork >= FkCancun: # EIP-4844 xp.chain.excessBlobGas = Opt.some(vmState.blockCtx.excessBlobGas) xp.chain.blobGasUsed = Opt.some(vmState.blobGasUsed) diff --git a/nimbus/core/withdrawals.nim b/nimbus/core/withdrawals.nim index e0553351c..c4a100e8c 100644 --- a/nimbus/core/withdrawals.nim +++ b/nimbus/core/withdrawals.nim @@ -16,7 +16,7 @@ import results, ../common/common proc validateWithdrawals*( com: CommonRef, header: BlockHeader, withdrawals: Opt[seq[Withdrawal]] ): Result[void, string] = - if com.forkGTE(Shanghai): + if com.isShanghaiOrLater(header.timestamp): if header.withdrawalsRoot.isNone: return err("Post-Shanghai block header must have withdrawalsRoot") elif withdrawals.isNone: diff --git a/nimbus/evm/state.nim b/nimbus/evm/state.nim index ef52b493f..4d0e0a128 100644 --- a/nimbus/evm/state.nim +++ b/nimbus/evm/state.nim @@ -54,7 +54,7 @@ func blockCtx(com: CommonRef, header: BlockHeader): baseFeePerGas: header.baseFeePerGas, prevRandao : header.prevRandao, difficulty : header.difficulty, - coinbase : com.minerAddress(header), + coinbase : header.coinbase, excessBlobGas: header.excessBlobGas.get(0'u64), ) @@ -225,22 +225,22 @@ proc init*( tracer = tracer) return true -proc coinbase*(vmState: BaseVMState): EthAddress = +func coinbase*(vmState: BaseVMState): EthAddress = vmState.blockCtx.coinbase -proc blockNumber*(vmState: BaseVMState): BlockNumber = +func blockNumber*(vmState: BaseVMState): BlockNumber = # it should return current block number # and not head.number vmState.parent.number + 1 -proc difficultyOrPrevRandao*(vmState: BaseVMState): UInt256 = +func difficultyOrPrevRandao*(vmState: BaseVMState): UInt256 = if vmState.com.consensus == ConsensusType.POS: # EIP-4399/EIP-3675 UInt256.fromBytesBE(vmState.blockCtx.prevRandao.data) else: vmState.blockCtx.difficulty -proc baseFeePerGas*(vmState: BaseVMState): UInt256 = +func baseFeePerGas*(vmState: BaseVMState): UInt256 = vmState.blockCtx.baseFeePerGas.get(0.u256) method getAncestorHash*( diff --git a/nimbus/sync/handlers/eth.nim b/nimbus/sync/handlers/eth.nim index 6e3f29c86..a308cfd68 100644 --- a/nimbus/sync/handlers/eth.nim +++ b/nimbus/sync/handlers/eth.nim @@ -566,52 +566,16 @@ method handleNewBlock*(ctx: EthWireRef, totalDifficulty: DifficultyInt): Result[void, string] {.gcsafe.} = - if ctx.enableTxPool != Enabled: - when trMissingOrDisabledGossipOk: - notEnabled("handleNewBlock") - return ok() - try: - if ctx.chain.com.forkGTE(MergeFork): - debug "Dropping peer for sending NewBlock after merge (EIP-3675)", - peer, blockNumber=blk.header.number, - blockHash=blk.header.blockHash, totalDifficulty - asyncSpawn banPeer(ctx.peerPool, peer, PEER_LONG_BANTIME) - return ok() - - if not ctx.newBlockHandler.handler.isNil: - ctx.newBlockHandler.handler( - ctx.newBlockHandler.arg, - peer, blk, totalDifficulty - ) - return ok() - except CatchableError as exc: - return err(exc.msg) + # We dropped support for non-merge networks + err("block broadcasts disallowed") method handleNewBlockHashes*(ctx: EthWireRef, peer: Peer, hashes: openArray[NewBlockHashesAnnounce]): Result[void, string] {.gcsafe.} = - if ctx.enableTxPool != Enabled: - when trMissingOrDisabledGossipOk: - notEnabled("handleNewBlockHashes") - return ok() - try: - if ctx.chain.com.forkGTE(MergeFork): - debug "Dropping peer for sending NewBlockHashes after merge (EIP-3675)", - peer, numHashes=hashes.len - asyncSpawn banPeer(ctx.peerPool, peer, PEER_LONG_BANTIME) - return ok() - - if not ctx.newBlockHashesHandler.handler.isNil: - ctx.newBlockHashesHandler.handler( - ctx.newBlockHashesHandler.arg, - peer, - hashes - ) - return ok() - except CatchableError as exc: - return err(exc.msg) + # We dropped support for non-merge networks + err("block announcements disallowed") # ------------------------------------------------------------------------------ # End diff --git a/tools/common/helpers.nim b/tools/common/helpers.nim index f7149848c..91d3dc55f 100644 --- a/tools/common/helpers.nim +++ b/tools/common/helpers.nim @@ -59,6 +59,7 @@ proc assignTime(c: ChainConfig, transitionFork: HardFork, t: EthTime) = let table = createForkTransitionTable(transitionFork, Opt.none(BlockNumber), Opt.some(t), Opt.none(DifficultyInt)) c.populateFromForkTransitionTable(table) + c.terminalTotalDifficulty = Opt.some(0.u256) func getChainConfig*(network: string, c: ChainConfig) = c.daoForkSupport = false @@ -110,6 +111,7 @@ func getChainConfig*(network: string, c: ChainConfig) = c.assignNumber(HardFork.GrayGlacier, BlockNumberZero) of $TestFork.Merge, $TestFork.Paris: c.assignNumber(HardFork.MergeFork, BlockNumberZero) + c.terminalTotalDifficulty = Opt.some(0.u256) of $TestFork.ArrowGlacierToParisAtDiffC0000: c.assignNumber(HardFork.GrayGlacier, BlockNumberZero) c.terminalTotalDifficulty = Opt.some(0xC0000.u256) diff --git a/tools/t8n/transition.nim b/tools/t8n/transition.nim index 32c5b4fa3..8bcd168d7 100644 --- a/tools/t8n/transition.nim +++ b/tools/t8n/transition.nim @@ -299,7 +299,6 @@ proc exec(ctx: var TransContext, vmState.stateDB.addBalance(withdrawal.address, withdrawal.weiAmount) let miner = ctx.env.currentCoinbase - let fork = vmState.com.toEVMFork coinbaseStateClearing(vmState, miner, stateReward.isSome()) let stateDB = vmState.stateDB @@ -320,7 +319,7 @@ proc exec(ctx: var TransContext, withdrawalsRoot : header.withdrawalsRoot ) - if fork >= FkCancun: + if vmState.com.isCancunOrLater(ctx.env.currentTimestamp): result.result.blobGasUsed = Opt.some vmState.blobGasUsed if ctx.env.currentExcessBlobGas.isSome: result.result.currentExcessBlobGas = ctx.env.currentExcessBlobGas @@ -459,7 +458,9 @@ proc transitionAction*(ctx: var TransContext, conf: T8NConf) = # un-set it if it has been set too early ctx.env.parentBeaconBlockRoot = Opt.none(Hash256) - if com.forkGTE(MergeFork): + let isMerged = config.terminalTotalDifficulty.isSome and + config.terminalTotalDifficulty.value == 0.u256 + if isMerged: if ctx.env.currentRandom.isNone: raise newError(ErrorConfig, "post-merge requires currentRandom to be defined in env")