(Fix) Replace abi.encodePacked with abi.encode #159
Conversation
| for (uint8 thresholdType = uint8(ThresholdTypes.Keys); thresholdType <= _thresholds.length; thresholdType++) { | ||
| uint256 thresholdValue = _thresholds[thresholdType - 1]; | ||
| uint256 thresholdValue = _thresholds[thresholdType - uint8(ThresholdTypes.Keys)]; |
There was a problem hiding this comment.
I think this won't help much: we're still assuming that ThresholdTypes.Keys is 1 and we still don't have guarantees from solidity on order and numeric values of enum members.
There was a problem hiding this comment.
@Eenae Maybe it is better for us not to use enums?
There was a problem hiding this comment.
It's up to you. I'm just saying that this particular change does't help much.
There was a problem hiding this comment.
@Eenae Solidity docs tell that valid values for enum of n members are 0 until n - 1: https://solidity.readthedocs.io/en/v0.4.24/miscellaneous.html#internals-cleaning-up-variables - so, enum in Solidity seems to be the same as in other programming languages. That still doesn't guarantee the order of enum values, but I think everything is ok with it because unit tests show that.
(Mandatory) Description
abi.encodePackedfunction has been replaced withabi.encodeto make bytes sequences forkeccak256more unique and to reduce hash collision probability.(Mandatory) What is it: (Fix), (Feature), or (Refactor)
(Fix)