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: replace receipt envelope encoded with trait #11742

Open
wants to merge 22 commits into
base: main
Choose a base branch
from

Conversation

hoank101
Copy link

@hoank101 hoank101 commented Oct 15, 2024

Closed #11737

@hoank101 hoank101 marked this pull request as draft October 15, 2024 09:48
crates/primitives/src/receipt.rs Outdated Show resolved Hide resolved
@hoank101 hoank101 marked this pull request as ready for review October 15, 2024 10:26
@hoank101 hoank101 requested a review from emhane as a code owner October 15, 2024 10:26
@hoank101 hoank101 requested a review from mattsse October 15, 2024 10:26
@hoank101
Copy link
Author

@mattsse @onbjerg can you help me review this PR?

crates/primitives/src/receipt.rs Outdated Show resolved Hide resolved
@hoank101 hoank101 requested a review from mattsse October 16, 2024 05:11
crates/primitives/src/receipt.rs Outdated Show resolved Hide resolved
crates/primitives/src/receipt.rs Outdated Show resolved Hide resolved
crates/rpc/rpc/src/debug.rs Outdated Show resolved Hide resolved
crates/primitives/src/receipt.rs Outdated Show resolved Hide resolved
crates/primitives/src/receipt.rs Outdated Show resolved Hide resolved
@mattsse mattsse added C-debt Refactor of code section that is hard to understand or maintain A-sdk Related to reth's use as a library labels Oct 16, 2024
hoank101 and others added 4 commits October 16, 2024 16:38
Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>
Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>
Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>
@hoank101 hoank101 requested a review from mattsse October 16, 2024 09:47
crates/primitives/src/receipt.rs Outdated Show resolved Hide resolved
@mattsse
Copy link
Collaborator

mattsse commented Oct 16, 2024

what needs to be changed here @Rjected ?

I also don't fully know

@Rjected
Copy link
Member

Rjected commented Oct 16, 2024

I think an encode_2718_len method could be implemented on ReceiptWithBloomEncoder:

        let mut payload_len = self.receipt_length();
        // account for eip-2718 type prefix and set the list
        if !matches!(self.receipt.tx_type, TxType::Legacy) {
            // we do NOT include a string header for 2718 envelope encoded receipts, but we do need to include the 2718 type byte
            payload_len += 1;
        }

        payload_len

@hoank101 hoank101 requested a review from Rjected October 18, 2024 15:33
Copy link
Member

@Rjected Rjected left a comment

Choose a reason for hiding this comment

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

just some nits, otherwise this looks good, thanks for adding the tests!

crates/primitives/src/receipt.rs Outdated Show resolved Hide resolved
crates/primitives/src/receipt.rs Outdated Show resolved Hide resolved
hoank101 and others added 2 commits October 19, 2024 09:58
Co-authored-by: Dan Cline <6798349+Rjected@users.noreply.github.com>
@hoank101 hoank101 requested a review from Rjected October 19, 2024 03:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sdk Related to reth's use as a library C-debt Refactor of code section that is hard to understand or maintain
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace receipt envelope encoded with trait
4 participants