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

4844: use hash_tree_root for tx hash #6385

Closed
wants to merge 1 commit into from

Conversation

lightclient
Copy link
Member

@lightclient lightclient commented Jan 26, 2023

I think we should move to hash_tree_root for calculating the transaction hash of 4844 transactions if we plan to migrate to an SSZ list of transactions.

The reason for staying with (and originally choosing) keccak is that it fits nicely into the expectation that the MPT hashing algorithm has for leaves greater than 32 bytes, it will simply take the keccak of it.

@github-actions github-actions bot added c-update Modifies an existing proposal s-draft This EIP is a Draft t-core labels Jan 26, 2023
@eth-bot
Copy link
Collaborator

eth-bot commented Jan 26, 2023

All tests passed; auto-merging...

(pass) eip-4844.md

classification
updateEIP
  • passed!

@vbuterin
Copy link
Contributor

Strongly support going in this direction. We have a medium-term goal of moving transactions as a whole away from RLP and Merkle Patricia trees to SSZ, and hash_tree_root is clearly what will be used for both signing hashes and txids long-term, so it makes more sense to move to it immediately than to have an awkward halfway-house where this one transaction type has one foot in the future SSZ system and another foot in the historical RLP system.

The main downside that I see is that this will break some infrastructure that depends on txid = keccak(serialize(tx)). I think the fact that we're introducing SSZ with this type of transaction is actually an argument for why now is exactly the time to make the change. EIP-4844 transactions are expected to be a specialized tool, which would be used by either (i) sophisticated actors publishing blobs for rollups, or (ii) sophisticated actors who just really want to be vanguard users of SSZ transactions; there are no existing applications and no expectation of regular users relying on EIP-4844 transactions behaving in any particular way. Hence, the costs of EIP-4844 transactions being incompatible with some existing infrastructure (even, if desired, eg. the getTransactionHashes RPC returning the "wrong" hash) are lower than they would otherwise be.

@etan-status
Copy link
Contributor

etan-status commented Feb 9, 2023

The signed hash must be protected against malleability

In this example, both hash_tree_root(foo) and hash_tree_root(bar) have the same value: b8061be2b4337ac42bc6ddb54395dbb1241b7fde97ae35539d2af32e7ba8ca5f

    type
      Foo = object
        a: uint64
        b: uint64
        c: uint64
      Bar = object
        a: uint64
        b: uint64
        c: uint64
        d: uint64

    let
      foo = Foo(
        a: 0x42,
        b: 0x42,
        c: 0x42)
      bar = Bar(
        a: 0x42,
        b: 0x42,
        c: 0x42,
        d: 0)

    debug "Foo", f = toHex(hash_tree_root(foo).data)
    debug "Bar", f = toHex(hash_tree_root(bar).data)

The transaction hash should be perpetually stable and unique.

In consensus, there is compute_signing_root. Also, all objects that are being referred by HTR have a stable offset that contains a field to derive version from, e.g., slot number for blocks.

Overall, the solution should be compatible with whatever we are deciding during ethereum/pm#721 regarding the overarching SSZ Transaction representation. While I am in favor of a hashing solution based on SHA256 / hash_tree_root for SSZ transactions, I do not see a real benefit in requiring the HTR and txid be the same value, because such a solution does not work well for either pre-SSZ transactions nor future transactions based on other hashing schemes. Technically, an SSZ hash_tree_root could simply be put into the current transactions MPT, same as any other transaction hash that is currently fed into it — this doesn't require a transactions SSZ structure.

As for security, if we decide to mix keccak256 and SHA256 hashes, cryptographic analysis may be necessary to determine the amount by which the hash collision probability is increased.

etan-status added a commit to etan-status/EIPs that referenced this pull request Feb 10, 2023
This PR builds on top of prior work from:
- @lightclient at ethereum#6385

The signature malleability issue in the original PR is addressed by
reusing the consensus `compute_signing_root` mechanism to link each
hash with the transaction's underlying `chain_id` and `tx_type`.

Note that this makes the transaction hashes different from the plain
`hash_tree_root` values. This means that if the `transactions_root` MPT
is replaced with SSZ (EIP-6404), that the `transaction_hash` would need
to be tracked separately, same as for legacy RLP-based transactions.
This is mainly a cosmetic issue, not a practical one. In an SSZ tx tree,
we could simply include both the HTR as well as the perpetual tx hash.

Cryptographic analysis may be necessary to determine the amount by which
the hash collision probability is increased, if we use different hashing
algorithms for transactions. On the other hand, using different algo for
SSZ transactions reduces the impact of a custom network defining 0x05 as
a RLP transaction that might serialize same as the blob SSZ transaction.
etan-status added a commit to etan-status/EIPs that referenced this pull request Feb 10, 2023
This PR builds on top of prior work from:
- @lightclient at ethereum#6385

The signature malleability issue in the original PR is addressed by
reusing the consensus `compute_signing_root` mechanism to link each
hash with the transaction's underlying `chain_id` and `tx_type`.

Note that this makes the transaction hashes different from the plain
`hash_tree_root` values. This means that if the `transactions_root` MPT
is replaced with SSZ (EIP-6404), that the `transaction_hash` would need
to be tracked separately, same as for legacy RLP-based transactions.
This is mainly a cosmetic issue, not a practical one. In an SSZ tx tree,
we could simply include both the HTR as well as the perpetual tx hash.

Cryptographic analysis may be necessary to determine the amount by which
the hash collision probability is increased, if we use different hashing
algorithms for transactions. On the other hand, using different algo for
SSZ transactions reduces the impact of a custom network defining 0x05 as
a RLP transaction that might serialize same as the blob SSZ transaction.
etan-status added a commit to etan-status/EIPs that referenced this pull request Feb 10, 2023
This PR builds on top of prior work from:
- @lightclient at ethereum#6385

The signature malleability issue in the original PR is addressed by
reusing the consensus `compute_signing_root` mechanism to link each
hash with the transaction's underlying `chain_id` and `tx_type`.

Note that this makes the transaction hashes different from the plain
`hash_tree_root` values. This means that if the `transactions_root` MPT
is replaced with SSZ (EIP-6404), that the `transaction_hash` would need
to be tracked separately, same as for legacy RLP-based transactions.
This is mainly a cosmetic issue, not a practical one. In an SSZ tx tree,
we could simply include both the HTR as well as the perpetual tx hash.

Cryptographic analysis may be necessary to determine the amount by which
the hash collision probability is increased, if we use different hashing
algorithms for transactions. On the other hand, using different algo for
SSZ transactions reduces the impact of a custom network defining 0x05 as
a RLP transaction that might serialize same as the blob SSZ transaction.
@etan-status
Copy link
Contributor

Attempt to fix malleability: #6493

etan-status added a commit to etan-status/EIPs that referenced this pull request Feb 10, 2023
This PR builds on top of prior work from:
- @lightclient at ethereum#6385

The signature malleability issue in the original PR is addressed by
reusing the consensus `compute_signing_root` mechanism to link each
hash with the transaction's underlying `chain_id` and `tx_type`.

Note that this makes the transaction hashes different from the plain
`hash_tree_root` values. This means that if the `transactions_root` MPT
is replaced with SSZ (EIP-6404), that the `transaction_hash` would need
to be tracked separately, same as for legacy RLP-based transactions.
This is mainly a cosmetic issue, not a practical one. In an SSZ tx tree,
we could simply include both the HTR as well as the perpetual tx hash.

Cryptographic analysis may be necessary to determine the amount by which
the hash collision probability is increased, if we use different hashing
algorithms for transactions. On the other hand, using different algo for
SSZ transactions reduces the impact of a custom network defining 0x05 as
a RLP transaction that might serialize same as the blob SSZ transaction.
etan-status added a commit to etan-status/EIPs that referenced this pull request Feb 10, 2023
This PR builds on top of prior work from:
- @lightclient at ethereum#6385

The signature malleability issue in the original PR is addressed by
reusing the consensus `compute_signing_root` mechanism to link each
hash with the transaction's underlying `chain_id` and `tx_type`.

Note that this makes the transaction hashes different from the plain
`hash_tree_root` values. This means that if the `transactions_root` MPT
is replaced with SSZ (EIP-6404), that the `transaction_hash` would need
to be tracked separately, same as for legacy RLP-based transactions.
This is mainly a cosmetic issue, not a practical one. In an SSZ tx tree,
we could simply include both the HTR as well as the perpetual tx hash.

Cryptographic analysis may be necessary to determine the amount by which
the hash collision probability is increased, if we use different hashing
algorithms for transactions. On the other hand, using different algo for
SSZ transactions reduces the impact of a custom network defining 0x05 as
a RLP transaction that might serialize same as the blob SSZ transaction.
@lightclient
Copy link
Member Author

Thanks @etan-status I understand the issue with malleability now. I will close this in favor of #6493

eth-bot pushed a commit that referenced this pull request Feb 26, 2023
* Update EIP-4844: `hash_tree_root` based transaction hashes

This PR builds on top of prior work from:
- @lightclient at #6385

The signature malleability issue in the original PR is addressed by
reusing the consensus `compute_signing_root` mechanism to link each
hash with the transaction's underlying `chain_id` and `tx_type`.

Note that this makes the transaction hashes different from the plain
`hash_tree_root` values. This means that if the `transactions_root` MPT
is replaced with SSZ (EIP-6404), that the `transaction_hash` would need
to be tracked separately, same as for legacy RLP-based transactions.
This is mainly a cosmetic issue, not a practical one. In an SSZ tx tree,
we could simply include both the HTR as well as the perpetual tx hash.

Cryptographic analysis may be necessary to determine the amount by which
the hash collision probability is increased, if we use different hashing
algorithms for transactions. On the other hand, using different algo for
SSZ transactions reduces the impact of a custom network defining 0x05 as
a RLP transaction that might serialize same as the blob SSZ transaction.

* Fix `signed_tx_hash`

* Use static chain ID for purpose of domain computation

* Extract SSZ signature scheme to EIP for reuse

* Add URL

* Split 4844 changes to separate PR

* Fix

* rm redundant test

* Apply most of @g11tech's review

Co-authored-by: g11tech <[email protected]>

---------

Co-authored-by: Gavin John <[email protected]>
Co-authored-by: g11tech <[email protected]>
fulldecent pushed a commit to fulldecent/EIPs that referenced this pull request Mar 13, 2023
* Update EIP-4844: `hash_tree_root` based transaction hashes

This PR builds on top of prior work from:
- @lightclient at ethereum#6385

The signature malleability issue in the original PR is addressed by
reusing the consensus `compute_signing_root` mechanism to link each
hash with the transaction's underlying `chain_id` and `tx_type`.

Note that this makes the transaction hashes different from the plain
`hash_tree_root` values. This means that if the `transactions_root` MPT
is replaced with SSZ (EIP-6404), that the `transaction_hash` would need
to be tracked separately, same as for legacy RLP-based transactions.
This is mainly a cosmetic issue, not a practical one. In an SSZ tx tree,
we could simply include both the HTR as well as the perpetual tx hash.

Cryptographic analysis may be necessary to determine the amount by which
the hash collision probability is increased, if we use different hashing
algorithms for transactions. On the other hand, using different algo for
SSZ transactions reduces the impact of a custom network defining 0x05 as
a RLP transaction that might serialize same as the blob SSZ transaction.

* Fix `signed_tx_hash`

* Use static chain ID for purpose of domain computation

* Extract SSZ signature scheme to EIP for reuse

* Add URL

* Split 4844 changes to separate PR

* Fix

* rm redundant test

* Apply most of @g11tech's review

Co-authored-by: g11tech <[email protected]>

---------

Co-authored-by: Gavin John <[email protected]>
Co-authored-by: g11tech <[email protected]>
axelcabee pushed a commit to axelcabee/EIPs that referenced this pull request May 6, 2023
* Update EIP-4844: `hash_tree_root` based transaction hashes

This PR builds on top of prior work from:
- @lightclient at ethereum#6385

The signature malleability issue in the original PR is addressed by
reusing the consensus `compute_signing_root` mechanism to link each
hash with the transaction's underlying `chain_id` and `tx_type`.

Note that this makes the transaction hashes different from the plain
`hash_tree_root` values. This means that if the `transactions_root` MPT
is replaced with SSZ (EIP-6404), that the `transaction_hash` would need
to be tracked separately, same as for legacy RLP-based transactions.
This is mainly a cosmetic issue, not a practical one. In an SSZ tx tree,
we could simply include both the HTR as well as the perpetual tx hash.

Cryptographic analysis may be necessary to determine the amount by which
the hash collision probability is increased, if we use different hashing
algorithms for transactions. On the other hand, using different algo for
SSZ transactions reduces the impact of a custom network defining 0x05 as
a RLP transaction that might serialize same as the blob SSZ transaction.

* Fix `signed_tx_hash`

* Use static chain ID for purpose of domain computation

* Extract SSZ signature scheme to EIP for reuse

* Add URL

* Split 4844 changes to separate PR

* Fix

* rm redundant test

* Apply most of @g11tech's review

Co-authored-by: g11tech <[email protected]>

---------

Co-authored-by: Gavin John <[email protected]>
Co-authored-by: g11tech <[email protected]>
GAEAlimited pushed a commit to GAEAlimited/EIPs that referenced this pull request Jun 19, 2024
* Update EIP-4844: `hash_tree_root` based transaction hashes

This PR builds on top of prior work from:
- @lightclient at ethereum#6385

The signature malleability issue in the original PR is addressed by
reusing the consensus `compute_signing_root` mechanism to link each
hash with the transaction's underlying `chain_id` and `tx_type`.

Note that this makes the transaction hashes different from the plain
`hash_tree_root` values. This means that if the `transactions_root` MPT
is replaced with SSZ (EIP-6404), that the `transaction_hash` would need
to be tracked separately, same as for legacy RLP-based transactions.
This is mainly a cosmetic issue, not a practical one. In an SSZ tx tree,
we could simply include both the HTR as well as the perpetual tx hash.

Cryptographic analysis may be necessary to determine the amount by which
the hash collision probability is increased, if we use different hashing
algorithms for transactions. On the other hand, using different algo for
SSZ transactions reduces the impact of a custom network defining 0x05 as
a RLP transaction that might serialize same as the blob SSZ transaction.

* Fix `signed_tx_hash`

* Use static chain ID for purpose of domain computation

* Extract SSZ signature scheme to EIP for reuse

* Add URL

* Split 4844 changes to separate PR

* Fix

* rm redundant test

* Apply most of @g11tech's review

Co-authored-by: g11tech <[email protected]>

---------

Co-authored-by: Gavin John <[email protected]>
Co-authored-by: g11tech <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c-update Modifies an existing proposal s-draft This EIP is a Draft t-core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants