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

Includes baseFee in receipt of RPC response #347

Merged
merged 10 commits into from
Apr 1, 2025

Conversation

Kourin1996
Copy link

@Kourin1996 Kourin1996 commented Mar 28, 2025

Closes https://github.com/celo-org/celo-blockchain-planning/issues/956
Closes #350

This PR adds baseFee field for CeloDynamicFeeTxV2 receipt because it's necessary for calculating receipt root remotely.

How to check

  1. Run op-geth locally with the change of this PR

  2. Switch to alecps/receiptRootCheck for running test code

  3. Copy core/types/receipt.go and core/types/gen_receipt_json.go in this PR into the branch (This is required for mapping JSON-RPC response properly in test code)

  4. Run go run ./receipt_root_test --debug <Local RPC URL> 31123401 and make sure it passes

How to test

I added a new script, compat_test/hash_consistency_test.go, which calculates BlockHash, TxHash, TxRoot, ReceiptRoot, and WithdrawalsRoot across multiple blocks starting from a specified height.

I ran a local op-geth node with the changes from the PR forAlfajores testnet and confirmed that the BlockHash, TxHash, TxRoot, ReceiptRoot, and WithdrawalsRoot of all L2 blocks were properly calculated based on RPC response locally

$ go test -v ./compat_test -tags compat_test -run TestHashConsistency -timeout 1h -op-geth-url ws://localhost:9994 --start-block 31056500 --block-interval 1

@Kourin1996 Kourin1996 requested review from palango and piersy March 28, 2025 13:00
@Kourin1996 Kourin1996 self-assigned this Mar 28, 2025
@palango
Copy link
Collaborator

palango commented Mar 28, 2025

@Kourin1996 This looks promising. Could you add a test case or extend an existing one (celo_api_test.go looks promising)

@@ -1722,6 +1722,11 @@ func marshalReceipt(receipt *types.Receipt, blockHash common.Hash, blockNumber u
if receipt.ContractAddress != (common.Address{}) {
fields["contractAddress"] = receipt.ContractAddress
}

if tx.Type() == types.CeloDynamicFeeTxV2Type {
fields["baseFee"] = (*hexutil.Big)(receipt.BaseFee)
Copy link

Choose a reason for hiding this comment

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

CeloDyamicFeeV2Txs ocurring before the L2 start won't have the BaseFee field on the receipt, so what happens here when receipt.BaseFee is nil?

Copy link
Author

Choose a reason for hiding this comment

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

it just sets null to baseFee. But added codes to omit the field if the tx type is CeloDynamicFeeTxV2 and the tx is for Celo1

@Kourin1996 Kourin1996 changed the title Includes baseFee in JSON-formatted receipt Includes baseFee in receipt of RPC response Mar 31, 2025
@Kourin1996
Copy link
Author

I tested all L2 blocks on mainnet and hashes can be calculated correctly from RPC response.

Copy link

@piersy piersy left a comment

Choose a reason for hiding this comment

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

Looks good, thanks @Kourin1996 !

@Kourin1996 Kourin1996 merged commit f17d355 into celo-rebase-12 Apr 1, 2025
7 checks passed
@Kourin1996 Kourin1996 deleted the Kourin1996/exports-base-fee-in-receipt branch April 1, 2025 09:21
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.

Recreating RLP receipts from RPC response misses baseFee
3 participants