poanetwork / posdao-contracts

Smart contracts for POSDAO (Proof of Stake Decentralized Autonomous Organization consensus), a DPOS consensus implemented in Solidity and running within EVM with swappable BFT consensus
Other
105 stars 49 forks source link

Revert "Fix initialization" #21

Closed varasev closed 5 years ago

varasev commented 5 years ago

I'm not agreed with the changes in ValidatorSetBase contract for the next functions:

@DemiMarie thanks for founding the bug. The proposed hack which uses overflows is a good workaround but is dangerous and more complex in the further supporting of the code.

The cons of such a solution are that a developer can read an index directly from the storage and forget to read an index by the getter. Also, this PR doesn't take into account the same situation for _removeFromPools function.

The fact that non-existent mapping item returns zero is normal for Solidity for architectural reasons. So, we should check for item's existence in another way.

I will add a simpler fix soon.

varasev commented 5 years ago

The fix is added: https://github.com/poanetwork/pos-contracts/commit/3f116f528f7a8a3ac8872f57fa021221becb1ed4#diff-7654c6ff5015e07981fd2db710fee7b7

Other commits related to reverted #16: https://github.com/poanetwork/pos-contracts/commit/2d0479c7bc2fcd04db7358234faf3055cdca5c23, https://github.com/poanetwork/pos-contracts/commit/a74ad7f4d7e0957029082c6c6bfe5c96c5a0d13e.