search

Zilliqa / zq2

Zilliqa 2.0 code base
Apache License 2.0
6 stars 0 forks source link

Nodes panic on removal of validator #1143

Open DrZoltanFazekas opened 3 days ago

DrZoltanFazekas commented 3 days ago

We saw nodes panicking because they tried to unwrap None in https://github.com/Zilliqa/zq2/blob/main/zilliqa/src/consensus.rs#L677 to apply rewards of a staker whom we removed from the validator set through a transaction calling deposit::tempRemoveStaker() in the current block.

https://github.com/Zilliqa/zq2/pull/1120 changes the way we determine the leader in apply_rewards() . It was based on the previous block and not the current block's state before: https://github.com/Zilliqa/zq2/blob/main/zilliqa/src/consensus.rs#L632-L634

With this change we would pick a different leader for the current block in case the actual block proposer were removed from the validator set in the current block. I think this is wrong since the actual proposer was selected from the validator set based on the previous block's state before the current block was proposed. The fact that the current block's proposer is removed from the validator set in the current block should only have an impact on the leader selection of the subsequent blocks.

Related to that, there is another problem because we're trying to get the reward address of the leader in https://github.com/Zilliqa/zq2/blob/main/zilliqa/src/consensus.rs#L640 and the reward addresses of the cosigners in https://github.com/Zilliqa/zq2/blob/main/zilliqa/src/consensus.rs#L652 (which was the problem we ran into) based on the current block's state, but their pub_key does not exist anymore in the deposit contract's current state since our transaction executed in the current block removed it.

As discussed, we would need to fetch the reward addresses based on the previous block's state in which the deposit contract had the respective pub_key before they were removed in the current block.

theo-zil commented 3 days ago

I believe an easy solution discussed was to apply the rewards as the first state change in the block. This would be a 1-line change (move the apply_rewards() call inside execute_block() up above the transaction execution section) and would effectively use the state of the previous block for the rewards and committee, before any of the current block's changes