Merge branch 'master' into tf/skip-unformattable-comments

AztecProtocol · May 9, 2024 · 32406ad · 32406ad
2 parents 662371c + cc59981
commit 32406ad
Show file tree

Hide file tree

Showing 5 changed files with 28 additions and 17 deletions.
diff --git a/yarn-project/end-to-end/src/e2e_avm_simulator.test.ts b/yarn-project/end-to-end/src/e2e_avm_simulator.test.ts
@@ -121,7 +121,7 @@ describe('e2e_avm_simulator', () => {
     });
   });
 
-  describe('ACVM interoperability', () => {
+  describe.skip('ACVM interoperability', () => {
     let avmContract: AvmAcvmInteropTestContract;
 
     beforeEach(async () => {
@@ -136,7 +136,7 @@ describe('e2e_avm_simulator', () => {
       expect(await avmContract.methods.call_avm_from_acvm().simulate()).toEqual(123456n);
     });
 
-    it.skip('Can call ACVM function from AVM', async () => {
+    it('Can call ACVM function from AVM', async () => {
       expect(await avmContract.methods.call_acvm_from_avm().simulate()).toEqual(123456n);
     });
 
@@ -146,7 +146,7 @@ describe('e2e_avm_simulator', () => {
       await avmContract.methods.assert_unsiloed_nullifier_acvm(nullifier).send().wait();
     });
 
-    it.skip('AVM nested call to ACVM sees settled nullifiers', async () => {
+    it('AVM nested call to ACVM sees settled nullifiers', async () => {
       const nullifier = new Fr(123456);
       await avmContract.methods.new_nullifier(nullifier).send().wait();
       await avmContract.methods
@@ -155,6 +155,7 @@ describe('e2e_avm_simulator', () => {
         .wait();
     });
 
+    // TODO: Enable (or delete) authwit tests once the AVM is fully functional.
     describe.skip('Authwit', () => {
       it('Works if authwit provided', async () => {
         const recipient = AztecAddress.random();

diff --git a/yarn-project/simulator/src/avm/avm_machine_state.ts b/yarn-project/simulator/src/avm/avm_machine_state.ts
@@ -136,14 +136,20 @@ export class AvmMachineState {
       throw new Error('Execution results are not ready! Execution is ongoing.');
     }
     let revertReason = undefined;
-    if (this.reverted && this.output.length > 0) {
-      try {
-        // We remove the first element which is the 'error selector'.
-        const revertOutput = this.output.slice(1);
-        // Try to interpret the output as a text string.
-        revertReason = new Error('Assertion failed: ' + String.fromCharCode(...revertOutput.map(fr => fr.toNumber())));
-      } catch (e) {
-        revertReason = new Error('<no output>');
+    if (this.reverted) {
+      if (this.output.length === 0) {
+        revertReason = new Error('Assertion failed.');
+      } else {
+        try {
+          // We remove the first element which is the 'error selector'.
+          const revertOutput = this.output.slice(1);
+          // Try to interpret the output as a text string.
+          revertReason = new Error(
+            'Assertion failed: ' + String.fromCharCode(...revertOutput.map(fr => fr.toNumber())),
+          );
+        } catch (e) {
+          revertReason = new Error('Assertion failed: <cannot interpret as string>');
+        }
       }
     }
     return new AvmContractCallResults(this.reverted, this.output, revertReason);

diff --git a/yarn-project/simulator/src/avm/avm_simulator.test.ts b/yarn-project/simulator/src/avm/avm_simulator.test.ts
@@ -115,7 +115,7 @@ describe('AVM simulator: transpiled Noir contracts', () => {
       const bytecode = getAvmTestContractBytecode('u128_from_integer_overflow');
       const results = await new AvmSimulator(initContext()).executeBytecode(bytecode);
       expect(results.reverted).toBe(true);
-      expect(results.revertReason?.message).toEqual(undefined);
+      expect(results.revertReason?.message).toEqual('Assertion failed.');
       // Note: compiler intrinsic messages (like below) are not known to the AVM
       //expect(results.revertReason?.message).toEqual("Assertion failed: call to assert_max_bit_size 'self.__assert_max_bit_size(bit_size)'");
     });

diff --git a/yarn-project/simulator/src/public/abstract_phase_manager.ts b/yarn-project/simulator/src/public/abstract_phase_manager.ts
@@ -283,10 +283,18 @@ export abstract class AbstractPhaseManager {
             )
           : current;
 
+        // Sanity check for a current upstream assumption.
+        // Consumers of the result seem to expect "reverted <=> revertReason !== undefined".
+        const functionSelector = result.execution.functionData.selector.toString();
+        if (result.reverted && !result.revertReason) {
+          throw new Error(
+            `Simulation of ${result.execution.contractAddress.toString()}:${functionSelector} reverted with no reason.`,
+          );
+        }
+
         // Accumulate gas used in this execution
         gasUsed = gasUsed.add(Gas.from(result.startGasLeft).sub(Gas.from(result.endGasLeft)));
 
-        const functionSelector = result.execution.functionData.selector.toString();
         if (result.reverted && !PhaseIsRevertible[this.phase]) {
           this.log.debug(
             `Simulation error on ${result.execution.contractAddress.toString()}:${functionSelector} with reason: ${

diff --git a/yarn-project/simulator/src/public/executor.ts b/yarn-project/simulator/src/public/executor.ts
@@ -168,10 +168,6 @@ async function executePublicFunctionAcvm(
   })();
 
   if (reverted) {
-    if (!revertReason) {
-      throw new Error('Reverted but no revert reason');
-    }
-
     return {
       execution,
       returnValues: [],