Skip to content

Conversation

@bjartek
Copy link
Contributor

@bjartek bjartek commented Oct 30, 2025

Closes #887

Description

Trying to store system transactions for each blockID and then fetching them from the transaction/transactionResult store when needed.


For contributor use:

  • Targeted PR against master branch
  • Linked to GitHub issue with discussion and accepted design OR link to spec that describes this work
  • Code follows the standards mentioned here
  • Updated relevant documentation
  • Re-reviewed Files changed in the GitHub PR explorer
  • Added appropriate labels

@bjartek
Copy link
Contributor Author

bjartek commented Nov 5, 2025

Any update on this one?

@jribbink jribbink changed the title taking a stab at fixing system transactions Update the latest Scheduled Transactions API changes Nov 5, 2025
@codecov-commenter
Copy link

codecov-commenter commented Nov 5, 2025

@jribbink
Copy link
Contributor

jribbink commented Nov 5, 2025

Hey @bjartek , sorry about the delay. I've added the additional remaining changes for the Scheduled Transactions API to this PR. Does it look good to you?

@jribbink
Copy link
Contributor

jribbink commented Nov 5, 2025

I just ran this through the same FCL integration test suite that I tested @peterargue's original flow-go changes against and everything worked.

@chasefleming chasefleming requested a review from Copilot November 5, 2025 23:52
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 updates the emulator to support scheduled transactions API by storing system transactions per block and providing retrieval methods via block ID and scheduled transaction ID. The implementation creates a global index mapping scheduled transaction IDs to their containing blocks.

Key Changes:

  • Added CollectionID field to transaction results to track which collection contains each transaction
  • Implemented system transaction storage and retrieval methods including scheduled transaction support
  • Updated test expectations to account for system transactions (system chunk and EVM) included in block transaction counts

Reviewed Changes

Copilot reviewed 18 out of 19 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
types/result.go Added CollectionID field to StorableTransactionResult
storage/store.go Added system transaction storage, scheduled transaction indexing methods, and SystemTransactions type
storage/store_test.go Added comprehensive tests for scheduled transaction indexing
storage/sqlite/createTables.sql Created database tables for system transactions and scheduled transaction index
storage/memstore/memstore.go Implemented in-memory storage for system transactions and scheduled transaction index
storage/encoding.go Added encoding/decoding functions for SystemTransactions
storage/mocks/store.go Updated mock store with new method signatures
emulator/blockchain.go Implemented system and scheduled transaction retrieval methods, updated block commit to store system transactions
emulator/transaction_test.go Updated test expectations to account for system transactions in block counts
emulator/system_transaction_test.go Added new tests for system transaction retrieval
emulator/emulator.go Added interface methods for system and scheduled transaction access
emulator/mocks/emulator.go Updated emulator mocks with new methods
convert/emu.go Added collectionID parameter to ToStorableResult function
adapters/access.go Implemented system and scheduled transaction access methods
internal/mocks/access.go Added mocks for scheduled transaction access methods
go.mod Updated flow-go and protobuf dependencies
storage/checkpoint/checkpoint.go Updated CommitBlock call with nil system transactions
adapters/streaming_integration_test.go Removed trailing whitespace

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jribbink jribbink enabled auto-merge (squash) November 6, 2025 00:32
@jribbink jribbink merged commit a539468 into onflow:master Nov 6, 2025
3 checks passed
@bjartek
Copy link
Contributor Author

bjartek commented Nov 6, 2025

was going to take a look at this now and see that it has been merged, i still struggle with migraines so energy comes sporadically

@jribbink
Copy link
Contributor

jribbink commented Nov 6, 2025

was going to take a look at this now and see that it has been merged, i still struggle with migraines so energy comes sporadically

No worries, I think it should be good, just wanted to double check though 👍

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.

GetTransactionsByBlockID is not working the same on testnet/mainnet and emulator

4 participants