Skip to content

Conversation

@lucca30
Copy link
Contributor

@lucca30 lucca30 commented Aug 27, 2025

Description

Implements Canonical Inclusion of StateSync Transactions in Block Bodies.
This change introduces a new typed system transaction (StateSyncTx) appended to blocks that execute StateSync events. The transaction itself has zero gas/fees and does not run EVM code, but anchors all StateSync outcomes into the canonical transaction/receipt set.

Key Effects

  • Commits StateSync membership and results to transactionsRoot, receiptsRoot, and logsBloom.
  • Provides canonical receipts and logs for observability (PIP-20) and replay semantics (PIP-36).
  • Enables trustless snap-sync and consistent validation of StateSync execution.
  • Requires a hard fork, as block roots and hashing change post-fork.

This proposal improves proofs, observability, and simplicity while maintaining economic neutrality and execution semantics.

Changes

  • Bugfix (non-breaking change that solves an issue)
  • Hotfix (change that solves an urgent issue, and requires immediate attention)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (change that is not backwards-compatible and/or changes current functionality)
  • Changes only for a subset of nodes

Breaking changes

It's a hardfork, so it demands stablishing a block where it starts.

@lucca30 lucca30 marked this pull request as ready for review September 29, 2025 17:11
@codecov
Copy link

codecov bot commented Oct 9, 2025

Codecov Report

❌ Patch coverage is 31.63265% with 134 lines in your changes missing coverage. Please review.
✅ Project coverage is 47.70%. Comparing base (c1e1773) to head (0b556fa).
⚠️ Report is 293 commits behind head on develop.

Files with missing lines Patch % Lines
consensus/bor/bor.go 10.00% 52 Missing and 2 partials ⚠️
core/types/tx_state_sync.go 51.61% 26 Missing and 4 partials ⚠️
core/parallel_state_processor.go 0.00% 12 Missing ⚠️
consensus/beacon/consensus.go 0.00% 11 Missing ⚠️
consensus/ethash/consensus.go 0.00% 6 Missing ⚠️
core/state_processor.go 58.33% 3 Missing and 2 partials ⚠️
core/blockchain.go 0.00% 1 Missing and 2 partials ⚠️
core/chain_makers.go 50.00% 3 Missing ⚠️
core/types/transaction_signing.go 0.00% 2 Missing and 1 partial ⚠️
core/state/statedb_hooked.go 0.00% 2 Missing ⚠️
... and 3 more
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1726      +/-   ##
===========================================
- Coverage    48.19%   47.70%   -0.49%     
===========================================
  Files          827      840      +13     
  Lines       135906   142707    +6801     
===========================================
+ Hits         65497    68076    +2579     
- Misses       66182    70181    +3999     
- Partials      4227     4450     +223     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@manav2401
Copy link
Member

The core logic LGTM. There are a few places where we might have to make changes.

  1. There are certain RPC methods which has custom logic for handling bor events. After this PR, we will have to maintain 2 separate logic (the post HF one would be very simple).
  2. Right now we use a separate transaction lookup which maps a transaction hash to bor receipt. That logic won't be used after HF. We need to confirm if queries done via tx hash works correctly or not (e.g. getTransactionReceipt by transaction hash). Ideally they should as the lookup entries are updated by iterating over block.Transactions which already has state-sync transactions.
  3. There will be some issues w/ snap sync post this HF as my changes related to eth/69 are merged. For e.g. right now, we exclude state-sync receipt from receipt root calculation but after the HF, we should be including it. If you want, I can take a look into this as I am mostly familiar with where the changes would be.
    Thanks!

@lucca30
Copy link
Contributor Author

lucca30 commented Oct 9, 2025

Thanks for the comments @manav2401

  1. There are certain RPC methods which has custom logic for handling bor events. After this PR, we will have to maintain 2 separate logic (the post HF one would be very simple).

I think I already checked all of them and just eth_getTransactionByHash showed an issue but not directly related to it. But it's crucial checking all of them specially on e2e.

  1. Right now we use a separate transaction lookup which maps a transaction hash to bor receipt. That logic won't be used after HF. We need to confirm if queries done via tx hash works correctly or not (e.g. getTransactionReceipt by transaction hash). Ideally they should as the lookup entries are updated by iterating over block.Transactions which already has state-sync transactions.
    Yeah, they not impact
  1. There will be some issues w/ snap sync post this HF as my changes related to eth/69 are merged. For e.g. right now, we exclude state-sync receipt from receipt root calculation but after the HF, we should be including it. If you want, I can take a look into this as I am mostly familiar with where the changes would be.

Yes, it will demands a HF check. As soon as one of the PR's got merged we can fix on the other one.

kamuikatsurgi and others added 2 commits October 17, 2025 17:42
* eth: include bor receipts in ReceiptHash post HF

* eth: rename to receiptListHash

* eth: extrat typecasting logic for better testing, fix type matching issue

* eth: add e2e tests for receipt delivery

* eth/protocols/eth: apply HF logic while handling receipt query over eth69

* core: skip split receipts post HF

* tests/bor: extend e2e test to check presence of state-sync in block
@manav2401
Copy link
Member

The core logic LGTM. There are a few places where we might have to make changes.

  1. There are certain RPC methods which has custom logic for handling bor events. After this PR, we will have to maintain 2 separate logic (the post HF one would be very simple).
  2. Right now we use a separate transaction lookup which maps a transaction hash to bor receipt. That logic won't be used after HF. We need to confirm if queries done via tx hash works correctly or not (e.g. getTransactionReceipt by transaction hash). Ideally they should as the lookup entries are updated by iterating over block.Transactions which already has state-sync transactions.
  3. There will be some issues w/ snap sync post this HF as my changes related to eth/69 are merged. For e.g. right now, we exclude state-sync receipt from receipt root calculation but after the HF, we should be including it. If you want, I can take a look into this as I am mostly familiar with where the changes would be.
    Thanks!

All 3 things seems to handled. First one is pending to be merged and needs some e2e and devnet testing.

manav2401 and others added 5 commits October 21, 2025 20:32
* fix: append tx only in FinalizeAndAssemble and use state instead of wrappedState

* consensus/bor: sort logs before extracting state-sync logs

* chore: revert statedb changes

---------

Co-authored-by: Manav Darji <[email protected]>
@marcello33 marcello33 requested a review from a team October 24, 2025 09:47
@kamuikatsurgi kamuikatsurgi requested a review from Copilot October 24, 2025 10:06
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements canonical inclusion of StateSync transactions in block bodies by introducing a new typed system transaction (StateSyncTx). After a hard fork activation, StateSync events are appended as transactions in block bodies, making them part of the canonical transaction/receipt sets. This allows StateSync outcomes to be committed to transactionsRoot, receiptsRoot, and logsBloom, improving observability, replay semantics, and enabling trustless snap-sync.

Key changes:

  • Introduces StateSyncTxType (0x7f) and StateSyncTx transaction type that carries StateSync data
  • Modifies consensus engine methods to return updated receipts from Finalize and FinalizeAndAssemble
  • Updates P2P receipt handling to conditionally include/exclude StateSync receipts based on hard fork activation

Reviewed Changes

Copilot reviewed 34 out of 35 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
core/types/tx_state_sync.go Implements the new StateSyncTx transaction type with encoding/decoding logic
core/types/state_data.go Changes StateSyncData.Data from string to []byte for binary data handling
consensus/bor/bor.go Appends StateSyncTx to block body post-fork and generates corresponding receipts
consensus/consensus.go Updates Engine interface to return receipts from Finalize/FinalizeAndAssemble
eth/protocols/eth/handlers.go Adjusts P2P receipt serving to handle StateSync receipts based on fork activation
internal/ethapi/api.go Skips separate BorReceipt fetching post-fork since they're in normal receipts
params/config.go Adds StateSyncBlock configuration field for hard fork activation

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@kamuikatsurgi kamuikatsurgi changed the title StateSync Tx on Body PIP-74: state-sync txs inclusion Oct 27, 2025
@kamuikatsurgi kamuikatsurgi added the squash and merge This PR will be squashed and merged label Oct 27, 2025
@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
5.4% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@manav2401 manav2401 merged commit 37438e4 into develop Oct 30, 2025
8 of 13 checks passed
@kamuikatsurgi kamuikatsurgi deleted the lmartins/state-sync-txs-on-block-body branch October 30, 2025 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

squash and merge This PR will be squashed and merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants