As part of `newPayload` block hash verification, the `transactionsRoot`
is computed by the EL. Because Merkle-Patricia Tries cannot contain `[]`
entries, MPT implementations typically treat setting a key to `[]` as
deleting the entry for the key. This means that if a CL receives a block
with `transactions` containing one or more zero-length transactions,
that such transactions will effectively be skipped when computing the
`transactionsRoot`. Note that `transactions` are opaque to the CL and
zero-length transactions are not filtered out before `newPayload`.
```python
# https://eips.ethereum.org/EIPS/eip-2718
def compute_trie_root_from_indexed_data(data):
"""
Computes the root hash of `patriciaTrie(rlp(Index) => Data)` for a data array.
"""
t = HexaryTrie(db={})
for i, obj in enumerate(data):
k = encode(i, big_endian_int)
t.set(k, obj) # Implicitly skipped if `obj == b''` (invalid RLP)
return t.root_hash
```
In any case, the `blockHash` validation may still succeed, resulting in
a potential `SYNCING/ACCEPTED` result to `newPayload` by spec.
Note, however, that there is an effective hash collision if a payload is
modified by appending one or more zero-length transactions to the end of
`transactions` list: In the trivial case, a block with zero transactions
has the same `transactionsRoot` (and `blockHash`) as one of a block with
one `[]` transaction (as that one is skipped).
This means that the same `blockHash` can refer to a valid block (without
extra `[]` transactions added), but also can refer to an invalid block.
Because `forkchoiceUpdated` refers to blocks by `blockHash`, outcome may
be nondeterministic and implementation dependent. If `forkchoiceUpdated`
deems the `blockHash` to refer to a `VALID` object (obtained from a src
that does not have the extra `[]` transactions, e.g., devp2p), then this
could result in honest attestations to a CL beacon block with invalid
`[]` transactions in its `ExecutionPayload`, risking finalizing it.
The problem can be avoided by returning `INVALID` in `newPayload` if
there are any zero-length `transactions` entries, preventing optimistic
import of such blocks by the CL.
The name `get_safe_execution_payload_hash` is misleading, as it returns
the execution block hash. There is no concept of a payload hash. Rename
the function, and also update some documentation for clarity.
* Moved configuration into network preset instead of constants.
Now that `MAX_CHUNK_SIZE` and `GOSSIP_MAX_SIZE` are in configuration, we no longer need separate constants to represent them in the spec when they change in Bellatrix.
I've changed the usage, and put the values into the presets, but I'm not sure if I've updated the descriptions in the best way...
This is following on from the work in #3375 where a number of constants got moved into configuration, so we no longer need these constants to be separately represented, they can simply be updated in presets.
* Update presets/minimal/bellatrix.yaml
Co-authored-by: Hsiao-Wei Wang <hsiaowei.eth@gmail.com>
* Update presets/mainnet/bellatrix.yaml
Co-authored-by: Hsiao-Wei Wang <hsiaowei.eth@gmail.com>
* Moved preset items into the correct section and updated TOC.
It looked like the items listed in configuration about the max size and chunk size were no longer needed since we're updating preset values now and the preset changes seem to only be listed in the changes at the top.
* review feedback
* hopefully correct this time! Moved the 2 fields from configs into presets completely as suggested.
* WIP - changing back to being in config and updating the phase 0 value... I think this should be close but want to see what's outstanding.
* fix intellij's formatting of table.
* more fixes
---------
Co-authored-by: Hsiao-Wei Wang <hsiaowei.eth@gmail.com>
* Remove the work-in-progress note in Bellatrix spec
Bellatrix is done and released.
* Remove work-in-progress notes in Bellatrix specs
* Remove work-in-progress notes in Bellatrix specs
* Remove work-in-progress notes in Bellatrix specs
The `fork_version` field in `LightClientUpdate` can be derived from the
`update.signature_slot` value by consulting the locally configured fork
schedule. The light client already needs access to the fork schedule to
determine the `GeneralizedIndex` values used for merkle proofs, and the
memory layouts of the structures (including `LightClientUpdate`). The
`fork_version` itself is network dependent and doesn't reveal that info.
Documentation on how to call `prepare_execution_payload` had the params
for `safe_block_hash` and `finalized_block` hash flipped compared to the
function definition. Also updated tests for consistency.