Skip to content

Commit

Permalink
chore: make transaction type fields private (#13915)
Browse files Browse the repository at this point in the history
Co-authored-by: Matthias Seitz <[email protected]>
  • Loading branch information
stevencartavia and mattsse authored Jan 29, 2025
1 parent ed593ae commit 2652ec8
Show file tree
Hide file tree
Showing 12 changed files with 63 additions and 25 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 @@ -169,7 +169,7 @@ impl<C: ChainSpecParser<ChainSpec = ChainSpec>> Command<C> {
.try_clone_into_recovered()
.map_err(|e| eyre::eyre!("failed to recover tx: {e}"))?;

let encoded_length = match &transaction.transaction {
let encoded_length = match transaction.transaction() {
Transaction::Eip4844(TxEip4844 { blob_versioned_hashes, .. }) => {
let blobs_bundle = blobs_bundle.as_mut().ok_or_else(|| {
eyre::eyre!("encountered a blob tx. `--blobs-bundle-path` must be provided")
Expand Down
1 change: 1 addition & 0 deletions crates/engine/tree/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -124,4 +124,5 @@ test-utils = [
"reth-prune-types?/test-utils",
"reth-trie-db/test-utils",
"reth-trie-parallel/test-utils",
"reth-ethereum-primitives/test-utils",
]
2 changes: 1 addition & 1 deletion crates/ethereum/payload/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ where
// invalid, which removes its dependent transactions from
// the iterator. This is similar to the gas limit condition
// for regular transactions above.
trace!(target: "payload_builder", tx=?tx.hash, ?sum_blob_gas_used, ?tx_blob_gas, "skipping blob transaction because it would exceed the max data gas per block");
trace!(target: "payload_builder", tx=?tx.hash(), ?sum_blob_gas_used, ?tx_blob_gas, "skipping blob transaction because it would exceed the max data gas per block");
best_txs.mark_invalid(
&pool_tx,
InvalidPoolTransactionError::ExceedsGasLimit(
Expand Down
4 changes: 4 additions & 0 deletions crates/ethereum/primitives/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ alloy-consensus = { workspace = true, features = ["serde", "arbitrary"] }

[features]
default = ["std"]
test-utils = [
"reth-codecs?/test-utils",
"reth-primitives-traits/test-utils",
]
alloy-compat = ["dep:alloy-network", "dep:alloy-serde", "dep:alloy-rpc-types"]
std = [
"alloy-consensus/std",
Expand Down
51 changes: 44 additions & 7 deletions crates/ethereum/primitives/src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,17 +282,19 @@ impl From<TypedTransaction> for Transaction {
/// Signed Ethereum transaction.
#[derive(Debug, Clone, Eq, Serialize, Deserialize, derive_more::AsRef, derive_more::Deref)]
#[cfg_attr(any(test, feature = "reth-codec"), reth_codecs::add_arbitrary_tests(rlp))]
#[cfg_attr(feature = "test-utils", derive(derive_more::DerefMut))]
#[serde(rename_all = "camelCase")]
pub struct TransactionSigned {
/// Transaction hash
#[serde(skip)]
pub hash: OnceLock<TxHash>,
hash: OnceLock<TxHash>,
/// The transaction signature values
pub signature: Signature,
signature: Signature,
/// Raw transaction info
#[deref]
#[as_ref]
pub transaction: Transaction,
#[cfg_attr(feature = "test-utils", deref_mut)]
transaction: Transaction,
}

impl Default for TransactionSigned {
Expand Down Expand Up @@ -328,13 +330,42 @@ impl TransactionSigned {
Self { hash: hash.into(), signature, transaction }
}

/// Consumes the type and returns the transaction.
#[inline]
pub fn into_transaction(self) -> Transaction {
self.transaction
}

/// Returns the transaction.
#[inline]
pub const fn transaction(&self) -> &Transaction {
&self.transaction
}

/// Returns the transaction hash.
#[inline]
pub fn hash(&self) -> &B256 {
self.hash.get_or_init(|| self.recalculate_hash())
}

/// Returns the transaction signature.
#[inline]
pub const fn signature(&self) -> &Signature {
&self.signature
}

/// Creates a new signed transaction from the given transaction and signature without the hash.
///
/// Note: this only calculates the hash on the first [`TransactionSigned::hash`] call.
pub fn new_unhashed(transaction: Transaction, signature: Signature) -> Self {
Self { hash: Default::default(), signature, transaction }
}

/// Splits the `TransactionSigned` into its transaction and signature.
pub fn split(self) -> (Transaction, Signature) {
(self.transaction, self.signature)
}

/// Converts from an EIP-4844 transaction to a [`PooledTransaction`] with the given sidecar.
///
/// Returns an `Err` containing the original `TransactionSigned` if the transaction is not
Expand Down Expand Up @@ -367,6 +398,12 @@ impl TransactionSigned {
_ => None,
}
}

/// Provides mutable access to the transaction.
#[cfg(feature = "test-utils")]
pub fn transaction_mut(&mut self) -> &mut Transaction {
&mut self.transaction
}
}

impl Typed2718 for TransactionSigned {
Expand Down Expand Up @@ -534,13 +571,13 @@ impl Decodable2718 for TransactionSigned {
}

impl Encodable for TransactionSigned {
fn length(&self) -> usize {
self.network_len()
}

fn encode(&self, out: &mut dyn alloy_rlp::BufMut) {
self.network_encode(out);
}

fn length(&self) -> usize {
self.network_len()
}
}

impl Decodable for TransactionSigned {
Expand Down
1 change: 1 addition & 0 deletions crates/primitives/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ test-utils = [
"reth-primitives-traits/test-utils",
"arbitrary",
"reth-codecs/test-utils",
"reth-ethereum-primitives/test-utils",
]
serde-bincode-compat = [
"alloy-eips/serde-bincode-compat",
Expand Down
6 changes: 2 additions & 4 deletions crates/rpc/rpc/src/eth/helpers/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ where
) -> Result<Self::Transaction, Self::Error> {
let from = tx.signer();
let hash = *tx.tx_hash();
let TransactionSigned { transaction, signature, .. } = tx.into_tx();
let signature = *tx.signature();

let inner: TxEnvelope = match transaction {
let inner: TxEnvelope = match tx.into_tx().into_transaction() {
reth_primitives::Transaction::Legacy(tx) => {
Signed::new_unchecked(tx, signature, hash).into()
}
Expand All @@ -63,8 +63,6 @@ where
reth_primitives::Transaction::Eip7702(tx) => {
Signed::new_unchecked(tx, signature, hash).into()
}
#[allow(unreachable_patterns)]
_ => unreachable!(),
};

let TransactionInfo {
Expand Down
3 changes: 0 additions & 3 deletions crates/stages/stages/src/stages/sender_recovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -672,9 +672,6 @@ mod tests {
for tx_id in body.tx_num_range() {
let transaction: TransactionSigned = provider
.transaction_by_id_unhashed(tx_id)?
.map(|tx| {
TransactionSigned::new_unhashed(tx.transaction, tx.signature)
})
.expect("no transaction entry");
let signer =
transaction.recover_signer().expect("failed to recover signer");
Expand Down
2 changes: 1 addition & 1 deletion crates/storage/provider/src/providers/static_file/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ mod tests {
writer.append_receipt(*next_tx_num, &receipt).unwrap();
} else {
// Used as ID for validation
tx.transaction.set_nonce(*next_tx_num);
tx.transaction_mut().set_nonce(*next_tx_num);
writer.append_transaction(*next_tx_num, &tx).unwrap();
}
*next_tx_num += 1;
Expand Down
2 changes: 1 addition & 1 deletion crates/transaction-pool/src/test_utils/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -913,7 +913,7 @@ impl TryFrom<Recovered<TransactionSigned>> for MockTransaction {
let size = transaction.size();

#[allow(unreachable_patterns)]
match transaction.transaction {
match transaction.into_transaction() {
Transaction::Legacy(TxLegacy {
chain_id,
nonce,
Expand Down
12 changes: 6 additions & 6 deletions crates/transaction-pool/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1326,7 +1326,7 @@ impl PoolTransaction for EthPooledTransaction {
///
/// This is also commonly referred to as the "Gas Fee Cap" (`GasFeeCap`).
fn max_fee_per_gas(&self) -> u128 {
self.transaction.transaction.max_fee_per_gas()
self.transaction.transaction().max_fee_per_gas()
}

fn access_list(&self) -> Option<&AccessList> {
Expand All @@ -1337,7 +1337,7 @@ impl PoolTransaction for EthPooledTransaction {
///
/// This will return `None` for non-EIP1559 transactions
fn max_priority_fee_per_gas(&self) -> Option<u128> {
self.transaction.transaction.max_priority_fee_per_gas()
self.transaction.transaction().max_priority_fee_per_gas()
}

fn max_fee_per_blob_gas(&self) -> Option<u128> {
Expand Down Expand Up @@ -1375,7 +1375,7 @@ impl PoolTransaction for EthPooledTransaction {

/// Returns a measurement of the heap usage of this type and all its internals.
fn size(&self) -> usize {
self.transaction.transaction.input().len()
self.transaction.transaction().input().len()
}

/// Returns the transaction type
Expand Down Expand Up @@ -1404,7 +1404,7 @@ impl EthPoolTransaction for EthPooledTransaction {
}

fn blob_count(&self) -> usize {
match &self.transaction.transaction {
match self.transaction.transaction() {
Transaction::Eip4844(tx) => tx.blob_versioned_hashes.len(),
_ => 0,
}
Expand Down Expand Up @@ -1437,14 +1437,14 @@ impl EthPoolTransaction for EthPooledTransaction {
sidecar: &BlobTransactionSidecar,
settings: &KzgSettings,
) -> Result<(), BlobTransactionValidationError> {
match &self.transaction.transaction {
match self.transaction.transaction() {
Transaction::Eip4844(tx) => tx.validate_blob(sidecar, settings),
_ => Err(BlobTransactionValidationError::NotBlobTransaction(self.tx_type())),
}
}

fn authorization_count(&self) -> usize {
match &self.transaction.transaction {
match self.transaction.transaction() {
Transaction::Eip7702(tx) => tx.authorization_list.len(),
_ => 0,
}
Expand Down
2 changes: 1 addition & 1 deletion testing/testing-utils/src/generators.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ pub fn random_block<R: Rng>(rng: &mut R, number: u64, block_params: BlockParams)
let tx_count = block_params.tx_count.unwrap_or_else(|| rng.gen::<u8>());
let transactions: Vec<TransactionSigned> =
(0..tx_count).map(|_| random_signed_tx(rng)).collect();
let total_gas = transactions.iter().fold(0, |sum, tx| sum + tx.transaction.gas_limit());
let total_gas = transactions.iter().fold(0, |sum, tx| sum + tx.transaction().gas_limit());

// Generate ommers
let ommers_count = block_params.ommers_count.unwrap_or_else(|| rng.gen_range(0..2));
Expand Down

0 comments on commit 2652ec8

Please sign in to comment.