chore: prep for switching AVM to checkpointed native trees - TS test cleanup and refactors #12023

dbanks12 · 2025-02-15T16:24:22Z

This PR is mostly setup for switching the AVM to use checkpointed native trees. Originally I had that in the same branch, but there were enough of these prerequisite changes to pull them out.

Orchestrator testing

prover-client's TestContext no longer mocks AVM simulation.

Context: while trying to transition the AVM to use native checkpointed trees, I ran into issues with the orchestrator public functions tests because the public processor will no longer update the trees for public functions. So, I think we should stop mocking the AVM simulation for these tests and let the AVM do its tree updates. We can then do two types of tests:

A quick test that doesn't actually deploy/run any real contract functions and instead tests the orchestrator with a mocked/garbage TX that has 1 revertible enqueued call. The AVM will fail to retrieve it's bytecode, but the TX can still be included. orchestrator_public_functions.test.ts now does this.
A real test that creates TXs that actually do a bunch of real token operations via non-revertible and revertible enqueued calls. orchestrator_multi_public_functions.test.ts now does this.

AVM & PublicProcessor testing

PublicTxSimulationTester lets you createTx separately from simulateTx (useful for testing PublicProcessor)
Add PublicTxSimulator test for avm bulk test
Add a PublicProcessor test of the token contract (construct, mint, many transfers)
Consolidate some duplicate helper functions in simulator/src/avm and simulator/src/public
Simplify process of registering/deploying contracts via avm *Tester classes, and optional contract address nullifier insertion
Contract Updates test now use the tester's register*() function instead of manually constructing contract classes and instances
Remove unused function from avm test contract

Misc

Making contract classes for tests is now deterministic (no more random public keys)
Use new/proper sha256 in benchmarking test contract

dbanks12 · 2025-02-18T20:01:11Z

yarn-project/bb-prover/src/avm_proving_tests/avm_check_circuit2.test.ts

@@ -58,9 +40,10 @@ describe('AVM WitGen & Circuit – check circuit', () => {
    },
    TIMEOUT,
  );
+  // FIXME(dbanks12): fails with "Lookup PERM_MAIN_ALU failed."


This comment was accidentally removed in a previous PR

dbanks12 · 2025-02-18T20:01:36Z

noir-projects/noir-contracts/contracts/benchmarking_contract/src/main.nr

-        std::hash::sha256(data)
+        sha256::sha256_var(data, data.len() as u64)


Created this fn last week and used the old stdlib sha

dbanks12 · 2025-02-18T20:01:43Z

noir-projects/noir-contracts/contracts/avm_test_contract/src/main.nr

-    #[public]
-    fn assert_timestamp(expected_timestamp: u64) {
-        let timestamp = context.timestamp();
-        assert(timestamp == expected_timestamp, "timestamp does not match");
-    }
-


dbanks12 · 2025-02-18T20:05:01Z

yarn-project/prover-client/src/orchestrator/orchestrator_public_functions.test.ts

      const tx = await mockTx(1234, {
-        numberOfNonRevertiblePublicCallRequests: 2,
+        numberOfRevertiblePublicCallRequests: 1,
      });
      tx.data.constants.historicalHeader = context.getBlockHeader(0);
      tx.data.constants.vkTreeRoot = getVKTreeRoot();


confirmed with @MirandaWood that this change should be okay

dbanks12 · 2025-02-18T20:05:10Z

yarn-project/simulator/src/avm/test_utils.ts

@@ -13,7 +13,6 @@ import { type PublicSideEffectTraceInterface } from '../public/side_effect_trace

 export async function mockGetBytecode(worldStateDB: WorldStateDB, bytecode: Buffer) {
  const commitment = await computePublicBytecodeCommitment(bytecode);
-  (worldStateDB as jest.Mocked<WorldStateDB>).getBytecode.mockResolvedValue(bytecode);


wasn't used

dbanks12 · 2025-02-18T20:05:37Z

yarn-project/simulator/src/public/fixtures/utils.ts

-
-export function getAvmTestContractFunctionSelector(functionName: string): Promise<FunctionSelector> {
-  const artifact = AvmTestContractArtifact.functions.find(f => f.name === functionName)!;
-  assert(!!artifact, `Function ${functionName} not found in AvmTestContractArtifact`);
-  const params = artifact.parameters;
-  return FunctionSelector.fromNameAndParameters(artifact.name, params);
-}
-
-export function getAvmTestContractArtifact(functionName: string): FunctionArtifact {
-  const artifact = AvmTestContractArtifact.functions.find(f => f.name === functionName)!;
-  assert(
-    !!artifact?.bytecode,
-    `No bytecode found for function ${functionName}. Try re-running bootstrap.sh on the repository root.`,
-  );
-  return artifact;
-}
-
-export function getAvmTestContractBytecode(functionName: string): Buffer {
-  const artifact = getAvmTestContractArtifact(functionName);
-  return artifact.bytecode;
-}
-
-export function getAvmTestContractPublicDispatchBytecode(): Buffer {
-  return getAvmTestContractBytecode(PUBLIC_DISPATCH_FN_NAME);
-}


these were duplicated from avm/fixtures

dbanks12 · 2025-02-18T20:06:38Z

This stack of pull requests is managed by Graphite. Learn more about stacking.

PhilWindle

This looks fine though I'm not familiar with most of the changes.

sirasistant

AVM related changes LGTM, only a small doubt regarding hrtime

sirasistant · 2025-02-19T10:51:38Z

yarn-project/simulator/src/public/public_tx_simulator.ts

@@ -132,6 +134,9 @@ export class PublicTxSimulator {
      tx.filterRevertedLogs(tx.data.forPublic!.nonRevertibleAccumulatedData);
    }

+    const endTime = process.hrtime.bigint();


I think the simulator package is going to be fully imported in a browser context (I don't think we have client/server separation in package.json entrypoints) so it's going to import this use of process. Maybe we are already polyfilling process?
We could just use performance.now like in the other tests, unless we need nanosecond precision

spalladino

LGTM! I glossed over the avm_proving_tests I'm not familiar with, but the rest looks ok.

Side note: I didn't know about the PublicSimulationTester capabilities to create valid txs, it's a really nice building block for testing! We need to make these more widely available.

dbanks12 added 10 commits February 15, 2025 16:23

chore: avm TS test cleanup and refactors

a0accc9

new test in orchestrator

33cf18b

Merge branch 'master' into db/testing-updates

20397fc

Merge branch 'master' into db/testing-updates

0e00834

fixes

caeec38

merge

504a63a

fixes

e6b5622

fix

b69a11d

more fixes

ba5fa4a

use new sha in benchmarking contract

8e5ac40

dbanks12 marked this pull request as ready for review February 18, 2025 17:34

dbanks12 requested a review from fcarreiro as a code owner February 18, 2025 17:34

dbanks12 added 4 commits February 18, 2025 18:09

Fixes

5f05e9f

fixes & cleanup

4a6972c

Merge branch 'master' into db/testing-updates

4c0c17f

fix noir contract

41c536b

dbanks12 requested review from spalladino and PhilWindle February 18, 2025 18:41

fix contract updates test

bc5806d

dbanks12 changed the title ~~chore: avm TS test cleanup and refactors~~ chore: prep for switching AVM to checkpointed native trees - TS test cleanup and refactors Feb 18, 2025

dbanks12 requested a review from sirasistant February 18, 2025 19:49

dbanks12 commented Feb 18, 2025

View reviewed changes

revert incorrect type change

6376465

fmt

5b9afbc

dbanks12 force-pushed the db/testing-updates branch from 5b9afbc to 27f2797 Compare February 18, 2025 20:19

dbanks12 requested review from jeanmon, IlyasRidhuan, Maddiaa0 and charlielye as code owners February 18, 2025 20:19

dbanks12 force-pushed the db/testing-updates branch from 27f2797 to 5b9afbc Compare February 18, 2025 20:22

dbanks12 added 2 commits February 18, 2025 20:24

Merge branch 'master' into db/testing-updates

e1adf6a

merge

0fe5ec2

dbanks12 removed request for fcarreiro, charlielye, IlyasRidhuan, Maddiaa0 and jeanmon February 19, 2025 03:08

PhilWindle reviewed Feb 19, 2025

View reviewed changes

sirasistant approved these changes Feb 19, 2025

View reviewed changes

spalladino reviewed Feb 19, 2025

View reviewed changes

fix feePayer

c1e8993

dbanks12 enabled auto-merge (squash) February 19, 2025 16:51

dbanks12 disabled auto-merge February 19, 2025 16:58

merge

a1758ed

dbanks12 enabled auto-merge (squash) February 19, 2025 20:01

dbanks12 merged commit c1cc3ed into master Feb 19, 2025
13 checks passed

dbanks12 deleted the db/testing-updates branch February 19, 2025 20:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: prep for switching AVM to checkpointed native trees - TS test cleanup and refactors #12023

chore: prep for switching AVM to checkpointed native trees - TS test cleanup and refactors #12023

dbanks12 commented Feb 15, 2025 •

edited

Loading

dbanks12 Feb 18, 2025

dbanks12 Feb 18, 2025

dbanks12 Feb 18, 2025

dbanks12 Feb 18, 2025

dbanks12 Feb 18, 2025

dbanks12 Feb 18, 2025

dbanks12 commented Feb 18, 2025

PhilWindle left a comment

sirasistant left a comment •

edited

Loading

sirasistant Feb 19, 2025

spalladino left a comment

		std::hash::sha256(data)
		sha256::sha256_var(data, data.len() as u64)

chore: prep for switching AVM to checkpointed native trees - TS test cleanup and refactors #12023

chore: prep for switching AVM to checkpointed native trees - TS test cleanup and refactors #12023

Conversation

dbanks12 commented Feb 15, 2025 • edited Loading

Orchestrator testing

AVM & PublicProcessor testing

Misc

dbanks12 Feb 18, 2025

Choose a reason for hiding this comment

dbanks12 Feb 18, 2025

Choose a reason for hiding this comment

dbanks12 Feb 18, 2025

Choose a reason for hiding this comment

dbanks12 Feb 18, 2025

Choose a reason for hiding this comment

dbanks12 Feb 18, 2025

Choose a reason for hiding this comment

dbanks12 Feb 18, 2025

Choose a reason for hiding this comment

dbanks12 commented Feb 18, 2025

PhilWindle left a comment

Choose a reason for hiding this comment

sirasistant left a comment • edited Loading

Choose a reason for hiding this comment

sirasistant Feb 19, 2025

Choose a reason for hiding this comment

spalladino left a comment

Choose a reason for hiding this comment

dbanks12 commented Feb 15, 2025 •

edited

Loading

sirasistant left a comment •

edited

Loading