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

Add EIP-6493: SSZ Transaction Signature Scheme #6493

Merged
merged 10 commits into from Feb 26, 2023

Conversation

etan-status
Copy link
Contributor

This PR builds on top of prior work from:

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.

@github-actions github-actions bot added c-update Modifies an existing proposal s-review This EIP is in Review t-core labels Feb 10, 2023
@eth-bot
Copy link
Collaborator

eth-bot commented Feb 10, 2023

All reviewers have approved. Auto merging...

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 Author

Overall, these items need to be mixed into any SHA256 based signature:

  1. The fact that this signature is for a transaction
  2. Schema version describing how the tx was serialized
  3. Spec namespace (to distinguish from Ethereum based L2 / Sidechains)
    All these items must be mixed in using static information based on the node configuration, not based on the dynamic network data.

Currently, this PR uses an approach as close as possible to the consensus specs:

  1. DOMAIN_EXECUTION_TRANSACTION (in consensus: DomainType)
  2. tx_type (in consensus: Version)
  3. chain_id (in consensus: genesis_validators_root)

It doesn't necessarily have to be these constants, and it doesn't necessarily have to be the same schema from consensus. While both these tx signatures and the consensus signatures are SSZ based, they are not signed using the same key. For example, we could also just run uuidgen for each new TX type, instead of deriving a Domain from (1), (2), and (3).

For the perpetual TX hash, it could be relaxed, but the same information needs to be mixed into the hash if the goal is to create globally unique TX hashes (across chains).

@etan-status
Copy link
Contributor Author

There is also https://eips.ethereum.org/EIPS/eip-2124, maybe the "genesis hash" would work to link the signature to the underlying spec document defining the tx type, as the consensus genesis_validators_root equivalent.

EIPS/eip-4844.md Outdated
Comment on lines 197 to 199
def signed_tx_hash(tx: SignedBlobTransaction) -> Root:
domain = compute_transaction_domain(BLOB_TX_TYPE, node.cfg.chain_id)
return compute_signing_root(tx, domain)
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 the signed_tx_hash, as it already includes the signature (which already signs over compute_transaction_domain), I think it may be possible to just use plain hash_tree_root. TBD.

@github-actions github-actions bot added c-new Creates a brand new proposal s-draft This EIP is a Draft labels Feb 25, 2023
@eth-bot eth-bot added a-review Waiting on author to review e-consensus Waiting on editor consensus labels Feb 25, 2023
@github-actions
Copy link

The commit 1e9e198 (as a parent of 0f13409) contains errors.
Please inspect the Run Summary for details.

@github-actions github-actions bot added the w-ci Waiting on CI to pass label Feb 25, 2023
@github-actions github-actions bot removed c-update Modifies an existing proposal s-review This EIP is in Review labels Feb 25, 2023
@etan-status etan-status changed the title Update EIP-4844: hash_tree_root based transaction hashes Add EIP-6493: SSZ Transaction Signature Scheme Feb 25, 2023
@eth-bot eth-bot added e-review Waiting on editor to review and removed a-review Waiting on author to review labels Feb 25, 2023
@github-actions github-actions bot removed the w-ci Waiting on CI to pass label Feb 25, 2023
@etan-status
Copy link
Contributor Author

etan-status commented Feb 25, 2023

@lightclient

I moved the scheme for SSZ transaction signatures to a separate EIP (6493).

Note that this representation is for the way how signatures are signed, identified, and represented before inclusion in a block. For post-block-inclusion, there is EIP-6404.

The new signature scheme proposed here is close to what the "SSZ Union" approach in EIP-6404's test section is referring to, so it doesn't conflict with the approach based on "SSZ Union". For "SSZ Normalized" approach, the transaction's mempool encoding is not linked to the post-block-inclusion encoding, so there is no concern either.

@etan-status
Copy link
Contributor Author

Also, @vbuterin , this is as close as I could get it to your post here: https://notes.ethereum.org/@vbuterin/transaction_ssz_refactoring
while fixing the malleability concerns as close as possible to how this was already solved in consensus.

Means, 1 more SHA256 needed to get from message.hash_tree_root() to sig_hash compared to your blog post there.

@etan-status
Copy link
Contributor Author

@etan-status etan-status marked this pull request as ready for review February 25, 2023 01:16
EIPS/eip-6493.md Outdated Show resolved Hide resolved
EIPS/eip-6493.md Outdated Show resolved Hide resolved
EIPS/eip-6493.md Outdated Show resolved Hide resolved
EIPS/eip-6493.md Outdated Show resolved Hide resolved
tx_to=ExecutionAddress(bytes.fromhex('d8da6bf26964af9d7eed9e03e53415d37aa96045')),
tx_value=3_141_592_653,
)
sig_hash = compute_example_sig_hash(message)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
sig_hash = compute_example_sig_hash(message)
sig_root = compute_example_sig_root(message)

would this be clearer to convey that this goes into signature calc

Co-authored-by: g11tech <develop@g11tech.io>
Copy link
Member

@Pandapip1 Pandapip1 left a comment

Choose a reason for hiding this comment

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

LGTM now (for a draft).

@eth-bot eth-bot enabled auto-merge (squash) February 26, 2023 00:47
@eth-bot eth-bot merged commit 52d6d45 into ethereum:master Feb 26, 2023
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 <develop@g11tech.io>

---------

Co-authored-by: Gavin John <gavinnjohn@gmail.com>
Co-authored-by: g11tech <develop@g11tech.io>
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 <develop@g11tech.io>

---------

Co-authored-by: Gavin John <gavinnjohn@gmail.com>
Co-authored-by: g11tech <develop@g11tech.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c-new Creates a brand new proposal e-consensus Waiting on editor consensus e-review Waiting on editor to review s-draft This EIP is a Draft t-core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants