-
Notifications
You must be signed in to change notification settings - Fork 286
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
feat: use OnceCell
for Signed::hash
#2025
base: main
Are you sure you want to change the base?
Conversation
329cc37
to
3638021
Compare
2b43842
to
f58fb6a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this,
we should make it very clear that this Signed type is specifically for transactions, it's already mentioned but I think we can make this even clearer
Why can't we use std here? |
we can but we'd still need once_cell in no-std |
@klkvr we can simply upstream this to alloy: |
6a74e0e
to
93003ba
Compare
We need So this means we need to somehow have a helper Also I've changed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
but blocking until we finalize #2138
fn into_signed(self, signature: Signature) -> Signed<Self> { | ||
let tx_hash = self.tx_hash(&signature); | ||
Signed::new_unchecked(self, signature, tx_hash) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah these we no longer need because part of SignableTransaction
Motivation
closes #1987
Lazily computes hash for the transaction in
Signed
. Methods relying on hash are now bounded byT: RlpEcdsaTx
which works for all of the transactions we have but would likely get trickier for custom signature usecasesSolution
PR Checklist