Further improvements to the mocking and testing tools

[igamigo]

Recently, various refactors of testing tools have taken place. Some of these were a consequence of external changes (to things such as miden-vm), and some were more related to making a more comprehensive set of tools that would allow for all types of tests: both high-level, more related to the domain entities of Miden, and "lower-level" scenarios such as testing internal kernel functions individually (which would not be possible from a user's perspective). A lot of these tests used very different, varied, ad-hoc sets of functions and tools to bootstrap testing code, probably as a consequence of many complementary pieces of code not being available at the time of writing them (and them not being updated after that).

As of right now, basically any test that implies code being executed as part of a transaction can get away with setting up a TransactionContext (which can act as a DataStore) in order to include any necessary entity. For building that context comprehensively, there is a builder struct, and also a MockChain struct. The idea of the MockChain struct is to provide a cohesive environment, where all mocked entities would make sense in a real blockchain scenario (the sequence of blocks is always valid and implies the validity of included notes and accounts).

All these changes were done mainly to accommodate the testing requirements on miden-base, but there are some remaining areas of improvement after the refactors.

Remaining improvements

Review the with_mocked_* Functions

The with_mocked_* functions should be reviewed and refactored. Some tests use specific environments, and to accommodate this, functions for creating notes with specific properties were added to TransactionContextBuilder. However, some of these functions are not currently being used (with_mock_notes_too_many_non_fungible_input), while others are used only in a single test (with_mock_notes_too_few_input and with_mock_notes_too_many_fungible_input). Only two of them are used across multiple tests. Additionally, they rely on specific, hardcoded entities. These should either be made more generally available or removed entirely. It may also be possible to improve NoteBuilder to make these functions unnecessary.

Implement a way to generate valid, random entities

It has come up a couple of times in the past (at least on the node and client) that it would make sense to be able to generate valid but random entities such as AccountId or TransactionId. This would allow for easier creation of valid test scenarios without requiring manually defined data.

Improve MockChain

The MockChain struct provides a valid environment against which transactions can be executed. It is versatile and useful for many testing scenarios, but it can be improved to be more widely usable, especially for external code beyond internal tests:

• Error handling and removing unwraps
  Right now, many operations within MockChain rely on unwraps, which should be replaced with proper error handling (related discussion).

• Simplifying data retrieval
  Alongside blockchain state trees (account, notes, and nullifiers), there are helper structs to retrieve the latest state of certain entities, such as accounts. While sufficient for basic testing, these helpers can fall short for more complex scenarios. For example, in the client a MockChain is used to create valid sync state responses. Scenarios like this often require access to complete note objects for specific blocks.

• Support for executing against arbitrary older blocks
  Currently, any transaction context built from a MockChain executes against the latest block. It would be beneficial to allow execution against older blocks. This would simplify testing scenarios such as P2IDR transactions.

• Revisit mutability
  Originally, the MockChain struct was conceived as a binary to precompute a scenario that could be serialized and reused (likely due to original PoW difficulty). It was later repurposed into its current form for testing by mocking a list of valid blocks. At present, it is a mutable struct where you can add entities and seal blocks. However, due to Miden's constraints, adding new entities dynamically in a way that makes sense is not straightforward - for example, you cannot obtain full Note objects from executed transactions because the VM does not necessarily retain all note details so having apply_executed_transaction is not necessarily useful. Perhaps a better way to go is to make MockChain not very mutable, and to be more in line with other pieces of testing code, have a builder struct that more declaratively expresses how the chain is composed.

I think some of the challenges with designing the testing framework come from the fact that it the process better driven by real testing scenarios. As such, as it is right now it serves a good purpose but if we want to make it better for external applications (say, for the Miden playground), improvements should also be discussed in those specific contexts.

[PhilippGackstatter]

Review the withmocked* Functions

Agreed that these should be refactored and some of them removed. I'm also not sure if the more heavily used ones like with_mock_notes_preserved need to be publicly exposed? Is it sufficient to have it only for our internal tests in miden-tx?

Implement a way to generate valid, random entities

Would AccountId::new_with_type_and_mode help here and then add an account with that ID to the MockChain, or is this about generating valid IDs in the context of the chain, i.e. add convenience functions to generate a valid account and transactions that directly exist in the chain and return their ID?

Error handling and removing unwraps

My understanding so far was that everything under testing is just for internal use and so we can expose functionality across crates without committing it to the public API. But since we want to make this usable for third partys as well, then we should indeed elevate the standards to use proper errors and error messages, and think more carefully about which APIs to make public (e.g. all the with_mock_* functions).

I like your suggestions, but I also haven't used MockChain enough to provide good insights. Perhaps the Account ID refactor will make me spend more time with it and if I notice any potential improvement, I'll post it here.