Skip to content

Commit 583b865

Browse files
spalladinospalladino
authored
fix: Validate txs protocol and vk tree roots (#13891) (#13903)
The `protocolContractTreeRoot` in txs was not checked by nodes when receiving a tx. This would lead to failures when trying to prove an epoch that contained such a tx. This PR adds a check to the tx metadata validator to check the protocol contract tree root. And since we're at it, we also check the protocol circuits tree root, so we can fail a tx earlier without having to verify the entire proof. This is a cherry-pick of #13891 onto alpha-tesnet
1 parent 4dc4e46 commit 583b865

File tree

11 files changed

+172
-143
lines changed

11 files changed

+172
-143
lines changed

‎yarn-project/aztec-node/package.json

+1
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@
7474
"@aztec/l1-artifacts": "workspace:^",
7575
"@aztec/merkle-tree": "workspace:^",
7676
"@aztec/node-lib": "workspace:^",
77+
"@aztec/noir-protocol-circuits-types": "workspace:^",
7778
"@aztec/p2p": "workspace:^",
7879
"@aztec/protocol-contracts": "workspace:^",
7980
"@aztec/prover-client": "workspace:^",

‎yarn-project/aztec-node/src/aztec-node/server.test.ts

+32-30
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
import { TestCircuitVerifier } from '@aztec/bb-prover';
22
import { EthAddress } from '@aztec/foundation/eth-address';
33
import { Fr } from '@aztec/foundation/fields';
4+
import { getVKTreeRoot } from '@aztec/noir-protocol-circuits-types/vk-tree';
45
import type { P2P } from '@aztec/p2p';
6+
import { protocolContractTreeRoot } from '@aztec/protocol-contracts';
57
import { computeFeePayerBalanceLeafSlot } from '@aztec/protocol-contracts/fee-juice';
68
import type { GlobalVariableBuilder } from '@aztec/sequencer-client';
79
import { AztecAddress } from '@aztec/stdlib/aztec-address';
@@ -14,7 +16,15 @@ import { RollupValidationRequests } from '@aztec/stdlib/kernel';
1416
import type { L1ToL2MessageSource } from '@aztec/stdlib/messaging';
1517
import { mockTx } from '@aztec/stdlib/testing';
1618
import { MerkleTreeId, PublicDataTreeLeaf, PublicDataTreeLeafPreimage } from '@aztec/stdlib/trees';
17-
import { BlockHeader, GlobalVariables, MaxBlockNumber, TX_ERROR_INVALID_BLOCK_NUMBER } from '@aztec/stdlib/tx';
19+
import {
20+
BlockHeader,
21+
GlobalVariables,
22+
MaxBlockNumber,
23+
TX_ERROR_DUPLICATE_NULLIFIER_IN_TX,
24+
TX_ERROR_INCORRECT_L1_CHAIN_ID,
25+
TX_ERROR_INCORRECT_ROLLUP_VERSION,
26+
TX_ERROR_INVALID_BLOCK_NUMBER,
27+
} from '@aztec/stdlib/tx';
1828

1929
import { readFileSync } from 'fs';
2030
import { type MockProxy, mock } from 'jest-mock-extended';
@@ -41,6 +51,10 @@ describe('aztec node', () => {
4151
numberOfNonRevertiblePublicCallRequests: 0,
4252
numberOfRevertiblePublicCallRequests: 0,
4353
feePayer,
54+
chainId,
55+
version: rollupVersion,
56+
vkTreeRoot: getVKTreeRoot(),
57+
protocolContractTreeRoot,
4458
});
4559
};
4660

@@ -131,10 +145,6 @@ describe('aztec node', () => {
131145
describe('tx validation', () => {
132146
it('tests that the node correctly validates double spends', async () => {
133147
const txs = await Promise.all([mockTxForRollup(0x10000), mockTxForRollup(0x20000)]);
134-
txs.forEach(tx => {
135-
tx.data.constants.txContext.chainId = chainId;
136-
tx.data.constants.txContext.version = rollupVersion;
137-
});
138148
const doubleSpendTx = txs[0];
139149
const doubleSpendWithExistingTx = txs[1];
140150
lastBlockNumber += 1;
@@ -144,7 +154,10 @@ describe('aztec node', () => {
144154
// We push a duplicate nullifier that was created in the same transaction
145155
doubleSpendTx.data.forRollup!.end.nullifiers[1] = doubleSpendTx.data.forRollup!.end.nullifiers[0];
146156

147-
expect(await node.isValidTx(doubleSpendTx)).toEqual({ result: 'invalid', reason: ['Duplicate nullifier in tx'] });
157+
expect(await node.isValidTx(doubleSpendTx)).toEqual({
158+
result: 'invalid',
159+
reason: [TX_ERROR_DUPLICATE_NULLIFIER_IN_TX],
160+
});
148161

149162
expect(await node.isValidTx(doubleSpendWithExistingTx)).toEqual({ result: 'valid' });
150163

@@ -169,37 +182,26 @@ describe('aztec node', () => {
169182

170183
it('tests that the node correctly validates chain id', async () => {
171184
const tx = await mockTxForRollup(0x10000);
172-
tx.data.constants.txContext.chainId = chainId;
173-
tx.data.constants.txContext.version = rollupVersion;
174-
175185
expect(await node.isValidTx(tx)).toEqual({ result: 'valid' });
176186

177187
// We make the chain id on the tx not equal to the configured chain id
178188
tx.data.constants.txContext.chainId = new Fr(1n + chainId.toBigInt());
179189

180-
expect(await node.isValidTx(tx)).toEqual({ result: 'invalid', reason: ['Incorrect chain id'] });
190+
expect(await node.isValidTx(tx)).toEqual({ result: 'invalid', reason: [TX_ERROR_INCORRECT_L1_CHAIN_ID] });
181191
});
182192

183193
it('tests that the node correctly validates rollup version', async () => {
184194
const tx = await mockTxForRollup(0x10000);
185-
tx.data.constants.txContext.chainId = chainId;
186-
tx.data.constants.txContext.version = rollupVersion;
187-
188195
expect(await node.isValidTx(tx)).toEqual({ result: 'valid' });
189196

190197
// We make the chain id on the tx not equal to the configured chain id
191198
tx.data.constants.txContext.version = new Fr(1n + rollupVersion.toBigInt());
192199

193-
expect(await node.isValidTx(tx)).toEqual({ result: 'invalid', reason: ['Incorrect rollup version'] });
200+
expect(await node.isValidTx(tx)).toEqual({ result: 'invalid', reason: [TX_ERROR_INCORRECT_ROLLUP_VERSION] });
194201
});
195202

196203
it('tests that the node correctly validates max block numbers', async () => {
197204
const txs = await Promise.all([mockTxForRollup(0x10000), mockTxForRollup(0x20000), mockTxForRollup(0x30000)]);
198-
txs.forEach(tx => {
199-
tx.data.constants.txContext.chainId = chainId;
200-
tx.data.constants.txContext.version = rollupVersion;
201-
});
202-
203205
const noMaxBlockNumberMetadata = txs[0];
204206
const invalidMaxBlockNumberMetadata = txs[1];
205207
const validMaxBlockNumberMetadata = txs[2];
@@ -226,19 +228,19 @@ describe('aztec node', () => {
226228
});
227229
});
228230

229-
describe('Node Info', () => {
230-
it('returns the correct node version', async () => {
231-
const releasePleaseVersionFile = readFileSync(
232-
resolve(dirname(fileURLToPath(import.meta.url)), '../../../../.release-please-manifest.json'),
233-
).toString();
234-
const releasePleaseVersion = JSON.parse(releasePleaseVersionFile)['.'];
235-
236-
const nodeInfo = await node.getNodeInfo();
237-
expect(nodeInfo.nodeVersion).toBe(releasePleaseVersion);
231+
describe('getters', () => {
232+
describe('node info', () => {
233+
it('returns the correct node version', async () => {
234+
const releasePleaseVersionFile = readFileSync(
235+
resolve(dirname(fileURLToPath(import.meta.url)), '../../../../.release-please-manifest.json'),
236+
).toString();
237+
const releasePleaseVersion = JSON.parse(releasePleaseVersionFile)['.'];
238+
239+
const nodeInfo = await node.getNodeInfo();
240+
expect(nodeInfo.nodeVersion).toBe(releasePleaseVersion);
241+
});
238242
});
239-
});
240243

241-
describe('getters', () => {
242244
describe('getBlockHeader', () => {
243245
let initialHeader: BlockHeader;
244246
let header1: BlockHeader;

‎yarn-project/aztec-node/tsconfig.json

+3
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,9 @@
3939
{
4040
"path": "../node-lib"
4141
},
42+
{
43+
"path": "../noir-protocol-circuits-types"
44+
},
4245
{
4346
"path": "../p2p"
4447
},

‎yarn-project/p2p/src/msg_validators/tx_validator/metadata_validator.test.ts

+46-29
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@ import { mockTx, mockTxForRollup } from '@aztec/stdlib/testing';
33
import type { AnyTx, Tx } from '@aztec/stdlib/tx';
44
import {
55
MaxBlockNumber,
6-
TX_ERROR_INCORRECT_CHAIN_ID,
6+
TX_ERROR_INCORRECT_L1_CHAIN_ID,
7+
TX_ERROR_INCORRECT_PROTOCOL_CONTRACT_TREE_ROOT,
78
TX_ERROR_INCORRECT_ROLLUP_VERSION,
9+
TX_ERROR_INCORRECT_VK_TREE_ROOT,
810
TX_ERROR_INVALID_BLOCK_NUMBER,
911
} from '@aztec/stdlib/tx';
1012

@@ -14,13 +16,25 @@ describe('MetadataTxValidator', () => {
1416
let blockNumber: Fr;
1517
let chainId: Fr;
1618
let rollupVersion: Fr;
19+
let vkTreeRoot: Fr;
20+
let protocolContractTreeRoot: Fr;
21+
22+
let seed: number = 1;
1723
let validator: MetadataTxValidator<AnyTx>;
1824

1925
beforeEach(() => {
2026
chainId = new Fr(1);
2127
blockNumber = new Fr(42);
2228
rollupVersion = new Fr(2);
23-
validator = new MetadataTxValidator(chainId, rollupVersion, blockNumber);
29+
vkTreeRoot = new Fr(3);
30+
protocolContractTreeRoot = new Fr(4);
31+
validator = new MetadataTxValidator({
32+
l1ChainId: chainId,
33+
rollupVersion,
34+
blockNumber,
35+
vkTreeRoot,
36+
protocolContractTreeRoot,
37+
});
2438
});
2539

2640
const expectValid = async (tx: Tx) => {
@@ -31,37 +45,33 @@ describe('MetadataTxValidator', () => {
3145
await expect(validator.validateTx(tx)).resolves.toEqual({ result: 'invalid', reason: [reason] });
3246
};
3347

34-
it('allows only transactions for the right chain', async () => {
35-
const goodTxs = await Promise.all([mockTx(1), mockTxForRollup(2)]);
36-
const badTxs = await Promise.all([mockTx(3), mockTxForRollup(4)]);
48+
const makeTxs = async () => {
49+
const opts = { chainId, version: rollupVersion, vkTreeRoot, protocolContractTreeRoot };
50+
const tx1 = await mockTx(seed++, opts);
51+
const tx2 = await mockTxForRollup(seed++, opts);
3752

38-
goodTxs.forEach(tx => {
39-
tx.data.constants.txContext.chainId = chainId;
40-
tx.data.constants.txContext.version = rollupVersion;
41-
});
53+
return [tx1, tx2];
54+
};
55+
56+
it('allows only transactions for the right chain', async () => {
57+
const goodTxs = await makeTxs();
58+
const badTxs = await makeTxs();
4259

4360
badTxs.forEach(tx => {
4461
tx.data.constants.txContext.chainId = chainId.add(new Fr(1));
45-
tx.data.constants.txContext.version = rollupVersion;
4662
});
4763

4864
await expectValid(goodTxs[0]);
4965
await expectValid(goodTxs[1]);
50-
await expectInvalid(badTxs[0], TX_ERROR_INCORRECT_CHAIN_ID);
51-
await expectInvalid(badTxs[1], TX_ERROR_INCORRECT_CHAIN_ID);
66+
await expectInvalid(badTxs[0], TX_ERROR_INCORRECT_L1_CHAIN_ID);
67+
await expectInvalid(badTxs[1], TX_ERROR_INCORRECT_L1_CHAIN_ID);
5268
});
5369

5470
it('allows only transactions for the right rollup', async () => {
55-
const goodTxs = await Promise.all([mockTx(1), mockTxForRollup(2)]);
56-
const badTxs = await Promise.all([mockTx(3), mockTxForRollup(4)]);
57-
58-
goodTxs.forEach(tx => {
59-
tx.data.constants.txContext.chainId = chainId;
60-
tx.data.constants.txContext.version = rollupVersion;
61-
});
71+
const goodTxs = await makeTxs();
72+
const badTxs = await makeTxs();
6273

6374
badTxs.forEach(tx => {
64-
tx.data.constants.txContext.chainId = chainId;
6575
tx.data.constants.txContext.version = rollupVersion.add(Fr.ONE);
6676
});
6777

@@ -71,28 +81,35 @@ describe('MetadataTxValidator', () => {
7181
await expectInvalid(badTxs[1], TX_ERROR_INCORRECT_ROLLUP_VERSION);
7282
});
7383

84+
it('allows only transactions with the right roots', async () => {
85+
const goodTxs = await makeTxs();
86+
const badTxs = await makeTxs();
87+
88+
badTxs[0].data.constants.vkTreeRoot = vkTreeRoot.add(new Fr(1));
89+
badTxs[1].data.constants.protocolContractTreeRoot = protocolContractTreeRoot.add(new Fr(1));
90+
91+
await expectValid(goodTxs[0]);
92+
await expectValid(goodTxs[1]);
93+
await expectInvalid(badTxs[0], TX_ERROR_INCORRECT_VK_TREE_ROOT);
94+
await expectInvalid(badTxs[1], TX_ERROR_INCORRECT_PROTOCOL_CONTRACT_TREE_ROOT);
95+
});
96+
7497
it.each([42, 43])('allows txs with valid max block number', async maxBlockNumber => {
75-
const goodTx = await mockTxForRollup(1);
76-
goodTx.data.constants.txContext.chainId = chainId;
77-
goodTx.data.constants.txContext.version = rollupVersion;
98+
const [goodTx] = await makeTxs();
7899
goodTx.data.rollupValidationRequests.maxBlockNumber = new MaxBlockNumber(true, new Fr(maxBlockNumber));
79100

80101
await expectValid(goodTx);
81102
});
82103

83104
it('allows txs with unset max block number', async () => {
84-
const goodTx = await mockTxForRollup(1);
85-
goodTx.data.constants.txContext.chainId = chainId;
86-
goodTx.data.constants.txContext.version = rollupVersion;
105+
const [goodTx] = await makeTxs();
87106
goodTx.data.rollupValidationRequests.maxBlockNumber = new MaxBlockNumber(false, Fr.ZERO);
88107

89108
await expectValid(goodTx);
90109
});
91110

92111
it('rejects txs with lower max block number', async () => {
93-
const badTx = await mockTxForRollup(1);
94-
badTx.data.constants.txContext.chainId = chainId;
95-
badTx.data.constants.txContext.version = rollupVersion;
112+
const [badTx] = await makeTxs();
96113
badTx.data.rollupValidationRequests.maxBlockNumber = new MaxBlockNumber(true, blockNumber.sub(new Fr(1)));
97114

98115
await expectInvalid(badTx, TX_ERROR_INVALID_BLOCK_NUMBER);

‎yarn-project/p2p/src/msg_validators/tx_validator/metadata_validator.ts

+47-11
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@ import type { Fr } from '@aztec/foundation/fields';
22
import { createLogger } from '@aztec/foundation/log';
33
import {
44
type AnyTx,
5-
TX_ERROR_INCORRECT_CHAIN_ID,
5+
TX_ERROR_INCORRECT_L1_CHAIN_ID,
6+
TX_ERROR_INCORRECT_PROTOCOL_CONTRACT_TREE_ROOT,
67
TX_ERROR_INCORRECT_ROLLUP_VERSION,
8+
TX_ERROR_INCORRECT_VK_TREE_ROOT,
79
TX_ERROR_INVALID_BLOCK_NUMBER,
810
Tx,
911
type TxValidationResult,
@@ -13,28 +15,62 @@ import {
1315
export class MetadataTxValidator<T extends AnyTx> implements TxValidator<T> {
1416
#log = createLogger('p2p:tx_validator:tx_metadata');
1517

16-
constructor(private chainId: Fr, private rollupVersion: Fr, private blockNumber: Fr) {}
18+
constructor(
19+
private values: { l1ChainId: Fr; rollupVersion: Fr; blockNumber: Fr; vkTreeRoot: Fr; protocolContractTreeRoot: Fr },
20+
) {}
1721

1822
async validateTx(tx: T): Promise<TxValidationResult> {
1923
const errors = [];
20-
if (!(await this.#hasCorrectChainId(tx))) {
21-
errors.push(TX_ERROR_INCORRECT_CHAIN_ID);
24+
if (!(await this.#hasCorrectL1ChainId(tx))) {
25+
errors.push(TX_ERROR_INCORRECT_L1_CHAIN_ID);
2226
}
2327
if (!(await this.#hasCorrectRollupVersion(tx))) {
2428
errors.push(TX_ERROR_INCORRECT_ROLLUP_VERSION);
2529
}
2630
if (!(await this.#isValidForBlockNumber(tx))) {
2731
errors.push(TX_ERROR_INVALID_BLOCK_NUMBER);
2832
}
33+
if (!(await this.#hasCorrectVkTreeRoot(tx))) {
34+
errors.push(TX_ERROR_INCORRECT_VK_TREE_ROOT);
35+
}
36+
if (!(await this.#hasCorrectProtocolContractTreeRoot(tx))) {
37+
errors.push(TX_ERROR_INCORRECT_PROTOCOL_CONTRACT_TREE_ROOT);
38+
}
2939
return errors.length > 0 ? { result: 'invalid', reason: errors } : { result: 'valid' };
3040
}
3141

32-
async #hasCorrectChainId(tx: T): Promise<boolean> {
33-
if (!tx.data.constants.txContext.chainId.equals(this.chainId)) {
42+
async #hasCorrectVkTreeRoot(tx: T): Promise<boolean> {
43+
// This gets implicitly tested in the proof validator, but we can get a much cheaper check here by looking early at the vk.
44+
if (!tx.data.constants.vkTreeRoot.equals(this.values.vkTreeRoot)) {
45+
this.#log.warn(
46+
`Rejecting tx ${await Tx.getHash(
47+
tx,
48+
)} because of incorrect vk tree root ${tx.data.constants.vkTreeRoot.toString()} != ${this.values.vkTreeRoot.toString()}`,
49+
);
50+
return false;
51+
} else {
52+
return true;
53+
}
54+
}
55+
56+
async #hasCorrectProtocolContractTreeRoot(tx: T): Promise<boolean> {
57+
if (!tx.data.constants.protocolContractTreeRoot.equals(this.values.protocolContractTreeRoot)) {
58+
this.#log.warn(
59+
`Rejecting tx ${await Tx.getHash(
60+
tx,
61+
)} because of incorrect protocol contract tree root ${tx.data.constants.protocolContractTreeRoot.toString()} != ${this.values.protocolContractTreeRoot.toString()}`,
62+
);
63+
return false;
64+
}
65+
return true;
66+
}
67+
68+
async #hasCorrectL1ChainId(tx: T): Promise<boolean> {
69+
if (!tx.data.constants.txContext.chainId.equals(this.values.l1ChainId)) {
3470
this.#log.warn(
3571
`Rejecting tx ${await Tx.getHash(
3672
tx,
37-
)} because of incorrect chain ${tx.data.constants.txContext.chainId.toNumber()} != ${this.chainId.toNumber()}`,
73+
)} because of incorrect L1 chain ${tx.data.constants.txContext.chainId.toNumber()} != ${this.values.l1ChainId.toNumber()}`,
3874
);
3975
return false;
4076
} else {
@@ -45,11 +81,11 @@ export class MetadataTxValidator<T extends AnyTx> implements TxValidator<T> {
4581
async #isValidForBlockNumber(tx: T): Promise<boolean> {
4682
const maxBlockNumber = tx.data.rollupValidationRequests.maxBlockNumber;
4783

48-
if (maxBlockNumber.isSome && maxBlockNumber.value < this.blockNumber) {
84+
if (maxBlockNumber.isSome && maxBlockNumber.value < this.values.blockNumber) {
4985
this.#log.warn(
5086
`Rejecting tx ${await Tx.getHash(tx)} for low max block number. Tx max block number: ${
5187
maxBlockNumber.value
52-
}, current block number: ${this.blockNumber}.`,
88+
}, current block number: ${this.values.blockNumber}.`,
5389
);
5490
return false;
5591
} else {
@@ -58,11 +94,11 @@ export class MetadataTxValidator<T extends AnyTx> implements TxValidator<T> {
5894
}
5995

6096
async #hasCorrectRollupVersion(tx: T): Promise<boolean> {
61-
if (!tx.data.constants.txContext.version.equals(this.rollupVersion)) {
97+
if (!tx.data.constants.txContext.version.equals(this.values.rollupVersion)) {
6298
this.#log.warn(
6399
`Rejecting tx ${await Tx.getHash(
64100
tx,
65-
)} because of incorrect rollup version ${tx.data.constants.txContext.version.toNumber()} != ${this.rollupVersion.toNumber()}`,
101+
)} because of incorrect rollup version ${tx.data.constants.txContext.version.toNumber()} != ${this.values.rollupVersion.toNumber()}`,
66102
);
67103
return false;
68104
} else {

0 commit comments

Comments
 (0)