Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

(Fix) Replace abi.encodePacked with abi.encode #159

Merged
merged 2 commits into from Aug 8, 2018
Merged

(Fix) Replace abi.encodePacked with abi.encode #159

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a876e40
(Fix) Safer using ThresholdTypes enum
varasev Aug 8, 2018
1e9b85c
(Fix) Replace abi.encodePacked with abi.encode
varasev Aug 8, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 5 additions & 5 deletions contracts/BallotsStorage.sol
View file
Open in desktop
Expand Up @@ -80,9 +80,9 @@ contract BallotsStorage is EternalStorage, EnumBallotTypes, EnumKeyTypes, EnumTh
) public onlyOwner {
require(!initDisabled());
require(_thresholds.length == uint256(ThresholdTypes.MetadataChange));
require(_thresholds.length <= 255);
require(_thresholds.length < 255);
for (uint8 thresholdType = uint8(ThresholdTypes.Keys); thresholdType <= _thresholds.length; thresholdType++) {
uint256 thresholdValue = _thresholds[thresholdType - 1];
uint256 thresholdValue = _thresholds[thresholdType - uint8(ThresholdTypes.Keys)];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Eenae Maybe it is better for us not to use enums?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's up to you. I'm just saying that this particular change does't help much.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

require(thresholdValue > 0);
_setThreshold(thresholdValue, thresholdType);
}
Expand Down Expand Up @@ -111,7 +111,7 @@ contract BallotsStorage is EternalStorage, EnumBallotTypes, EnumKeyTypes, EnumTh
onlyVotingToChangeThreshold
returns(bool)
{
if (_thresholdType == 0) return false;
if (_thresholdType == uint8(ThresholdTypes.Invalid)) return false;
if (_thresholdType > uint8(ThresholdTypes.MetadataChange)) return false;
if (_newValue == 0) return false;
if (_newValue == getBallotThreshold(_thresholdType)) return false;
Expand All @@ -121,7 +121,7 @@ contract BallotsStorage is EternalStorage, EnumBallotTypes, EnumKeyTypes, EnumTh
}

function getBallotThreshold(uint8 _ballotType) public view returns(uint256) {
return uintStorage[keccak256(abi.encodePacked(BALLOT_THRESHOLDS, _ballotType))];
return uintStorage[keccak256(abi.encode(BALLOT_THRESHOLDS, _ballotType))];
}

function getVotingToChangeThreshold() public view returns(address) {
Expand Down Expand Up @@ -233,7 +233,7 @@ contract BallotsStorage is EternalStorage, EnumBallotTypes, EnumKeyTypes, EnumTh

function _setThreshold(uint256 _newValue, uint8 _thresholdType) internal {
uintStorage[
keccak256(abi.encodePacked(BALLOT_THRESHOLDS, _thresholdType))
keccak256(abi.encode(BALLOT_THRESHOLDS, _thresholdType))
] = _newValue;
}
}
40 changes: 20 additions & 20 deletions contracts/KeysManager.sol
View file
Open in desktop
Expand Up @@ -136,31 +136,31 @@ contract KeysManager is EternalStorage, IKeysManager {

function getInitialKeyStatus(address _initialKey) public view returns(uint8) {
return uint8(uintStorage[
keccak256(abi.encodePacked(INITIAL_KEY_STATUS, _initialKey))
keccak256(abi.encode(INITIAL_KEY_STATUS, _initialKey))
]);
}

function miningKeyByPayout(address _payoutKey) public view returns(address) {
return addressStorage[
keccak256(abi.encodePacked(MINING_KEY_BY_PAYOUT, _payoutKey))
keccak256(abi.encode(MINING_KEY_BY_PAYOUT, _payoutKey))
];
}

function miningKeyByVoting(address _votingKey) public view returns(address) {
return addressStorage[
keccak256(abi.encodePacked(MINING_KEY_BY_VOTING, _votingKey))
keccak256(abi.encode(MINING_KEY_BY_VOTING, _votingKey))
];
}

function miningKeyHistory(address _miningKey) public view returns(address) {
return addressStorage[
keccak256(abi.encodePacked(MINING_KEY_HISTORY, _miningKey))
keccak256(abi.encode(MINING_KEY_HISTORY, _miningKey))
];
}

function successfulValidatorClone(address _miningKey) public view returns(bool) {
return boolStorage[
keccak256(abi.encodePacked(SUCCESSFUL_VALIDATOR_CLONE, _miningKey))
keccak256(abi.encode(SUCCESSFUL_VALIDATOR_CLONE, _miningKey))
];
}

Expand Down Expand Up @@ -310,7 +310,7 @@ contract KeysManager is EternalStorage, IKeysManager {

function isMiningActive(address _key) public view returns(bool) {
return boolStorage[
keccak256(abi.encodePacked(VALIDATOR_KEYS, _key, IS_MINING_ACTIVE))
keccak256(abi.encode(VALIDATOR_KEYS, _key, IS_MINING_ACTIVE))
];
}

Expand All @@ -321,25 +321,25 @@ contract KeysManager is EternalStorage, IKeysManager {

function isVotingActiveByMiningKey(address _miningKey) public view returns(bool) {
return boolStorage[
keccak256(abi.encodePacked(VALIDATOR_KEYS, _miningKey, IS_VOTING_ACTIVE))
keccak256(abi.encode(VALIDATOR_KEYS, _miningKey, IS_VOTING_ACTIVE))
];
}

function isPayoutActive(address _miningKey) public view returns(bool) {
return boolStorage[
keccak256(abi.encodePacked(VALIDATOR_KEYS, _miningKey, IS_PAYOUT_ACTIVE))
keccak256(abi.encode(VALIDATOR_KEYS, _miningKey, IS_PAYOUT_ACTIVE))
];
}

function getVotingByMining(address _miningKey) public view returns(address) {
return addressStorage[
keccak256(abi.encodePacked(VALIDATOR_KEYS, _miningKey, VOTING_KEY))
keccak256(abi.encode(VALIDATOR_KEYS, _miningKey, VOTING_KEY))
];
}

function getPayoutByMining(address _miningKey) public view returns(address) {
return addressStorage[
keccak256(abi.encodePacked(VALIDATOR_KEYS, _miningKey, PAYOUT_KEY))
keccak256(abi.encode(VALIDATOR_KEYS, _miningKey, PAYOUT_KEY))
];
}

Expand Down Expand Up @@ -536,63 +536,63 @@ contract KeysManager is EternalStorage, IKeysManager {

function _setInitialKeyStatus(address _initialKey, uint8 _status) private {
uintStorage[
keccak256(abi.encodePacked(INITIAL_KEY_STATUS, _initialKey))
keccak256(abi.encode(INITIAL_KEY_STATUS, _initialKey))
] = _status;
}

function _setVotingKey(address _key, address _miningKey) private {
addressStorage[
keccak256(abi.encodePacked(VALIDATOR_KEYS, _miningKey, VOTING_KEY))
keccak256(abi.encode(VALIDATOR_KEYS, _miningKey, VOTING_KEY))
] = _key;
}

function _setPayoutKey(address _key, address _miningKey) private {
addressStorage[
keccak256(abi.encodePacked(VALIDATOR_KEYS, _miningKey, PAYOUT_KEY))
keccak256(abi.encode(VALIDATOR_KEYS, _miningKey, PAYOUT_KEY))
] = _key;
}

function _setIsMiningActive(bool _active, address _miningKey) private {
boolStorage[
keccak256(abi.encodePacked(VALIDATOR_KEYS, _miningKey, IS_MINING_ACTIVE))
keccak256(abi.encode(VALIDATOR_KEYS, _miningKey, IS_MINING_ACTIVE))
] = _active;
}

function _setIsVotingActive(bool _active, address _miningKey) private {
boolStorage[
keccak256(abi.encodePacked(VALIDATOR_KEYS, _miningKey, IS_VOTING_ACTIVE))
keccak256(abi.encode(VALIDATOR_KEYS, _miningKey, IS_VOTING_ACTIVE))
] = _active;
}

function _setIsPayoutActive(bool _active, address _miningKey) private {
boolStorage[
keccak256(abi.encodePacked(VALIDATOR_KEYS, _miningKey, IS_PAYOUT_ACTIVE))
keccak256(abi.encode(VALIDATOR_KEYS, _miningKey, IS_PAYOUT_ACTIVE))
] = _active;
}

function _setMiningKeyByPayout(address _payoutKey, address _miningKey) private {
if (_payoutKey == address(0)) return;
addressStorage[
keccak256(abi.encodePacked(MINING_KEY_BY_PAYOUT, _payoutKey))
keccak256(abi.encode(MINING_KEY_BY_PAYOUT, _payoutKey))
] = _miningKey;
}

function _setMiningKeyByVoting(address _votingKey, address _miningKey) private {
if (_votingKey == address(0)) return;
addressStorage[
keccak256(abi.encodePacked(MINING_KEY_BY_VOTING, _votingKey))
keccak256(abi.encode(MINING_KEY_BY_VOTING, _votingKey))
] = _miningKey;
}

function _setMiningKeyHistory(address _key, address _oldMiningKey) private {
addressStorage[
keccak256(abi.encodePacked(MINING_KEY_HISTORY, _key))
keccak256(abi.encode(MINING_KEY_HISTORY, _key))
] = _oldMiningKey;
}

function _setSuccessfulValidatorClone(bool _success, address _miningKey) private {
boolStorage[
keccak256(abi.encodePacked(SUCCESSFUL_VALIDATOR_CLONE, _miningKey))
keccak256(abi.encode(SUCCESSFUL_VALIDATOR_CLONE, _miningKey))
] = _success;
}

Expand Down
4 changes: 2 additions & 2 deletions contracts/RewardByBlock.sol
View file
Open in desktop
Expand Up @@ -93,7 +93,7 @@ contract RewardByBlock is EternalStorage, IRewardByBlock {

function extraReceiversAmounts(address _receiver) public view returns(uint256) {
return uintStorage[
keccak256(abi.encodePacked(EXTRA_RECEIVERS_AMOUNTS, _receiver))
keccak256(abi.encode(EXTRA_RECEIVERS_AMOUNTS, _receiver))
];
}

Expand Down Expand Up @@ -138,7 +138,7 @@ contract RewardByBlock is EternalStorage, IRewardByBlock {

function _setExtraReceiverAmount(address _receiver, uint256 _amount) private {
uintStorage[
keccak256(abi.encodePacked(EXTRA_RECEIVERS_AMOUNTS, _receiver))
keccak256(abi.encode(EXTRA_RECEIVERS_AMOUNTS, _receiver))
] = _amount;
}
}