Skip to content

Commit 9daa6da

Browse files
spalladinospalladino
committed
fix: remove detailed revert codes
Alternative to #22265
1 parent 8ba2c51 commit 9daa6da

File tree

6 files changed

+153
-185
lines changed

6 files changed

+153
-185
lines changed

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

Lines changed: 93 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,98 @@ 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+
const outrageousPublicAmountAliceDoesNotHave = t.ALICE_INITIAL_BANANAS * 5n;
337+
338+
// Send a tx that will revert in BOTH app logic and teardown.
339+
const { receipt } = await bananaCoin.methods
340+
.transfer_in_public(aliceAddress, sequencerAddress, outrageousPublicAmountAliceDoesNotHave, 0)
341+
.send({
342+
from: aliceAddress,
343+
fee: {
344+
paymentMethod: new BuggedTeardownFeePaymentMethod(bananaFPC.address, aliceAddress, wallet, gasSettings),
345+
},
346+
wait: { dontThrowOnRevert: true },
347+
});
348+
349+
expect(receipt.executionResult).toBe(TxExecutionResult.BOTH_REVERTED);
350+
expect(receipt.transactionFee).toBeGreaterThan(0n);
351+
352+
await t.context.watcher.trigger();
353+
await t.cheatCodes.rollup.advanceToNextEpoch();
354+
const provenTimeout =
355+
(t.context.config.aztecProofSubmissionEpochs + 1) *
356+
t.context.config.aztecEpochDuration *
357+
t.context.config.aztecSlotDuration;
358+
await waitForProven(aztecNode, receipt, { provenTimeout });
320359
});
321360
});
322361

362+
/**
363+
* Fee payment method whose teardown always reverts because max_fee is set to 0.
364+
* The FPC's _pay_refund will assert `0 >= actual_fee` which always fails since actual_fee > 0.
365+
* The setup transfer of 0 tokens succeeds (and the authwit matches the 0 amount).
366+
*/
367+
class BuggedTeardownFeePaymentMethod extends PublicFeePaymentMethod {
368+
override async getExecutionPayload(): Promise<ExecutionPayload> {
369+
const zeroFee = new Fr(0n);
370+
const authwitNonce = Fr.random();
371+
372+
const asset = await this.getAsset();
373+
374+
// Authorize the FPC to transfer 0 tokens (matches the 0 max_fee we'll pass).
375+
const setPublicAuthWitInteraction = await SetPublicAuthwitContractInteraction.create(
376+
this.wallet,
377+
this.sender,
378+
{
379+
caller: this.paymentContract,
380+
call: FunctionCall.from({
381+
name: 'transfer_in_public',
382+
to: asset,
383+
selector: await FunctionSelector.fromSignature('transfer_in_public((Field),(Field),u128,Field)'),
384+
type: FunctionType.PUBLIC,
385+
hideMsgSender: false,
386+
isStatic: false,
387+
args: [this.sender.toField(), this.paymentContract.toField(), zeroFee, authwitNonce],
388+
returnTypes: [],
389+
}),
390+
},
391+
true,
392+
);
393+
394+
return new ExecutionPayload(
395+
[
396+
...(await setPublicAuthWitInteraction.request()).calls,
397+
FunctionCall.from({
398+
name: 'fee_entrypoint_public',
399+
to: this.paymentContract,
400+
selector: await FunctionSelector.fromSignature('fee_entrypoint_public(u128,Field)'),
401+
type: FunctionType.PRIVATE,
402+
hideMsgSender: false,
403+
isStatic: false,
404+
args: [zeroFee, authwitNonce],
405+
returnTypes: [],
406+
}),
407+
],
408+
[],
409+
[],
410+
[],
411+
this.paymentContract,
412+
);
413+
}
414+
}
415+
323416
class BuggedSetupFeePaymentMethod extends PublicFeePaymentMethod {
324417
override async getExecutionPayload(): Promise<ExecutionPayload> {
325418
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)