nimiq / core-rs-albatross

Rust implementation of the Albatross protocol
https://nimiq.com
Other
161 stars 63 forks source link

Wrong validator set for pk_tree_root calculation #326

Closed brunoffranca closed 2 years ago

brunoffranca commented 3 years ago

On election blocks we are using the current validator set to calculate the pk_tree_root, it should be instead the next validator set.

fiaxh commented 3 years ago

Reopening the issue since the respective commits have been reverted. They caused #343.

brunoffranca commented 3 years ago

By using the correct validator set we uncovered another problem. The Handel aggregation keeps a map from proposal hashes to the corresponding multisignatures. This allows us to keep track and aggregate all of the votes that we receive even if they are for different proposals. So far, the proposal hash was all the information that we needed in order to verify the votes that we received. We also need the block number, round number, step (which are equal for all proposals) and pk_tree_root (which was equal for all proposals when we were using the current validator set, but now that we use the next validator set it differs across proposals). The Handel aggregation then needs a way of keeping track of the pk_tree_root for each proposal. We discussed it a bit and have two solutions:

  1. Remove the pk_tree_root from the TendermintVote. For this we could modify the way we calculate the hash header to be header_hash = H(H(header)||pk_tree_root). This way the Handel aggregation doesn't need to be changed at all. But the nano-zkp circuit would need to be changed. This means that the circuit will need to do one extra hash, but we probably have the space for that.
  2. Make the Handel aggregation explicitly keep track of the pk_tree_root. However, since the pk_tree_root is currently 95 bytes, we probably want add an extra hash step (using Blake2s) to get the size down to 32 bytes. This solution requires us to alter both the Handel aggregation and the nano-zkp circuit (only if we want that size optimization).

Since this pk_tree_root stuff is only necessary for nano nodes, we will first release the devnet and then fix this.