Fix system transaction result indexing & event block/transaction indices #907
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jribbink
force-pushed
the
jribbink/fix-system-txns
branch
from
da93d56 to
1b8ed8e
Compare
jribbink
changed the title
jribbink
changed the title
jribbink
requested review from
chasefleming,
janezpodhostnik,
mfbz and
peterargue
as code owners
jribbink
added
the
Improvement
chasefleming
approved these changes
Nov 10, 2025
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR fixes inaccuracies in system transaction result indexing by introducing composite keys (blockID + txID) for storage and retrieval. This resolves issues where system transactions with duplicate IDs across blocks would overwrite each other's results. Additionally, the PR corrects event block/transaction indices to ensure each system transaction receives a unique sequential index.
Key changes:
- System transaction results now stored with composite keys (blockID, txID) instead of txID alone
- System transaction indices are now computed sequentially after user transactions
- Refactored
CommitBlockto separate system transactions from regular transactions in storage
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| storage/store.go | Added SystemTransactionResultByID method and composite key generation; refactored CommitBlock signature to accept separate system transaction parameters |
| storage/mocks/store.go | Updated mock methods to match new CommitBlock and SystemTransactionResultByID signatures |
| storage/memstore/memstore.go | Added in-memory storage for system transaction results with composite keys; updated CommitBlock implementation |
| storage/checkpoint/checkpoint.go | Updated CommitBlock call with additional nil parameters for system transactions |
| emulator/system_transaction_test.go | Added comprehensive tests verifying system transaction events and unique transaction indices |
| emulator/blockchain.go | Refactored system transaction execution to assign correct sequential indices; updated result retrieval to use composite keys |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
6 tasks
#911) * Initial plan * Improve comment clarity for system chunk transaction index calculation Co-authored-by: jribbink <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: jribbink <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #903
Description
Fixes inaccuracies in the scheduled/system tx result indexing.
For contributor use:
masterbranchFiles changedin the GitHub PR explorer