-
Notifications
You must be signed in to change notification settings - Fork 124
Description
We have a test helper procedure called create_mock_notes_procedure(notes: &[Note]) -> String which returns a string of MASM code, defining a proc.create_mock_notes. It writes the provided notes directly into transaction kernel memory, which seems odd. We (now) have much better ways to create notes in the transaction kernel as @igamigo mentioned elsewhere, and I don't see the advantage of doing it this way.
We use this procedure in four tests:
test_epilogue: This test primarily tests that the epilogue finalizes the transaction correctly and produces the expected output stack state. The use ofcreate_mock_notesseems to be primarily so we can execute arbitrary code and don't have to run a full transaction. Imo, running a full transaction would give us better coverage as it would also test the transaction executor (i.e. how it extracts transaction outputs) and we could avoidcreate_mock_notes.test_compute_output_note_id: Tests that the kernel computes the output note ID correctly. This could be replaced by a full transaction as well, asserting thatexecuted_transaction.output_notes().commitment()matches the expectation. Again, better coverage and less low-level test setup.test_epilogue_asset_preservation_violation_too_few_inputandtest_epilogue_asset_preservation_violation_too_many_fungible_input:- Tests that the tx kernel asset preservation rules are working. Both of these could and should be tested with a full transaction, making sure that the preservation rules are enforced for regular transactions, not when executing arbitrary code, which does not reflect how end users call the kernel.
This should allow us to remove create_mock_notes.
The larger theme here is that I think we should execute full transactions whenever possible, especially when testing the behavior of the transaction kernel. Using execute_code is fine for unit-like tests (related #1513), e.g. checking that $kernel::account_id::validate fails on invalid account IDs (e.g. as done in test_account_validate_id).