New finalizer with multi parent support #3388
Conversation
nzpr
force-pushed
the
new-finalizer
branch
2 times, most recently
from
29e61e9
to
9c07529
Compare
nzpr
force-pushed
the
new-finalizer
branch
6 times, most recently
from
6fe19f6
to
7cd30cb
Compare
casper/src/main/scala/coop/rchain/casper/finality/Finalizer.scala
Outdated
Show resolved
Hide resolved
casper/src/main/scala/coop/rchain/casper/finality/Finalizer.scala
Outdated
Show resolved
Hide resolved
nzpr
force-pushed
the
new-finalizer
branch
9 times, most recently
from
3929626
to
c38d66a
Compare
nzpr
force-pushed
the
new-finalizer
branch
5 times, most recently
from
964f6d5
to
6cdd205
Compare
block-storage/src/main/scala/coop/rchain/blockstorage/dag/BlockDagRepresentationSyntax.scala
Show resolved
Hide resolved
| newLfbEffect(lfb.blockHash).as(lfb.blockHash) <* | ||
| dag | ||
| .withAncestors(lfb.blockHash, dag.isFinalized(_).not) | ||
| .flatMap(_.toList.traverse(finalisationEffect)) |
There was a problem hiding this comment.
Because we don't have atomic operation on LMDB, here is possible for node to be killed between saving directly finalized block newLfbEffect and related operations in finalisationEffect.
Is it better to save LFB at the end? Can this be more safe so that finalisationEffect can be executed again?
There was a problem hiding this comment.
Makes sense, indeed, otherwise if node is killed finalisationEffect might be never called for some messages that are ancestor of LFB. I'l push fix in commit addressing comments
| // in theory if each stake is high enough, e.g. Long.MaxValue, sum of them might result in negative value | ||
| require(activeStakeTotal > 0, message = "Long overflow when computing total stake") | ||
| require(activeStakeAgreeing > 0, message = "Long overflow when computing total stake") | ||
| activeStakeAgreeing > activeStakeTotal.toFloat / 2 |
There was a problem hiding this comment.
How is this calculation here in finalizer? I don't see that was in previous calculation. This should be information from safety oracle.
There was a problem hiding this comment.
This is because previous finalizer were just running clique oracle on all LFB children. This particular logic were not on that version. New version traverses down from tips until it finds messages worth to be computed faultTolerance agains. If you see clique oracle code - if less than half of a stake is agreed on message, max clique is not computed for this message, and minimal fault tolerance is returned straight away. So this message for sure cannot be finalized.
Do you mean this function better to be moved inside CliqueOracle?
There was a problem hiding this comment.
My concern is if we are sure here that agreeing validators cannot have equivocations.
There was a problem hiding this comment.
I'm not sure I understand this. There is always only one main parent, right? So agreeing chains are well defined. We need to get to the point where enough of these chains join, and find first message that pass fault toleranceThreshold.
This is replacement for #3366 as it has been closed by bors.
Old finalizer is not suitable for multi parent as it is possible that last finalized block is stuck. This happen due to the fact that only children that have current LFB as a main parent are subject to be next LFB.
Finalizer introduced in this PR seeks for LFB only using tips and DAG and input data, without involving requirement to know previous LFB. Due to this change, algorithm for finalization is rewritten significantly, keeping the main logic of following CliqueOracle output do decide if some message is finalized .