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

Make ExecutionOutcome generic over receipt collection type #11108

Open
1 task done
Tracked by #11109 ...
emhane opened this issue Sep 22, 2024 · 6 comments · May be fixed by #12448
Open
1 task done
Tracked by #11109 ...

Make ExecutionOutcome generic over receipt collection type #11108

emhane opened this issue Sep 22, 2024 · 6 comments · May be fixed by #12448
Assignees
Labels
A-consensus Related to the consensus engine A-execution Related to the Execution and EVM A-op-reth Related to Optimism and op-reth A-sdk Related to reth's use as a library C-debt Refactor of code section that is hard to understand or maintain

Comments

@emhane
Copy link
Member

emhane commented Sep 22, 2024

Describe the feature

Make ExecutionOutcome generic over receipt collection. Require trait bound ExecutionOutcome<T: Receipts> in AutoSealBuilder, in order to remove optimism feature from the AutoSealBuilder. Somehow the ExecutionOutcome must be linked to the Executor so it can be customised at sdk level instead, probably requiring trait bound Executor<Output: Into<ExecutionOutcome<T: Receipts>>>.

The trait Receipts doesn't exist yet, and needs to be smthg like

pub trait Receipts {
    fn receipts_root_slow(&self) -> Result<Option<B256>, Self::Error>;
}

so the optimism receipts collection would have to look smthg like

struct OpReceipts<'a> {
    timestamp: u64,
    block_number: u64,
    chain_spec: &'a ChainSpec,
    receipts: Vec<ReceiptWithBloom<OpReceipt>>,
}

and simple ethereum type

struct Receipts {
    block_number: u64,
    receipts: Vec<ReceiptWithBloom<Receipt>>,
}

Additional context

No response

Tasks

  1. A-op-reth C-debt D-complex
    klkvr
@emhane emhane added C-debt Refactor of code section that is hard to understand or maintain A-execution Related to the Execution and EVM A-consensus Related to the consensus engine A-op-reth Related to Optimism and op-reth A-sdk Related to reth's use as a library labels Sep 22, 2024
@emhane emhane self-assigned this Sep 22, 2024
@emhane emhane changed the title Add trait method Executor::receipts_root_slow Make ExecutionOutcome generic over Receipt Sep 22, 2024
@emhane emhane changed the title Make ExecutionOutcome generic over Receipt Make ExecutionOutcome generic over receipt type Sep 22, 2024
@emhane emhane changed the title Make ExecutionOutcome generic over receipt type Make ExecutionOutcome generic over receipt collection type Sep 22, 2024
@emhane
Copy link
Member Author

emhane commented Sep 22, 2024

@garwahl , if this description makes sense to you, you're very welcome to implement it and close #10932

@garwahl
Copy link
Contributor

garwahl commented Sep 23, 2024

@emhane I'm sure it'll make more sense as I dig in 😅 I can give it a crack, I'm sure I'll have more questions as I go

@emhane emhane assigned garwahl and unassigned emhane Sep 23, 2024
@emhane
Copy link
Member Author

emhane commented Sep 25, 2024

hey @garwahl did you start with this? will scope this out into smaller issues otherwise.

would like to point out that the sketch of the solution in the description uses a vector of receipts with bloom, when it should actually be a vector of vectors of receipts (without bloom).

@garwahl
Copy link
Contributor

garwahl commented Sep 25, 2024

hey @garwahl did you start with this? will scope this out into smaller issues otherwise.

would like to point out that the sketch of the solution in the description uses a vector of receipts with bloom, when it should actually be a vector of vectors of receipts (without bloom).

Hey not yet, no worries feel free to break it up into smaller tasks and I'll jump on those

@greged93
Copy link
Contributor

greged93 commented Nov 3, 2024

@emhane is this still ongoing? Happy to take a look into it

@emhane
Copy link
Member Author

emhane commented Nov 4, 2024

will need @mattsse input here since he closed #11314 which solved this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-consensus Related to the consensus engine A-execution Related to the Execution and EVM A-op-reth Related to Optimism and op-reth A-sdk Related to reth's use as a library C-debt Refactor of code section that is hard to understand or maintain
Projects
Status: In Progress
3 participants