Add test cases for EnumerableGuardianMap. #6
Conversation
|
|
||
| function test_Set_RevertWhen_MaxNumberOfGuardiansReached() public { | ||
| bool result; | ||
| // TODO: Can it be acceptable to number 33? |
There was a problem hiding this comment.
Regarding current implementation, it allows to store 33 guardians. Is that expected behavior?
There was a problem hiding this comment.
Ah yes good catch, expected behaviour should be 32. Could you change to assertion in the library to ensure that the max number is 32?
There was a problem hiding this comment.
@JohnGuilding Fixed.
| } | ||
|
|
||
| function test_Keys_ReturnsEmptyArrayOfKeys() public view { |
There was a problem hiding this comment.
cool, can you remove this test case then?
| addresses[0] = vm.addr(1); | ||
| addresses[1] = vm.addr(2); | ||
| addresses[2] = vm.addr(3); | ||
| guardiansStorage[accountAddress].removeAll(addresses); |
There was a problem hiding this comment.
Can you add an assert to check that the values are no longer in storage?
| ) { | ||
| addresses[i] = vm.addr(i + 1); | ||
| } | ||
| guardiansStorage[accountAddress].removeAll(addresses); |
There was a problem hiding this comment.
Same here, can you add an assert to check that the values are no longer in storage?
| guardiansStorage[accountAddress].set({ | ||
| key: guardian1, | ||
| value: GuardianStorage(GuardianStatus.REQUESTED, guardianWeights[1]) | ||
| }); |
There was a problem hiding this comment.
Can you add an assert here
| assertEq(recoveryRequest.executeAfter, 0); | ||
| assertEq(recoveryRequest.executeBefore, 0); | ||
| assertEq(recoveryRequest.currentWeight, 1); | ||
| assertEq(recoveryRequest.calldataHashString, ""); |
There was a problem hiding this comment.
If you merge the latest changes in, this has been changed to bytes32 from a string
No description provided.