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.

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

extend BlockHeader for Capella #562

Merged
merged 5 commits into from
Nov 25, 2022
Merged

Conversation

etan-status
Copy link
Contributor

Adds Withdrawal type according to EIP-4895, and extends BlockHeader accordingly. Also adds RLP encoding support for Withdrawal to enable building BlockHeader (used by Nimbus-CL in empty block prod fallback).

Adds `Withdrawal` type according to EIP-4895, and extends `BlockHeader`
accordingly. Also adds RLP encoding support for `Withdrawal` to enable
building `BlockHeader` (used by Nimbus-CL in empty block prod fallback).
@etan-status etan-status requested a review from mratsim November 24, 2022 14:58
@etan-status
Copy link
Contributor Author

shallowCopy error on Nim devel already existed before this patch, and is present on master as well.

@etan-status
Copy link
Contributor Author

Tested on Mainnet (Bellatrix) with nimbus_light_client by adding this check to the validation logic

doAssert payload.block_hash == rlpHash payload.toBlockHeader()

Also tested on Bellatrix / Capella against ethereum/consensus-specs#3126 by validating all sample block files against the Nimbus implementation.

From consensus-specs PR checkout:

make install_test && doctoc specs && doctoc ssz && make lint && \
    make citest && make test && make clean && make generate_tests

Test tool (adjust path in bottom):

# beacon_chain
# Copyright (c) 2022 Status Research & Development GmbH
# Licensed and distributed under either of
#   * MIT license (license terms in the root directory or at https://opensource.org/licenses/MIT).
#   * Apache v2 license (license terms in the root directory or at https://www.apache.org/licenses/LICENSE-2.0).
# at your option. This file may not be copied, modified, or distributed except according to those terms.

{.used.}

import
  # Standard library
  std/[os, sequtils, streams],
  # Status libraries
  stew/[bitops2, saturation_arith], snappy, unittest2,
  eth/common/[eth_types, eth_types_rlp],
  eth/trie/[db, hexary],
  eth/rlp,
  chronicles,
  # Third-party
  yaml,
  # Beacon chain internals
  beacon_chain/eth1/eth1_monitor,
  beacon_chain/spec/datatypes/[altair, bellatrix, capella],
  beacon_chain/spec/helpers,
  # Test utilities
  tests/testutil,
  tests/consensus_spec/fixtures_utils

proc calcRootHashRlp*(items: openArray[seq[byte]]): Hash256 =
  var tr = initHexaryTrie(newMemoryDB())
  for i, t in items:
    tr.put(rlp.encode(i), t)
  return tr.rootHash()

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

func toExecutionWithdrawal*(
    withdrawal: capella.Withdrawal): eth_types.Withdrawal =
  eth_types.Withdrawal(
    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 toBlockHeader*(
    payload: bellatrix.ExecutionPayload | capella.ExecutionPayload
): eth_types.BlockHeader =
  let
    txRoot = calcRootHashRlp(seq[seq[byte]](payload.transactions))
    withdrawalsRoot =
      when payload is bellatrix.ExecutionPayload:
        none(Hash256)
      else:
        some payload.computeWithdrawalsTrieRoot()

  eth_types.BlockHeader(
    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)

proc goBlck(path: string, blck: auto) =
  if not blck.message.is_execution_block:
    return

  test path:
    let
      consensusPayload = blck.message.body.execution_payload
      elBlockHeader = consensusPayload.toBlockHeader()
      expectedBlockHash = rlpHash elBlockHeader
    check consensusPayload.block_hash == expectedBlockHash

proc goFile(path: string) =
  try:
    goBlck path, sszDecodeEntireInput(
      snappy.decode(readFileBytes(path), MaxObjectSize),
      bellatrix.SignedBeaconBlock)
  except SerializationError:
    discard

  try:
    goBlck path, sszDecodeEntireInput(
      snappy.decode(readFileBytes(path), MaxObjectSize),
      capella.SignedBeaconBlock)
  except SerializationError:
    discard

proc goDir(dir: string) =
  for kind, path in walkDir(dir, relative = true, checkDir = true):
    if kind == pcDir:
      goDir(dir/path)
    else:
      goFile(dir/path)

suite "Payload tests":
  goDir("/path/to/consensus-spec-tests")

@etan-status etan-status requested a review from zah November 24, 2022 21:05
let abytes = rlp.encode(a)
let aa = rlp.decode(abytes, Withdrawal)
check aa == a

proc suite2() =
Copy link
Contributor

Choose a reason for hiding this comment

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

I was going to remark to also add an additional test for a header with the withdrawalsRoot but it looks like we don't have header at all covered here (neither with the fee addition EIP in the past apparently), so never mind.
This probably (?) does get tested on nimbus-eth1 level, and you've got it covered on the consensus test part.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, hence the additional test against the EF consensus-specs-tests, which includes withdrawals. It is a bit suboptimal as I wrote both sides (the implementation on consensus-specs, as well as the nimbus one), but this is a feature not currently being used, so problems would be discovered in internal devnets the latest.

@kdeme
Copy link
Contributor

kdeme commented Nov 25, 2022

shallowCopy error on Nim devel already existed before this patch, and is present on master as well.

Yes, can be ignored for now. We need to resolve some issue with the p2p macro for nim-devel.

@etan-status etan-status merged commit 6499ee2 into master Nov 25, 2022
@etan-status etan-status deleted the dev/etan/sf-cwithdrawals branch November 25, 2022 10:07
etan-status added a commit to status-im/nimbus-eth2 that referenced this pull request Nov 25, 2022
Implements `emptyPayloadToBlockHeader` for Capella.
status-im/nim-eth#562
etan-status added a commit to status-im/nimbus-eth1 that referenced this pull request Nov 25, 2022
The `BlockHeader` structure in `nim-eth` was updated with support for
EIP-4895 (withdrawals). To enable the `nim-eth` bump, the ingress of
`BlockHeader` structures has been hardened to reject headers that have
the new `withdrawalsRoot` field until proper withdrawals support exists.
status-im/nim-eth#562
etan-status added a commit to status-im/nimbus-eth2 that referenced this pull request Nov 25, 2022
Implements `emptyPayloadToBlockHeader` for Capella.
status-im/nim-eth#562
etan-status added a commit to status-im/nimbus-eth2 that referenced this pull request Nov 25, 2022
Implements `emptyPayloadToBlockHeader` for Capella.
status-im/nim-eth#562
etan-status added a commit to status-im/nimbus-eth2 that referenced this pull request Nov 25, 2022
Implements `emptyPayloadToBlockHeader` for Capella.
status-im/nim-eth#562
etan-status added a commit to status-im/nimbus-eth2 that referenced this pull request Nov 25, 2022
Implements `emptyPayloadToBlockHeader` for Capella.
status-im/nim-eth#562
etan-status added a commit to status-im/nimbus-eth2 that referenced this pull request Nov 25, 2022
Implements `emptyPayloadToBlockHeader` for Capella.
status-im/nim-eth#562
etan-status added a commit to status-im/nimbus-eth2 that referenced this pull request Nov 25, 2022
Implements `emptyPayloadToBlockHeader` for Capella.
status-im/nim-eth#562
etan-status added a commit to status-im/nimbus-eth2 that referenced this pull request Nov 25, 2022
Implements `emptyPayloadToBlockHeader` for Capella.
status-im/nim-eth#562
etan-status added a commit to status-im/nimbus-eth2 that referenced this pull request Nov 25, 2022
Implements `emptyPayloadToBlockHeader` for Capella.
status-im/nim-eth#562
etan-status added a commit to status-im/nimbus-eth1 that referenced this pull request Nov 26, 2022
The `BlockHeader` structure in `nim-eth` was updated with support for
EIP-4895 (withdrawals). To enable the `nim-eth` bump, the ingress of
`BlockHeader` structures has been hardened to reject headers that have
the new `withdrawalsRoot` field until proper withdrawals support exists.
status-im/nim-eth#562
etan-status added a commit to status-im/nimbus-eth2 that referenced this pull request Nov 28, 2022
Implements `emptyPayloadToBlockHeader` for Capella.
status-im/nim-eth#562
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants