test: Add more high-level methods to MockChain
#807
Conversation
miden-tx/src/testing/mock_chain.rs
Outdated
| pub fn add_executed_transaction(&mut self, transaction: ExecutedTransaction) { | ||
| let mut account = transaction.initial_account().clone(); | ||
| account.apply_delta(transaction.account_delta()).unwrap(); | ||
|
|
||
| // disregard private accounts, so it's easier to retrieve data | ||
| let account_update_details = AccountUpdateDetails::New(account.clone()); | ||
|
|
||
| let block_account_update = BlockAccountUpdate::new( | ||
| transaction.account_id(), | ||
| account.hash(), | ||
| account_update_details, | ||
| vec![transaction.id()], | ||
| ); | ||
| self.pending_objects.updated_accounts.push(block_account_update); | ||
|
|
||
| for note in transaction.input_notes().iter() { | ||
| // TODO: check that nullifiers are not duplicate | ||
| self.pending_objects.created_nullifiers.push(note.nullifier()); | ||
| self.removed_notes.push(note.id()); | ||
| } | ||
|
|
||
| // TODO: check that notes are not duplicate | ||
| let output_notes: Vec<OutputNote> = transaction.output_notes().iter().cloned().collect(); | ||
| self.pending_objects.output_note_batches.push(output_notes); | ||
| } |
There was a problem hiding this comment.
This method doesn't seems to be currently used - but it seems like it could be quite useful. Are there no tests where we could use it?
| pub struct MockChainBuilder { | ||
| accounts: Vec<Account>, | ||
| notes: Vec<Note>, | ||
| starting_block_num: u32, | ||
| } |
There was a problem hiding this comment.
The relation between MockChain and MockChain builder is a bit unclear to me. It probably makes sense to write some documentation about how these supposed to be used, but also, if we have a MockChainBuilder:
- Shouldn't all mutators on the
MockChainbe private to this module? - Does
MockChainneedpending_objectsfield?
Basically, if we have a builder we usually add things to the builder and then build an object that doesn't need to be mutated further - but here it seems like the pattern is a bit different?
In general, I like the idea of having a separate builder - but I think if we go this way, we should move things like add_nullifier(), add_new_wallet(), seal_block() etc. to be on the builder rather than on the MockChain.
There was a problem hiding this comment.
I think organization of this file could be improved a bit (i.e., by grouping related methods into sections and maybe splitting things out into several files under the same module).
Also, should improve documentation here quite a bit both to document individual functions but also to describe how these structs are expected to be used. But we can probably do this after we align on the high-level interfaces (e.g., see comment on MockChain vs. MockChainBuilder).
…den/miden-base into igamigo-hl-mockchain
…den/miden-base into igamigo-hl-mockchain
…gonMiden/miden-base into igamigo-hl-mockchain
…gonMiden/miden-base into igamigo-hl-mockchain
There was a problem hiding this comment.
Is this file used somewhere? If not, we should delete it.
There was a problem hiding this comment.
There was a problem hiding this comment.
Ah - I see! Let's then rename miden-tx/src/testing/tx_context.rs into miden-tx/src/testing/tx_context/mod.rs.
There was a problem hiding this comment.
Just saw this was already addressed. Sorry I missed it before!
Addresses #446
Closes #788