Skip to content

Commit 50728af

Browse files
authored
fix: address tmnt-144 (#16436)
Please read [contributing guidelines](CONTRIBUTING.md) and remove this line. For audit-related pull requests, please use the [audit PR template](?expand=1&template=audit.md).
2 parents 267e2b0 + 3efcf01 commit 50728af

File tree

9 files changed

+196
-95
lines changed

9 files changed

+196
-95
lines changed

l1-contracts/gas_benchmark.md

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -16,23 +16,23 @@
1616

1717
| Function | Avg Gas | Max Gas | Calldata Size | Calldata Gas |
1818
|----------------------|---------|---------|---------------|--------------|
19-
| propose | 152,636 | 169,163 | 1,060 | 16,960 |
20-
| submitEpochRootProof | 569,702 | 592,918 | 3,812 | 60,992 |
21-
| setupEpoch | 40,827 | 108,414 | - | - |
19+
| propose | 142,569 | 157,723 | 1,060 | 16,960 |
20+
| submitEpochRootProof | 567,842 | 591,073 | 3,812 | 60,992 |
21+
| setupEpoch | 31,585 | 108,397 | - | - |
2222

23-
**Avg Gas Cost per Second**: 923.0 gas/second
23+
**Avg Gas Cost per Second**: 869.2 gas/second
2424
*Epoch duration*: 2h 33m 36s
2525

2626
### Validators (IGNITION)
2727

2828
| Function | Avg Gas | Max Gas | Calldata Size | Calldata Gas |
2929
|----------------------|---------|---------|---------------|--------------|
30-
| propose | 210,504 | 227,206 | 2,852 | 45,632 |
31-
| submitEpochRootProof | 680,098 | 703,302 | 5,092 | 81,472 |
32-
| aggregate3 | 257,879 | 281,656 | - | - |
33-
| setupEpoch | 47,388 | 327,101 | - | - |
30+
| propose | 206,977 | 226,338 | 2,852 | 45,632 |
31+
| submitEpochRootProof | 679,690 | 701,700 | 5,092 | 81,472 |
32+
| aggregate3 | 256,679 | 286,474 | - | - |
33+
| setupEpoch | 38,145 | 327,074 | - | - |
3434

35-
**Avg Gas Cost per Second**: 1,249.1 gas/second
35+
**Avg Gas Cost per Second**: 1,229.6 gas/second
3636
*Epoch duration*: 2h 33m 36s
3737

3838

@@ -52,22 +52,22 @@
5252

5353
| Function | Avg Gas | Max Gas | Calldata Size | Calldata Gas |
5454
|----------------------|---------|---------|---------------|--------------|
55-
| propose | 230,591 | 247,557 | 1,060 | 16,960 |
56-
| submitEpochRootProof | 689,363 | 728,218 | 3,812 | 60,992 |
57-
| setupEpoch | 42,002 | 110,935 | - | - |
55+
| propose | 219,926 | 235,984 | 1,060 | 16,960 |
56+
| submitEpochRootProof | 687,107 | 726,001 | 3,812 | 60,992 |
57+
| setupEpoch | 32,376 | 108,397 | - | - |
5858

59-
**Avg Gas Cost per Second**: 7,638.6 gas/second
59+
**Avg Gas Cost per Second**: 7,330.1 gas/second
6060
*Epoch duration*: 0h 19m 12s
6161

6262
### Validators (Alpha)
6363

6464
| Function | Avg Gas | Max Gas | Calldata Size | Calldata Gas |
6565
|----------------------|---------|---------|---------------|--------------|
66-
| propose | 337,553 | 355,991 | 4,580 | 73,280 |
67-
| submitEpochRootProof | 897,057 | 935,029 | 6,308 | 100,928 |
68-
| aggregate3 | 389,910 | 411,588 | - | - |
69-
| setupEpoch | 59,354 | 544,742 | - | - |
66+
| propose | 334,025 | 351,385 | 4,580 | 73,280 |
67+
| submitEpochRootProof | 895,025 | 933,081 | 6,308 | 100,928 |
68+
| aggregate3 | 386,141 | 411,884 | - | - |
69+
| setupEpoch | 49,728 | 542,190 | - | - |
7070

71-
**Avg Gas Cost per Second**: 10,985.4 gas/second
71+
**Avg Gas Cost per Second**: 10,875.5 gas/second
7272
*Epoch duration*: 0h 19m 12s
7373

l1-contracts/gas_benchmark_results.json

Lines changed: 56 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -3,125 +3,125 @@
33
"no_validators": {
44
"propose": {
55
"calls": 100,
6-
"min": 149608,
7-
"mean": 152636,
8-
"median": 152418,
9-
"max": 169163,
6+
"min": 139463,
7+
"mean": 142569,
8+
"median": 142433,
9+
"max": 157723,
1010
"calldata_size": 1060,
1111
"calldata_gas": 16960
1212
},
1313
"setupEpoch": {
1414
"calls": 100,
15-
"min": 37692,
16-
"mean": 40827,
17-
"median": 39692,
18-
"max": 108414
15+
"min": 29235,
16+
"mean": 31585,
17+
"median": 29235,
18+
"max": 108397
1919
},
2020
"submitEpochRootProof": {
2121
"calls": 2,
22-
"min": 546487,
23-
"mean": 569702,
24-
"median": 569702,
25-
"max": 592918,
22+
"min": 544611,
23+
"mean": 567842,
24+
"median": 567842,
25+
"max": 591073,
2626
"calldata_size": 3812,
2727
"calldata_gas": 60992
2828
}
2929
},
3030
"validators": {
3131
"propose": {
3232
"calls": 100,
33-
"min": 204064,
34-
"mean": 210504,
35-
"median": 209860,
36-
"max": 227206,
33+
"min": 200706,
34+
"mean": 206977,
35+
"median": 207326,
36+
"max": 226338,
3737
"calldata_size": 2852,
3838
"calldata_gas": 45632
3939
},
4040
"setupEpoch": {
4141
"calls": 100,
42-
"min": 37692,
43-
"mean": 47388,
44-
"median": 39692,
45-
"max": 327101
42+
"min": 29235,
43+
"mean": 38145,
44+
"median": 29235,
45+
"max": 327074
4646
},
4747
"submitEpochRootProof": {
4848
"calls": 2,
49-
"min": 656895,
50-
"mean": 680098,
51-
"median": 680098,
52-
"max": 703302,
49+
"min": 657681,
50+
"mean": 679690,
51+
"median": 679690,
52+
"max": 701700,
5353
"calldata_size": 5092,
5454
"calldata_gas": 81472
5555
},
5656
"aggregate3": {
5757
"calls": 100,
58-
"min": 247273,
59-
"mean": 257879,
60-
"median": 255561,
61-
"max": 281656
58+
"min": 243742,
59+
"mean": 256679,
60+
"median": 253598,
61+
"max": 286474
6262
}
6363
}
6464
},
6565
"alpha": {
6666
"no_validators": {
6767
"propose": {
6868
"calls": 100,
69-
"min": 221621,
70-
"mean": 230591,
71-
"median": 229876,
72-
"max": 247557,
69+
"min": 212503,
70+
"mean": 219926,
71+
"median": 219136,
72+
"max": 235984,
7373
"calldata_size": 1060,
7474
"calldata_gas": 16960
7575
},
7676
"setupEpoch": {
7777
"calls": 100,
78-
"min": 37692,
79-
"mean": 42002,
80-
"median": 39692,
81-
"max": 110935
78+
"min": 29235,
79+
"mean": 32376,
80+
"median": 29235,
81+
"max": 108397
8282
},
8383
"submitEpochRootProof": {
8484
"calls": 3,
85-
"min": 669888,
86-
"mean": 689363,
87-
"median": 669984,
88-
"max": 728218,
85+
"min": 667612,
86+
"mean": 687107,
87+
"median": 667708,
88+
"max": 726001,
8989
"calldata_size": 3812,
9090
"calldata_gas": 60992
9191
}
9292
},
9393
"validators": {
9494
"propose": {
9595
"calls": 100,
96-
"min": 323268,
97-
"mean": 337553,
98-
"median": 337214,
99-
"max": 355991,
96+
"min": 318448,
97+
"mean": 334025,
98+
"median": 334471,
99+
"max": 351385,
100100
"calldata_size": 4580,
101101
"calldata_gas": 73280
102102
},
103103
"setupEpoch": {
104104
"calls": 100,
105-
"min": 37692,
106-
"mean": 59354,
107-
"median": 39692,
108-
"max": 544742
105+
"min": 29235,
106+
"mean": 49728,
107+
"median": 29235,
108+
"max": 542190
109109
},
110110
"submitEpochRootProof": {
111111
"calls": 3,
112-
"min": 876819,
113-
"mean": 897057,
114-
"median": 879324,
115-
"max": 935029,
112+
"min": 874800,
113+
"mean": 895025,
114+
"median": 877195,
115+
"max": 933081,
116116
"calldata_size": 6308,
117117
"calldata_gas": 100928
118118
},
119119
"aggregate3": {
120120
"calls": 100,
121-
"min": 366564,
122-
"mean": 389910,
123-
"median": 389226,
124-
"max": 411588
121+
"min": 362550,
122+
"mean": 386141,
123+
"median": 384626,
124+
"max": 411884
125125
}
126126
}
127127
}

l1-contracts/src/governance/proposer/EmpireBase.sol

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,8 @@ abstract contract EmpireBase is EIP712, IEmpire {
107107
using CompressedTimeMath for CompressedSlot;
108108

109109
// EIP-712 type hash for the Signal struct
110-
bytes32 public constant SIGNAL_TYPEHASH = keccak256("Signal(address payload,uint256 nonce,uint256 round)");
110+
bytes32 public constant SIGNAL_TYPEHASH =
111+
keccak256("Signal(address payload,uint256 nonce,uint256 round,address instance)");
111112

112113
// The number of signals needed for a payload to be considered submittable.
113114
uint256 public immutable QUORUM_SIZE;
@@ -269,7 +270,7 @@ abstract contract EmpireBase is EIP712, IEmpire {
269270
}
270271

271272
function getSignalSignatureDigest(IPayload _payload, address _signaler, uint256 _round) public view returns (bytes32) {
272-
return _hashTypedDataV4(keccak256(abi.encode(SIGNAL_TYPEHASH, _payload, nonces[_signaler], _round)));
273+
return _hashTypedDataV4(keccak256(abi.encode(SIGNAL_TYPEHASH, _payload, nonces[_signaler], _round, getInstance())));
273274
}
274275

275276
// Virtual functions

l1-contracts/test/governance/governance-proposer/scenario/15123.t.sol

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,10 @@ contract Test15123 is GovernanceProposerBase {
8080
bytes32 domainSeparator =
8181
keccak256(abi.encode(TYPE_HASH, hashedName, hashedVersion, block.chainid, address(governanceProposer)));
8282
bytes32 digest = MessageHashUtils.toTypedDataHash(
83-
domainSeparator, keccak256(abi.encode(governanceProposer.SIGNAL_TYPEHASH(), _payload, _nonce, _round))
83+
domainSeparator,
84+
keccak256(
85+
abi.encode(governanceProposer.SIGNAL_TYPEHASH(), _payload, _nonce, _round, governanceProposer.getInstance())
86+
)
8487
);
8588

8689
(uint8 v, bytes32 r, bytes32 s) = vm.sign(_privateKey, digest);
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
// SPDX-License-Identifier: UNLICENSED
2+
pragma solidity >=0.8.27;
3+
4+
import {GovernanceProposerBase} from "../Base.t.sol";
5+
6+
import {IPayload} from "@aztec/governance/interfaces/IPayload.sol";
7+
import {Slot, Timestamp} from "@aztec/core/libraries/TimeLib.sol";
8+
import {Fakerollup} from "../mocks/Fakerollup.sol";
9+
import {IRollup} from "@aztec/core/interfaces/IRollup.sol";
10+
import {Signature} from "@aztec/shared/libraries/SignatureLib.sol";
11+
import {MessageHashUtils} from "@oz/utils/cryptography/MessageHashUtils.sol";
12+
13+
// https://linear.app/aztec-labs/issue/TMNT-144/spearbit-gov-finding-3-missing-instance-in-digest
14+
contract TestTmnt144 is GovernanceProposerBase {
15+
using MessageHashUtils for bytes32;
16+
17+
IPayload internal proposal = IPayload(address(0xdeadbeef));
18+
address internal proposer = address(0);
19+
Fakerollup internal rollup1;
20+
Fakerollup internal rollup2;
21+
22+
uint256 internal privateKey = 0x1234567890;
23+
Signature internal signature;
24+
25+
function setUp() public override {
26+
super.setUp();
27+
}
28+
29+
function test_replay() external {
30+
rollup1 = new Fakerollup();
31+
32+
proposer = vm.addr(privateKey);
33+
rollup1.setProposer(proposer);
34+
35+
vm.prank(registry.getGovernance());
36+
registry.addRollup(IRollup(address(rollup1)));
37+
vm.warp(Timestamp.unwrap(rollup1.getTimestampForSlot(Slot.wrap(1))));
38+
39+
// Create a signature for the proposer
40+
uint256 round = governanceProposer.getCurrentRound();
41+
signature = createSignature(privateKey, address(proposal), round);
42+
43+
// For some reason, before he signals, the time progresses and he is no longer the proposer!
44+
// It is not even the same round actually
45+
vm.warp(Timestamp.unwrap(rollup1.getTimestampForSlot(Slot.wrap(2 + governanceProposer.ROUND_SIZE()))));
46+
rollup1.setProposer(address(0xdead));
47+
assertNotEq(round, governanceProposer.getCurrentRound());
48+
49+
// We therefore expect the tx to revert, but not before it was broadcast, so anyone can pick up the signature
50+
vm.expectRevert();
51+
governanceProposer.signalWithSig(proposal, signature);
52+
53+
// A new rollup swings by, and gets added to the registry!
54+
rollup2 = new Fakerollup();
55+
rollup2.setProposer(proposer);
56+
vm.prank(registry.getGovernance());
57+
registry.addRollup(IRollup(address(rollup2)));
58+
59+
vm.warp(Timestamp.unwrap(rollup2.getTimestampForSlot(Slot.wrap(1))));
60+
61+
// Some random dude that picked up the earlier signature sends it to vote using the new rollup!
62+
assertEq(governanceProposer.signalCount(address(rollup2), 0, proposal), 0, "invalid number of votes");
63+
64+
vm.expectRevert();
65+
governanceProposer.signalWithSig(proposal, signature);
66+
67+
assertEq(governanceProposer.signalCount(address(rollup2), 0, proposal), 0, "invalid number of votes");
68+
}
69+
70+
function createSignature(uint256 _privateKey, address _payload, uint256 _round)
71+
internal
72+
view
73+
returns (Signature memory)
74+
{
75+
address signer = vm.addr(_privateKey);
76+
bytes32 digest = governanceProposer.getSignalSignatureDigest(IPayload(_payload), signer, _round);
77+
78+
(uint8 v, bytes32 r, bytes32 s) = vm.sign(_privateKey, digest);
79+
80+
return Signature({v: v, r: r, s: s});
81+
}
82+
}

l1-contracts/test/governance/governance-proposer/voteWithsig.t.sol

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,9 @@ contract SignalWithSigTest is GovernanceProposerBase {
6363

6464
// We jump into the future since slot 0, will behave as if already signald in
6565
vm.warp(Timestamp.unwrap(validatorSelection.getTimestampForSlot(Slot.wrap(1))));
66+
67+
// Also we need to generate a signature because it is using the address of the instance as part of it.
68+
signature = createSignature(privateKey, proposal);
6669
_;
6770
}
6871

@@ -359,20 +362,7 @@ contract SignalWithSigTest is GovernanceProposerBase {
359362

360363
function getDigest(uint256 _privateKey, IPayload _payload, uint256 _round) internal view returns (bytes32) {
361364
address p = vm.addr(_privateKey);
362-
uint256 nonce = governanceProposer.nonces(p);
363-
bytes32 domainSeparator = keccak256(
364-
abi.encode(
365-
keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"),
366-
keccak256(bytes("EmpireBase")),
367-
keccak256(bytes("1")),
368-
block.chainid,
369-
address(governanceProposer)
370-
)
371-
);
372-
bytes32 digest = MessageHashUtils.toTypedDataHash(
373-
domainSeparator, keccak256(abi.encode(governanceProposer.SIGNAL_TYPEHASH(), _payload, nonce, _round))
374-
);
375-
return digest;
365+
return governanceProposer.getSignalSignatureDigest(_payload, p, _round);
376366
}
377367

378368
function createSignature(uint256 _privateKey, IPayload _payload) internal view returns (Signature memory) {

0 commit comments

Comments
 (0)