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

refactor: move hash memoization out of signed #1485

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/consensus/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ pub mod transaction;
#[cfg(feature = "kzg")]
pub use transaction::BlobTransactionValidationError;
pub use transaction::{
SignableTransaction, Transaction, TxEip1559, TxEip2930, TxEip4844, TxEip4844Variant,
Enveloped, SignableTransaction, Transaction, TxEip1559, TxEip2930, TxEip4844, TxEip4844Variant,
TxEip4844WithSidecar, TxEip7702, TxEnvelope, TxLegacy, TxType, TypedTransaction,
};

Expand Down
16 changes: 4 additions & 12 deletions crates/consensus/src/signed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ pub struct Signed<T, Sig = Signature> {
tx: T,
#[cfg_attr(feature = "serde", serde(flatten))]
signature: Sig,
#[doc(alias = "tx_hash", alias = "transaction_hash")]
hash: B256,
}

impl<T, Sig> Signed<T, Sig> {
Expand All @@ -26,15 +24,9 @@ impl<T, Sig> Signed<T, Sig> {
&self.signature
}

/// Returns a reference to the transaction hash.
#[doc(alias = "tx_hash", alias = "transaction_hash")]
pub const fn hash(&self) -> &B256 {
&self.hash
}

/// Splits the transaction into parts.
pub fn into_parts(self) -> (T, Sig, B256) {
(self.tx, self.signature, self.hash)
pub fn into_parts(self) -> (T, Sig) {
(self.tx, self.signature)
}

/// Returns the transaction without signature.
Expand All @@ -45,8 +37,8 @@ impl<T, Sig> Signed<T, Sig> {

impl<T: SignableTransaction<Sig>, Sig> Signed<T, Sig> {
/// Instantiate from a transaction and signature. Does not verify the signature.
pub const fn new_unchecked(tx: T, signature: Sig, hash: B256) -> Self {
Self { tx, signature, hash }
pub const fn new_unchecked(tx: T, signature: Sig) -> Self {
Self { tx, signature }
}

/// Calculate the signing hash for the transaction.
Expand Down
17 changes: 7 additions & 10 deletions crates/consensus/src/transaction/eip1559.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use crate::{EncodableSignature, SignableTransaction, Signed, Transaction, TxType};
use alloc::vec::Vec;
use alloy_eips::{eip2930::AccessList, eip7702::SignedAuthorization};
use alloy_primitives::{keccak256, Bytes, ChainId, Parity, Signature, TxKind, B256, U256};
use alloy_primitives::{Bytes, ChainId, Parity, Signature, TxKind, B256, U256};
use alloy_rlp::{BufMut, Decodable, Encodable, Header};
use core::mem;

Expand Down Expand Up @@ -345,11 +344,7 @@ impl SignableTransaction<Signature> for TxEip1559 {
// signature.
let signature = signature.with_parity_bool();

let mut buf = Vec::with_capacity(self.encoded_len_with_signature(&signature, false));
self.encode_with_signature(&signature, &mut buf, false);
let hash = keccak256(&buf);

Signed::new_unchecked(self, signature, hash)
Signed::new_unchecked(self, signature)
}
}

Expand Down Expand Up @@ -414,8 +409,8 @@ mod tests {
hex!("0d5688ac3897124635b6cf1bc0e29d6dfebceebdc10a54d74f2ef8b56535b682")
);

let signed_tx = tx.into_signed(sig);
assert_eq!(*signed_tx.hash(), hash, "Expected same hash");
let signed_tx: crate::TxEnvelope = tx.into_signed(sig).into();
assert_eq!(signed_tx.tx_hash(), hash, "Expected same hash");
assert_eq!(signed_tx.recover_signer().unwrap(), signer, "Recovering signer should pass.");
}

Expand Down Expand Up @@ -446,7 +441,9 @@ mod tests {
tx.encode_with_signature_fields(&sig, &mut buf);
let decoded = TxEip1559::decode_signed_fields(&mut &buf[..]).unwrap();
assert_eq!(decoded, tx.into_signed(sig));
assert_eq!(*decoded.hash(), hash);

let envelope = crate::TxEnvelope::from(decoded);
assert_eq!(envelope.tx_hash(), hash);
}
}

Expand Down
11 changes: 3 additions & 8 deletions crates/consensus/src/transaction/eip2930.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use crate::{EncodableSignature, SignableTransaction, Signed, Transaction, TxType};
use alloc::vec::Vec;
use alloy_eips::{eip2930::AccessList, eip7702::SignedAuthorization};
use alloy_primitives::{keccak256, Bytes, ChainId, Parity, Signature, TxKind, B256, U256};
use alloy_primitives::{Bytes, ChainId, Parity, Signature, TxKind, B256, U256};
use alloy_rlp::{length_of_length, BufMut, Decodable, Encodable, Header};
use core::mem;

Expand Down Expand Up @@ -312,11 +311,7 @@ impl SignableTransaction<Signature> for TxEip2930 {
// signature.
let signature = signature.with_parity_bool();

let mut buf = Vec::with_capacity(self.encoded_len_with_signature(&signature, false));
self.encode_with_signature(&signature, &mut buf, false);
let hash = keccak256(&buf);

Signed::new_unchecked(self, signature, hash)
Signed::new_unchecked(self, signature)
}
}

Expand Down Expand Up @@ -391,7 +386,7 @@ mod tests {

let tx = request.into_signed(signature);

let envelope = TxEnvelope::Eip2930(tx);
let envelope: TxEnvelope = tx.into();

let mut encoded = Vec::new();
envelope.encode(&mut encoded);
Expand Down
46 changes: 14 additions & 32 deletions crates/consensus/src/transaction/eip4844.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::{EncodableSignature, SignableTransaction, Signed, Transaction, TxType

use alloc::vec::Vec;
use alloy_eips::{eip2930::AccessList, eip4844::DATA_GAS_PER_BLOB, eip7702::SignedAuthorization};
use alloy_primitives::{keccak256, Address, Bytes, ChainId, Parity, Signature, TxKind, B256, U256};
use alloy_primitives::{Address, Bytes, ChainId, Parity, Signature, TxKind, B256, U256};
use alloy_rlp::{length_of_length, BufMut, Decodable, Encodable, Header};
use core::mem;

Expand Down Expand Up @@ -185,15 +185,15 @@ impl TxEip4844Variant {
let header = Header::decode(&mut current_buf)?;
if header.list {
let tx = TxEip4844WithSidecar::decode_signed_fields(buf)?;
let (tx, signature, hash) = tx.into_parts();
return Ok(Signed::new_unchecked(tx.into(), signature, hash));
let (tx, signature) = tx.into_parts();
return Ok(Signed::new_unchecked(tx.into(), signature));
}

// Since there is not a second list header, this is a historical 4844 transaction without a
// sidecar.
let tx = TxEip4844::decode_signed_fields(buf)?;
let (tx, signature, hash) = tx.into_parts();
Ok(Signed::new_unchecked(tx.into(), signature, hash))
let (tx, signature) = tx.into_parts();
Ok(Signed::new_unchecked(tx.into(), signature))
}
}

Expand Down Expand Up @@ -329,13 +329,7 @@ impl SignableTransaction<Signature> for TxEip4844Variant {
// signature.
let signature = signature.with_parity_bool();

let payload_length = 1 + self.fields_len() + signature.rlp_vrs_len();
let mut buf = Vec::with_capacity(payload_length);
// we use the inner tx to encode the fields
self.tx().encode_with_signature(&signature, &mut buf, false);
let hash = keccak256(&buf);

Signed::new_unchecked(self, signature, hash)
Signed::new_unchecked(self, signature)
}
}

Expand Down Expand Up @@ -685,11 +679,7 @@ impl SignableTransaction<Signature> for TxEip4844 {
// signature.
let signature = signature.with_parity_bool();

let mut buf = Vec::with_capacity(self.encoded_len_with_signature(&signature, false));
self.encode_with_signature(&signature, &mut buf, false);
let hash = keccak256(&buf);

Signed::new_unchecked(self, signature, hash)
Signed::new_unchecked(self, signature)
}
}

Expand Down Expand Up @@ -919,11 +909,11 @@ impl TxEip4844WithSidecar {
});
}

let (tx, signature, hash) = inner_tx.into_parts();
let (tx, signature) = inner_tx.into_parts();

// create unchecked signed tx because these checks should have happened during construction
// of `Signed<TxEip4844>` in `TxEip4844::decode_signed_fields`
Ok(Signed::new_unchecked(Self::from_tx_and_sidecar(tx, sidecar), signature, hash))
Ok(Signed::new_unchecked(Self::from_tx_and_sidecar(tx, sidecar), signature))
}
}

Expand Down Expand Up @@ -953,16 +943,7 @@ impl SignableTransaction<Signature> for TxEip4844WithSidecar {
// signature.
let signature = signature.with_parity_bool();

let mut buf = Vec::with_capacity(self.tx.encoded_len_with_signature(&signature, false));
// The sidecar is NOT included in the signed payload, only the transaction fields and the
// type byte. Include the type byte.
//
// Include the transaction fields, making sure to __not__ use the sidecar, and __not__
// encode a header.
self.tx.encode_with_signature(&signature, &mut buf, false);
let hash = keccak256(&buf);

Signed::new_unchecked(self, signature, hash)
Signed::new_unchecked(self, signature)
}
}

Expand Down Expand Up @@ -1075,12 +1056,13 @@ mod tests {
let actual_signed = tx.into_signed(signature);

// the hashes should be the same
assert_eq!(expected_signed.hash(), actual_signed.hash());

// convert to envelopes
let expected_envelope: TxEnvelope = expected_signed.into();
let actual_envelope: TxEnvelope = actual_signed.into();

assert_eq!(expected_envelope.tx_hash(), actual_envelope.tx_hash());

// now encode the transaction and check the length
let len = expected_envelope.length();
let mut buf = Vec::with_capacity(len);
Expand Down Expand Up @@ -1124,9 +1106,9 @@ mod tests {
)
.unwrap();

let signed = variant.into_signed(signature);
let signed = TxEnvelope::from(variant.into_signed(signature));
assert_eq!(
*signed.hash(),
signed.tx_hash(),
b256!("93fc9daaa0726c3292a2e939df60f7e773c6a6a726a61ce43f4a217c64d85e87")
);
}
Expand Down
8 changes: 2 additions & 6 deletions crates/consensus/src/transaction/eip7702.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use alloy_eips::{
eip2930::AccessList,
eip7702::{constants::EIP7702_TX_TYPE_ID, SignedAuthorization},
};
use alloy_primitives::{keccak256, Address, Bytes, ChainId, Parity, Signature, TxKind, B256, U256};
use alloy_primitives::{Address, Bytes, ChainId, Parity, Signature, TxKind, B256, U256};
use alloy_rlp::{BufMut, Decodable, Encodable, Header};
use core::mem;

Expand Down Expand Up @@ -355,11 +355,7 @@ impl SignableTransaction<Signature> for TxEip7702 {
// signature.
let signature = signature.with_parity_bool();

let mut buf = Vec::with_capacity(self.encoded_len_with_signature(&signature, false));
self.encode_with_signature(&signature, &mut buf, false);
let hash = keccak256(&buf);

Signed::new_unchecked(self, signature, hash)
Signed::new_unchecked(self, signature)
}
}

Expand Down
Loading