chore: prep for switching AVM to checkpointed native trees - TS test cleanup and refactors #12023
+541
−457
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dbanks12
changed the title
dbanks12
commented
Feb 18, 2025
| @@ -58,9 +40,10 @@ describe('AVM WitGen & Circuit – check circuit', () => { | |||
| }, | |||
| TIMEOUT, | |||
| ); | |||
| // FIXME(dbanks12): fails with "Lookup PERM_MAIN_ALU failed." | |||
There was a problem hiding this comment.
This comment was accidentally removed in a previous PR
dbanks12
commented
Feb 18, 2025
| std::hash::sha256(data) | ||
| sha256::sha256_var(data, data.len() as u64) |
There was a problem hiding this comment.
Created this fn last week and used the old stdlib sha
dbanks12
commented
Feb 18, 2025
Comment on lines
-403
to
-408
| #[public] | ||
| fn assert_timestamp(expected_timestamp: u64) { | ||
| let timestamp = context.timestamp(); | ||
| assert(timestamp == expected_timestamp, "timestamp does not match"); | ||
| } | ||
|
|
dbanks12
commented
Feb 18, 2025
Comment on lines
58
to
62
| const tx = await mockTx(1234, { | ||
| numberOfNonRevertiblePublicCallRequests: 2, | ||
| numberOfRevertiblePublicCallRequests: 1, | ||
| }); | ||
| tx.data.constants.historicalHeader = context.getBlockHeader(0); | ||
| tx.data.constants.vkTreeRoot = getVKTreeRoot(); |
There was a problem hiding this comment.
confirmed with @MirandaWood that this change should be okay
dbanks12
commented
Feb 18, 2025
| @@ -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); | |||
dbanks12
commented
Feb 18, 2025
Comment on lines
-86
to
-110
|
|
||
| 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); | ||
| } |
There was a problem hiding this comment.
these were duplicated from avm/fixtures
This stack of pull requests is managed by Graphite. Learn more about stacking. |
dbanks12
force-pushed
the
db/testing-updates
branch
from
5b9afbc to
27f2797
Compare
dbanks12
requested review from
jeanmon,
IlyasRidhuan,
Maddiaa0 and
charlielye
as code owners
dbanks12
force-pushed
the
db/testing-updates
branch
from
27f2797 to
5b9afbc
Compare
dbanks12
removed request for
fcarreiro,
charlielye,
IlyasRidhuan,
Maddiaa0 and
jeanmon
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
TestContextno 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:
orchestrator_public_functions.test.tsnow does this.orchestrator_multi_public_functions.test.tsnow does this.AVM & PublicProcessor testing
PublicTxSimulationTesterlets youcreateTxseparately fromsimulateTx(useful for testingPublicProcessor)PublicTxSimulatortest for avm bulk testPublicProcessortest of the token contract (construct, mint, many transfers)simulator/src/avmandsimulator/src/public*Testerclasses, and optional contract address nullifier insertionregister*()function instead of manually constructing contract classes and instancesMisc