coder5876 / simple-multisig

Simple multisig for Ethereum using detached signatures
MIT License
247 stars 108 forks source link

Check that threshold is positive instead of non-zero #22

Closed ripper234 closed 6 years ago

ripper234 commented 6 years ago

This is semantically equivalent because threadshold is unsigned, so this proposed change should have zero effect on the code.

However, I believe that it is slightly more legible/simple this way. Reasoning about threshold_ <= owners_.length && threshold_ >= 0 is easier than reasoning about threshold_ <= owners_.length && threshold_ != 0 because you don't have to think about threshold's type. The semantic we are trying to check is is that threashold is within the range [0, owners_.length), and I believe my suggestion captures this semantic slightly better.

ripper234 commented 6 years ago

Contained in #27