feat: Speed up transaction execution #10172

PhilWindle · 2024-11-24T23:15:44Z

This PR make a number of optimisation related to the speed up of transaction execution. Namely:

We don't re-initialise the instruction set mapping with each instruction decode.
We now compute public bytecode commitments at the point of receiving a contract and persist them, meaning they don't need to be computed at the point of execution.
We only store and iterate opcode and program counter tally information when in debug.
Function names are also cached at the point contract artifacts are shared with the node.
World state status summary and previous block archive roots are cached to reduce the impact on the world state DB whilst execution is taking place.

fcarreiro · 2024-11-25T09:54:48Z

yarn-project/circuits.js/src/contract/contract_class_id.ts

-    bytecodeCommitment = poseidon2Hash([encodedBytecode[i + 1], bytecodeCommitment]);
-  }
-  return bytecodeCommitment;
+  return bytecodeLength == 0 ? new Fr(0) : poseidon2HashAccumulate(encodedBytecode.slice(1, bytecodeLength + 1));


@IlyasRidhuan do we have any test (e.g. fixed hash) to know that this still works exactly as before?

Not directly - but it'll be covered in the avm_proving.test because the witgen will compute it in the loop and ASSERTs they are the same

fcarreiro · 2024-11-25T09:57:26Z

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

      assert(
        contractClass,
        `Contract class not found in DB, but a contract instance was found with this class ID (${instance.contractClassId}). This should not happen!`,
      );

+      assert(


@dbanks12 we'll have to add this to the list of errors to handle properly. (same with the contract class)

I don't think we want to handle this error. This means something is broken because the contract instance "exists", but it's contract class or bytecode are missing in the DB. So there is a bug somewhere.

fcarreiro · 2024-11-25T09:58:08Z

yarn-project/simulator/src/avm/serialization/instruction_serialization.ts

@@ -103,7 +103,7 @@ type OperandNativeType = number | bigint;
 type OperandWriter = (value: any) => void;

 // Specifies how to read and write each operand type.
-const OPERAND_SPEC = new Map<OperandType, [number, () => OperandNativeType, OperandWriter]>([
+const OPERAND_SPEC = new Map<OperandType, [number, (offset: number) => OperandNativeType, OperandWriter]>([


@PhilWindle does having the offset here help a lot?

We could probably remove this if we think this is undesirable. I flamegraphed the buffer/cursor stuff and creating subarrays for each instruction decode came out as the most costly part of this process. So I switched it to basically keep the same buffer with a single offset that is moved around. However, I later found that this buffer/cursor latency was marginal in the grand scheme of the decoding process.

No worries, it's ok if it helps.

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

fcarreiro

LGTM for the avm side. I'll let Palla aprove the archiver/db side.

spalladino

Looks good! There are some changes I'd push for, but they are not critical and can be addressed in another PR if you prefer.

spalladino · 2024-11-26T12:52:29Z

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

+    if (process.env.LOG_LEVEL === 'debug') {
+      this.tallyPrintFunction = this.printOpcodeTallies;
+      this.tallyInstructionFunction = this.tallyInstruction;
+    }


This will ignore namespaces set in the debug env var, but I can take care of it as part of the upcoming logs refactor.

spalladino · 2024-11-26T12:53:28Z

yarn-project/simulator/src/avm/serialization/buffer_cursor.ts

@@ -23,6 +27,11 @@ export class BufferCursor {
    assert(n < this._buffer.length);
  }

+  public peakUint8(): number {


Suggested change

public peakUint8(): number {

public peekUint8(): number {

spalladino · 2024-11-26T13:01:17Z

yarn-project/archiver/src/archiver/kv_archiver_store/kv_archiver_store.ts

@@ -46,6 +46,7 @@ export class KVArchiverDataStore implements ArchiverDataStore {
  #contractClassStore: ContractClassStore;
  #contractInstanceStore: ContractInstanceStore;
  #contractArtifactStore: ContractArtifactsStore;
+  private functionNames = new Map<string, string>();


Shouldn't this be persisted, as opposed to an in-memory Map? If we do it memory-only as a cache, then the getter should recalculate it on a miss.

As discussed. These are currently in memory only as they are a debugging feature that is perhaps not a full feature of the node or it's api. I've commented that this needs to be reviewed along with other similar features.

spalladino · 2024-11-26T13:04:55Z

yarn-project/archiver/src/archiver/kv_archiver_store/kv_archiver_store.ts

+    contract.functions.forEach(f => {
+      this.functionNames.set(FunctionSelector.fromNameAndParameters(f.name, f.parameters).toString(), f.name);
+    });


This will cause issues down the road with selector clashes. A selector is just 4 bytes (we're borrowing this from Ethereum), so it's relatively easy to stumble upon a collision. Indexing the functionNames collection by selector only will mean that the most recently stored selector will overwrite any previous ones.

Unfortunately, I think the only recourse is to store both contract class id and selector as key.

Fun fact: proxies built using the DELEGATECALL pattern in Ethereum were subject to a vulnerability caused by selector clashing.

https://forum.openzeppelin.com/t/beware-of-the-proxy-learn-how-to-exploit-function-clashing/1070

spalladino · 2024-11-26T13:06:33Z

yarn-project/archiver/src/archiver/archiver.ts

+  getBytecodeCommitment(contractClassId: Fr): Promise<Fr | undefined> {
+    return this.store.getBytecodeCommitment(contractClassId);
+  }


I don't think there are any cases where we want the bytecode commitment but not the bytecode, right? If so, instead of having a separate getter for the bytecode commitment, I'd just store it along with the rest of the contract class in the same store, so we don't have to query for them separately. But this is just a cleanup, no need to do now.

…kages into pw/check-times-2

As #10172 got merged right after #10176 it had not run the new test against the updated master so the issue with fee payment was not caught. To fix it, we fetch the current base fees and use those in our settings.

🤖 I have created a release *beep* *boop* --- <details><summary>aztec-package: 0.65.1</summary> ## [0.65.1](aztec-package-v0.65.0...aztec-package-v0.65.1) (2024-11-27) ### Miscellaneous * Delete old serialization methods ([#9951](#9951)) ([10d3f6f](10d3f6f)) </details> <details><summary>barretenberg.js: 0.65.1</summary> ## [0.65.1](barretenberg.js-v0.65.0...barretenberg.js-v0.65.1) (2024-11-27) ### Features * Speed up transaction execution ([#10172](#10172)) ([da265b6](da265b6)) ### Bug Fixes * Add pako as a dependency in bb.js ([#10186](#10186)) ([b773c14](b773c14)) </details> <details><summary>aztec-packages: 0.65.1</summary> ## [0.65.1](aztec-packages-v0.65.0...aztec-packages-v0.65.1) (2024-11-27) ### Features * Add total mana used to header ([#9868](#9868)) ([2478d19](2478d19)) * Assert metrics in network tests ([#10215](#10215)) ([9380c0f](9380c0f)) * Avm inserts nullifiers from private ([#10129](#10129)) ([3fc0c7c](3fc0c7c)) * Burn congestion fee ([#10231](#10231)) ([20a33f2](20a33f2)) * Configure world state block history ([#10216](#10216)) ([01eb392](01eb392)) * Integrate fee into rollup ([#10176](#10176)) ([12744d6](12744d6)) * Speed up transaction execution ([#10172](#10172)) ([da265b6](da265b6)) * Using current gas prices in cli-wallet ([#10105](#10105)) ([15ffeea](15ffeea)) ### Bug Fixes * Add pako as a dependency in bb.js ([#10186](#10186)) ([b773c14](b773c14)) * **avm:** Execution test ordering ([#10226](#10226)) ([49b4a6c](49b4a6c)) * Deploy preview master ([#10227](#10227)) ([321a175](321a175)) * Use current base fee for public fee payment ([#10230](#10230)) ([f081d80](f081d80)) ### Miscellaneous * Add traces and histograms to avm simulator ([#10233](#10233)) ([e83726d](e83726d)), closes [#10146](#10146) * Avm-proving and avm-integration tests do not require simulator to export function with jest mocks ([#10228](#10228)) ([f28fcdb](f28fcdb)) * **avm:** Handle parsing error ([#10203](#10203)) ([3c623fc](3c623fc)), closes [#9770](#9770) * **avm:** Zero initialization in avm public inputs and execution test fixes ([#10238](#10238)) ([0c7c4c9](0c7c4c9)) * Bump timeout for after-hook for data store test again ([#10240](#10240)) ([52047f0](52047f0)) * CIVC VK ([#10223](#10223)) ([089c34c](089c34c)) * Declare global types ([#10206](#10206)) ([7b2e343](7b2e343)) * Delete old serialization methods ([#9951](#9951)) ([10d3f6f](10d3f6f)) * Fix migration notes ([#10252](#10252)) ([05bdcd5](05bdcd5)) * Pull out some sync changes ([#10245](#10245)) ([1bfc15e](1bfc15e)) * Remove docs from sync ([#10241](#10241)) ([eeea0aa](eeea0aa)) * Replace relative paths to noir-protocol-circuits ([e7690ca](e7690ca)) </details> <details><summary>barretenberg: 0.65.1</summary> ## [0.65.1](barretenberg-v0.65.0...barretenberg-v0.65.1) (2024-11-27) ### Features * Add total mana used to header ([#9868](#9868)) ([2478d19](2478d19)) * Configure world state block history ([#10216](#10216)) ([01eb392](01eb392)) * Speed up transaction execution ([#10172](#10172)) ([da265b6](da265b6)) ### Bug Fixes * **avm:** Execution test ordering ([#10226](#10226)) ([49b4a6c](49b4a6c)) ### Miscellaneous * **avm:** Handle parsing error ([#10203](#10203)) ([3c623fc](3c623fc)), closes [#9770](#9770) * **avm:** Zero initialization in avm public inputs and execution test fixes ([#10238](#10238)) ([0c7c4c9](0c7c4c9)) * CIVC VK ([#10223](#10223)) ([089c34c](089c34c)) * Pull out some sync changes ([#10245](#10245)) ([1bfc15e](1bfc15e)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

🤖 I have created a release *beep* *boop* --- <details><summary>aztec-package: 0.65.1</summary> ## [0.65.1](AztecProtocol/aztec-packages@aztec-package-v0.65.0...aztec-package-v0.65.1) (2024-11-27) ### Miscellaneous * Delete old serialization methods ([#9951](AztecProtocol/aztec-packages#9951)) ([10d3f6f](AztecProtocol/aztec-packages@10d3f6f)) </details> <details><summary>barretenberg.js: 0.65.1</summary> ## [0.65.1](AztecProtocol/aztec-packages@barretenberg.js-v0.65.0...barretenberg.js-v0.65.1) (2024-11-27) ### Features * Speed up transaction execution ([#10172](AztecProtocol/aztec-packages#10172)) ([da265b6](AztecProtocol/aztec-packages@da265b6)) ### Bug Fixes * Add pako as a dependency in bb.js ([#10186](AztecProtocol/aztec-packages#10186)) ([b773c14](AztecProtocol/aztec-packages@b773c14)) </details> <details><summary>aztec-packages: 0.65.1</summary> ## [0.65.1](AztecProtocol/aztec-packages@aztec-packages-v0.65.0...aztec-packages-v0.65.1) (2024-11-27) ### Features * Add total mana used to header ([#9868](AztecProtocol/aztec-packages#9868)) ([2478d19](AztecProtocol/aztec-packages@2478d19)) * Assert metrics in network tests ([#10215](AztecProtocol/aztec-packages#10215)) ([9380c0f](AztecProtocol/aztec-packages@9380c0f)) * Avm inserts nullifiers from private ([#10129](AztecProtocol/aztec-packages#10129)) ([3fc0c7c](AztecProtocol/aztec-packages@3fc0c7c)) * Burn congestion fee ([#10231](AztecProtocol/aztec-packages#10231)) ([20a33f2](AztecProtocol/aztec-packages@20a33f2)) * Configure world state block history ([#10216](AztecProtocol/aztec-packages#10216)) ([01eb392](AztecProtocol/aztec-packages@01eb392)) * Integrate fee into rollup ([#10176](AztecProtocol/aztec-packages#10176)) ([12744d6](AztecProtocol/aztec-packages@12744d6)) * Speed up transaction execution ([#10172](AztecProtocol/aztec-packages#10172)) ([da265b6](AztecProtocol/aztec-packages@da265b6)) * Using current gas prices in cli-wallet ([#10105](AztecProtocol/aztec-packages#10105)) ([15ffeea](AztecProtocol/aztec-packages@15ffeea)) ### Bug Fixes * Add pako as a dependency in bb.js ([#10186](AztecProtocol/aztec-packages#10186)) ([b773c14](AztecProtocol/aztec-packages@b773c14)) * **avm:** Execution test ordering ([#10226](AztecProtocol/aztec-packages#10226)) ([49b4a6c](AztecProtocol/aztec-packages@49b4a6c)) * Deploy preview master ([#10227](AztecProtocol/aztec-packages#10227)) ([321a175](AztecProtocol/aztec-packages@321a175)) * Use current base fee for public fee payment ([#10230](AztecProtocol/aztec-packages#10230)) ([f081d80](AztecProtocol/aztec-packages@f081d80)) ### Miscellaneous * Add traces and histograms to avm simulator ([#10233](AztecProtocol/aztec-packages#10233)) ([e83726d](AztecProtocol/aztec-packages@e83726d)), closes [#10146](AztecProtocol/aztec-packages#10146) * Avm-proving and avm-integration tests do not require simulator to export function with jest mocks ([#10228](AztecProtocol/aztec-packages#10228)) ([f28fcdb](AztecProtocol/aztec-packages@f28fcdb)) * **avm:** Handle parsing error ([#10203](AztecProtocol/aztec-packages#10203)) ([3c623fc](AztecProtocol/aztec-packages@3c623fc)), closes [#9770](AztecProtocol/aztec-packages#9770) * **avm:** Zero initialization in avm public inputs and execution test fixes ([#10238](AztecProtocol/aztec-packages#10238)) ([0c7c4c9](AztecProtocol/aztec-packages@0c7c4c9)) * Bump timeout for after-hook for data store test again ([#10240](AztecProtocol/aztec-packages#10240)) ([52047f0](AztecProtocol/aztec-packages@52047f0)) * CIVC VK ([#10223](AztecProtocol/aztec-packages#10223)) ([089c34c](AztecProtocol/aztec-packages@089c34c)) * Declare global types ([#10206](AztecProtocol/aztec-packages#10206)) ([7b2e343](AztecProtocol/aztec-packages@7b2e343)) * Delete old serialization methods ([#9951](AztecProtocol/aztec-packages#9951)) ([10d3f6f](AztecProtocol/aztec-packages@10d3f6f)) * Fix migration notes ([#10252](AztecProtocol/aztec-packages#10252)) ([05bdcd5](AztecProtocol/aztec-packages@05bdcd5)) * Pull out some sync changes ([#10245](AztecProtocol/aztec-packages#10245)) ([1bfc15e](AztecProtocol/aztec-packages@1bfc15e)) * Remove docs from sync ([#10241](AztecProtocol/aztec-packages#10241)) ([eeea0aa](AztecProtocol/aztec-packages@eeea0aa)) * Replace relative paths to noir-protocol-circuits ([e7690ca](AztecProtocol/aztec-packages@e7690ca)) </details> <details><summary>barretenberg: 0.65.1</summary> ## [0.65.1](AztecProtocol/aztec-packages@barretenberg-v0.65.0...barretenberg-v0.65.1) (2024-11-27) ### Features * Add total mana used to header ([#9868](AztecProtocol/aztec-packages#9868)) ([2478d19](AztecProtocol/aztec-packages@2478d19)) * Configure world state block history ([#10216](AztecProtocol/aztec-packages#10216)) ([01eb392](AztecProtocol/aztec-packages@01eb392)) * Speed up transaction execution ([#10172](AztecProtocol/aztec-packages#10172)) ([da265b6](AztecProtocol/aztec-packages@da265b6)) ### Bug Fixes * **avm:** Execution test ordering ([#10226](AztecProtocol/aztec-packages#10226)) ([49b4a6c](AztecProtocol/aztec-packages@49b4a6c)) ### Miscellaneous * **avm:** Handle parsing error ([#10203](AztecProtocol/aztec-packages#10203)) ([3c623fc](AztecProtocol/aztec-packages@3c623fc)), closes [#9770](AztecProtocol/aztec-packages#9770) * **avm:** Zero initialization in avm public inputs and execution test fixes ([#10238](AztecProtocol/aztec-packages#10238)) ([0c7c4c9](AztecProtocol/aztec-packages@0c7c4c9)) * CIVC VK ([#10223](AztecProtocol/aztec-packages#10223)) ([089c34c](AztecProtocol/aztec-packages@089c34c)) * Pull out some sync changes ([#10245](AztecProtocol/aztec-packages#10245)) ([1bfc15e](AztecProtocol/aztec-packages@1bfc15e)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

PhilWindle added 7 commits November 21, 2024 09:28

First set of stats

bf362b8

WIP

759143b

WIP

c52e2fd

WIP

3c8e324

WIP

7d46400

WIP

2843505

WIP

0da28dc

PhilWindle requested review from fcarreiro and dbanks12 as code owners November 24, 2024 23:15

WIP

b9dc6ac

PhilWindle added the e2e-all CI: Enables this CI job. label Nov 24, 2024

PhilWindle added 3 commits November 24, 2024 23:31

Merge remote-tracking branch 'origin/master' into pw/check-times-2

92cdace

WIP

a0c21fc

Formatting

f932cf3

fcarreiro reviewed Nov 25, 2024

View reviewed changes

This was linked to issues Nov 25, 2024

Store cache public bytecode commitments #10137

Closed

Rationalise requests to world state during simulation #10138

Closed

Speed up instruction decoding #10142

Closed

PhilWindle added 7 commits November 25, 2024 13:36

Keep instruction set iniitialisation within AVM simulator

05159b8

Function bind fixes

d01a9e3

Merge remote-tracking branch 'origin/master' into pw/check-times-2

95adcfc

Merge remote-tracking branch 'origin/master' into pw/check-times-2

3b73deb

New test

8455cbf

Test fixes

57f5749

Test fixes

0cc95a4

fcarreiro approved these changes Nov 26, 2024

View reviewed changes

spalladino reviewed Nov 26, 2024

View reviewed changes

PhilWindle added 2 commits November 26, 2024 13:46

Review fixes

3e5c8c5

Review fixes

332894f

PhilWindle enabled auto-merge (squash) November 26, 2024 13:51

PhilWindle and others added 6 commits November 26, 2024 14:49

Merge branch 'master' into pw/check-times-2

41bca92

Also process world state errors on the job queue

a5e9c16

Merge branch 'pw/check-times-2' of github.com:AztecProtocol/aztec-pac…

cd6ba1c

…kages into pw/check-times-2

Test fix

98b957d

Updated comment

9439407

Merge branch 'pw/check-times-2' of github.com:AztecProtocol/aztec-pac…

ae26b46

…kages into pw/check-times-2

PhilWindle merged commit da265b6 into master Nov 26, 2024
103 checks passed

PhilWindle deleted the pw/check-times-2 branch November 26, 2024 20:01

AztecBot mentioned this pull request Nov 26, 2024

chore(master): Release 0.65.1 #10219

Merged

LHerskind mentioned this pull request Nov 26, 2024

fix: use current base fee for public fee payment #10230

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Speed up transaction execution #10172

feat: Speed up transaction execution #10172

PhilWindle commented Nov 24, 2024 •

edited

Loading

fcarreiro Nov 25, 2024

IlyasRidhuan Nov 25, 2024

fcarreiro Nov 25, 2024

dbanks12 Nov 25, 2024

fcarreiro Nov 25, 2024

PhilWindle Nov 25, 2024

fcarreiro Nov 25, 2024

fcarreiro left a comment

spalladino left a comment

spalladino Nov 26, 2024

spalladino Nov 26, 2024

spalladino Nov 26, 2024

PhilWindle Nov 26, 2024

spalladino Nov 26, 2024

spalladino Nov 26, 2024

spalladino Nov 26, 2024

feat: Speed up transaction execution #10172

feat: Speed up transaction execution #10172

Conversation

PhilWindle commented Nov 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fcarreiro left a comment

Choose a reason for hiding this comment

spalladino left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PhilWindle commented Nov 24, 2024 •

edited

Loading