search

chainflip-io / chainflip-backend

The Chainflip backend repo, including the Chainflip Node and CFE.
50 stars 14 forks source link

Inconsistent Consensus on External Chain Blocks for `vault_key_rotated` #738

Closed kylezs closed 2 years ago

kylezs commented 2 years ago

Describe the changes you'd like

The vault_key_rotated call currently takes a block_number arg. This call is executed once we've had 2/3 of nodes agree on this call (through the witnesser-api). This means that we must have 2/3 nodes all agree on the exact same block. This seems extremely unlikely (at least to happen consistently), and particularly for faster chains.

I propose we use (what I'll call) Highest Common Supermajority Vote (HCSV). Basically, we use the highest block number that we reached a 2/3 consensus on. For example. let's say we have 5 nodes and they vote to for block numbers: [5, 4, 3, 4, 5] respectively. This would mean we execute the call with 4 as the HCSV. This is because a vote for 5 counts as a vote for all previous blocks too (since of course, if you're synced up to 5, you're synced up to 4). This way we can be sure to resolve to a reasonable block, even if the numbers don't exactly align.

Does this issue have a Clubhouse Story?

Not yet, labelling as discussion for now.

Describe alternatives you've considered

Pray and hope we get exact block number matches.

Additional context

N/A.

Tag specific people for comment

@andyjsbell @morelazers

morelazers commented 2 years ago

Wat.

The point of the block number argument is that it would be set to the block which contained the rotation transaction, not just whatever block is the latest when you execute eth_getBlock.

kylezs commented 2 years ago

...Yeah, that's actually pretty obvious. is there any where we do vote on block numbers? I feel like there was somewhere in the discussion yesterday where deciding on which block to end was mentioned which got me thinking on this (seemingly non- now) issue. Will close it.