Skip to content

Commit

Permalink
fix(public): stack trace short term hack (#9024)
Browse files Browse the repository at this point in the history
Fixes the public stack trace, to unblock releases.

BEFORE
```
    Simulation error: Assertion failed: Not implemented 'false'

      248 |     on_behalf_of: AztecAddress
      249 | ) {
    > 250 |     assert(false, "Not implemented");
          |            ^
      251 |     let inner_hash = compute_inner_authwit_hash(
      252 |         [(*context).msg_sender().to_field(), (*context).selector().to_field(), (*context).get_args_hash()]
      253 |     );

      at false (../../../noir-projects/aztec-nr/authwit/src/auth.nr:250:12)
      at assert_current_call_valid_authwit_public(&mut context, from) (../../../noir-projects/noir-contracts/contracts/token_contract/src/main.nr:243:13)
      at aztec (../../../noir-projects/noir-contracts/contracts/token_contract/src/main.nr:14:3)
      at Token.public_dispatch
      at Uniswap.public_dispatch
```

AFTER
```
    Simulation error: Assertion failed: Not implemented 'false'

      248 |     on_behalf_of: AztecAddress
      249 | ) {
    > 250 |     assert(false, "Not implemented");
          |            ^
      251 |     let inner_hash = compute_inner_authwit_hash(
      252 |         [(*context).msg_sender().to_field(), (*context).selector().to_field(), (*context).get_args_hash()]
      253 |     );

  at false (../../../noir-projects/aztec-nr/authwit/src/auth.nr:250:12)
  at assert_current_call_valid_authwit_public(&mut context, from) (../../../noir-projects/noir-contracts/contracts/token_contract/src/main.nr:259:13)
  at aztec (../../../noir-projects/noir-contracts/contracts/token_contract/src/main.nr:14:3)
  at Token.burn_public
  at TokenBridge.exit_to_l1_public
  at Uniswap._approve_bridge_and_exit_input_asset_to_L1
```

Part of #8985.
  • Loading branch information
fcarreiro authored Oct 4, 2024
1 parent 6c30453 commit f2ea42c
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 7 deletions.
7 changes: 6 additions & 1 deletion yarn-project/pxe/src/pxe_service/pxe_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import {
AztecAddress,
type CompleteAddress,
type L1_TO_L2_MSG_TREE_HEIGHT,
PUBLIC_DISPATCH_SELECTOR,
type PartialAddress,
computeContractAddressFromInstance,
computeContractClassId,
Expand Down Expand Up @@ -748,9 +749,13 @@ export class PXEService implements PXE {
if (err instanceof SimulationError) {
const callStack = err.getCallStack();
const originalFailingFunction = callStack[callStack.length - 1];
// TODO(https://github.com/AztecProtocol/aztec-packages/issues/8985): Properly fix this.
// To be able to resolve the assertion message, we need to use the information from the public dispatch function,
// no matter what the call stack selector points to (since we've modified it to point to the target function).
// We should remove this because the AVM (or public protocol) shouldn't be aware of the public dispatch calling convention.
const debugInfo = await this.contractDataOracle.getFunctionDebugMetadata(
originalFailingFunction.contractAddress,
originalFailingFunction.functionSelector,
FunctionSelector.fromField(new Fr(PUBLIC_DISPATCH_SELECTOR)),
);
const noirCallStack = err.getNoirCallStack();
if (debugInfo) {
Expand Down
6 changes: 3 additions & 3 deletions yarn-project/sequencer-client/src/sequencer/sequencer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -491,9 +491,9 @@ export class Sequencer {
await this.publishL2Block(block, attestations, txHashes, proofQuote);
this.metrics.recordPublishedBlock(workDuration);
this.log.info(
`Submitted rollup block ${block.number} with ${
processedTxs.length
} transactions duration=${workDuration}ms (Submitter: ${this.publisher.getSenderAddress()})`,
`Submitted rollup block ${block.number} with ${processedTxs.length} transactions duration=${Math.ceil(
workDuration,
)}ms (Submitter: ${this.publisher.getSenderAddress()})`,
);
} catch (err) {
this.metrics.recordFailedBlock();
Expand Down
14 changes: 11 additions & 3 deletions yarn-project/simulator/src/avm/errors.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { type FailingFunction, type NoirCallStack } from '@aztec/circuit-types';
import { type AztecAddress, type Fr } from '@aztec/circuits.js';
import { type AztecAddress, Fr, FunctionSelector, PUBLIC_DISPATCH_SELECTOR } from '@aztec/circuits.js';

import { ExecutionError } from '../common/errors.js';
import { type AvmContext } from './avm_context.js';
Expand Down Expand Up @@ -109,13 +109,21 @@ export class AvmRevertReason extends ExecutionError {
* @param nestedError - the error that caused this one (if this is not the root-cause itself)
*/
function createRevertReason(message: string, context: AvmContext, nestedError?: AvmRevertReason): AvmRevertReason {
// TODO(https://github.com/AztecProtocol/aztec-packages/issues/8985): Properly fix this.
// If the function selector is the public dispatch selector, we need to extract the actual function selector from the calldata.
// We should remove this because the AVM (or public protocol) shouldn't be aware of the public dispatch calling convention.
let functionSelector = context.environment.functionSelector;
const internalCallStack = context.machineState.internalCallStack;
if (functionSelector.toField().equals(new Fr(PUBLIC_DISPATCH_SELECTOR)) && context.environment.calldata.length > 0) {
functionSelector = FunctionSelector.fromField(context.environment.calldata[0]);
}
return new AvmRevertReason(
message,
/*failingFunction=*/ {
contractAddress: context.environment.address,
functionSelector: context.environment.functionSelector,
functionSelector: functionSelector,
},
/*noirCallStack=*/ [...context.machineState.internalCallStack, context.machineState.pc].map(pc => `0.${pc}`),
/*noirCallStack=*/ [...internalCallStack, context.machineState.pc].map(pc => `0.${pc}`),
/*options=*/ { cause: nestedError },
);
}
Expand Down

0 comments on commit f2ea42c

Please sign in to comment.