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

primitives: use alloy Receipts with Option #12162

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
5 changes: 4 additions & 1 deletion crates/blockchain-tree/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,10 @@ mod tests {
// Create a chain with the block and its receipts
let chain = AppendableChain::new(Chain::new(
vec![block.clone()],
ExecutionOutcome { receipts: receipts.clone().into(), ..Default::default() },
ExecutionOutcome {
receipts: receipts.clone().into_iter().map(Option::Some).collect::<Vec<_>>().into(),
tcoratger marked this conversation as resolved.
Show resolved Hide resolved
..Default::default()
},
Default::default(),
));

Expand Down
3 changes: 2 additions & 1 deletion crates/chain-state/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,8 @@ impl TestBlockBuilder {
Vec::new(),
);

execution_outcome.with_receipts(Receipts::from(receipts))
execution_outcome
.with_receipts(receipts.into_iter().map(Option::Some).collect::<Vec<_>>().into())
}
}
/// A test `ChainEventSubscriptions`
Expand Down
36 changes: 30 additions & 6 deletions crates/evm/execution-types/src/execution_outcome.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,10 +197,22 @@ impl ExecutionOutcome {
#[cfg(feature = "optimism")]
panic!("This should not be called in optimism mode. Use `optimism_receipts_root_slow` instead.");
#[cfg(not(feature = "optimism"))]
self.receipts.root_slow(
self.block_number_to_index(_block_number)?,
reth_primitives::proofs::calculate_receipt_root_no_memo,
)
{
// TODO: This is a temporary solution until we eliminate the `Option` from `Receipts`.
// We can then apply directly:
// ```rust
// self.receipts
// .receipt_vec
// .get(self.block_number_to_index(_block_number)?)
// .map(|receipts| reth_primitives::proofs::calculate_receipt_root_no_memo(receipts))
// ```
let receipts = self.receipts().receipt_vec
[self.block_number_to_index(_block_number)?]
.iter()
.map(Option::as_ref)
.collect::<Option<Vec<_>>>()?;
Some(reth_primitives::proofs::calculate_receipt_root_no_memo(receipts.as_slice()))
}
}

/// Returns the receipt root for all recorded receipts.
Expand All @@ -211,7 +223,19 @@ impl ExecutionOutcome {
block_number: BlockNumber,
f: impl FnOnce(&[&Receipt]) -> B256,
) -> Option<B256> {
self.receipts.root_slow(self.block_number_to_index(block_number)?, f)
// TODO: This is a temporary solution until we eliminate the `Option` from `Receipts`.
// We can then apply directly:
// ```rust
// self.receipts
// .receipt_vec
// .get(self.block_number_to_index(block_number)?)
// .map(|receipts| f(receipts))
// ```
let receipts = self.receipts.receipt_vec[self.block_number_to_index(block_number)?]
.iter()
.map(Option::as_ref)
.collect::<Option<Vec<_>>>()?;
Some(f(receipts.as_slice()))
}

/// Returns reference to receipts.
Expand Down Expand Up @@ -356,7 +380,7 @@ impl From<(BlockExecutionOutput<Receipt>, BlockNumber)> for ExecutionOutcome {
fn from(value: (BlockExecutionOutput<Receipt>, BlockNumber)) -> Self {
Self {
bundle: value.0.state,
receipts: Receipts::from(value.0.receipts),
receipts: value.0.receipts.into_iter().map(Some).collect::<Vec<_>>().into(),
Comment on lines -359 to +383
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this new syntax is more complicated, I'm missing the point of this change

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before we had this implemented in reth

impl From<Vec<Receipt>> for Receipts {
    fn from(block_receipts: Vec<Receipt>) -> Self {
        Self { receipt_vec: vec![block_receipts.into_iter().map(Option::Some).collect()] }
    }
}

but now the situation is more generic in alloy with a generic parameter T https://github.com/alloy-rs/alloy/blob/86812dba8507e6f7741f666303a53cafef93c8c1/crates/consensus/src/receipt/receipts.rs#L81 so that this is not available anymore. But this should just be a temporary situation since we would like to remove soon the Option<> in the definition of Receipts in reth after the merge of this PR and some follow up work

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have Receipts<T> in reth too now #12358, #12454

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The goal is was to use directly the alloy type, just importing it:

  1. Use the Receipts alloy type everywhere with pub type Receipts = alloy_consensus::Receipts<Option<Receipt>>;
  2. Do some work to arrive at pub type Receipts = alloy_consensus::Receipts<Receipt>; (especially on pruning which is blocker right now)

But if we want to integrate a new Receipts type into reth maybe this PR is not relevant anymore

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would hold off a bit with this pr until we have gotten further underway with #12454

first_block: value.1,
requests: vec![value.0.requests],
}
Expand Down
8 changes: 7 additions & 1 deletion crates/exex/exex/src/backfill/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,13 @@ pub(crate) fn to_execution_outcome(
) -> ExecutionOutcome {
ExecutionOutcome {
bundle: block_execution_output.state.clone(),
receipts: block_execution_output.receipts.clone().into(),
receipts: block_execution_output
.receipts
.clone()
.into_iter()
.map(Option::Some)
.collect::<Vec<_>>()
.into(),
first_block: block_number,
requests: vec![block_execution_output.requests.clone()],
}
Expand Down
58 changes: 3 additions & 55 deletions crates/primitives/src/receipt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ use alloy_consensus::constants::{
EIP1559_TX_TYPE_ID, EIP2930_TX_TYPE_ID, EIP4844_TX_TYPE_ID, EIP7702_TX_TYPE_ID,
};
use alloy_eips::eip2718::Encodable2718;
use alloy_primitives::{Bloom, Log, B256};
use alloy_primitives::{Bloom, Log};
use alloy_rlp::{length_of_length, Decodable, Encodable, RlpDecodable, RlpEncodable};
use bytes::{Buf, BufMut};
use core::{cmp::Ordering, ops::Deref};
use derive_more::{DerefMut, From, IntoIterator};
use derive_more::{From, IntoIterator};
#[cfg(feature = "reth-codec")]
use reth_codecs::Compact;
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -67,59 +67,7 @@ impl Receipt {
}

/// A collection of receipts organized as a two-dimensional vector.
#[derive(
Clone,
Debug,
PartialEq,
Eq,
Default,
Serialize,
Deserialize,
From,
derive_more::Deref,
DerefMut,
IntoIterator,
)]
pub struct Receipts {
/// A two-dimensional vector of optional `Receipt` instances.
pub receipt_vec: Vec<Vec<Option<Receipt>>>,
}

impl Receipts {
/// Returns the length of the `Receipts` vector.
pub fn len(&self) -> usize {
self.receipt_vec.len()
}

/// Returns `true` if the `Receipts` vector is empty.
pub fn is_empty(&self) -> bool {
self.receipt_vec.is_empty()
}

/// Push a new vector of receipts into the `Receipts` collection.
pub fn push(&mut self, receipts: Vec<Option<Receipt>>) {
self.receipt_vec.push(receipts);
}

/// Retrieves all recorded receipts from index and calculates the root using the given closure.
pub fn root_slow(&self, index: usize, f: impl FnOnce(&[&Receipt]) -> B256) -> Option<B256> {
let receipts =
self.receipt_vec[index].iter().map(Option::as_ref).collect::<Option<Vec<_>>>()?;
Some(f(receipts.as_slice()))
}
}

impl From<Vec<Receipt>> for Receipts {
fn from(block_receipts: Vec<Receipt>) -> Self {
Self { receipt_vec: vec![block_receipts.into_iter().map(Option::Some).collect()] }
}
}

impl FromIterator<Vec<Option<Receipt>>> for Receipts {
fn from_iter<I: IntoIterator<Item = Vec<Option<Receipt>>>>(iter: I) -> Self {
iter.into_iter().collect::<Vec<_>>().into()
}
}
pub type Receipts = alloy_consensus::Receipts<Option<Receipt>>;

impl From<Receipt> for ReceiptWithBloom {
fn from(receipt: Receipt) -> Self {
Expand Down
10 changes: 8 additions & 2 deletions crates/storage/provider/src/providers/blockchain_provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -921,8 +921,14 @@ mod tests {
.map(|block| {
let senders = block.senders().expect("failed to recover senders");
let block_receipts = receipts.get(block.number as usize).unwrap().clone();
let execution_outcome =
ExecutionOutcome { receipts: block_receipts.into(), ..Default::default() };
let execution_outcome = ExecutionOutcome {
receipts: block_receipts
.into_iter()
.map(Option::Some)
.collect::<Vec<_>>()
.into(),
..Default::default()
};

ExecutedBlock::new(
Arc::new(block.clone()),
Expand Down
Loading