Skip to content

Commit dc83e00

Browse files
spalladinospalladino
committed
fix: remove detailed revert codes
Alternative to #22265
1 parent b594593 commit dc83e00

File tree

6 files changed

+168
-185
lines changed

6 files changed

+168
-185
lines changed

‎yarn-project/end-to-end/src/e2e_fees/failures.test.ts‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@ import { FunctionSelector } from '@aztec/aztec.js/abi';
22
import type { AztecAddress } from '@aztec/aztec.js/addresses';
33
import { EthAddress } from '@aztec/aztec.js/addresses';
44
import { SetPublicAuthwitContractInteraction } from '@aztec/aztec.js/authorization';
5+
import { waitForProven } from '@aztec/aztec.js/contracts';
56
import { PrivateFeePaymentMethod, PublicFeePaymentMethod } from '@aztec/aztec.js/fee';
67
import { Fr } from '@aztec/aztec.js/fields';
8+
import type { AztecNode } from '@aztec/aztec.js/node';
79
import { TxExecutionResult } from '@aztec/aztec.js/tx';
810
import type { Wallet } from '@aztec/aztec.js/wallet';
911
import type { FPCContract } from '@aztec/noir-contracts.js/FPC';
@@ -23,6 +25,7 @@ describe('e2e_fees failures', () => {
2325
let bananaCoin: BananaCoin;
2426
let bananaFPC: FPCContract;
2527
let gasSettings: GasSettings;
28+
let aztecNode: AztecNode;
2629
const coinbase = EthAddress.random();
2730

2831
const t = new FeesTest('failures', 3, { coinbase });
@@ -31,6 +34,7 @@ describe('e2e_fees failures', () => {
3134
await t.setup();
3235
await t.applyFPCSetup();
3336
({ wallet, aliceAddress, sequencerAddress, bananaCoin, bananaFPC, gasSettings } = t);
37+
aztecNode = t.aztecNode;
3438

3539
// Prove up until the current state by just marking it as proven.
3640
// Then turn off the watcher to prevent it from keep proving
@@ -317,9 +321,113 @@ describe('e2e_fees failures', () => {
317321
[aliceAddress, bananaFPC.address, sequencerAddress],
318322
[initialAliceGas, initialFPCGas - receipt.transactionFee!, initialSequencerGas],
319323
);
324+
325+
// Prove the block containing the teardown-reverted tx (revert_code = 2).
326+
await t.context.watcher.trigger();
327+
await t.cheatCodes.rollup.advanceToNextEpoch();
328+
const provenTimeout =
329+
(t.context.config.aztecProofSubmissionEpochs + 1) *
330+
t.context.config.aztecEpochDuration *
331+
t.context.config.aztecSlotDuration;
332+
await waitForProven(aztecNode, receipt, { provenTimeout });
333+
});
334+
335+
it('proves transaction where both app logic and teardown revert', async () => {
336+
/**
337+
* Regression test for a bug where the circuit encodes revert_code as 0 or 1 (boolean),
338+
* but the TS side preserves the full RevertCode enum (BOTH_REVERTED = 3).
339+
* This causes the tx start marker in the blob data to differ, which cascades into
340+
* a spongeBlobHash mismatch in the block header.
341+
*
342+
* We trigger BOTH_REVERTED by:
343+
* - App logic: transfer more tokens than Alice has (reverts in public app logic)
344+
* - Teardown: use a bugged fee payment method whose teardown transfers an impossible amount
345+
*
346+
* See: noir-projects/noir-protocol-circuits/sponge-blob-revert-code-bug.md
347+
*/
348+
const outrageousPublicAmountAliceDoesNotHave = t.ALICE_INITIAL_BANANAS * 5n;
349+
350+
// Send a tx that will revert in BOTH app logic and teardown.
351+
const { receipt } = await bananaCoin.methods
352+
.transfer_in_public(aliceAddress, sequencerAddress, outrageousPublicAmountAliceDoesNotHave, 0)
353+
.send({
354+
from: aliceAddress,
355+
fee: {
356+
paymentMethod: new BuggedTeardownFeePaymentMethod(bananaFPC.address, aliceAddress, wallet, gasSettings),
357+
},
358+
wait: { dontThrowOnRevert: true },
359+
});
360+
361+
expect(receipt.executionResult).toBe(TxExecutionResult.BOTH_REVERTED);
362+
expect(receipt.transactionFee).toBeGreaterThan(0n);
363+
364+
// Now prove the block containing this tx via the real prover node.
365+
// The prover node will fail with "Block header mismatch" if the revert_code encoding
366+
// differs between the circuit (which uses 1) and the TS (which uses 3).
367+
await t.context.watcher.trigger();
368+
await t.cheatCodes.rollup.advanceToNextEpoch();
369+
const provenTimeout =
370+
(t.context.config.aztecProofSubmissionEpochs + 1) *
371+
t.context.config.aztecEpochDuration *
372+
t.context.config.aztecSlotDuration;
373+
await waitForProven(aztecNode, receipt, { provenTimeout });
320374
});
321375
});
322376

377+
/**
378+
* Fee payment method whose teardown always reverts because max_fee is set to 0.
379+
* The FPC's _pay_refund will assert `0 >= actual_fee` which always fails since actual_fee > 0.
380+
* The setup transfer of 0 tokens succeeds (and the authwit matches the 0 amount).
381+
*/
382+
class BuggedTeardownFeePaymentMethod extends PublicFeePaymentMethod {
383+
override async getExecutionPayload(): Promise<ExecutionPayload> {
384+
const zeroFee = new Fr(0n);
385+
const authwitNonce = Fr.random();
386+
387+
const asset = await this.getAsset();
388+
389+
// Authorize the FPC to transfer 0 tokens (matches the 0 max_fee we'll pass).
390+
const setPublicAuthWitInteraction = await SetPublicAuthwitContractInteraction.create(
391+
this.wallet,
392+
this.sender,
393+
{
394+
caller: this.paymentContract,
395+
call: FunctionCall.from({
396+
name: 'transfer_in_public',
397+
to: asset,
398+
selector: await FunctionSelector.fromSignature('transfer_in_public((Field),(Field),u128,Field)'),
399+
type: FunctionType.PUBLIC,
400+
hideMsgSender: false,
401+
isStatic: false,
402+
args: [this.sender.toField(), this.paymentContract.toField(), zeroFee, authwitNonce],
403+
returnTypes: [],
404+
}),
405+
},
406+
true,
407+
);
408+
409+
return new ExecutionPayload(
410+
[
411+
...(await setPublicAuthWitInteraction.request()).calls,
412+
FunctionCall.from({
413+
name: 'fee_entrypoint_public',
414+
to: this.paymentContract,
415+
selector: await FunctionSelector.fromSignature('fee_entrypoint_public(u128,Field)'),
416+
type: FunctionType.PRIVATE,
417+
hideMsgSender: false,
418+
isStatic: false,
419+
args: [zeroFee, authwitNonce],
420+
returnTypes: [],
421+
}),
422+
],
423+
[],
424+
[],
425+
[],
426+
this.paymentContract,
427+
);
428+
}
429+
}
430+
323431
class BuggedSetupFeePaymentMethod extends PublicFeePaymentMethod {
324432
override async getExecutionPayload(): Promise<ExecutionPayload> {
325433
const maxFee = this.gasSettings.getFeeLimit();

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

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -174,14 +174,8 @@ export class PublicTxContext {
174174
}
175175
if (phase === TxExecutionPhase.SETUP) {
176176
this.log.warn(`Setup phase reverted! The transaction will be thrown out.`);
177-
} else if (phase === TxExecutionPhase.APP_LOGIC) {
178-
this.revertCode = RevertCode.APP_LOGIC_REVERTED;
179-
} else if (phase === TxExecutionPhase.TEARDOWN) {
180-
if (this.revertCode.equals(RevertCode.APP_LOGIC_REVERTED)) {
181-
this.revertCode = RevertCode.BOTH_REVERTED;
182-
} else {
183-
this.revertCode = RevertCode.TEARDOWN_REVERTED;
184-
}
177+
} else if (phase === TxExecutionPhase.APP_LOGIC || phase === TxExecutionPhase.TEARDOWN) {
178+
this.revertCode = RevertCode.REVERTED;
185179
}
186180
}
187181

‎yarn-project/stdlib/src/avm/__snapshots__/revert_code.test.ts.snap‎

Lines changed: 1 addition & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Jest Snapshot v1, https://goo.gl/fbAQLP
1+
// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing
22

33
exports[`revert_code should serialize RevertCode<0> properly 1`] = `
44
{
@@ -101,105 +101,3 @@ exports[`revert_code should serialize RevertCode<1> properly 2`] = `
101101
`;
102102

103103
exports[`revert_code should serialize RevertCode<1> properly 3`] = `"0x0000000000000000000000000000000000000000000000000000000000000001"`;
104-
105-
exports[`revert_code should serialize RevertCode<2> properly 1`] = `
106-
{
107-
"data": [
108-
0,
109-
0,
110-
0,
111-
0,
112-
0,
113-
0,
114-
0,
115-
0,
116-
0,
117-
0,
118-
0,
119-
0,
120-
0,
121-
0,
122-
0,
123-
0,
124-
0,
125-
0,
126-
0,
127-
0,
128-
0,
129-
0,
130-
0,
131-
0,
132-
0,
133-
0,
134-
0,
135-
0,
136-
0,
137-
0,
138-
0,
139-
2,
140-
],
141-
"type": "Buffer",
142-
}
143-
`;
144-
145-
exports[`revert_code should serialize RevertCode<2> properly 2`] = `
146-
{
147-
"data": [
148-
2,
149-
],
150-
"type": "Buffer",
151-
}
152-
`;
153-
154-
exports[`revert_code should serialize RevertCode<2> properly 3`] = `"0x0000000000000000000000000000000000000000000000000000000000000002"`;
155-
156-
exports[`revert_code should serialize RevertCode<3> properly 1`] = `
157-
{
158-
"data": [
159-
0,
160-
0,
161-
0,
162-
0,
163-
0,
164-
0,
165-
0,
166-
0,
167-
0,
168-
0,
169-
0,
170-
0,
171-
0,
172-
0,
173-
0,
174-
0,
175-
0,
176-
0,
177-
0,
178-
0,
179-
0,
180-
0,
181-
0,
182-
0,
183-
0,
184-
0,
185-
0,
186-
0,
187-
0,
188-
0,
189-
0,
190-
3,
191-
],
192-
"type": "Buffer",
193-
}
194-
`;
195-
196-
exports[`revert_code should serialize RevertCode<3> properly 2`] = `
197-
{
198-
"data": [
199-
3,
200-
],
201-
"type": "Buffer",
202-
}
203-
`;
204-
205-
exports[`revert_code should serialize RevertCode<3> properly 3`] = `"0x0000000000000000000000000000000000000000000000000000000000000003"`;

‎yarn-project/stdlib/src/avm/revert_code.test.ts‎

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -4,31 +4,31 @@ import { jsonStringify } from '@aztec/foundation/json-rpc';
44
import { RevertCode } from './revert_code.js';
55

66
describe('revert_code', () => {
7-
it.each([RevertCode.OK, RevertCode.APP_LOGIC_REVERTED, RevertCode.TEARDOWN_REVERTED, RevertCode.BOTH_REVERTED])(
8-
'should serialize %s properly',
9-
revertCode => {
10-
expect(revertCode.getSerializedLength()).toBe(1);
7+
it.each([RevertCode.OK, RevertCode.REVERTED])('should serialize %s properly', revertCode => {
8+
expect(revertCode.getSerializedLength()).toBe(1);
119

12-
const hashPreimage = revertCode.toHashPreimage();
13-
expect(hashPreimage).toMatchSnapshot();
14-
expect(hashPreimage.length).toBe(32);
10+
const hashPreimage = revertCode.toHashPreimage();
11+
expect(hashPreimage).toMatchSnapshot();
12+
expect(hashPreimage.length).toBe(32);
1513

16-
const buf = revertCode.toBuffer();
17-
expect(buf).toMatchSnapshot();
18-
expect(RevertCode.fromBuffer(buf)).toEqual(revertCode);
14+
const buf = revertCode.toBuffer();
15+
expect(buf).toMatchSnapshot();
16+
expect(RevertCode.fromBuffer(buf)).toEqual(revertCode);
1917

20-
const field = revertCode.toField();
21-
expect(field).toMatchSnapshot();
22-
expect(RevertCode.fromField(field)).toEqual(revertCode);
23-
expect(RevertCode.fromFields([field])).toEqual(revertCode);
18+
const field = revertCode.toField();
19+
expect(field).toMatchSnapshot();
20+
expect(RevertCode.fromField(field)).toEqual(revertCode);
21+
expect(RevertCode.fromFields([field])).toEqual(revertCode);
2422

25-
const json = jsonStringify(revertCode);
26-
expect(RevertCode.schema.parse(JSON.parse(json))).toEqual(revertCode);
27-
},
28-
);
23+
const json = jsonStringify(revertCode);
24+
expect(RevertCode.schema.parse(JSON.parse(json))).toEqual(revertCode);
25+
});
2926

30-
it('should throw when deserializing from invalid buffer', () => {
31-
expect(() => RevertCode.fromBuffer(Buffer.from([42]))).toThrow();
32-
expect(() => RevertCode.fromField(new Fr(42))).toThrow();
27+
it('should coerce values >= 1 to REVERTED', () => {
28+
expect(RevertCode.fromNumber(2)).toEqual(RevertCode.REVERTED);
29+
expect(RevertCode.fromNumber(3)).toEqual(RevertCode.REVERTED);
30+
expect(RevertCode.fromNumber(42)).toEqual(RevertCode.REVERTED);
31+
expect(RevertCode.fromField(new Fr(42))).toEqual(RevertCode.REVERTED);
32+
expect(RevertCode.fromBuffer(Buffer.from([5]))).toEqual(RevertCode.REVERTED);
3333
});
3434
});

0 commit comments

Comments
 (0)