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

Synchronously check all transactions to have non-zero length #573

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Commits on Aug 14, 2024

  1. Synchronously check all transactions to have non-zero length

    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.
    etan-status committed Aug 14, 2024
    Configuration menu
    Copy the full SHA
    15bc86e View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    180eb4c View commit details
    Browse the repository at this point in the history