fix issue 256, 257 #300
fix issue 256, 257 #300
Conversation
webelf101
requested review from
jackzampolin,
jtremback and
zmanian
as code owners
There was a problem hiding this comment.
The refactor to use the signature struct is good. We used parallel arrays when writing this because at the time thats all that solidity supported in our use case.
The other changes, which i have commented on, are a waste of gas. If you would like i can get on a call and explain the threat model and why these changes are not necessary. We could probably go even further and remove some existing requires. I'm happy to help guide that effort.
| @@ -230,14 +228,12 @@ contract Gravity is ReentrancyGuard { | |||
| ); | |||
|
|
|||
| // Check that new validators and powers set is well-formed | |||
| require(_newValidators.length == _newPowers.length, "Malformed new validator set"); | |||
| require(_newValidators.length == _newPowers.length || _newValidators.length == 0, "Malformed new validator set"); | |||
There was a problem hiding this comment.
This is the first substantive change in this PR (other than the signature struct refactor). I guess it doesn't hurt but it's also not very useful. This entire require statement could probably go, and probably should. I just never got around to removing it.
There was a problem hiding this comment.
This update is related to the issue #257
Should I ignore this issue? @jtremback
No description provided.