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.

Sign up for GitHub

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

Jump to bottom

[Feature] Use lazy hashing for Signed #1987

Open
mattsse opened this issue Feb 1, 2025 · 0 comments · May be fixed by #2025
Open

[Feature] Use lazy hashing for Signed #1987

mattsse opened this issue Feb 1, 2025 · 0 comments · May be fixed by #2025
Labels
c-consensus Pertaining to the consensus crate enhancement New feature or request
Milestone
v1.0

Comments

@mattsse
Copy link
Member

mattsse commented Feb 1, 2025

Component

consensus, eips, genesis

Describe the feature you would like

currently the signed type mandates a hash:

alloy/crates/consensus/src/signed.rs

Lines 15 to 16 in dc3a7fe

#[doc(alias = "tx_hash", alias = "transaction_hash")]
hash: B256,

ideally we can relax this and instead compute the hash on demand if missing via a OnceLock

https://github.com/paradigmxyz/reth/blob/36807928311a91db559db228c3d25615152b4c8b/crates/ethereum/primitives/src/transaction.rs#L298-L300

https://github.com/paradigmxyz/reth/blob/36807928311a91db559db228c3d25615152b4c8b/crates/ethereum/primitives/src/transaction.rs#L355-L359

the signed value is currently just a type T, to compute the hash on demand we'd either need to introduce a new trait (basically Sealable) or we require that T: Transaction
I think we could make T: Transaction because this type is already intended for transactions:

alloy/crates/consensus/src/signed.rs

Lines 6 to 9 in dc3a7fe

/// A transaction with a signature and hash seal.
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub struct Signed<T, Sig = Signature> {

TODO

  • replace B256 with OnceLock

Additional context

No response
@mattsse mattsse added blocked This cannot move forward until something else changes enhancement New feature or request discuss needs discussion labels Feb 1, 2025
@mattsse mattsse added this to Alloy Feb 3, 2025
@github-project-automation github-project-automation bot moved this to Todo in Alloy Feb 3, 2025
@mattsse mattsse linked a pull request Feb 7, 2025 that will close this issue
Open
3 tasks
@yash-atreya yash-atreya added the c-consensus Pertaining to the consensus crate label Feb 20, 2025
@yash-atreya yash-atreya added this to the v1.0 milestone Feb 20, 2025
@yash-atreya yash-atreya removed the discuss needs discussion label Feb 28, 2025
@jenpaff jenpaff moved this from Todo to Ready for Review in Alloy Mar 3, 2025
@jenpaff jenpaff removed the blocked This cannot move forward until something else changes label Mar 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
c-consensus Pertaining to the consensus crate enhancement New feature or request
Projects
Alloy
Status: Ready for Review
Milestone
v1.0
Development

Successfully merging a pull request may close this issue.

feat: use OnceCell for Signed::hash alloy-rs/alloy
3 participants
@jenpaff @mattsse @yash-atreya