Skip to content

Commit

Permalink
feat: convert hash field to OnceLock<TxHash> on TransactionSigned (
Browse files Browse the repository at this point in the history
…paradigmxyz#12596)

Co-authored-by: joshieDo <[email protected]>
  • Loading branch information
stevencartavia and joshieDo authored Nov 21, 2024
1 parent 6f6fb00 commit 4442b5d
Show file tree
Hide file tree
Showing 29 changed files with 127 additions and 87 deletions.
2 changes: 1 addition & 1 deletion bin/reth/src/commands/debug_cmd/build_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ impl<C: ChainSpecParser<ChainSpec = ChainSpec>> Command<C> {
let encoded_length = pooled.encode_2718_len();

// insert the blob into the store
blob_store.insert(transaction.hash, sidecar)?;
blob_store.insert(transaction.hash(), sidecar)?;

encoded_length
}
Expand Down
20 changes: 16 additions & 4 deletions crates/chain-state/src/notifications.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ impl<T: Clone + Sync + Send + 'static> Stream for ForkChoiceStream<T> {
#[cfg(test)]
mod tests {
use super::*;
use alloy_primitives::B256;
use alloy_primitives::{b256, B256};
use reth_execution_types::ExecutionOutcome;
use reth_primitives::{Receipt, Receipts, TransactionSigned, TxType};

Expand Down Expand Up @@ -332,7 +332,11 @@ mod tests {
block_receipts[0].0,
BlockReceipts {
block: block1.num_hash(),
tx_receipts: vec![(B256::default(), receipt1)]
tx_receipts: vec![(
// Transaction hash of a Transaction::default()
b256!("20b5378c6fe992c118b557d2f8e8bbe0b7567f6fe5483a8f0f1c51e93a9d91ab"),
receipt1
)]
}
);

Expand Down Expand Up @@ -403,7 +407,11 @@ mod tests {
block_receipts[0].0,
BlockReceipts {
block: old_block1.num_hash(),
tx_receipts: vec![(B256::default(), old_receipt)]
tx_receipts: vec![(
// Transaction hash of a Transaction::default()
b256!("20b5378c6fe992c118b557d2f8e8bbe0b7567f6fe5483a8f0f1c51e93a9d91ab"),
old_receipt
)]
}
);
// Confirm this is from the reverted segment.
Expand All @@ -415,7 +423,11 @@ mod tests {
block_receipts[1].0,
BlockReceipts {
block: new_block1.num_hash(),
tx_receipts: vec![(B256::default(), new_receipt)]
tx_receipts: vec![(
// Transaction hash of a Transaction::default()
b256!("20b5378c6fe992c118b557d2f8e8bbe0b7567f6fe5483a8f0f1c51e93a9d91ab"),
new_receipt
)]
}
);
// Confirm this is from the committed segment.
Expand Down
2 changes: 1 addition & 1 deletion crates/engine/util/src/reorg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ where
// Treat error as fatal
Err(error) => {
return Err(RethError::Execution(BlockExecutionError::Validation(
BlockValidationError::EVM { hash: tx.hash, error: Box::new(error) },
BlockValidationError::EVM { hash: tx.hash(), error: Box::new(error) },
)))
}
};
Expand Down
2 changes: 1 addition & 1 deletion crates/ethereum/payload/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ where
// grab the blob sidecars from the executed txs
blob_sidecars = pool
.get_all_blobs_exact(
executed_txs.iter().filter(|tx| tx.is_eip4844()).map(|tx| tx.hash).collect(),
executed_txs.iter().filter(|tx| tx.is_eip4844()).map(|tx| tx.hash()).collect(),
)
.map_err(PayloadBuilderError::other)?;

Expand Down
2 changes: 1 addition & 1 deletion crates/evm/execution-types/src/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ impl ChainBlocks<'_> {
/// Returns an iterator over all transaction hashes in the block
#[inline]
pub fn transaction_hashes(&self) -> impl Iterator<Item = TxHash> + '_ {
self.blocks.values().flat_map(|block| block.transactions().map(|tx| tx.hash))
self.blocks.values().flat_map(|block| block.transactions().map(|tx| tx.hash()))
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/net/network/src/transactions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2178,7 +2178,7 @@ mod tests {
.await;

assert!(!pool.is_empty());
assert!(pool.get(&signed_tx.hash).is_some());
assert!(pool.get(signed_tx.hash_ref()).is_some());
handle.terminate().await;
}

Expand Down
2 changes: 1 addition & 1 deletion crates/net/network/tests/it/txgossip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ async fn test_4844_tx_gossip_penalization() {
let peer0_reputation_after =
peer1.peer_handle().peer_by_id(*peer0.peer_id()).await.unwrap().reputation();
assert_ne!(peer0_reputation_before, peer0_reputation_after);
assert_eq!(received, txs[1].transaction().hash);
assert_eq!(received, txs[1].transaction().hash());

// this will return an [`Empty`] error because blob txs are disallowed to be broadcasted
assert!(peer1_tx_listener.try_recv().is_err());
Expand Down
2 changes: 1 addition & 1 deletion crates/optimism/rpc/src/eth/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ where
.enumerate()
.map(|(idx, (ref tx, receipt))| -> Result<_, _> {
let meta = TransactionMeta {
tx_hash: tx.hash,
tx_hash: tx.hash(),
index: idx as u64,
block_hash,
block_number,
Expand Down
3 changes: 2 additions & 1 deletion crates/optimism/rpc/src/eth/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@ where
tx_info: TransactionInfo,
) -> Result<Self::Transaction, Self::Error> {
let from = tx.signer();
let TransactionSigned { transaction, signature, hash } = tx.into_signed();
let hash = tx.hash();
let TransactionSigned { transaction, signature, .. } = tx.into_signed();
let mut deposit_receipt_version = None;
let mut deposit_nonce = None;

Expand Down
2 changes: 1 addition & 1 deletion crates/primitives/src/alloy_compat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ impl TryFrom<AnyRpcTransaction> for TransactionSigned {
_ => return Err(ConversionError::Custom("unknown transaction type".to_string())),
};

Ok(Self { transaction, signature, hash })
Ok(Self { transaction, signature, hash: hash.into() })
}
}

Expand Down
61 changes: 39 additions & 22 deletions crates/primitives/src/transaction/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,22 @@ use alloy_primitives::{
keccak256, Address, Bytes, ChainId, PrimitiveSignature as Signature, TxHash, TxKind, B256, U256,
};
use alloy_rlp::{Decodable, Encodable, Error as RlpError, Header};
use core::mem;
use core::{
hash::{Hash, Hasher},
mem,
};
use derive_more::{AsRef, Deref};
use once_cell as _;
#[cfg(not(feature = "std"))]
use once_cell::sync::Lazy as LazyLock;
use once_cell::sync::{Lazy as LazyLock, OnceCell as OnceLock};
#[cfg(feature = "optimism")]
use op_alloy_consensus::DepositTransaction;
use rayon::prelude::{IntoParallelIterator, ParallelIterator};
use reth_primitives_traits::InMemorySize;
use serde::{Deserialize, Serialize};
use signature::decode_with_eip155_chain_id;
#[cfg(feature = "std")]
use std::sync::LazyLock;
use std::sync::{LazyLock, OnceLock};

pub use error::{
InvalidTransactionError, TransactionConversionError, TryFromRecoveredTransactionError,
Expand Down Expand Up @@ -1078,10 +1081,11 @@ impl From<TransactionSigned> for TransactionSignedNoHash {

/// Signed transaction.
#[cfg_attr(any(test, feature = "reth-codec"), reth_codecs::add_arbitrary_tests(rlp))]
#[derive(Debug, Clone, PartialEq, Eq, Hash, AsRef, Deref, Serialize, Deserialize)]
#[derive(Debug, Clone, Eq, AsRef, Deref, Serialize, Deserialize)]
pub struct TransactionSigned {
/// Transaction hash
pub hash: TxHash,
#[serde(skip)]
pub hash: OnceLock<TxHash>,
/// The transaction signature values
pub signature: Signature,
/// Raw transaction info
Expand All @@ -1106,6 +1110,21 @@ impl AsRef<Self> for TransactionSigned {
}
}

impl Hash for TransactionSigned {
fn hash<H: Hasher>(&self, state: &mut H) {
self.signature.hash(state);
self.transaction.hash(state);
}
}

impl PartialEq for TransactionSigned {
fn eq(&self, other: &Self) -> bool {
self.signature == other.signature &&
self.transaction == other.transaction &&
self.hash_ref() == other.hash_ref()
}
}

// === impl TransactionSigned ===

impl TransactionSigned {
Expand All @@ -1120,13 +1139,13 @@ impl TransactionSigned {
}

/// Transaction hash. Used to identify transaction.
pub const fn hash(&self) -> TxHash {
self.hash
pub fn hash(&self) -> TxHash {
*self.hash_ref()
}

/// Reference to transaction hash. Used to identify transaction.
pub const fn hash_ref(&self) -> &TxHash {
&self.hash
pub fn hash_ref(&self) -> &TxHash {
self.hash.get_or_init(|| self.recalculate_hash())
}

/// Recover signer from signature and hash.
Expand Down Expand Up @@ -1259,9 +1278,7 @@ impl TransactionSigned {
///
/// This will also calculate the transaction hash using its encoding.
pub fn from_transaction_and_signature(transaction: Transaction, signature: Signature) -> Self {
let mut initial_tx = Self { transaction, hash: Default::default(), signature };
initial_tx.hash = initial_tx.recalculate_hash();
initial_tx
Self { transaction, signature, hash: Default::default() }
}

/// Decodes legacy transaction from the data buffer into a tuple.
Expand Down Expand Up @@ -1321,7 +1338,8 @@ impl TransactionSigned {
// so decoding methods do not need to manually advance the buffer
pub fn decode_rlp_legacy_transaction(data: &mut &[u8]) -> alloy_rlp::Result<Self> {
let (transaction, hash, signature) = Self::decode_rlp_legacy_transaction_tuple(data)?;
let signed = Self { transaction: Transaction::Legacy(transaction), hash, signature };
let signed =
Self { transaction: Transaction::Legacy(transaction), hash: hash.into(), signature };
Ok(signed)
}
}
Expand All @@ -1330,7 +1348,7 @@ impl SignedTransaction for TransactionSigned {
type Transaction = Transaction;

fn tx_hash(&self) -> &TxHash {
&self.hash
self.hash_ref()
}

fn transaction(&self) -> &Self::Transaction {
Expand Down Expand Up @@ -1608,19 +1626,19 @@ impl Decodable2718 for TransactionSigned {
TxType::Legacy => Err(Eip2718Error::UnexpectedType(0)),
TxType::Eip2930 => {
let (tx, signature, hash) = TxEip2930::rlp_decode_signed(buf)?.into_parts();
Ok(Self { transaction: Transaction::Eip2930(tx), signature, hash })
Ok(Self { transaction: Transaction::Eip2930(tx), signature, hash: hash.into() })
}
TxType::Eip1559 => {
let (tx, signature, hash) = TxEip1559::rlp_decode_signed(buf)?.into_parts();
Ok(Self { transaction: Transaction::Eip1559(tx), signature, hash })
Ok(Self { transaction: Transaction::Eip1559(tx), signature, hash: hash.into() })
}
TxType::Eip7702 => {
let (tx, signature, hash) = TxEip7702::rlp_decode_signed(buf)?.into_parts();
Ok(Self { transaction: Transaction::Eip7702(tx), signature, hash })
Ok(Self { transaction: Transaction::Eip7702(tx), signature, hash: hash.into() })
}
TxType::Eip4844 => {
let (tx, signature, hash) = TxEip4844::rlp_decode_signed(buf)?.into_parts();
Ok(Self { transaction: Transaction::Eip4844(tx), signature, hash })
Ok(Self { transaction: Transaction::Eip4844(tx), signature, hash: hash.into() })
}
#[cfg(feature = "optimism")]
TxType::Deposit => Ok(Self::from_transaction_and_signature(
Expand Down Expand Up @@ -1661,7 +1679,6 @@ impl<'a> arbitrary::Arbitrary<'a> for TransactionSigned {

#[cfg(feature = "optimism")]
let signature = if transaction.is_deposit() { TxDeposit::signature() } else { signature };

Ok(Self::from_transaction_and_signature(transaction, signature))
}
}
Expand Down Expand Up @@ -1900,7 +1917,7 @@ pub mod serde_bincode_compat {
impl<'a> From<&'a super::TransactionSigned> for TransactionSigned<'a> {
fn from(value: &'a super::TransactionSigned) -> Self {
Self {
hash: value.hash,
hash: value.hash(),
signature: value.signature,
transaction: Transaction::from(&value.transaction),
}
Expand All @@ -1910,7 +1927,7 @@ pub mod serde_bincode_compat {
impl<'a> From<TransactionSigned<'a>> for super::TransactionSigned {
fn from(value: TransactionSigned<'a>) -> Self {
Self {
hash: value.hash,
hash: value.hash.into(),
signature: value.signature,
transaction: value.transaction.into(),
}
Expand Down Expand Up @@ -2203,7 +2220,7 @@ mod tests {
) {
let expected = TransactionSigned::from_transaction_and_signature(transaction, signature);
if let Some(hash) = hash {
assert_eq!(hash, expected.hash);
assert_eq!(hash, expected.hash());
}
assert_eq!(bytes.len(), expected.length());

Expand Down
34 changes: 19 additions & 15 deletions crates/primitives/src/transaction/pooled.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,17 +69,18 @@ impl PooledTransactionsElement {
/// [`PooledTransactionsElement`]. Since [`BlobTransaction`] is disallowed to be broadcasted on
/// p2p, return an err if `tx` is [`Transaction::Eip4844`].
pub fn try_from_broadcast(tx: TransactionSigned) -> Result<Self, TransactionSigned> {
let hash = tx.hash();
match tx {
TransactionSigned { transaction: Transaction::Legacy(tx), signature, hash } => {
TransactionSigned { transaction: Transaction::Legacy(tx), signature, .. } => {
Ok(Self::Legacy { transaction: tx, signature, hash })
}
TransactionSigned { transaction: Transaction::Eip2930(tx), signature, hash } => {
TransactionSigned { transaction: Transaction::Eip2930(tx), signature, .. } => {
Ok(Self::Eip2930 { transaction: tx, signature, hash })
}
TransactionSigned { transaction: Transaction::Eip1559(tx), signature, hash } => {
TransactionSigned { transaction: Transaction::Eip1559(tx), signature, .. } => {
Ok(Self::Eip1559 { transaction: tx, signature, hash })
}
TransactionSigned { transaction: Transaction::Eip7702(tx), signature, hash } => {
TransactionSigned { transaction: Transaction::Eip7702(tx), signature, .. } => {
Ok(Self::Eip7702 { transaction: tx, signature, hash })
}
// Not supported because missing blob sidecar
Expand All @@ -99,9 +100,10 @@ impl PooledTransactionsElement {
tx: TransactionSigned,
sidecar: BlobTransactionSidecar,
) -> Result<Self, TransactionSigned> {
let hash = tx.hash();
Ok(match tx {
// If the transaction is an EIP-4844 transaction...
TransactionSigned { transaction: Transaction::Eip4844(tx), signature, hash } => {
TransactionSigned { transaction: Transaction::Eip4844(tx), signature, .. } => {
// Construct a `PooledTransactionsElement::BlobTransaction` with provided sidecar.
Self::BlobTransaction(BlobTransaction {
signature,
Expand Down Expand Up @@ -187,23 +189,25 @@ impl PooledTransactionsElement {
/// Returns the inner [`TransactionSigned`].
pub fn into_transaction(self) -> TransactionSigned {
match self {
Self::Legacy { transaction, signature, hash } => {
TransactionSigned { transaction: Transaction::Legacy(transaction), signature, hash }
}
Self::Legacy { transaction, signature, hash } => TransactionSigned {
transaction: Transaction::Legacy(transaction),
signature,
hash: hash.into(),
},
Self::Eip2930 { transaction, signature, hash } => TransactionSigned {
transaction: Transaction::Eip2930(transaction),
signature,
hash,
hash: hash.into(),
},
Self::Eip1559 { transaction, signature, hash } => TransactionSigned {
transaction: Transaction::Eip1559(transaction),
signature,
hash,
hash: hash.into(),
},
Self::Eip7702 { transaction, signature, hash } => TransactionSigned {
transaction: Transaction::Eip7702(transaction),
signature,
hash,
hash: hash.into(),
},
Self::BlobTransaction(blob_tx) => blob_tx.into_parts().0,
}
Expand Down Expand Up @@ -460,7 +464,7 @@ impl Decodable2718 for PooledTransactionsElement {
}
tx_type => {
let typed_tx = TransactionSigned::typed_decode(tx_type, buf)?;

let hash = typed_tx.hash();
match typed_tx.transaction {
Transaction::Legacy(_) => Err(RlpError::Custom(
"legacy transactions should not be a result of typed decoding",
Expand All @@ -473,17 +477,17 @@ impl Decodable2718 for PooledTransactionsElement {
Transaction::Eip2930(tx) => Ok(Self::Eip2930 {
transaction: tx,
signature: typed_tx.signature,
hash: typed_tx.hash,
hash
}),
Transaction::Eip1559(tx) => Ok(Self::Eip1559 {
transaction: tx,
signature: typed_tx.signature,
hash: typed_tx.hash,
hash
}),
Transaction::Eip7702(tx) => Ok(Self::Eip7702 {
transaction: tx,
signature: typed_tx.signature,
hash: typed_tx.hash,
hash
}),
#[cfg(feature = "optimism")]
Transaction::Deposit(_) => Err(RlpError::Custom("Optimism deposit transaction cannot be decoded to PooledTransactionsElement").into())
Expand Down
Loading

0 comments on commit 4442b5d

Please sign in to comment.