Skip to content

Commit 07650da

Browse files
LHerskindLHerskind
committed
fix: address tmnt 149
1 parent eb8ca7a commit 07650da

File tree

9 files changed

+57
-16
lines changed

9 files changed

+57
-16
lines changed

‎l1-contracts/src/governance/libraries/AddressSnapshotLib.sol‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ struct Index {
2828
// AddressSnapshotLib
2929
error AddressSnapshotLib__IndexOutOfBounds(uint256 index, uint256 size); // 0xd789b71a
3030
error AddressSnapshotLib__AddressNotInSet(address addr);
31+
error AddressSnapshotLib__CannotAddAddressZero();
3132

3233
/**
3334
* @title AddressSnapshotLib
@@ -52,6 +53,7 @@ library AddressSnapshotLib {
5253
* @return bool True if the address was added, false if it was already present
5354
*/
5455
function add(SnapshottedAddressSet storage _self, address _address) internal returns (bool) {
56+
require(_address != address(0), AddressSnapshotLib__CannotAddAddressZero());
5557
// Prevent against double insertion
5658
if (_self.addressToCurrentIndex[_address].exists) {
5759
return false;

‎l1-contracts/test/governance/governance/address_snapshots/AddressSnapshotsBase.t.sol‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ contract AddressSnapshotsBase is Test {
6969
// Ensure addresses within _addrSet1 are unique
7070
vm.assume(_addrs.length > 0 && _addrs.length < 16);
7171
for (uint256 i = 0; i < _addrs.length; i++) {
72+
vm.assume(_addrs[i] != address(0));
7273
for (uint256 j = 0; j < i; j++) {
7374
vm.assume(_addrs[i] != _addrs[j]);
7475
}

‎l1-contracts/test/governance/governance/address_snapshots/add.t.sol‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,22 @@ import {AddressSnapshotsBase} from "./AddressSnapshotsBase.t.sol";
66
import {
77
AddressSnapshotLib,
88
SnapshottedAddressSet,
9-
AddressSnapshotLib__IndexOutOfBounds
9+
AddressSnapshotLib__IndexOutOfBounds,
10+
AddressSnapshotLib__CannotAddAddressZero
1011
} from "@aztec/governance/libraries/AddressSnapshotLib.sol";
1112

1213
contract AddressSnapshotAddTest is AddressSnapshotsBase {
13-
function test_WhenValidatorIsNotInTheSet(address _addr, uint16 _add2) public {
14+
function test_WhenAddressIsZero() public {
15+
vm.expectRevert(abi.encodeWithSelector(AddressSnapshotLib__CannotAddAddressZero.selector));
16+
validatorSet.add(address(0));
17+
}
18+
19+
modifier whenAddressIsNotZero(address _addr) {
20+
vm.assume(_addr != address(0));
21+
_;
22+
}
23+
24+
function test_WhenValidatorIsNotInTheSet(address _addr, uint16 _add2) public whenAddressIsNotZero(_addr) {
1425
// It returns true
1526
// It increases the length
1627
// It creates a checkpoint for the next epoch
@@ -57,7 +68,7 @@ contract AddressSnapshotAddTest is AddressSnapshotsBase {
5768
assertEq(validatorSet.getAddressFromIndexAtTimestamp(0, ts2), _addr);
5869
}
5970

60-
function test_WhenValidatorIsAlreadyInTheSet(address _addr) public {
71+
function test_WhenValidatorIsAlreadyInTheSet(address _addr) public whenAddressIsNotZero(_addr) {
6172
// It returns false
6273

6374
validatorSet.add(_addr);

‎l1-contracts/test/governance/governance/address_snapshots/add.tree‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
AddTest
2-
├── when validator is not in the set
3-
│ ├── it returns true
4-
│ ├── it increases the length
5-
│ ├── it creates a checkpoint for the next epoch
6-
│ └── it does not change the current epoch
7-
├── when validator is already in the set
8-
│ └── it returns false
9-
└── when validator has been removed from the set
10-
└── it can be added again
11-
2+
├── when address is zero
3+
│ └── it reverts
4+
└── when address is not zero
5+
├── when validator is not in the set
6+
│ ├── it returns true
7+
│ ├── it increases the length
8+
│ ├── it creates a checkpoint for the next epoch
9+
│ └── it does not change the current epoch
10+
├── when validator is already in the set
11+
│ └── it returns false
12+
└── when validator has been removed from the set
13+
└── it can be added again

‎l1-contracts/test/governance/gse/gse/base.sol‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ contract WithGSE is TestBase {
2323
}
2424

2525
function cheat_deposit(address _instance, address _attester, address _withdrawer, bool _onBonus) public {
26+
vm.assume(_attester != address(0));
27+
2628
uint256 activationThreshold = gse.ACTIVATION_THRESHOLD();
2729

2830
vm.prank(stakingAsset.owner());

‎l1-contracts/test/governance/gse/gse/deposit.t.sol‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ contract DepositTest is WithGSE {
1717

1818
vm.prank(_instance);
1919
vm.expectRevert(abi.encodeWithSelector(Errors.GSE__NotRollup.selector, _instance));
20-
gse.deposit(address(0), address(0), BN254Lib.g1Zero(), BN254Lib.g2Zero(), BN254Lib.g1Zero(), onBonus);
20+
gse.deposit(address(1), address(0), BN254Lib.g1Zero(), BN254Lib.g2Zero(), BN254Lib.g1Zero(), onBonus);
2121
}
2222

2323
modifier whenCallerIsRegisteredRollup(address _instance) {
@@ -45,7 +45,7 @@ contract DepositTest is WithGSE {
4545

4646
vm.prank(_instance1);
4747
vm.expectRevert(abi.encodeWithSelector(Errors.GSE__NotLatestRollup.selector, _instance1));
48-
gse.deposit(address(0), address(0), BN254Lib.g1Zero(), BN254Lib.g2Zero(), BN254Lib.g1Zero(), onBonus);
48+
gse.deposit(address(1), address(0), BN254Lib.g1Zero(), BN254Lib.g2Zero(), BN254Lib.g1Zero(), onBonus);
4949
}
5050

5151
modifier givenCallerIsLatest() {
@@ -58,6 +58,7 @@ contract DepositTest is WithGSE {
5858
address _withdrawer
5959
) external whenCallerIsRegisteredRollup(_instance) givenOnBonusEqTrue givenCallerIsLatest {
6060
// it reverts
61+
vm.assume(_attester != address(0));
6162

6263
uint256 activationThreshold = gse.ACTIVATION_THRESHOLD();
6364

@@ -81,6 +82,7 @@ contract DepositTest is WithGSE {
8182
givenCallerIsLatest
8283
{
8384
// it reverts
85+
vm.assume(_attester != address(0));
8486

8587
uint256 activationThreshold = gse.ACTIVATION_THRESHOLD();
8688

@@ -114,6 +116,8 @@ contract DepositTest is WithGSE {
114116
// it deposits staking asset to governance
115117
// it emits Deposit event
116118

119+
vm.assume(_attester != address(0));
120+
117121
uint256 activationThreshold = gse.ACTIVATION_THRESHOLD();
118122

119123
vm.prank(stakingAsset.owner());
@@ -165,6 +169,8 @@ contract DepositTest is WithGSE {
165169
// the only diff is `onBonus = false` here. We could consider slamming them together, but to be
166170
// explicit we are not doing that.
167171

172+
vm.assume(_attester != address(0));
173+
168174
uint256 activationThreshold = gse.ACTIVATION_THRESHOLD();
169175

170176
vm.prank(stakingAsset.owner());
@@ -187,6 +193,8 @@ contract DepositTest is WithGSE {
187193
) external whenCallerIsRegisteredRollup(_instance) givenOnBonusEqFalse {
188194
// it reverts
189195

196+
vm.assume(_attester != address(0));
197+
190198
// Again, this one is essentially same as test_GivenAttesterAlreadyRegisteredOnBonus but with false.
191199

192200
uint256 activationThreshold = gse.ACTIVATION_THRESHOLD();
@@ -224,6 +232,8 @@ contract DepositTest is WithGSE {
224232
// it deposits staking asset to governance
225233
// it emits Deposit event
226234

235+
vm.assume(_attester != address(0));
236+
227237
uint256 activationThreshold = gse.ACTIVATION_THRESHOLD();
228238

229239
vm.prank(stakingAsset.owner());
@@ -275,6 +285,8 @@ contract DepositTest is WithGSE {
275285
// it deposits staking asset to governance
276286
// it emits Deposit event
277287

288+
vm.assume(_attester != address(0));
289+
278290
// @todo exactly the same as above, with only diff being not latest
279291

280292
uint256 activationThreshold = gse.ACTIVATION_THRESHOLD();

‎l1-contracts/test/governance/gse/gse/depositBN254.t.sol‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ contract DepositBN254Test is WithGSE {
6262
// it reverts
6363
vm.prank(_instance);
6464
vm.expectRevert(abi.encodeWithSelector(BN254Lib.NotOnCurve.selector, 0, 0));
65-
gse.deposit(address(0), address(0), BN254Lib.g1Zero(), BN254Lib.g2Zero(), BN254Lib.g1Zero(), _moveWithLatestRollup);
65+
gse.deposit(address(1), address(0), BN254Lib.g1Zero(), BN254Lib.g2Zero(), BN254Lib.g1Zero(), _moveWithLatestRollup);
6666
}
6767

6868
function test_WhenTheDepositKeysPassTheProofOfPossessionCheck(
@@ -72,6 +72,8 @@ contract DepositBN254Test is WithGSE {
7272
address _withdrawer,
7373
bool _moveWithLatestRollup
7474
) external whenCallerIsRegisteredRollup(_instance) {
75+
vm.assume(_attester1 != address(0));
76+
vm.assume(_attester2 != address(0));
7577
vm.assume(_attester1 != _attester2);
7678
// it adds the keys if they are new
7779
uint256 activationThreshold = gse.ACTIVATION_THRESHOLD();

‎l1-contracts/test/governance/gse/gse/vote.t.sol‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ contract VoteTest is WithGSE {
2626
) external {
2727
// it reverts
2828

29+
vm.assume(_attester != address(0));
30+
2931
(address voter, uint256 availablePower) = _prepare(_instance, _attester, _delegatee, _delegate);
3032
uint256 amount = bound(_amount, availablePower + 1, type(uint256).max);
3133

@@ -47,6 +49,8 @@ contract VoteTest is WithGSE {
4749
// it uses delegation power for proposal
4850
// it votes in governance
4951

52+
vm.assume(_attester != address(0));
53+
5054
Proposal memory proposal = governance.getProposal(0);
5155
assertEq(proposal.summedBallot.yea, 0);
5256
assertEq(proposal.summedBallot.nay, 0);
@@ -70,6 +74,7 @@ contract VoteTest is WithGSE {
7074
returns (address, uint256)
7175
{
7276
vm.assume(_instance != address(0) && _instance != gse.getBonusInstanceAddress());
77+
vm.assume(_attester != address(0));
7378

7479
vm.prank(gse.owner());
7580
gse.addRollup(_instance);

‎l1-contracts/test/governance/gse/gse/voteWithBonus.t.sol‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ contract VoteWithBonusTest is WithGSE {
4646
{
4747
// it reverts
4848

49+
vm.assume(_attester != address(0));
50+
4951
uint256 availablePower = _prepare(_instance, _attester);
5052
uint256 amount = bound(_amount, availablePower + 1, type(uint256).max);
5153

@@ -65,6 +67,8 @@ contract VoteWithBonusTest is WithGSE {
6567
// it uses delegation power for bonus
6668
// it votes in governance
6769

70+
vm.assume(_attester != address(0));
71+
6872
Proposal memory proposal = governance.getProposal(0);
6973
assertEq(proposal.summedBallot.yea, 0);
7074
assertEq(proposal.summedBallot.nay, 0);

0 commit comments

Comments
 (0)