Skip to content
This repository was archived by the owner on Feb 6, 2026. It is now read-only.

fix: support encodable receipt to use alloy calculate_receipt_root#74

Draft
pedrohba1 wants to merge 4 commits into
mainfrom
fix-calculate-receipt-root
Draft

fix: support encodable receipt to use alloy calculate_receipt_root#74
pedrohba1 wants to merge 4 commits into
mainfrom
fix-calculate-receipt-root

Conversation

@pedrohba1

Copy link
Copy Markdown
Contributor

Example now works from this. Although, this leaves state_root unused. Should I just remove the unused code in this case?

@suchapalaver suchapalaver left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@pedrohba1 see my suggestions. But also, let's revert the original change in another PR before we do this, as discussed 🙏

Comment thread crates/firehose-protos-examples/examples/receipt_root.rs
Comment thread crates/firehose-protos/src/ethereum_v2/eth_block.rs Outdated
Comment thread crates/firehose-protos/src/ethereum_v2/eth_block.rs Outdated
}

impl Typed2718 for FullReceipt {
fn ty(&self) -> u8 {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please give this a more meaningful name 🙏

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This one I unfortunately cannot change the name. I need this ty function from Typed2718 from alloy-eip. Looks weird, but alloy-eip does call a type ty , which is quite annying that now we have tx_type , r#type and ty all in 3 different external crates which means the same in all.
On our codebase though it seems to be always tx_type, I'm good with it.

I could do this though:

impl FullReceipt {
    pub fn transaction_type(&self) -> u8 {
        self.ty() // Calls the `Typed2718` implementation
    }
}

impl Typed2718 for FullReceipt {
    fn ty(&self) -> u8 {
        self.tx_type.ty()
    }
}

But I don't think it is necessary since we can already use full_receipt.tx_type the way the code is in the way it is implemented.

@pedrohba1

Copy link
Copy Markdown
Contributor Author

@pedrohba1 see my suggestions. But also, let's revert the original change in another PR before we do this, as discussed 🙏

This is the PR attempting to the original change. it does revert the please-release merge, I ask for some attention into reading the PR, because I proposed keeping some of the changes in another PR linked there.

@suchapalaver
suchapalaver marked this pull request as draft February 17, 2025 15:29
@suchapalaver

Copy link
Copy Markdown
Contributor

@pedrohba1 converting this to draft while there are still CI failures 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants