Skip to content

Consider removing create_mock_notes #1845

@PhilippGackstatter

Description

@PhilippGackstatter

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 of create_mock_notes seems 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 avoid create_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 that executed_transaction.output_notes().commitment() matches the expectation. Again, better coverage and less low-level test setup.
  • test_epilogue_asset_preservation_violation_too_few_input and test_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).

Metadata

Metadata

Assignees

No one assigned

    Labels

    kernelsRelated to transaction, batch, or block kernelstestsImprovements to testing

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions