Is your feature request related to a problem? Please describe
Initial discussion started here: https://github.com/dtr-org/unit-e/pull/852#discussion_r272167308
When node syncs, it receives headers+commits. At this step, we verify that commits are correct, but we don't check that they are produced by finalizers which are in finalizer set. For instance, after the deposit, finalizer will be able to vote in current_dynasty+3. However, if we don't check that finalizer is part of the active set, that finalizer can vote right after the deposit and potentially create its own finalization.
During the full sync, the node can be fooled and moved to the wrong fork but once it starts downloading blocks, the issue will be discovered, and blocks will be rejected as we perform the following check in ContextualCheckFinalizerCommit.
However, we don't have full blocks when we run fast sync and it can lead to constructing malicious finalization state and downloading a wrong snapshot.
Describe the solution you'd like
When we process finalizer commits (regardless of how we sync), we should always check that commits we receive are from the eligible finalizers. In this case, we can secure the fast sync as it would require 2/3 of finalizers to misbehave to attack the system.
Additional context
Currently, we have the following checks:
ContextualCheckFinalizerCommit -> CheckFinalizerCommit.
CheckFinalizerCommit checks the format of commits.
ContextualCheckFinalizerCommit checks that signatures are correct (but not tx signature), finalizers are part of finalizer set, and inputs spend UTXO.
Would be better to introduce another level of check:
FullCheckFinalizerCommit -> ContextualCheckFinalizerCommit -> CheckFinalizerCommit.
CheckFinalizerCommit stays unchanged.
From ContextualCheckFinalizerCommit we remove input check as this function will be used for both, full/fast sync. But we extend it and check the full signature of the transaction.
FullCheckFinalizerCommit calls two other functions + check the input, that it is part of UTXO.
Is your feature request related to a problem? Please describe Initial discussion started here: https://github.com/dtr-org/unit-e/pull/852#discussion_r272167308 When node syncs, it receives headers+commits. At this step, we verify that commits are correct, but we don't check that they are produced by finalizers which are in finalizer set. For instance, after the deposit, finalizer will be able to vote in
current_dynasty+3. However, if we don't check that finalizer is part of the active set, that finalizer can vote right after the deposit and potentially create its own finalization.During the full sync, the node can be fooled and moved to the wrong fork but once it starts downloading blocks, the issue will be discovered, and blocks will be rejected as we perform the following check in
ContextualCheckFinalizerCommit.However, we don't have full blocks when we run fast sync and it can lead to constructing malicious finalization state and downloading a wrong snapshot.
Describe the solution you'd like When we process finalizer commits (regardless of how we sync), we should always check that commits we receive are from the eligible finalizers. In this case, we can secure the fast sync as it would require 2/3 of finalizers to misbehave to attack the system.
Additional context Currently, we have the following checks:
ContextualCheckFinalizerCommit->CheckFinalizerCommit.CheckFinalizerCommitchecks the format of commits.ContextualCheckFinalizerCommitchecks that signatures are correct (but not tx signature), finalizers are part of finalizer set, and inputs spend UTXO.Would be better to introduce another level of check:
FullCheckFinalizerCommit->ContextualCheckFinalizerCommit->CheckFinalizerCommit.CheckFinalizerCommitstays unchanged. FromContextualCheckFinalizerCommitwe remove input check as this function will be used for both, full/fast sync. But we extend it and check the full signature of the transaction.FullCheckFinalizerCommitcalls two other functions + check the input, that it is part of UTXO.