feat: separate event handling into data extraction and handling

[PhilippGackstatter]

Until now, we basically had the following event handling setup:

• From exec or prover host, call into the base host to handle a transaction event.
• Base host extracts the event's data and either handles it or returns TransactionEventHandling::Unhandled with the data for the executor or prover host to further handle it.

The change in this PR is to:

• From exec or prover host, extract the data necessary for handling an event into a TransactionEvent.
• In each host, handle an event by delegating to the base host for shared functionality (e.g. account delta updates) or by doing additional logic like requesting data from the DataStore.

Overall, this second approach appears cleaner because data extraction and handling logic are separated. It comes at the cost of a bit more code duplication in the sense that both exec and prover host match on every single TransactionEvent, but this also provides the increase in clarity since events are essentially handled in a single place.

Changes

• A way to refer to an individual transaction event is still useful and so I did not remove the current TransactionEvent but rather renamed it to TransactionEventId.
  • This will probably also integrate nicer with Use EventName for TransactionEvent #2043.
  • This is useful for the MockHost for instance (see crates/miden-testing/src/mock_host.rs).
• Introduces TransactionEvent in miden-tx to contain the data extract from the stack or advice provider for handling it in the executor or prover host.
  • This is currently an internal type, because it doesn't need to be public and is quite low-level.
  • This must be in miden-tx because it relies on TransactionKernelProcess, TransactionKernelError and LinkMap, all of which are in miden-tx. It may be possible to move all these to miden-lib but not sure there's a benefit.
• Much of this PR is just code that moves, because the base host methods that previously extracted and handled events are now split.

Implementation Notes

• Some TransactionEventIds are mapped to the same TransactionEvent, e.g. AccountVaultBeforeGetBalance, AccountVaultBeforeHasNonFungibleAsset, AccountVaultBeforeAddAsset , AccountVaultBeforeRemoveAsset are all mapped to TransactionEvent::AccountVaultBeforeAssetAccess.
• Some events required more than just data extraction from the ProcessState, such as checking whether a merkle path is already present in the advice provider. At the same time, TransactionEvent::extract_from_process does not have access to the base host and so it cannot run the same logic as the base host did before, which is why some odd fields are necessary such as is_witness_present in TransactionEvent::AccountVaultBeforeAssetAccess.
• Porting most events was straightforward with the exception of these that were refactored more heavily and would be good to review more carefully:
  • NoteAfterCreated
  • AuthRequest and Unauthorized
  • AccountPushProcedureIndex
  • The other events should have been split into data extraction and handling logic with fairly minimal changes.
• Deliberately includes a &ProcessState into the returned Future from on_event in the executor host. This is not technically necessary, but I wonder if we should try to see if this works with building the web client. If it doesn't, it would be good to have a check/test in miden-base that alerts us if we do this anyway.
• TransactionProgress is only tracked in the executor host as the prover host doesn't do anything useful with it anyway and so we can save a few lines of event handling code. Totally possible to do this anyway, though.

closes #1709

[Copilot]

Pull Request Overview

This PR refactors the transaction event handling system by renaming TransactionEvent to TransactionEventId and introducing a new internal TransactionEvent enum that encapsulates both the event type and its associated data. This separation improves code organization by splitting event data extraction from event handling logic.

Key changes:

• Renamed TransactionEvent enum to TransactionEventId to represent event identifiers
• Created new TransactionEvent enum in tx_event.rs that combines event IDs with their data
• Refactored event handlers to accept extracted data as parameters instead of reading from ProcessState
• Simplified return types in LinkMap event handlers by removing unnecessary Result wrapper

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
crates/miden-tx/src/host/tx_event.rs	New file containing TransactionEvent enum with data extraction logic from process state
crates/miden-tx/src/prover/prover_host.rs	Updated to use new event handling pattern with extracted data
crates/miden-tx/src/host/mod.rs	Refactored event handlers to accept pre-extracted data as parameters
crates/miden-tx/src/executor/exec_host.rs	Updated async event handling to use new pattern with data extraction
crates/miden-tx/src/host/link_map.rs	Simplified return types by removing unnecessary Result wrappers
crates/miden-tx/src/host/kernel_process.rs	Added new helper methods for account code commitment and merkle path checking
crates/miden-tx/src/host/account_procedures.rs	Simplified API to accept pre-extracted data instead of ProcessState
crates/miden-lib/src/transaction/tx_event_id.rs	Renamed from events.rs, contains TransactionEventId enum
crates/miden-lib/src/transaction/mod.rs	Updated exports to reflect TransactionEventId rename
crates/miden-testing/src/mock_host.rs	Updated to use TransactionEventId instead of TransactionEvent
crates/miden-tx/src/prover/mod.rs	Updated destructuring of into_parts() return value
crates/miden-tx/src/errors/mod.rs	Changed MissingNote error type from u64 to usize
CHANGELOG.md	Added entry documenting the breaking change

---

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

[bobbinth]

Looks great! Thank you! I left some comments inline.

[bobbinth]

I would try to move it both the handling of stdlib events and TransactionEvent construction out of the async context. It will be a bit more code - but I think it is conceptually cleaner this way:

1. First we handle stdlib events.
2. Then we read w/e data we need from the process/host.
3. And only then we enter async context to make any required async requests.

[PhilippGackstatter]

I moved it outside so you can see what it looks like, but because we cannot return anything outside the async block, this is not as pretty overall.

[bobbinth]

Yes, makes sense - but I'd still probably go with this approach (the additional code complexity is pretty small IMO).

[bobbinth]

Looks good! Thank you! I left a few more small comments inline.

[bobbinth]

All looks good! Thank you!