Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bump nim-eth, extend empty block fallback for Capella #4357

Merged
merged 2 commits into from
Nov 28, 2022
Merged

bump nim-eth, extend empty block fallback for Capella #4357

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6744cd7
bump `nim-eth`, extend empty block fallback for Capella
etan-status Nov 24, 2022
341f6bb
RLP dependency
etan-status Nov 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions AllTests-mainnet.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -447,11 +447,12 @@ OK: 12/12 Fail: 0/12 Skip: 0/12
OK: 1/1 Fail: 0/1 Skip: 0/1
## Spec helpers
```diff
+ build_empty_execution_payload OK
+ build_empty_execution_payload - Bellatrix OK
+ build_empty_execution_payload - Capella OK
+ build_proof - BeaconState OK
+ integer_squareroot OK
```
OK: 3/3 Fail: 0/3 Skip: 0/3
OK: 4/4 Fail: 0/4 Skip: 0/4
## Specific field types
```diff
+ root update OK
Expand Down Expand Up @@ -613,4 +614,4 @@ OK: 2/2 Fail: 0/2 Skip: 0/2
OK: 9/9 Fail: 0/9 Skip: 0/9

---TOTAL---
OK: 338/343 Fail: 0/343 Skip: 5/343
OK: 339/344 Fail: 0/344 Skip: 5/344
115 changes: 94 additions & 21 deletions beacon_chain/spec/helpers.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import
stew/[bitops2, byteutils, endians2, objects, saturation_arith],
chronicles,
eth/eip1559, eth/common/[eth_types, eth_types_rlp],
eth/rlp, eth/trie/[db, hexary],
# Internal
./datatypes/[phase0, altair, bellatrix, capella],
"."/[eth2_merkleization, forks, ssz_codec]
Expand All @@ -29,6 +30,7 @@ export
forks, eth2_merkleization, ssz_codec

type
ExecutionWithdrawal = eth_types.Withdrawal
ExecutionBlockHeader = eth_types.BlockHeader

FinalityCheckpoints* = object
Expand Down Expand Up @@ -360,32 +362,101 @@ func compute_timestamp_at_slot*(state: ForkyBeaconState, slot: Slot): uint64 =
let slots_since_genesis = slot - GENESIS_SLOT
state.genesis_time + slots_since_genesis * SECONDS_PER_SLOT

func gweiToWei*(gwei: Gwei): UInt256 =
gwei.u256 * 1_000_000_000.u256

func toExecutionWithdrawal*(
withdrawal: capella.Withdrawal): ExecutionWithdrawal =
ExecutionWithdrawal(
index: withdrawal.index,
validatorIndex: withdrawal.validatorIndex,
address: EthAddress withdrawal.address.data,
amount: gweiToWei withdrawal.amount)

# https://eips.ethereum.org/EIPS/eip-4895
proc computeWithdrawalsTrieRoot*(
payload: capella.ExecutionPayload): Hash256 =
if payload.withdrawals.len == 0:
return EMPTY_ROOT_HASH

var tr = initHexaryTrie(newMemoryDB())
for i, withdrawal in payload.withdrawals:
try:
tr.put(rlp.encode(i), rlp.encode(toExecutionWithdrawal(withdrawal)))
except RlpError as exc:
doAssert false, "HexaryTrie.put failed: " & $exc.msg
tr.rootHash()

proc emptyPayloadToBlockHeader*(
payload: bellatrix.ExecutionPayload): ExecutionBlockHeader =
payload: bellatrix.ExecutionPayload | capella.ExecutionPayload
): ExecutionBlockHeader =
static: # `GasInt` is signed. We only use it for hashing.
doAssert sizeof(GasInt) == sizeof(payload.gas_limit)
doAssert sizeof(GasInt) == sizeof(payload.gas_used)

## This function assumes that the payload is empty!
doAssert payload.transactions.len == 0

let
txRoot = EMPTY_ROOT_HASH
withdrawalsRoot =
when payload is bellatrix.ExecutionPayload:
none(Hash256)
else:
some payload.computeWithdrawalsTrieRoot()

ExecutionBlockHeader(
parentHash : payload.parent_hash,
ommersHash : EMPTY_UNCLE_HASH,
coinbase : EthAddress payload.fee_recipient.data,
stateRoot : payload.state_root,
txRoot : EMPTY_ROOT_HASH,
receiptRoot : payload.receipts_root,
bloom : payload.logs_bloom.data,
difficulty : default(DifficultyInt),
blockNumber : payload.block_number.u256,
gasLimit : GasInt payload.gas_limit,
gasUsed : GasInt payload.gas_used,
timestamp : fromUnix(int64.saturate payload.timestamp),
extraData : payload.extra_data.asSeq,
mixDigest : payload.prev_randao, # EIP-4399 redefine `mixDigest` -> `prevRandao`
nonce : default(BlockNonce),
fee : some payload.base_fee_per_gas
)
parentHash : payload.parent_hash,
ommersHash : EMPTY_UNCLE_HASH,
coinbase : EthAddress payload.fee_recipient.data,
stateRoot : payload.state_root,
txRoot : txRoot,
receiptRoot : payload.receipts_root,
bloom : payload.logs_bloom.data,
difficulty : default(DifficultyInt),
blockNumber : payload.block_number.u256,
gasLimit : cast[GasInt](payload.gas_limit),
gasUsed : cast[GasInt](payload.gas_used),
timestamp : fromUnix(int64.saturate payload.timestamp),
extraData : payload.extra_data.asSeq,
mixDigest : payload.prev_randao, # EIP-4399 `mixDigest` -> `prevRandao`
nonce : default(BlockNonce),
fee : some payload.base_fee_per_gas,
withdrawalsRoot: withdrawalsRoot)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it work to unconditionally include this, regardless of fork or engine API version?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the RLP serializer checks whether the field is a some or a none, and emits the correct block hash.

I tested the logic using the test script here:
status-im/nim-eth#562 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For Capella blocks you need engine API v2 calls. For pre-Capella blocks, you may also use v1 but v2 also works.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fortunately, once ELs support Capella in any production capacity, then Nimbus can start getting rid of the v1 calls then. Presumably gating on Gnosis capella fork schedule, but fundamentally see no reason to keep using v1 calls, ever, once the ELs support v2 calls.


func build_empty_execution_payload*(
state: bellatrix.BeaconState,
feeRecipient: Eth1Address): bellatrix.ExecutionPayload =
## Assuming a pre-state of the same slot, build a valid ExecutionPayload
## without any transactions.
let
latest = state.latest_execution_payload_header
timestamp = compute_timestamp_at_slot(state, state.slot)
randao_mix = get_randao_mix(state, get_current_epoch(state))
base_fee = calcEip1599BaseFee(GasInt.saturate latest.gas_limit,
GasInt.saturate latest.gas_used,
latest.base_fee_per_gas)

var payload = bellatrix.ExecutionPayload(
parent_hash: latest.block_hash,
fee_recipient: bellatrix.ExecutionAddress(data: distinctBase(feeRecipient)),
state_root: latest.state_root, # no changes to the state
receipts_root: EMPTY_ROOT_HASH,
block_number: latest.block_number + 1,
prev_randao: randao_mix,
gas_limit: latest.gas_limit, # retain same limit
gas_used: 0, # empty block, 0 gas
timestamp: timestamp,
base_fee_per_gas: base_fee)

payload.block_hash = rlpHash emptyPayloadToBlockHeader(payload)

payload

func build_empty_execution_payload*[BS, EP](
state: BS, feeRecipient: Eth1Address): EP =
proc build_empty_execution_payload*(
state: capella.BeaconState,
feeRecipient: Eth1Address,
expectedWithdrawals: seq[capella.Withdrawal]): capella.ExecutionPayload =
## Assuming a pre-state of the same slot, build a valid ExecutionPayload
## without any transactions.
let
Expand All @@ -396,7 +467,7 @@ func build_empty_execution_payload*[BS, EP](
GasInt.saturate latest.gas_used,
latest.base_fee_per_gas)

var payload = EP(
var payload = capella.ExecutionPayload(
parent_hash: latest.block_hash,
fee_recipient: bellatrix.ExecutionAddress(data: distinctBase(feeRecipient)),
state_root: latest.state_root, # no changes to the state
Expand All @@ -407,6 +478,8 @@ func build_empty_execution_payload*[BS, EP](
gas_used: 0, # empty block, 0 gas
timestamp: timestamp,
base_fee_per_gas: base_fee)
for withdrawal in expectedWithdrawals:
doAssert payload.withdrawals.add withdrawal

payload.block_hash = rlpHash emptyPayloadToBlockHeader(payload)

Expand Down
7 changes: 4 additions & 3 deletions beacon_chain/validators/validator_duties.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,9 +388,10 @@ proc getExecutionPayload[T](

template empty_execution_payload(): auto =
withState(proposalState[]):
when stateFork >= BeaconStateFork.Bellatrix:
build_empty_execution_payload[typeof forkyState.data, T](
forkyState.data, feeRecipient)
when stateFork >= BeaconStateFork.Capella:
raiseAssert $capellaImplementationMissing
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this PR add such an implementation?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem here is that the various when return different types. So, either the entire function needs to change to ForkyState, or there needs to be a ForkedExecutionPayload. Too big to just extend the empty payload fallback.

Also, regardless of empty vs non-empty, we need to pass expected withdrawals into this function so that engine_getPayload or the empty block fallback are correct. Again too big for the scope of this PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plan is to make the whole makeBeaconBlockForHeadAndSlot call tree generic, avoid yet another forked type.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But, sure, agree with scope of PR.

elif stateFork >= BeaconStateFork.Bellatrix:
build_empty_execution_payload(forkyState.data, feeRecipient)
else:
default(T)

Expand Down
66 changes: 57 additions & 9 deletions tests/test_helpers.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@
import
# Status libraries
stew/bitops2,
eth/common/eth_types as commonEthTypes, eth/common/eth_types_rlp,
web3/ethtypes,
# Beacon chain internals
../beacon_chain/spec/[forks, helpers, state_transition],
../beacon_chain/spec/datatypes/bellatrix,
../beacon_chain/spec/datatypes/[bellatrix, capella],
# Test utilities
./unittest2, mocking/mock_genesis

Expand Down Expand Up @@ -61,20 +62,67 @@ suite "Spec helpers":
i += 1
process(state, state.numLeaves)

test "build_empty_execution_payload":
test "build_empty_execution_payload - Bellatrix":
var cfg = defaultRuntimeConfig
cfg.ALTAIR_FORK_EPOCH = GENESIS_EPOCH
cfg.BELLATRIX_FORK_EPOCH = GENESIS_EPOCH

let state = newClone(initGenesisState(cfg = cfg).bellatrixData)

template testCase(recipient: Eth1Address): untyped =
block:
let payload = build_empty_execution_payload[
typeof state[].data, bellatrix.ExecutionPayload](
state[].data, recipient)
check payload.fee_recipient ==
bellatrix.ExecutionAddress(data: distinctBase(recipient))
proc testCase(recipient: Eth1Address) =
let payload = build_empty_execution_payload(state[].data, recipient)
check payload.fee_recipient ==
bellatrix.ExecutionAddress(data: distinctBase(recipient))

testCase default(Eth1Address)
testCase Eth1Address.fromHex("0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b")

test "build_empty_execution_payload - Capella":
var cfg = defaultRuntimeConfig
cfg.ALTAIR_FORK_EPOCH = GENESIS_EPOCH
cfg.BELLATRIX_FORK_EPOCH = GENESIS_EPOCH
cfg.CAPELLA_FORK_EPOCH = GENESIS_EPOCH

let
state = newClone(initGenesisState(cfg = cfg).capellaData)
recipient = Eth1Address.fromHex(
"0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b")

proc testCase(withdrawals: seq[capella.Withdrawal]) =
let payload = build_empty_execution_payload(
state[].data, recipient, withdrawals)
check payload.fee_recipient ==
bellatrix.ExecutionAddress(data: distinctBase(recipient))
for i, withdrawal in withdrawals:
check payload.withdrawals[i] == withdrawal

let elHeader = emptyPayloadToBlockHeader(payload)
check elHeader.withdrawalsRoot.isSome
if withdrawals.len == 0:
check elHeader.withdrawalsRoot.get == EMPTY_ROOT_HASH
else:
check elHeader.withdrawalsRoot.get != EMPTY_ROOT_HASH
check elHeader.blockHash == payload.block_hash

var bellatrixHeader = elHeader
bellatrixHeader.withdrawalsRoot.reset()
check elHeader.blockHash != rlpHash bellatrixHeader

testCase @[]
testCase @[
capella.Withdrawal(
index: 42,
validatorIndex: 1337,
address: bellatrix.ExecutionAddress(data: distinctBase(recipient)),
amount: 25.Gwei)]
testCase @[
capella.Withdrawal(
index: 1,
validatorIndex: 1,
address: bellatrix.ExecutionAddress(data: distinctBase(recipient)),
amount: 1.Gwei),
capella.Withdrawal(
index: 2,
validatorIndex: 2,
address: bellatrix.ExecutionAddress(data: distinctBase(recipient)),
amount: 2.Gwei)]
4 changes: 1 addition & 3 deletions tests/testblockutil.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,7 @@ proc addTestBlock*(
cfg.BELLATRIX_FORK_EPOCH * SLOTS_PER_EPOCH + 10:
if is_merge_transition_complete(forkyState.data):
const feeRecipient = default(Eth1Address)
build_empty_execution_payload[
bellatrix.BeaconState, bellatrix.ExecutionPayload](
forkyState.data, feeRecipient)
build_empty_execution_payload(forkyState.data, feeRecipient)
else:
build_empty_merge_execution_payload(forkyState.data)
else:
Expand Down
2 changes: 1 addition & 1 deletion vendor/nim-eth
Open in desktop
Submodule nim-eth updated 59 files
+1 −1 doc/rlp.md
+4 −4 doc/trie.md
+23 −16 eth/common/eth_types.nim
+21 −6 eth/common/eth_types_rlp.nim
+2 −2 eth/db/kvstore.nim
+4 −4 eth/db/kvstore_sqlite3.nim
+1 −1 eth/keyfile/keyfile.nim
+1 −1 eth/keys.nim
+2 −2 eth/net/nat.nim
+1 −1 eth/net/utils.nim
+47 −15 eth/p2p/auth.nim
+1 −1 eth/p2p/discovery.nim
+48 −12 eth/p2p/discoveryv5/dcli.nim
+1 −1 eth/p2p/discoveryv5/encoding.nim
+101 −73 eth/p2p/discoveryv5/enr.nim
+1 −1 eth/p2p/discoveryv5/ip_vote.nim
+1 −1 eth/p2p/discoveryv5/lru.nim
+1 −1 eth/p2p/discoveryv5/nodes_verification.nim
+3 −3 eth/p2p/discoveryv5/protocol.nim
+4 −4 eth/p2p/discoveryv5/routing_table.nim
+11 −10 eth/p2p/ecies.nim
+2 −2 eth/p2p/kademlia.nim
+8 −8 eth/p2p/p2p_protocol_dsl.nim
+1 −1 eth/p2p/peer_pool.nim
+16 −13 eth/p2p/private/p2p_types.nim
+11 −14 eth/p2p/rlpx.nim
+6 −10 eth/p2p/rlpxcrypt.nim
+9 −4 eth/rlp.nim
+1 −1 eth/rlp/writer.nim
+1 −1 eth/trie/db.nim
+4 −4 eth/utp/delay_histogram.nim
+2 −2 eth/utp/growable_buffer.nim
+8 −8 eth/utp/ledbat_congestion_control.nim
+4 −4 eth/utp/packets.nim
+3 −3 eth/utp/utp.nim
+2 −2 eth/utp/utp_discv5_protocol.nim
+3 −3 eth/utp/utp_protocol.nim
+7 −7 eth/utp/utp_router.nim
+66 −66 eth/utp/utp_socket.nim
+3 −3 tests/db/test_kvstore_sqlite3.nim
+1 −1 tests/fuzzing/discovery/fuzz.nim
+3 −3 tests/fuzzing/discoveryv5/fuzz_decode_packet.nim
+1 −1 tests/p2p/discv5_test_helper.nim
+79 −1 tests/p2p/test_auth.nim
+2 −2 tests/p2p/test_discovery.nim
+22 −3 tests/p2p/test_discoveryv5.nim
+4 −4 tests/p2p/test_discoveryv5_encoding.nim
+1 −1 tests/p2p/test_enr.nim
+16 −6 tests/p2p/test_rlpx_thunk.json
+2 −2 tests/p2p/test_routing_table.nim
+36 −4 tests/rlp/test_api_usage.nim
+13 −0 tests/rlp/test_common.nim
+3 −3 tests/utp/test_buffer.nim
+2 −2 tests/utp/test_clock_drift_calculator.nim
+12 −12 tests/utp/test_protocol.nim
+3 −3 tests/utp/test_protocol_integration.nim
+4 −4 tests/utp/test_utp_router.nim
+55 −55 tests/utp/test_utp_socket.nim
+3 −3 tests/utp/test_utp_socket_sack.nim