Skip to content

feat: store Receipts#313

Merged
mpaulucci merged 20 commits into
mainfrom
get-block-receipts
Sep 3, 2024
Merged

feat: store Receipts#313
mpaulucci merged 20 commits into
mainfrom
get-block-receipts

Conversation

@avilagaston9

@avilagaston9 avilagaston9 commented Aug 28, 2024

Copy link
Copy Markdown
Contributor

Motivation

After executing all block transactions, we need to store the resulting Receipts.

Description

  • Creates a Receipt from the ExecutionResult when executing a transaction. This includes handling logs and Bloom computation.
  • Returns Vec<Receipt> as the result of execute_block in the evm crate.
  • Checks the gas_used against the BlockHeader.
  • Stores the Receipts in the db.
  • Adds some changes for testing with Hive:
    • Moves RpcReceipt to the Rpc crate.
    • Calculates blob_gas_price from the parent_header.
    • Fixes other formatting issues.

Closes #271
Closes #276

@avilagaston9 avilagaston9 changed the title fix: eth_getBlockReceipts endpoint feat: store Receipts Sep 2, 2024
@avilagaston9 avilagaston9 marked this pull request as ready for review September 2, 2024 16:09
@avilagaston9 avilagaston9 requested a review from a team as a code owner September 2, 2024 16:09
&& gas_limit >= GAS_LIMIT_MINIMUM
}

// Calculates the base fee per blob gas for the current block based on it's parent excess blob gas

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

init: t's weird that all these helper functions are here (not only the ones you added)

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.

I agree. Maybe we need a core::utils module to hold this logic?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I wouldnt use a generic utils module. Maybe gas_helpers or 4844_helpers? We can also leave them here for now until we think of a better place

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.

I have created #324 for this.

Comment thread crates/core/types/receipt.rs Outdated
#[serde(rename = "type")]
pub tx_type: TxType,
#[serde(with = "crate::serde_utils::bool")]
#[serde(with = "crate::serde_utils::bool", rename = "status")]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I wonder if we should do all this rpc specific stuff inside the rpc crate (add a RpcReceipt type or something) and implement from/into to map to the "core" Receipt. Not in scope for this PR though. WDYT?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ah, I see there is one further down. Why does this one have the serde implementations?

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.

The RpcReceipt is a wrapper around this Receipt and some metadata:

pub struct RpcReceipt {
    pub receipt: Receipt,
    pub tx_info: RpcReceiptTxInfo,
    pub block_info: RpcReceiptBlockInfo,
    ...
}

Should I unwrap the Receipt and store its fields directly?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Maybe you could create a type like ReceiptInfo inside RPC that has the serde attributes. And then you can do a from/into the core Receipt. So that we have the Rpc stuff separated from the core stuff. Thoughts?

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.

Agreed!

@mpaulucci mpaulucci added this pull request to the merge queue Sep 3, 2024
Merged via the queue into main with commit 649dd6f Sep 3, 2024
@mpaulucci mpaulucci deleted the get-block-receipts branch September 3, 2024 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hive Testing: eth_getTransactionReceipt Hive Testing: eth_getBlockReceipts

2 participants