From 088181b55f2b05b67a634b60c099d1691cfd5c01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mamy=20Andr=C3=A9-Ratsimbazafy?= Date: Thu, 2 Apr 2020 22:41:33 +0200 Subject: [PATCH 01/22] initial fork-choice refactor --- beacon_chain/fork_choice/README.md | 5 + beacon_chain/fork_choice/fork_choice.nim | 576 ++++++++++++++++++ .../fork_choice/fork_choice_types.nim | 121 ++++ beacon_chain/fork_choice/proto_array.nim | 456 ++++++++++++++ vendor/lmdb | 1 + vendor/nim-rocksdb | 1 + 6 files changed, 1160 insertions(+) create mode 100644 beacon_chain/fork_choice/README.md create mode 100644 beacon_chain/fork_choice/fork_choice.nim create mode 100644 beacon_chain/fork_choice/fork_choice_types.nim create mode 100644 beacon_chain/fork_choice/proto_array.nim create mode 160000 vendor/lmdb create mode 160000 vendor/nim-rocksdb diff --git a/beacon_chain/fork_choice/README.md b/beacon_chain/fork_choice/README.md new file mode 100644 index 0000000000..1f8f742d16 --- /dev/null +++ b/beacon_chain/fork_choice/README.md @@ -0,0 +1,5 @@ +# Fork choice implementations + +References: +- https://github.com/ethereum/eth2.0-specs/blob/v0.10.1/specs/phase0/fork-choice.md +- https://github.com/protolambda/lmd-ghost diff --git a/beacon_chain/fork_choice/fork_choice.nim b/beacon_chain/fork_choice/fork_choice.nim new file mode 100644 index 0000000000..ad87f99efc --- /dev/null +++ b/beacon_chain/fork_choice/fork_choice.nim @@ -0,0 +1,576 @@ +# beacon_chain +# Copyright (c) 2018-2020 Status Research & Development GmbH +# Licensed and distributed under either of +# * MIT license (license terms in the root directory or at https://opensource.org/licenses/MIT). +# * Apache v2 license (license terms in the root directory or at https://www.apache.org/licenses/LICENSE-2.0). +# at your option. This file may not be copied, modified, or distributed except according to those terms. + +import + # Standard library + std/tables, std/options, std/typetraits, + # Status libraries + nimcrypto/hash, stew/result, + # Internal + ../spec/[datatypes, digest], + # Fork choice + ./fork_choice_types, ./proto_array + +# https://github.com/ethereum/eth2.0-specs/blob/v0.10.1/specs/phase0/fork-choice.md +# This is a port of https://github.com/sigp/lighthouse/pull/804 +# which is a port of "Proto-Array": https://github.com/protolambda/lmd-ghost +# +# See also the port of the port: https://github.com/protolambda/eth2-py-hacks/blob/ec9395d371903d855e1488d04b8fe89fd5be0ad9/proto_array.py + +const DefaultPruneThreshold = 256 + +# Forward declarations +# ---------------------------------------------------------------------- + +func compute_deltas( + deltas: var openarray[Delta], + indices: Table[Eth2Digest, Index], + votes: var openArray[VoteTracker], + old_balances: openarray[Gwei], + new_balances: openarray[Gwei] + ): ForkChoiceError {.raises: [KeyError].} + +# Fork choice routines +# ---------------------------------------------------------------------- + +func initForkChoice( + finalized_block_slot: Slot, + finalized_block_state_root: Eth2Digest, + justified_epoch: Epoch, + finalized_epoch: Epoch, + finalized_root: Eth2Digest + ): Result[ForkChoice, string] {.raises: [KeyError].} = + ## Initialize a fork choice context + var proto_array = ProtoArray( + prune_threshold: DefaultPruneThreshold, + justified_epoch: justified_epoch, + finalized_epoch: finalized_epoch + ) + + let err = proto_array.on_block( + finalized_block_slot, + finalized_root, + none(Eth2Digest), + finalized_block_state_root, + justified_epoch, + finalized_epoch + ) + if err.kind != fcSuccess: + result.err("Failed to add finalized block to proto_array: " & $err) + return + result.ok(ForkChoice(proto_array: proto_array)) + + +func extend[T](s: var seq[T], minLen: int) = + ## Extend a sequence so that it can contains at least `minLen` elements. + ## If it's already bigger, the sequence is unmodified. + ## The extension is zero-initialized + let curLen = s.len + let diff = minLen - curLen + if diff > 0: + # Note: seq has a length and a capacity. + # If the new length is less than the original capacity + # => setLen will not zeroMem + # If the capacity was too small + # => reallocation occurs + # => the fresh buffer is zeroMem-ed + # In the second case our own zeroMem is redundant + # but this should happen rarely as we reuse the buffer + # most of the time + s.setLen(minLen) + zeroMem(s[minLen].addr, diff * sizeof(T)) + + +func process_attestation*( + self: var ForkChoice, + validator_index: ValidatorIndex, + block_root: Eth2Digest, + target_epoch: Epoch + ): Result[void, string] {.raises: [].} = + ## Add an attestation to the fork choice context + self.votes.extend(validator_index.int) + + template vote: untyped {.dirty.} = self.votes[validator_index.int] + # alias + + if target_epoch > vote.next_epoch or vote == default(VoteTracker): + # TODO: the "default" condition is probably unneeded + vote.next_root = block_root + vote.next_epoch = target_epoch + + +func process_block*( + self: var ForkChoice, + slot: Slot, + block_root: Eth2Digest, + parent_root: Eth2Digest, + state_root: Eth2Digest, + justified_epoch: Epoch, + finalized_epoch: Epoch + ): Result[void, string] {.raises: [KeyError].} = + ## Add a block to the fork choice context + let err = self.proto_array.on_block( + slot, block_root, some(parent_root), state_root, justified_epoch, finalized_epoch + ) + if err.kind != fcSuccess: + result.err("process_block_error: " & $err) + return + result.ok() + + +func find_head*( + self: var ForkChoice, + justified_epoch: Epoch, + justified_root: Eth2Digest, + finalized_epoch: Epoch, + justified_state_balances: seq[Gwei] + ): Result[Eth2Digest, string] {.raises: [UnpackError, KeyError].} = + ## Returns the new blockchain head + + # Compute deltas with previous call + # we might want to reuse the `deltas` buffer across calls + var deltas = newSeq[Delta](self.proto_array.indices.len) + let delta_err = deltas.compute_deltas( + indices = self.proto_array.indices, + votes = self.votes, + old_balances = self.balances, + new_balances = justified_state_balances + ) + if delta_err.kind != fcSuccess: + result.err("find_head compute_deltas failed: " & $delta_err) + return + + # Apply score changes + let score_err = self.proto_array.apply_score_changes( + deltas, justified_epoch, finalized_epoch + ) + if score_err.kind != fcSuccess: + result.err("find_head apply_score_changes failed: " & $score_err) + + # Find the best block + self.balances = justified_state_balances + var new_head{.noInit.}: Eth2Digest + let ghost_err = self.proto_array.find_head(new_head, justified_root) + if ghost_err.kind != fcSuccess: + result.err("find_head failed: " & $ghost_err) + return + + result.ok(new_head) + + +func maybe_prune*( + self: var ForkChoice, finalized_root: Eth2Digest + ): Result[void, string] {.raises: [KeyError].} = + ## Prune blocks preceding the finalized root as they are now unneeded. + let err = self.proto_array.maybe_prune(finalized_root) + if err.kind != fcSuccess: + result.err("find_head maybe_pruned failed: " & $err) + + +func compute_deltas( + deltas: var openarray[Delta], + indices: Table[Eth2Digest, Index], + votes: var openArray[VoteTracker], + old_balances: openarray[Gwei], + new_balances: openarray[Gwei] + ): ForkChoiceError {.raises: [KeyError].} = + ## Update `deltas` + ## between old and new balances + ## between votes + ## + ## `deltas.len` must match `indices.len` (lenght match) + ## + ## Error: + ## - If a value in indices is greater than `indices.len` + ## - If a `Eth2Digest` in `votes` does not exist in `indices` + ## except for the `default(Eth2Digest)` (i.e. zero hash) + + for val_index, vote in votes.mpairs(): + # No need to create a score change if the validator has never voted + # or if votes are for the zero hash (alias to the genesis block) + if vote.current_root == default(Eth2Digest) and vote.next_root == default(Eth2Digest): + continue + + # If the validator was not included in `old_balances` (i.e. did not exist) + # its balance is zero + let old_balance = if val_index < old_balances.len: old_balances[val_index] + else: 0 + + # If the validator is not known in the `new_balances` then use balance of zero + # + # It is possible that there is a vote for an unknown validator if we change our + # justified state to a new state with a higher epoch on a different fork + # because that fork may have on-boarded less validators than the previous fork. + # + # Note that attesters are not different as they are activated only under finality + let new_balance = if val_index < new_balances.len: new_balances[val_index] + else: 0 + + if vote.current_root != vote.next_root or old_balance != new_balance: + # Ignore the current or next vote if it is not known in `indices`. + # We assume that it is outside of our tree (i.e., pre-finalization) and therefore not interesting. + if vote.current_root in indices: + let index = indices[vote.current_root] + if index >= deltas.len: + return ForkChoiceError( + kind: fcErrInvalidNodeDelta, + index: index + ) + deltas[index] -= Delta old_balance + # Note that delta can be negative + # TODO: is int64 big enough? + + if vote.next_root in indices: + let index = indices[vote.next_root] + if index >= deltas.len: + return ForkChoiceError( + kind: fcErrInvalidNodeDelta, + index: index + ) + deltas[index] += Delta new_balance + # Note that delta can be negative + # TODO: is int64 big enough? + + vote.current_root = vote.next_root + + # debugEcho "deltas(", vote.next_root, ") = ", if vote.next_root in indices: $deltas[indices[vote.next_root]] + # else: "nil" + + return ForkChoiceSuccess + +# Sanity checks +# ---------------------------------------------------------------------- +# Sanity checks on internal private procedures + +when isMainModule: + import nimcrypto/[sha2, utils] + + func eth2hash(index: int): Eth2Digest = + sha256.digest(cast[array[sizeof(int), byte]](index)) + + proc tZeroHash() = + echo " fork_choice compute_deltas - test zero votes" + + const validator_count = 16 + var deltas = newSeqUninitialized[Delta](validator_count) + + var indices: Table[Eth2Digest, Index] + var votes: seq[VoteTracker] + var old_balances: seq[Gwei] + var new_balances: seq[Gwei] + + for i in 0 ..< validator_count: + indices.add eth2hash(i), i + votes.add default(VoteTracker) + old_balances.add 0 + new_balances.add 0 + + let err = deltas.compute_deltas( + indices, votes, old_balances, new_balances + ) + + doAssert err.kind == fcSuccess, "compute_deltas finished with error: " & $err + + doAssert deltas == newSeq[Delta](validator_count), "deltas should be zeros" + + for vote in votes: + doAssert vote.current_root == vote.next_root, "The vote should have been updated" + + + proc tAll_voted_the_same() = + echo " fork_choice compute_deltas - test all same votes" + + const + Balance = Gwei(42) + validator_count = 16 + var deltas = newSeqUninitialized[Delta](validator_count) + + var indices: Table[Eth2Digest, Index] + var votes: seq[VoteTracker] + var old_balances: seq[Gwei] + var new_balances: seq[Gwei] + + for i in 0 ..< validator_count: + indices.add eth2hash(i), i + votes.add VoteTracker( + current_root: default(Eth2Digest), + next_root: eth2hash(0), # Get a non-zero hash + next_epoch: Epoch(0) + ) + old_balances.add Balance + new_balances.add Balance + + let err = deltas.compute_deltas( + indices, votes, old_balances, new_balances + ) + + doAssert err.kind == fcSuccess, "compute_deltas finished with error: " & $err + + for i, delta in deltas.pairs: + if i == 0: + doAssert delta == Delta(Balance * validator_count), "The 0th root should have a delta" + else: + doAssert delta == 0, "The non-0 indexes should have a zero delta" + + for vote in votes: + doAssert vote.current_root == vote.next_root, "The vote should have been updated" + + + proc tDifferent_votes() = + echo " fork_choice compute_deltas - test all different votes" + + const + Balance = Gwei(42) + validator_count = 16 + var deltas = newSeqUninitialized[Delta](validator_count) + + var indices: Table[Eth2Digest, Index] + var votes: seq[VoteTracker] + var old_balances: seq[Gwei] + var new_balances: seq[Gwei] + + for i in 0 ..< validator_count: + indices.add eth2hash(i), i + votes.add VoteTracker( + current_root: default(Eth2Digest), + next_root: eth2hash(i), # Each vote for a different root + next_epoch: Epoch(0) + ) + old_balances.add Balance + new_balances.add Balance + + let err = deltas.compute_deltas( + indices, votes, old_balances, new_balances + ) + + doAssert err.kind == fcSuccess, "compute_deltas finished with error: " & $err + + for i, delta in deltas.pairs: + doAssert delta == Delta(Balance), "Each root should have a delta" + + for vote in votes: + doAssert vote.current_root == vote.next_root, "The vote should have been updated" + + + proc tMoving_votes() = + echo " fork_choice compute_deltas - test moving votes" + + const + Balance = Gwei(42) + validator_count = 16 + TotalDeltas = Delta(Balance * validator_count) + var deltas = newSeqUninitialized[Delta](validator_count) + + var indices: Table[Eth2Digest, Index] + var votes: seq[VoteTracker] + var old_balances: seq[Gwei] + var new_balances: seq[Gwei] + + for i in 0 ..< validator_count: + indices.add eth2hash(i), i + votes.add VoteTracker( + # Move vote from root 0 to root 1 + current_root: eth2hash(0), + next_root: eth2hash(1), + next_epoch: Epoch(0) + ) + old_balances.add Balance + new_balances.add Balance + + let err = deltas.compute_deltas( + indices, votes, old_balances, new_balances + ) + + doAssert err.kind == fcSuccess, "compute_deltas finished with error: " & $err + + for i, delta in deltas.pairs: + if i == 0: + doAssert delta == -TotalDeltas, "0th root should have a negative delta" + elif i == 1: + doAssert delta == TotalDeltas, "1st root should have a positive delta" + else: + doAssert delta == 0, "The non-0 and non-1 indexes should have a zero delta" + + for vote in votes: + doAssert vote.current_root == vote.next_root, "The vote should have been updated" + + + proc tMove_out_of_tree() = + echo " fork_choice compute_deltas - test votes for unknown subtree" + + const Balance = Gwei(42) + + var indices: Table[Eth2Digest, Index] + var votes: seq[VoteTracker] + + # Add a block + indices.add eth2hash(1), 0 + + # 2 validators + var deltas = newSeqUninitialized[Delta](2) + let old_balances = @[Balance, Balance] + let new_balances = @[Balance, Balance] + + # One validator moves their vote from the block to the zero hash + votes.add VoteTracker( + current_root: eth2hash(1), + next_root: default(Eth2Digest), + next_epoch: Epoch(0) + ) + + # One validator moves their vote from the block to something outside of the tree + votes.add VoteTracker( + current_root: eth2hash(1), + next_root: eth2hash(1337), + next_epoch: Epoch(0) + ) + + let err = deltas.compute_deltas( + indices, votes, old_balances, new_balances + ) + + doAssert err.kind == fcSuccess, "compute_deltas finished with error: " & $err + + doAssert deltas[0] == -Delta(Balance)*2, "The 0th block should have lost both balances." + + for vote in votes: + doAssert vote.current_root == vote.next_root, "The vote should have been updated" + + + proc tChanging_balances() = + echo " fork_choice compute_deltas - test changing balances" + + const + OldBalance = Gwei(42) + NewBalance = OldBalance * 2 + validator_count = 16 + TotalOldDeltas = Delta(OldBalance * validator_count) + TotalNewDeltas = Delta(NewBalance * validator_count) + var deltas = newSeqUninitialized[Delta](validator_count) + + var indices: Table[Eth2Digest, Index] + var votes: seq[VoteTracker] + var old_balances: seq[Gwei] + var new_balances: seq[Gwei] + + for i in 0 ..< validator_count: + indices.add eth2hash(i), i + votes.add VoteTracker( + # Move vote from root 0 to root 1 + current_root: eth2hash(0), + next_root: eth2hash(1), + next_epoch: Epoch(0) + ) + old_balances.add OldBalance + new_balances.add NewBalance + + let err = deltas.compute_deltas( + indices, votes, old_balances, new_balances + ) + + doAssert err.kind == fcSuccess, "compute_deltas finished with error: " & $err + + for i, delta in deltas.pairs: + if i == 0: + doAssert delta == -TotalOldDeltas, "0th root should have a negative delta" + elif i == 1: + doAssert delta == TotalNewDeltas, "1st root should have a positive delta" + else: + doAssert delta == 0, "The non-0 and non-1 indexes should have a zero delta" + + for vote in votes: + doAssert vote.current_root == vote.next_root, "The vote should have been updated" + + + proc tValidator_appears() = + echo " fork_choice compute_deltas - test validator appears" + + const Balance = Gwei(42) + + var indices: Table[Eth2Digest, Index] + var votes: seq[VoteTracker] + + # Add 2 blocks + indices.add eth2hash(1), 0 + indices.add eth2hash(2), 1 + + # 1 validator at the start, 2 at the end + var deltas = newSeqUninitialized[Delta](2) + let old_balances = @[Balance] + let new_balances = @[Balance, Balance] + + # Both moves vote from Block 1 to 2 + for _ in 0 ..< 2: + votes.add VoteTracker( + current_root: eth2hash(1), + next_root: eth2hash(2), + next_epoch: Epoch(0) + ) + + + let err = deltas.compute_deltas( + indices, votes, old_balances, new_balances + ) + + doAssert err.kind == fcSuccess, "compute_deltas finished with error: " & $err + + doAssert deltas[0] == -Delta(Balance), "Block 1 should have lost only 1 balance" + doAssert deltas[1] == Delta(Balance)*2, "Block 2 should have gained 2 balances" + + for vote in votes: + doAssert vote.current_root == vote.next_root, "The vote should have been updated" + + + proc tValidator_disappears() = + echo " fork_choice compute_deltas - test validator disappears" + + const Balance = Gwei(42) + + var indices: Table[Eth2Digest, Index] + var votes: seq[VoteTracker] + + # Add 2 blocks + indices.add eth2hash(1), 0 + indices.add eth2hash(2), 1 + + # 1 validator at the start, 2 at the end + var deltas = newSeqUninitialized[Delta](2) + let old_balances = @[Balance, Balance] + let new_balances = @[Balance] + + # Both moves vote from Block 1 to 2 + for _ in 0 ..< 2: + votes.add VoteTracker( + current_root: eth2hash(1), + next_root: eth2hash(2), + next_epoch: Epoch(0) + ) + + + let err = deltas.compute_deltas( + indices, votes, old_balances, new_balances + ) + + doAssert err.kind == fcSuccess, "compute_deltas finished with error: " & $err + + doAssert deltas[0] == -Delta(Balance)*2, "Block 1 should have lost 2 balances" + doAssert deltas[1] == Delta(Balance), "Block 2 should have gained 1 balance" + + for vote in votes: + doAssert vote.current_root == vote.next_root, "The vote should have been updated" + + + # ---------------------------------------------------------------------- + + echo "fork_choice internal tests for compute_deltas" + tZeroHash() + tAll_voted_the_same() + tDifferent_votes() + tMoving_votes() + tChanging_balances() + tValidator_appears() + tValidator_disappears() diff --git a/beacon_chain/fork_choice/fork_choice_types.nim b/beacon_chain/fork_choice/fork_choice_types.nim new file mode 100644 index 0000000000..4597f0653e --- /dev/null +++ b/beacon_chain/fork_choice/fork_choice_types.nim @@ -0,0 +1,121 @@ + +# beacon_chain +# Copyright (c) 2018-2020 Status Research & Development GmbH +# Licensed and distributed under either of +# * MIT license (license terms in the root directory or at https://opensource.org/licenses/MIT). +# * Apache v2 license (license terms in the root directory or at https://www.apache.org/licenses/LICENSE-2.0). +# at your option. This file may not be copied, modified, or distributed except according to those terms. + +import + # Standard library + std/tables, std/options, + # Internal + ../spec/[datatypes, digest] + +# https://github.com/ethereum/eth2.0-specs/blob/v0.10.1/specs/phase0/fork-choice.md +# This is a port of https://github.com/sigp/lighthouse/pull/804 +# which is a port of "Proto-Array": https://github.com/protolambda/lmd-ghost + +# ProtoArray low-level types +# ---------------------------------------------------------------------- + +type + FcErrKind* = enum + ## Fork Choice Error Kinds + fcSuccess + fcErrFinalizedNodeUnknown + fcErrJustifiedNodeUnknown + fcErrInvalidFinalizedRootCHange + fcErrInvalidNodeIndex + fcErrInvalidParentIndex + fcErrInvalidBestChildIndex + fcErrInvalidJustifiedIndex + fcErrInvalidBestDescendant + fcErrInvalidParentDelta + fcErrInvalidNodeDelta + fcErrDeltaUnderflow + fcErrIndexUnderflow + fcErrInvalidDeltaLen + fcErrRevertedFinalizedEpoch + fcErrInvalidBestNode + + FcUnderflowKind* = enum + ## Fork Choice Overflow Kinds + fcUnderflowIndices = "Indices Overflow" + fcUnderflowBestChild = "Best Child Overflow" + fcUnderflowBestDescendant = "Best Descendant Overflow" + + Index* = int + Delta* = int + ## Delta indices + + ForkChoiceError* = object + case kind*: FcErrKind + of fcSuccess: + discard + of fcErrFinalizedNodeUnknown, + fcErrJustifiedNodeUnknown: + block_root*: Eth2Digest + of fcErrInvalidFinalizedRootChange: + discard + of fcErrInvalidNodeIndex, + fcErrInvalidParentIndex, + fcErrInvalidBestChildIndex, + fcErrInvalidJustifiedIndex, + fcErrInvalidBestDescendant, + fcErrInvalidParentDelta, + fcErrInvalidNodeDelta, + fcErrDeltaUnderflow: + index*: Index + of fcErrIndexUnderflow: + underflowKind*: FcUnderflowKind + of fcErrInvalidDeltaLen: + deltasLen*: int + indicesLen*: int + of fcErrRevertedFinalizedEpoch: + current_finalized_epoch*: Epoch + new_finalized_epoch*: Epoch + of fcErrInvalidBestNode: + start_root*: Eth2Digest + justified_epoch*: Epoch + finalized_epoch*: Epoch + head_root*: Eth2Digest + head_justified_epoch*: Epoch + head_finalized_epoch*: Epoch + + ProtoArray* = object + prune_threshold*: int + justified_epoch*: Epoch + finalized_epoch*: Epoch + nodes*: seq[ProtoNode] + indices*: Table[Eth2Digest, Index] + + ProtoNode* = object + slot*: Slot + state_root*: Eth2Digest + root*: Eth2Digest + parent_delta*: Option[Delta] + justified_epoch*: Epoch + finalized_epoch*: Epoch + weight*: int64 + best_child*: Option[Index] + best_descendant*: Option[Index] + +const ForkChoiceSuccess* = ForkChoiceError(kind: fcSuccess) + +# Fork choice high-level types +# ---------------------------------------------------------------------- + +type + VoteTracker* = object + current_root*: Eth2Digest + next_root*: Eth2Digest + next_epoch*: Epoch + + ForkChoice* = object + # Note: Lighthouse is protecting all fields with Reader-Writer locks. + # However, given the nature of the fields, I suspect sharing those fields + # will lead to thread contention. For now, stay single-threaded. - Mamy + proto_array*: ProtoArray + votes*: seq[VoteTracker] + balances*: seq[Gwei] diff --git a/beacon_chain/fork_choice/proto_array.nim b/beacon_chain/fork_choice/proto_array.nim new file mode 100644 index 0000000000..cbfe796c92 --- /dev/null +++ b/beacon_chain/fork_choice/proto_array.nim @@ -0,0 +1,456 @@ +# beacon_chain +# Copyright (c) 2018-2020 Status Research & Development GmbH +# Licensed and distributed under either of +# * MIT license (license terms in the root directory or at https://opensource.org/licenses/MIT). +# * Apache v2 license (license terms in the root directory or at https://www.apache.org/licenses/LICENSE-2.0). +# at your option. This file may not be copied, modified, or distributed except according to those terms. + +import + # Standard library + std/tables, std/options, std/typetraits, + # Status libraries + nimcrypto/hash, + # Internal + ../spec/[datatypes, digest], + # Fork choice + ./fork_choice_types + +# https://github.com/ethereum/eth2.0-specs/blob/v0.10.1/specs/phase0/fork-choice.md +# This is a port of https://github.com/sigp/lighthouse/pull/804 +# which is a port of "Proto-Array": https://github.com/protolambda/lmd-ghost +# +# See also the port of the port: https://github.com/protolambda/eth2-py-hacks/blob/ec9395d371903d855e1488d04b8fe89fd5be0ad9/proto_array.py + +# Helper +# ---------------------------------------------------------------------- + +func tiebreak(a, b: Eth2Digest): bool = + ## Fork-Choice tie-break between 2 digests + ## Currently implemented as `>=` (greater or equal) + ## on the binary representation + for i in 0 ..< a.data.len: + if a.data[i] < b.data[i]: + return false + return true + +# Forward declarations +# ---------------------------------------------------------------------- + +func maybe_update_best_child_and_descendant(self: var ProtoArray, parent_index: Index, child_index: Index): ForkChoiceError {.raises: [].} +func node_is_viable_for_head(self: ProtoArray, node: ProtoNode): bool {.raises: [].} +func node_leads_to_viable_head(self: ProtoArray, node: ProtoNode): tuple[viable: bool, err: ForkChoiceError] {.raises: [].} + +# ProtoArray routines +# ---------------------------------------------------------------------- + +func apply_score_changes*( + self: var ProtoArray, + deltas: var openarray[Delta], + justified_epoch: Epoch, + finalized_epoch: Epoch + ): ForkChoiceError {.raises: [UnpackError].}= + ## Iterate backwards through the array, touching all nodes and their parents + ## and potentially the best-child of each parent. + ## + ## The structure of `self.nodes` array ensures that the child of each node + ## is always touched before it's aprent. + ## + ## For each node the following is done: + ## + ## 1. Update the node's weight with the corresponding delta. + ## 2. Backpropagate each node's delta to its parent's delta. + ## 3. Compare the current node with the parent's best-child, + ## updating if the current node should become the best-child + ## 4. If required, update the parent's best-descendant with the current node or its best-descendant + # TODO: remove spurious raised exceptions + if deltas.len != self.indices.len: + return ForkChoiceError( + kind: fcErrInvalidDeltaLen, + deltasLen: deltas.len, + indicesLen: self.indices.len + ) + + self.justified_epoch = justified_epoch + self.finalized_epoch = finalized_epoch + + # Iterate backwards through all the indices in `self.nodes` + for node_index in countdown(self.nodes.len - 1, 0): + template node: untyped {.dirty.}= self.nodes[node_index] + ## Alias + # This cannot raise the IndexError exception, how to tell compiler? + + if node.root == default(Eth2Digest): + continue + + if node_index notin {0..deltas.len-1}: + # TODO: Here `deltas.len == self.indices.len` from the previous check + # and we can probably assume that + # `self.indices.len == self.nodes.len` by construction + # and avoid this check in a loop or altogether + return ForkChoiceError( + kind: fcErrInvalidNodeDelta, + index: node_index + ) + let node_delta = deltas[node_index] + + # Apply the delta to the node + # We fail fast if underflow, which shouldn't happen. + # Note that delta can be negative. + let weight = node.weight - node_delta + if weight < 0: + return ForkChoiceError( + kind: fcErrDeltaUnderflow, + index: node_index + ) + node.weight = weight + + # If the node has a parent, try to update its best-child and best-descendant + if node.parent_delta.isSome(): + # TODO: Nim `options` module could use some {.inline.} + # and a mutable overload for unsafeGet + # and a "no exceptions" (only panics) implementation. + let parent_index = node.parent_delta.unsafeGet() + if parent_index notin {0..deltas.len-1}: + return ForkChoiceError( + kind: fcErrInvalidParentDelta, + index: parent_index + ) + + # Back-propagate the nodes delta to its parent. + node.parent_delta.get() += node_delta + + let err = self.maybe_update_best_child_and_descendant(parent_index, node_index) + if err.kind != fcSuccess: + return err + + return ForkChoiceSuccess + + +func on_block*( + self: var ProtoArray, + slot: Slot, + root: Eth2Digest, + parent: Option[Eth2Digest], + state_root: Eth2Digest, + justified_epoch: Epoch, + finalized_epoch: Epoch + ): ForkChoiceError {.raises: [KeyError].} = + ## Register a block with the fork choice + ## A `none` parent is only valid for Genesis + # TODO: fix exceptions raised + + # If the block is already known, ignore it + if root in self.indices: + return ForkChoiceSuccess + + let node_index = self.nodes.len + + let parent_index = block: + if parent.isNone: + none(int) + elif parent.unsafeGet() notin self.indices: + # Is this possible? + none(int) + else: # TODO: How to tell the compiler not to raise KeyError + some(self.indices[parent.unsafeGet()]) + + let node = ProtoNode( + slot: slot, + state_root: state_root, + root: root, + parent_delta: parent_index, + justified_epoch: justified_epoch, + finalized_epoch: finalized_epoch, + weight: 0, + best_child: none(int), + best_descendant: none(int) + ) + + self.indices[node.root] = node_index + self.nodes.add node # TODO: if this is costly, we can setLen + construct the node in-place + + if parent_index.isSome(): + let err = self.maybe_update_best_child_and_descendant(parent_index.unsafeGet(), node_index) + if err.kind != fcSuccess: + return err + + return ForkChoiceSuccess + +func find_head*( + self: var ProtoArray, + head: var Eth2Digest, + justified_root: Eth2Digest + ): ForkChoiceError {.raises: [KeyError].} = + ## Follows the best-descendant links to find the best-block (i.e. head-block) + ## + ## ⚠️ Warning + ## The result may not be accurate if `on_new_block` + ## is not followed by `apply_score_changes` as `on_new_block` does not + ## update the whole tree. + if justified_root notin self.indices: + return ForkChoiceError( + kind: fcErrJustifiedNodeUnknown, + block_root: justified_root + ) + let justified_index = self.indices[justified_root] # TODO: this can't throw KeyError + + if justified_index notin {0..self.nodes.len-1}: + return ForkChoiceError( + kind: fcErrInvalidJustifiedIndex, + index: justified_index + ) + template justified_node: untyped {.dirty.} = self.nodes[justified_index] + # Alias, TODO: no exceptions + + let best_descendant_index = block: + if justified_node.best_descendant.isSome(): + justified_node.best_descendant.unsafeGet() + else: + justified_index + + if best_descendant_index notin {0..self.nodes.len-1}: + return ForkChoiceError( + kind: fcErrInvalidBestDescendant, + index: best_descendant_index + ) + template best_node: untyped {.dirty.} = self.nodes[best_descendant_index] + # Alias, TODO: no exceptions + + # Perform a sanity check to ensure the node can be head + if not self.node_is_viable_for_head(best_node): + return ForkChoiceError( + kind: fcErrInvalidBestNode, + start_root: justified_root, + justified_epoch: self.justified_epoch, + finalized_epoch: self.finalized_epoch, + head_root: justified_node.root, + head_justified_epoch: justified_node.justified_epoch, + head_finalized_epoch: justified_node.finalized_epoch + ) + + head = best_node.root + return ForkChoiceSuccess + + +func maybe_prune*( + self: var ProtoArray, + finalized_root: Eth2Digest + ): ForkChoiceError {.raises: [KeyError].} = + ## Update the tree with new finalization information. + ## The tree is pruned if and only if: + ## - The `finalized_root` and finalized epoch are different from current + ## - The number of nodes in `self` is at least `self.prune_threshold` + ## + ## Returns error if: + ## - The finalized epoch is less than the current one + ## - The finalized epoch matches the current one but the finalized root is different + ## - Internal error due to invalid indices in `self` + # TODO: Exceptions + + if finalized_root notin self.indices: + return ForkChoiceError( + kind: fcErrFinalizedNodeUnknown, + block_root: finalized_root + ) + let finalized_index = self.indices[finalized_root] + + if finalized_index < self.prune_threshold: + # Pruning small numbers of nodes incurs more overhead than leaving them as is + return ForkChoiceSuccess + + # Remove the `self.indices` key/values for the nodes slated for deletion + if finalized_index notin {0..self.nodes.len-1}: + return ForkChoiceError( + kind: fcErrInvalidNodeIndex, + index: finalized_index + ) + for node_index in 0 ..< finalized_index: + self.indices.del(self.nodes[node_index].root) + + # Drop all nodes prior to finalization. + # This is done in-place with `moveMem` to avoid costly reallocations. + static: doAssert ProtoNode.supportsCopyMem(), "ProtoNode must be a trivial type" + let tail = self.nodes.len - finalized_index + # TODO: can we have an unallocated `self.nodes`? i.e. self.nodes[0] is nil + moveMem(self.nodes[0].addr, self.nodes[finalized_index].addr, tail * sizeof(ProtoNode)) + self.nodes.setLen(tail) + + # Adjust the indices map + for index in self.indices.mvalues(): + index -= finalized_index + if index < 0: + return ForkChoiceError( + kind: fcErrIndexUnderflow, + underflowKind: fcUnderflowIndices + ) + + # Iterate through all the existing nodes and adjust their indices to match + # the new layout of `self.nodes` + for node in self.nodes.mitems(): + # If `node.parent` is less than `finalized_index`, set it to None + if node.parent_delta.isSome(): + let new_parent_delta = node.parent_delta.unsafeGet() - finalized_index + if new_parent_delta < 0: + node.parent_delta = none(Delta) + else: + node.parent_delta = some(new_parent_delta) + + if node.best_child.isSome(): + let new_best_child = node.best_child.unsafeGet() - finalized_index + if new_best_child < 0: + return ForkChoiceError( + kind: fcErrIndexUnderflow, + underflowKind: fcUnderflowBestChild + ) + node.best_child = some(new_best_child) + + if node.best_descendant.isSome(): + let new_best_descendant = node.best_descendant.unsafeGet() - finalized_index + if new_best_descendant < 0: + return ForkChoiceError( + kind: fcErrIndexUnderflow, + underflowKind: fcUnderflowBestDescendant + ) + node.best_descendant = some(new_best_descendant) + + return ForkChoiceSuccess + + +func maybe_update_best_child_and_descendant( + self: var ProtoArray, + parent_index: Index, + child_index: Index): ForkChoiceError {.raises: [].} = + ## Observe the parent at `parent_index` with respect to the child at `child_index` and + ## potentiatlly modify the `parent.best_child` and `parent.best_descendant` values + ## + ## There are four scenarios: + ## + ## 1. The child is already the best child + ## but it's now invalid due to a FFG change and should be removed. + ## 2. The child is already the best child + ## and the parent is updated with the new best descendant + ## 3. The child is not the best child but becomes the best child + ## 4. The child is not the best child and does not become the best child + if child_index notin {0..self.nodes.len-1}: + return ForkChoiceError( + kind: fcErrInvalidNodeIndex, + index: child_index + ) + if parent_index notin {0..self.nodes.len-1}: + return ForkChoiceError( + kind: fcErrInvalidNodeIndex, + index: parent_index + ) + + # Aliases + template child: untyped {.dirty.} = self.nodes[child_index] + template parent: untyped {.dirty.} = self.nodes[parent_index] + + let (child_leads_to_viable_head, err) = self.node_leads_to_viable_head(child) + if err.kind != fcSuccess: + return err + + let # Aliases to the 3 possible (best_child, best_descendant) tuples + change_to_none = (none(Index), none(Index)) + change_to_child = ( + some(child_index), + # Nim `options` module doesn't implement option `or` + if child.best_descendant.isSome(): child.best_descendant + else: some(child_index) + ) + no_change = (parent.best_child, parent.best_descendant) + + # TODO: state-machine? The control-flow is messy + + let (new_best_child, new_best_descendant) = block: + if parent.best_child.isSome: + let best_child_index = parent.best_child.unsafeGet() + if best_child_index == child_index and not child_leads_to_viable_head: + # The child is already the best-child of the parent + # but it's not viable to be the head block => remove it + change_to_none + elif best_child_index == child_index: + # If the child is the best-child already, set it again to ensure + # that the best-descendant of the parent is up-to-date. + change_to_child + else: + if best_child_index notin {0..self.nodes.len-1}: + return ForkChoiceError( + kind: fcErrInvalidBestDescendant, + index: best_child_index + ) + let best_child = self.nodes[best_child_index] + + let (best_child_leads_to_viable_head, err) = self.node_leads_to_viable_head(best_child) + if err.kind != fcSuccess: + return err + + if child_leads_to_viable_head and not best_child_leads_to_viable_head: + # The child leads to a viable head, but the current best-child doesn't + change_to_child + elif not child_leads_to_viable_head and best_child_leads_to_viable_head: + # The best child leads to a viable head, but the child doesn't + no_change + elif child.weight == best_child.weight: + # Tie-breaker of equal weights by root + if child.root.tiebreak(best_child.root): + change_to_child + else: + change_to_none + else: # Choose winner by weight + if child.weight >= best_child.weight: + change_to_child + else: + change_to_none + else: + if child_leads_to_viable_head: + # There is no current best-child and the child is viable + change_to_child + else: + # There is no current best-child but the child is not viable + no_change + + self.nodes[parent_index].best_child = new_best_child + self.nodes[parent_index].best_descendant = new_best_descendant + + +func node_leads_to_viable_head( + self: ProtoArray, node: ProtoNode + ): tuple[viable: bool, err: ForkChoiceError] {.raises: [].} = + ## Indicates if the node itself or its best-descendant are viable + ## for blockchain head + let best_descendant_is_viable_for_head = block: + if node.best_descendant.isSome(): + let best_descendant_index = node.best_descendant.unsafeGet() + if best_descendant_index notin {0..self.nodes.len-1}: + return ( + false, + ForkChoiceError( + kind: fcErrInvalidBestDescendant, + index: best_descendant_index + ) + ) + let best_descendant = self.nodes[best_descendant_index] + self.node_is_viable_for_head(best_descendant) + else: + false + + return ( + best_descendant_is_viable_for_head or + self.node_is_viable_for_head(node), + ForkChoiceSuccess + ) + +func node_is_viable_for_head(self: ProtoArray, node: ProtoNode): bool {.raises: [].} = + ## This is the equivalent of `filter_block_tree` function in eth2 spec + ## https://github.com/ethereum/eth2.0-specs/blob/v0.10.0/specs/phase0/fork-choice.md#filter_block_tree + ## + ## Any node that has a different finalized or justified epoch + ## should not be viable for the head. + ( + (node.justified_epoch == self.justified_epoch) or + (node.justified_epoch == Epoch(0)) + ) and ( + (node.finalized_epoch == self.finalized_epoch) or + (node.finalized_epoch == Epoch(0)) + ) diff --git a/vendor/lmdb b/vendor/lmdb new file mode 160000 index 0000000000..c8ecc17b38 --- /dev/null +++ b/vendor/lmdb @@ -0,0 +1 @@ +Subproject commit c8ecc17b38e164e6a728d66a9b1d05bc18dd3ace diff --git a/vendor/nim-rocksdb b/vendor/nim-rocksdb new file mode 160000 index 0000000000..08fec021c0 --- /dev/null +++ b/vendor/nim-rocksdb @@ -0,0 +1 @@ +Subproject commit 08fec021c0f28f63d1221d40a655078b5b923d1b From 7759055c89700c329037d415ef5db002e22b85dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mamy=20Andr=C3=A9-Ratsimbazafy?= Date: Fri, 3 Apr 2020 20:30:12 +0200 Subject: [PATCH 02/22] Add fork_choice test for "no votes" --- beacon_chain/fork_choice/fork_choice.nim | 50 ++--- beacon_chain/fork_choice/proto_array.nim | 4 +- tests/fork_choice/interpreter.nim | 105 +++++++++ tests/fork_choice/scenarios/no_votes.nim | 270 +++++++++++++++++++++++ 4 files changed, 402 insertions(+), 27 deletions(-) create mode 100644 tests/fork_choice/interpreter.nim create mode 100644 tests/fork_choice/scenarios/no_votes.nim diff --git a/beacon_chain/fork_choice/fork_choice.nim b/beacon_chain/fork_choice/fork_choice.nim index ad87f99efc..c3cc4f4ab7 100644 --- a/beacon_chain/fork_choice/fork_choice.nim +++ b/beacon_chain/fork_choice/fork_choice.nim @@ -37,7 +37,7 @@ func compute_deltas( # Fork choice routines # ---------------------------------------------------------------------- -func initForkChoice( +func initForkChoice*( finalized_block_slot: Slot, finalized_block_state_root: Eth2Digest, justified_epoch: Epoch, @@ -249,7 +249,7 @@ func compute_deltas( when isMainModule: import nimcrypto/[sha2, utils] - func eth2hash(index: int): Eth2Digest = + func fakeHash(index: int): Eth2Digest = sha256.digest(cast[array[sizeof(int), byte]](index)) proc tZeroHash() = @@ -264,7 +264,7 @@ when isMainModule: var new_balances: seq[Gwei] for i in 0 ..< validator_count: - indices.add eth2hash(i), i + indices.add fakeHash(i), i votes.add default(VoteTracker) old_balances.add 0 new_balances.add 0 @@ -295,10 +295,10 @@ when isMainModule: var new_balances: seq[Gwei] for i in 0 ..< validator_count: - indices.add eth2hash(i), i + indices.add fakeHash(i), i votes.add VoteTracker( current_root: default(Eth2Digest), - next_root: eth2hash(0), # Get a non-zero hash + next_root: fakeHash(0), # Get a non-zero hash next_epoch: Epoch(0) ) old_balances.add Balance @@ -334,10 +334,10 @@ when isMainModule: var new_balances: seq[Gwei] for i in 0 ..< validator_count: - indices.add eth2hash(i), i + indices.add fakeHash(i), i votes.add VoteTracker( current_root: default(Eth2Digest), - next_root: eth2hash(i), # Each vote for a different root + next_root: fakeHash(i), # Each vote for a different root next_epoch: Epoch(0) ) old_balances.add Balance @@ -371,11 +371,11 @@ when isMainModule: var new_balances: seq[Gwei] for i in 0 ..< validator_count: - indices.add eth2hash(i), i + indices.add fakeHash(i), i votes.add VoteTracker( # Move vote from root 0 to root 1 - current_root: eth2hash(0), - next_root: eth2hash(1), + current_root: fakeHash(0), + next_root: fakeHash(1), next_epoch: Epoch(0) ) old_balances.add Balance @@ -408,7 +408,7 @@ when isMainModule: var votes: seq[VoteTracker] # Add a block - indices.add eth2hash(1), 0 + indices.add fakeHash(1), 0 # 2 validators var deltas = newSeqUninitialized[Delta](2) @@ -417,15 +417,15 @@ when isMainModule: # One validator moves their vote from the block to the zero hash votes.add VoteTracker( - current_root: eth2hash(1), + current_root: fakeHash(1), next_root: default(Eth2Digest), next_epoch: Epoch(0) ) # One validator moves their vote from the block to something outside of the tree votes.add VoteTracker( - current_root: eth2hash(1), - next_root: eth2hash(1337), + current_root: fakeHash(1), + next_root: fakeHash(1337), next_epoch: Epoch(0) ) @@ -458,11 +458,11 @@ when isMainModule: var new_balances: seq[Gwei] for i in 0 ..< validator_count: - indices.add eth2hash(i), i + indices.add fakeHash(i), i votes.add VoteTracker( # Move vote from root 0 to root 1 - current_root: eth2hash(0), - next_root: eth2hash(1), + current_root: fakeHash(0), + next_root: fakeHash(1), next_epoch: Epoch(0) ) old_balances.add OldBalance @@ -495,8 +495,8 @@ when isMainModule: var votes: seq[VoteTracker] # Add 2 blocks - indices.add eth2hash(1), 0 - indices.add eth2hash(2), 1 + indices.add fakeHash(1), 0 + indices.add fakeHash(2), 1 # 1 validator at the start, 2 at the end var deltas = newSeqUninitialized[Delta](2) @@ -506,8 +506,8 @@ when isMainModule: # Both moves vote from Block 1 to 2 for _ in 0 ..< 2: votes.add VoteTracker( - current_root: eth2hash(1), - next_root: eth2hash(2), + current_root: fakeHash(1), + next_root: fakeHash(2), next_epoch: Epoch(0) ) @@ -534,8 +534,8 @@ when isMainModule: var votes: seq[VoteTracker] # Add 2 blocks - indices.add eth2hash(1), 0 - indices.add eth2hash(2), 1 + indices.add fakeHash(1), 0 + indices.add fakeHash(2), 1 # 1 validator at the start, 2 at the end var deltas = newSeqUninitialized[Delta](2) @@ -545,8 +545,8 @@ when isMainModule: # Both moves vote from Block 1 to 2 for _ in 0 ..< 2: votes.add VoteTracker( - current_root: eth2hash(1), - next_root: eth2hash(2), + current_root: fakeHash(1), + next_root: fakeHash(2), next_epoch: Epoch(0) ) diff --git a/beacon_chain/fork_choice/proto_array.nim b/beacon_chain/fork_choice/proto_array.nim index cbfe796c92..614cda57e1 100644 --- a/beacon_chain/fork_choice/proto_array.nim +++ b/beacon_chain/fork_choice/proto_array.nim @@ -396,12 +396,12 @@ func maybe_update_best_child_and_descendant( if child.root.tiebreak(best_child.root): change_to_child else: - change_to_none + no_change else: # Choose winner by weight if child.weight >= best_child.weight: change_to_child else: - change_to_none + no_change else: if child_leads_to_viable_head: # There is no current best-child and the child is viable diff --git a/tests/fork_choice/interpreter.nim b/tests/fork_choice/interpreter.nim new file mode 100644 index 0000000000..066fa4d313 --- /dev/null +++ b/tests/fork_choice/interpreter.nim @@ -0,0 +1,105 @@ +# beacon_chain +# Copyright (c) 2018 Status Research & Development GmbH +# Licensed and distributed under either of +# * MIT license (license terms in the root directory or at https://opensource.org/licenses/MIT). +# * Apache v2 license (license terms in the root directory or at https://www.apache.org/licenses/LICENSE-2.0). +# at your option. This file may not be copied, modified, or distributed except according to those terms. + +import + # Standard library + std/strformat, std/tables, std/options, + # Status libraries + stew/result, nimcrypto/[sha2, utils], + # Internals + ../../beacon_chain/spec/[datatypes, digest], + ../../beacon_chain/fork_choice/[fork_choice, fork_choice_types] + +export result, datatypes, digest, fork_choice, fork_choice_types, tables, options + +func fakeHash*(index: int): Eth2Digest = + ## Create fake hashes + sha256.digest(cast[array[sizeof(int), byte]](index)) + +# The fork choice tests are quite complex. +# For flexibility in block arrival, timers, operations sequencing, ... +# we create a small interpreter that will trigger events in proper order +# before fork choice. + +type + OpKind* = enum + FindHead + InvalidFindHead + ProcessBlock + ProcessAttestation + Prune + + Operation* = object + # variant specific fields + case kind*: OpKind + of FindHead, InvalidFindHead: + justified_epoch*: Epoch + justified_root*: Eth2Digest + finalized_epoch*: Epoch + justified_state_balances*: seq[Gwei] + expected_head*: Eth2Digest + of ProcessBlock: + slot*: Slot + root*: Eth2Digest + parent_root*: Eth2Digest + blk_justified_epoch*: Epoch + blk_finalized_epoch*: Epoch + of ProcessAttestation: + validator_index*: ValidatorIndex + block_root*: Eth2Digest + target_epoch*: Epoch + of Prune: # ProtoArray specific + finalized_root*: Eth2Digest + prune_threshold*: int + expected_len*: int + +func apply(ctx: var ForkChoice, id: int, op: Operation) = + ## Apply the specified operation to a ForkChoice context + ## ``id`` is additional debugging info. It is the + ## operation index. + + case op.kind + of FindHead, InvalidFindHead: + let r = ctx.find_head( + op.justified_epoch, + op.justified_root, + op.finalized_epoch, + op.justified_state_balances + ) + if op.kind == FindHead: + doAssert r.isOk(), &"find_head (op #{id}) returned an error: {r.error}" + doAssert r.get() == op.expected_head, &"find_head (op #{id}) returned an incorrect result: {r.get()} (expected: {op.expected_head})" + else: + doAssert r.isErr(), "find_head was unexpectedly suvvessful" + of ProcessBlock: + let r = ctx.process_block( + slot = op.slot, + block_root = op.root, + parent_root = op.parent_root, + state_root = default(Eth2Digest), + justified_epoch = op.blk_justified_epoch, + finalized_epoch = op.blk_finalized_epoch + ) + doAssert r.isOk(), &"process_block (op #{id}) returned an error: {r.error}" + of ProcessAttestation: + let r = ctx.process_attestation( + validator_index = op.validator_index, + block_root = op.block_root, + target_epoch = op.target_epoch + ) + doAssert r.isOk(), &"process_attestation (op #{id}) returned an error: {r.error}" + of Prune: + ctx.proto_array.prune_threshold = op.prune_threshold + let r = ctx.maybe_prune(op.finalized_root) + doAssert r.isOk(), &"prune (op #{id}) returned an error: {r.error}" + doAssert ctx.proto_array.nodes.len == op.expected_len, + &"prune (op #{id}): the resulting length ({ctx.proto_array.nodes.len}) was not expected ({op.expected_len})" + +func run*(ctx: var ForkChoice, ops: seq[Operation]) = + ## Apply a sequence of fork-choice operations on a store + for i, op in ops: + ctx.apply(i, op) diff --git a/tests/fork_choice/scenarios/no_votes.nim b/tests/fork_choice/scenarios/no_votes.nim new file mode 100644 index 0000000000..06e8a139df --- /dev/null +++ b/tests/fork_choice/scenarios/no_votes.nim @@ -0,0 +1,270 @@ +# beacon_chain +# Copyright (c) 2018 Status Research & Development GmbH +# Licensed and distributed under either of +# * MIT license (license terms in the root directory or at https://opensource.org/licenses/MIT). +# * Apache v2 license (license terms in the root directory or at https://www.apache.org/licenses/LICENSE-2.0). +# at your option. This file may not be copied, modified, or distributed except according to those terms. + +import ../interpreter + +proc setup_no_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = + let balances = newSeq[Gwei](16) + let GenesisRoot = fakeHash(0) + + # Head should be genesis + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(1), + justified_root: GenesisRoot, + finalized_epoch: Epoch(1), + justified_state_balances: balances, + expected_head: GenesisRoot + ) + + # Add block 2 + # + # 0 + # / + # 2 + result.ops.add Operation( + kind: ProcessBlock, + slot: Slot(0), + root: fakeHash(2), + parent_root: GenesisRoot, + blk_justified_epoch: Epoch(1), + blk_finalized_epoch: Epoch(1) + ) + + # Head should be 2 + # + # 0 + # / + # 2 <- head + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(1), + justified_root: GenesisRoot, + finalized_epoch: Epoch(1), + justified_state_balances: balances, + expected_head: fakeHash(2) + ) + + # Add block 1 + # + # 0 + # / \ + # 2 1 + result.ops.add Operation( + kind: ProcessBlock, + slot: Slot(0), + root: fakeHash(1), + parent_root: GenesisRoot, + blk_justified_epoch: Epoch(1), + blk_finalized_epoch: Epoch(1) + ) + + # Head is still 2 due to tiebreaker as fakeHash(2) (0xD8...) > fakeHash(1) (0x7C...) + # + # 0 + # / \ + # head-> 2 1 + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(1), + justified_root: GenesisRoot, + finalized_epoch: Epoch(1), + justified_state_balances: balances, + expected_head: fakeHash(2) + ) + + # Add block 3 + # + # 0 + # / \ + # 2 1 + # | + # 3 + result.ops.add Operation( + kind: ProcessBlock, + slot: Slot(0), + root: fakeHash(3), + parent_root: fakeHash(1), + blk_justified_epoch: Epoch(1), + blk_finalized_epoch: Epoch(1) + ) + + # Head is still 2 + # + # 0 + # / \ + # head-> 2 1 + # | + # 3 + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(1), + justified_root: GenesisRoot, + finalized_epoch: Epoch(1), + justified_state_balances: balances, + expected_head: fakeHash(2) + ) + + # Add block 4 + # + # 0 + # / \ + # 2 1 + # | | + # 4 3 + result.ops.add Operation( + kind: ProcessBlock, + slot: Slot(0), + root: fakeHash(4), + parent_root: fakeHash(2), + blk_justified_epoch: Epoch(1), + blk_finalized_epoch: Epoch(1) + ) + + # Check that head is 4 + # + # 0 + # / \ + # 2 1 + # | | + # head-> 4 3 + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(1), + justified_root: GenesisRoot, + finalized_epoch: Epoch(1), + justified_state_balances: balances, + expected_head: fakeHash(4) + ) + + # Add block 5 with justified epoch of 2 + # + # 0 + # / \ + # 2 1 + # | | + # 4 3 + # | + # 5 <- justified epoch = 2 + result.ops.add Operation( + kind: ProcessBlock, + slot: Slot(0), + root: fakeHash(5), + parent_root: fakeHash(4), + blk_justified_epoch: Epoch(2), + blk_finalized_epoch: Epoch(1) + ) + + # Ensure the head is still 4 whilst the justified epoch is 0. + # + # 0 + # / \ + # 2 1 + # | | + # head-> 4 3 + # | + # 5 + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(1), + justified_root: GenesisRoot, + finalized_epoch: Epoch(1), + justified_state_balances: balances, + expected_head: fakeHash(4) + ) + + # Ensure that there is an error when starting from a block with the wrong justified epoch + # 0 + # / \ + # 2 1 + # | | + # 4 3 + # | + # 5 <- starting from 5 with justified epoch 1 should error. + result.ops.add Operation( + kind: InvalidFindHead, + justified_epoch: Epoch(1), # <--- Wrong epoch + justified_root: fakeHash(5), + finalized_epoch: Epoch(1), + justified_state_balances: balances + ) + + # Set the justified epoch to 2 and the start block to 5 and ensure 5 is the head. + # 0 + # / \ + # 2 1 + # | | + # 4 3 + # | + # 5 <- head + justified + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(2), + justified_root: fakeHash(5), + finalized_epoch: Epoch(1), + justified_state_balances: balances, + expected_head: fakeHash(5) + ) + + # Add block 6 + # + # 0 + # / \ + # 2 1 + # | | + # 4 3 + # | + # 5 <- justified root + # | + # 6 + result.ops.add Operation( + kind: ProcessBlock, + slot: Slot(0), + root: fakeHash(6), + parent_root: fakeHash(5), + blk_justified_epoch: Epoch(2), + blk_finalized_epoch: Epoch(1) + ) + + # Ensure 6 is the head + # 0 + # / \ + # 2 1 + # | | + # 4 3 + # | + # 5 <- justified root + # | + # 6 <- head + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(2), + justified_root: fakeHash(5), + finalized_epoch: Epoch(1), + justified_state_balances: balances, + expected_head: fakeHash(6) + ) + + # ---------------------------------- + # Initialize the fork choice context + result.fork_choice = initForkChoice( + finalized_block_slot = Slot(0), + finalized_block_state_root = default(Eth2Digest), + justified_epoch = Epoch(1), + finalized_epoch = Epoch(1), + finalized_root = GenesisRoot + ).get() + +proc test_no_votes() = + echo " fork_choice - testing no votes" + # for i in 0 ..< 6: + # echo " block (", i, ") hash: ", fakeHash(i) + + var (ctx, ops) = setup_no_votes() + ctx.run(ops) + +test_no_votes() From b2c70346f68985a7f72e5c6e32a95069eb3ddfd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mamy=20Andr=C3=A9-Ratsimbazafy?= Date: Mon, 6 Apr 2020 16:01:15 +0200 Subject: [PATCH 03/22] Initial test with voting: fix handling of unknown validators and parent blocks --- beacon_chain/fork_choice/fork_choice.nim | 5 +- .../fork_choice/fork_choice_types.nim | 2 +- beacon_chain/fork_choice/proto_array.nim | 44 +++-- tests/fork_choice/interpreter.nim | 7 +- tests/fork_choice/scenarios/no_votes.nim | 24 +-- tests/fork_choice/scenarios/votes.nim | 152 ++++++++++++++++++ 6 files changed, 208 insertions(+), 26 deletions(-) create mode 100644 tests/fork_choice/scenarios/votes.nim diff --git a/beacon_chain/fork_choice/fork_choice.nim b/beacon_chain/fork_choice/fork_choice.nim index c3cc4f4ab7..bdc7ebe66f 100644 --- a/beacon_chain/fork_choice/fork_choice.nim +++ b/beacon_chain/fork_choice/fork_choice.nim @@ -82,7 +82,7 @@ func extend[T](s: var seq[T], minLen: int) = # but this should happen rarely as we reuse the buffer # most of the time s.setLen(minLen) - zeroMem(s[minLen].addr, diff * sizeof(T)) + zeroMem(s[curLen].addr, diff * sizeof(T)) func process_attestation*( @@ -92,7 +92,7 @@ func process_attestation*( target_epoch: Epoch ): Result[void, string] {.raises: [].} = ## Add an attestation to the fork choice context - self.votes.extend(validator_index.int) + self.votes.extend(validator_index.int + 1) template vote: untyped {.dirty.} = self.votes[validator_index.int] # alias @@ -102,6 +102,7 @@ func process_attestation*( vote.next_root = block_root vote.next_epoch = target_epoch + result.ok() func process_block*( self: var ForkChoice, diff --git a/beacon_chain/fork_choice/fork_choice_types.nim b/beacon_chain/fork_choice/fork_choice_types.nim index 4597f0653e..3a8857d65c 100644 --- a/beacon_chain/fork_choice/fork_choice_types.nim +++ b/beacon_chain/fork_choice/fork_choice_types.nim @@ -94,7 +94,7 @@ type slot*: Slot state_root*: Eth2Digest root*: Eth2Digest - parent_delta*: Option[Delta] + parent*: Option[Index] justified_epoch*: Epoch finalized_epoch*: Epoch weight*: int64 diff --git a/beacon_chain/fork_choice/proto_array.nim b/beacon_chain/fork_choice/proto_array.nim index 614cda57e1..7f51a1c6b8 100644 --- a/beacon_chain/fork_choice/proto_array.nim +++ b/beacon_chain/fork_choice/proto_array.nim @@ -95,8 +95,8 @@ func apply_score_changes*( # Apply the delta to the node # We fail fast if underflow, which shouldn't happen. - # Note that delta can be negative. - let weight = node.weight - node_delta + # Note that delta can be negative but weight cannot + let weight = node.weight + node_delta if weight < 0: return ForkChoiceError( kind: fcErrDeltaUnderflow, @@ -104,12 +104,15 @@ func apply_score_changes*( ) node.weight = weight + # debugecho " deltas[", node_index, "] = ", deltas[node_index] + # debugecho " node.weight = ", node.weight + # If the node has a parent, try to update its best-child and best-descendant - if node.parent_delta.isSome(): + if node.parent.isSome(): # TODO: Nim `options` module could use some {.inline.} # and a mutable overload for unsafeGet # and a "no exceptions" (only panics) implementation. - let parent_index = node.parent_delta.unsafeGet() + let parent_index = node.parent.unsafeGet() if parent_index notin {0..deltas.len-1}: return ForkChoiceError( kind: fcErrInvalidParentDelta, @@ -117,7 +120,7 @@ func apply_score_changes*( ) # Back-propagate the nodes delta to its parent. - node.parent_delta.get() += node_delta + deltas[parent_index] += node_delta let err = self.maybe_update_best_child_and_descendant(parent_index, node_index) if err.kind != fcSuccess: @@ -158,7 +161,7 @@ func on_block*( slot: slot, state_root: state_root, root: root, - parent_delta: parent_index, + parent: parent_index, justified_epoch: justified_epoch, finalized_epoch: finalized_epoch, weight: 0, @@ -187,6 +190,7 @@ func find_head*( ## The result may not be accurate if `on_new_block` ## is not followed by `apply_score_changes` as `on_new_block` does not ## update the whole tree. + debugEcho " self.find_head(head = 0x", head, ", justified_root = 0x", justified_root, ")" if justified_root notin self.indices: return ForkChoiceError( kind: fcErrJustifiedNodeUnknown, @@ -202,6 +206,8 @@ func find_head*( template justified_node: untyped {.dirty.} = self.nodes[justified_index] # Alias, TODO: no exceptions + debugEcho " self.nodes[justified_index (", justified_index, ")] = ", justified_node + let best_descendant_index = block: if justified_node.best_descendant.isSome(): justified_node.best_descendant.unsafeGet() @@ -216,6 +222,8 @@ func find_head*( template best_node: untyped {.dirty.} = self.nodes[best_descendant_index] # Alias, TODO: no exceptions + debugEcho " self.nodes[best_descendant_index (", best_descendant_index, ")] = ", best_node + # Perform a sanity check to ensure the node can be head if not self.node_is_viable_for_head(best_node): return ForkChoiceError( @@ -229,6 +237,7 @@ func find_head*( ) head = best_node.root + debugEcho "Head found: 0x", head return ForkChoiceSuccess @@ -288,12 +297,12 @@ func maybe_prune*( # the new layout of `self.nodes` for node in self.nodes.mitems(): # If `node.parent` is less than `finalized_index`, set it to None - if node.parent_delta.isSome(): - let new_parent_delta = node.parent_delta.unsafeGet() - finalized_index - if new_parent_delta < 0: - node.parent_delta = none(Delta) + if node.parent.isSome(): + let new_parent = node.parent.unsafeGet() - finalized_index + if new_parent < 0: + node.parent = none(Index) else: - node.parent_delta = some(new_parent_delta) + node.parent = some(new_parent) if node.best_child.isSome(): let new_best_child = node.best_child.unsafeGet() - finalized_index @@ -331,6 +340,9 @@ func maybe_update_best_child_and_descendant( ## and the parent is updated with the new best descendant ## 3. The child is not the best child but becomes the best child ## 4. The child is not the best child and does not become the best child + + debugEcho " self.maybe_update_best_child_and_descendant(parent = ", parent_index, ", child = ", child_index, ")" + if child_index notin {0..self.nodes.len-1}: return ForkChoiceError( kind: fcErrInvalidNodeIndex, @@ -346,6 +358,8 @@ func maybe_update_best_child_and_descendant( template child: untyped {.dirty.} = self.nodes[child_index] template parent: untyped {.dirty.} = self.nodes[parent_index] + debugEcho " child: ", child + let (child_leads_to_viable_head, err) = self.node_leads_to_viable_head(child) if err.kind != fcSuccess: return err @@ -360,6 +374,10 @@ func maybe_update_best_child_and_descendant( ) no_change = (parent.best_child, parent.best_descendant) + debugEcho " change_to_none = ", change_to_none + debugEcho " change_to_child = ", change_to_child + debugEcho " no_change = ", no_change + # TODO: state-machine? The control-flow is messy let (new_best_child, new_best_descendant) = block: @@ -410,9 +428,13 @@ func maybe_update_best_child_and_descendant( # There is no current best-child but the child is not viable no_change + debugEcho " new_best_child = ", new_best_child + debugEcho " new_best_descendant = ", new_best_descendant + self.nodes[parent_index].best_child = new_best_child self.nodes[parent_index].best_descendant = new_best_descendant + return ForkChoiceSuccess func node_leads_to_viable_head( self: ProtoArray, node: ProtoNode diff --git a/tests/fork_choice/interpreter.nim b/tests/fork_choice/interpreter.nim index 066fa4d313..27880a8645 100644 --- a/tests/fork_choice/interpreter.nim +++ b/tests/fork_choice/interpreter.nim @@ -73,8 +73,10 @@ func apply(ctx: var ForkChoice, id: int, op: Operation) = if op.kind == FindHead: doAssert r.isOk(), &"find_head (op #{id}) returned an error: {r.error}" doAssert r.get() == op.expected_head, &"find_head (op #{id}) returned an incorrect result: {r.get()} (expected: {op.expected_head})" + debugEcho " Successfully found head: ", op.expected_head else: - doAssert r.isErr(), "find_head was unexpectedly suvvessful" + doAssert r.isErr(), "find_head was unexpectedly successful" + debugEcho " Successfully detected an invalid head" of ProcessBlock: let r = ctx.process_block( slot = op.slot, @@ -85,6 +87,7 @@ func apply(ctx: var ForkChoice, id: int, op: Operation) = finalized_epoch = op.blk_finalized_epoch ) doAssert r.isOk(), &"process_block (op #{id}) returned an error: {r.error}" + debugEcho " Processed block 0x", op.root, " with parent 0x", op.parent_root, " and justified epoch ", op.blk_justified_epoch of ProcessAttestation: let r = ctx.process_attestation( validator_index = op.validator_index, @@ -92,12 +95,14 @@ func apply(ctx: var ForkChoice, id: int, op: Operation) = target_epoch = op.target_epoch ) doAssert r.isOk(), &"process_attestation (op #{id}) returned an error: {r.error}" + debugEcho " Processed attestation for validator ", op.validator_index, " targeting ", op.block_root, " at epoch ", op.target_epoch of Prune: ctx.proto_array.prune_threshold = op.prune_threshold let r = ctx.maybe_prune(op.finalized_root) doAssert r.isOk(), &"prune (op #{id}) returned an error: {r.error}" doAssert ctx.proto_array.nodes.len == op.expected_len, &"prune (op #{id}): the resulting length ({ctx.proto_array.nodes.len}) was not expected ({op.expected_len})" + debugEcho " Maybe_pruned block preceding finalized block 0x", op.finalized_root func run*(ctx: var ForkChoice, ops: seq[Operation]) = ## Apply a sequence of fork-choice operations on a store diff --git a/tests/fork_choice/scenarios/no_votes.nim b/tests/fork_choice/scenarios/no_votes.nim index 06e8a139df..a91ee9a0e4 100644 --- a/tests/fork_choice/scenarios/no_votes.nim +++ b/tests/fork_choice/scenarios/no_votes.nim @@ -11,6 +11,17 @@ proc setup_no_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = let balances = newSeq[Gwei](16) let GenesisRoot = fakeHash(0) + # Initialize the fork choice context + result.fork_choice = initForkChoice( + finalized_block_slot = Slot(0), + finalized_block_state_root = default(Eth2Digest), + justified_epoch = Epoch(1), + finalized_epoch = Epoch(1), + finalized_root = GenesisRoot + ).get() + + # ---------------------------------- + # Head should be genesis result.ops.add Operation( kind: FindHead, @@ -49,7 +60,7 @@ proc setup_no_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = expected_head: fakeHash(2) ) - # Add block 1 + # Add block 1 as a fork # # 0 # / \ @@ -249,20 +260,11 @@ proc setup_no_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = expected_head: fakeHash(6) ) - # ---------------------------------- - # Initialize the fork choice context - result.fork_choice = initForkChoice( - finalized_block_slot = Slot(0), - finalized_block_state_root = default(Eth2Digest), - justified_epoch = Epoch(1), - finalized_epoch = Epoch(1), - finalized_root = GenesisRoot - ).get() - proc test_no_votes() = echo " fork_choice - testing no votes" # for i in 0 ..< 6: # echo " block (", i, ") hash: ", fakeHash(i) + # echo " ------------------------------------------------------" var (ctx, ops) = setup_no_votes() ctx.run(ops) diff --git a/tests/fork_choice/scenarios/votes.nim b/tests/fork_choice/scenarios/votes.nim new file mode 100644 index 0000000000..edc4d17685 --- /dev/null +++ b/tests/fork_choice/scenarios/votes.nim @@ -0,0 +1,152 @@ +# beacon_chain +# Copyright (c) 2018 Status Research & Development GmbH +# Licensed and distributed under either of +# * MIT license (license terms in the root directory or at https://opensource.org/licenses/MIT). +# * Apache v2 license (license terms in the root directory or at https://www.apache.org/licenses/LICENSE-2.0). +# at your option. This file may not be copied, modified, or distributed except according to those terms. + +import ../interpreter + +proc setup_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = + let balances = @[Gwei(1), Gwei(1)] + let GenesisRoot = fakeHash(0) + + # Initialize the fork choice context + result.fork_choice = initForkChoice( + finalized_block_slot = Slot(0), + finalized_block_state_root = default(Eth2Digest), + justified_epoch = Epoch(1), + finalized_epoch = Epoch(1), + finalized_root = GenesisRoot + ).get() + + # ---------------------------------- + + # Head should be genesis + # result.ops.add Operation( + # kind: FindHead, + # justified_epoch: Epoch(1), + # justified_root: GenesisRoot, + # finalized_epoch: Epoch(1), + # justified_state_balances: balances, + # expected_head: GenesisRoot + # ) + + # Add block 2 + # + # 0 + # / + # 2 + result.ops.add Operation( + kind: ProcessBlock, + slot: Slot(0), + root: fakeHash(2), + parent_root: GenesisRoot, + blk_justified_epoch: Epoch(1), + blk_finalized_epoch: Epoch(1) + ) + + # Head should be 2 + # + # 0 + # / + # 2 <- head + # result.ops.add Operation( + # kind: FindHead, + # justified_epoch: Epoch(1), + # justified_root: GenesisRoot, + # finalized_epoch: Epoch(1), + # justified_state_balances: balances, + # expected_head: fakeHash(2) + # ) + + # Add block 1 as a fork + # + # 0 + # / \ + # 2 1 + result.ops.add Operation( + kind: ProcessBlock, + slot: Slot(0), + root: fakeHash(1), + parent_root: GenesisRoot, + blk_justified_epoch: Epoch(1), + blk_finalized_epoch: Epoch(1) + ) + + # Head is still 2 due to tiebreaker as fakeHash(2) (0xD8...) > fakeHash(1) (0x7C...) + # + # 0 + # / \ + # head-> 2 1 + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(1), + justified_root: GenesisRoot, + finalized_epoch: Epoch(1), + justified_state_balances: balances, + expected_head: fakeHash(2) + ) + + # Add a vote to block 1 + # + # 0 + # / \ + # 2 1 <- +vote + result.ops.add Operation( + kind: ProcessAttestation, + validator_index: ValidatorIndex(0), + block_root: fakeHash(1), + target_epoch: Epoch(2) + ) + + # Head is now 1 as 1 has an extra vote + # + # 0 + # / \ + # 2 1 <- head + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(1), + justified_root: GenesisRoot, + finalized_epoch: Epoch(1), + justified_state_balances: balances, + expected_head: fakeHash(1) + ) + + # Add a vote to block 2 + # + # 0 + # / \ + # +vote-> 2 1 + result.ops.add Operation( + kind: ProcessAttestation, + validator_index: ValidatorIndex(1), + block_root: fakeHash(2), + target_epoch: Epoch(2) + ) + + # Head is back to 2 due to tiebreaker as fakeHash(2) (0xD8...) > fakeHash(1) (0x7C...) + # + # 0 + # / \ + # head-> 2 1 + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(1), + justified_root: GenesisRoot, + finalized_epoch: Epoch(1), + justified_state_balances: balances, + expected_head: fakeHash(2) + ) + +proc test_votes() = + echo " fork_choice - testing with votes" + for i in 0 ..< 11: + echo " block (", i, ") hash: ", fakeHash(i) + echo " ------------------------------------------------------" + + var (ctx, ops) = setup_votes() + ctx.run(ops) + +test_votes() From cc712b7848a95c98927c410f09249545d7f1efc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mamy=20Andr=C3=A9-Ratsimbazafy?= Date: Mon, 6 Apr 2020 16:12:34 +0200 Subject: [PATCH 04/22] Fix tiebreak of votes --- beacon_chain/fork_choice/proto_array.nim | 22 ++++++++++++++++++++-- tests/fork_choice/interpreter.nim | 2 +- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/beacon_chain/fork_choice/proto_array.nim b/beacon_chain/fork_choice/proto_array.nim index 7f51a1c6b8..6941ea306b 100644 --- a/beacon_chain/fork_choice/proto_array.nim +++ b/beacon_chain/fork_choice/proto_array.nim @@ -30,8 +30,8 @@ func tiebreak(a, b: Eth2Digest): bool = ## on the binary representation for i in 0 ..< a.data.len: if a.data[i] < b.data[i]: - return false - return true + return true + return false # Forward declarations # ---------------------------------------------------------------------- @@ -410,10 +410,15 @@ func maybe_update_best_child_and_descendant( # The best child leads to a viable head, but the child doesn't no_change elif child.weight == best_child.weight: + debugEcho "Reached tiebreak" + debugEcho " child.root 0x", child.root + debugEcho " best_child.root 0x", best_child.root + debugEcho " child.root.tiebreak(best_child.root): ", child.root.tiebreak(best_child.root) # Tie-breaker of equal weights by root if child.root.tiebreak(best_child.root): change_to_child else: + debugEcho "----> no change" no_change else: # Choose winner by weight if child.weight >= best_child.weight: @@ -476,3 +481,16 @@ func node_is_viable_for_head(self: ProtoArray, node: ProtoNode): bool {.raises: (node.finalized_epoch == self.finalized_epoch) or (node.finalized_epoch == Epoch(0)) ) + +# Sanity checks +# ---------------------------------------------------------------------- +# Sanity checks on internal private procedures + +when isMainModule: + + import nimcrypto/[hash, utils] + + let a = Eth2Digest.fromHex("0xD86E8112F3C4C4442126F8E9F44F16867DA487F29052BF91B810457DB34209A4") # sha256(2) + let b = Eth2Digest.fromHex("0x7C9FA136D4413FA6173637E883B6998D32E1D675F88CDDFF9DCBCF331820F4B8") # sha256(1) + + doAssert tiebreak(a, b) diff --git a/tests/fork_choice/interpreter.nim b/tests/fork_choice/interpreter.nim index 27880a8645..11b0fd17f6 100644 --- a/tests/fork_choice/interpreter.nim +++ b/tests/fork_choice/interpreter.nim @@ -61,7 +61,7 @@ func apply(ctx: var ForkChoice, id: int, op: Operation) = ## Apply the specified operation to a ForkChoice context ## ``id`` is additional debugging info. It is the ## operation index. - + debugEcho " ---------------------------------------------------------------------------------" case op.kind of FindHead, InvalidFindHead: let r = ctx.find_head( From e9d9c73979186f8de7b480e2cb2f2db1f8c1e4d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mamy=20Andr=C3=A9-Ratsimbazafy?= Date: Mon, 6 Apr 2020 16:21:16 +0200 Subject: [PATCH 05/22] Cleanup debugging traces --- beacon_chain/fork_choice/fork_choice.nim | 4 ---- beacon_chain/fork_choice/proto_array.nim | 23 ----------------------- tests/fork_choice/interpreter.nim | 9 ++++----- 3 files changed, 4 insertions(+), 32 deletions(-) diff --git a/beacon_chain/fork_choice/fork_choice.nim b/beacon_chain/fork_choice/fork_choice.nim index bdc7ebe66f..3d266e4fe3 100644 --- a/beacon_chain/fork_choice/fork_choice.nim +++ b/beacon_chain/fork_choice/fork_choice.nim @@ -237,10 +237,6 @@ func compute_deltas( # TODO: is int64 big enough? vote.current_root = vote.next_root - - # debugEcho "deltas(", vote.next_root, ") = ", if vote.next_root in indices: $deltas[indices[vote.next_root]] - # else: "nil" - return ForkChoiceSuccess # Sanity checks diff --git a/beacon_chain/fork_choice/proto_array.nim b/beacon_chain/fork_choice/proto_array.nim index 6941ea306b..469527a2de 100644 --- a/beacon_chain/fork_choice/proto_array.nim +++ b/beacon_chain/fork_choice/proto_array.nim @@ -190,7 +190,6 @@ func find_head*( ## The result may not be accurate if `on_new_block` ## is not followed by `apply_score_changes` as `on_new_block` does not ## update the whole tree. - debugEcho " self.find_head(head = 0x", head, ", justified_root = 0x", justified_root, ")" if justified_root notin self.indices: return ForkChoiceError( kind: fcErrJustifiedNodeUnknown, @@ -206,8 +205,6 @@ func find_head*( template justified_node: untyped {.dirty.} = self.nodes[justified_index] # Alias, TODO: no exceptions - debugEcho " self.nodes[justified_index (", justified_index, ")] = ", justified_node - let best_descendant_index = block: if justified_node.best_descendant.isSome(): justified_node.best_descendant.unsafeGet() @@ -222,8 +219,6 @@ func find_head*( template best_node: untyped {.dirty.} = self.nodes[best_descendant_index] # Alias, TODO: no exceptions - debugEcho " self.nodes[best_descendant_index (", best_descendant_index, ")] = ", best_node - # Perform a sanity check to ensure the node can be head if not self.node_is_viable_for_head(best_node): return ForkChoiceError( @@ -237,7 +232,6 @@ func find_head*( ) head = best_node.root - debugEcho "Head found: 0x", head return ForkChoiceSuccess @@ -340,9 +334,6 @@ func maybe_update_best_child_and_descendant( ## and the parent is updated with the new best descendant ## 3. The child is not the best child but becomes the best child ## 4. The child is not the best child and does not become the best child - - debugEcho " self.maybe_update_best_child_and_descendant(parent = ", parent_index, ", child = ", child_index, ")" - if child_index notin {0..self.nodes.len-1}: return ForkChoiceError( kind: fcErrInvalidNodeIndex, @@ -358,8 +349,6 @@ func maybe_update_best_child_and_descendant( template child: untyped {.dirty.} = self.nodes[child_index] template parent: untyped {.dirty.} = self.nodes[parent_index] - debugEcho " child: ", child - let (child_leads_to_viable_head, err) = self.node_leads_to_viable_head(child) if err.kind != fcSuccess: return err @@ -374,10 +363,6 @@ func maybe_update_best_child_and_descendant( ) no_change = (parent.best_child, parent.best_descendant) - debugEcho " change_to_none = ", change_to_none - debugEcho " change_to_child = ", change_to_child - debugEcho " no_change = ", no_change - # TODO: state-machine? The control-flow is messy let (new_best_child, new_best_descendant) = block: @@ -410,15 +395,10 @@ func maybe_update_best_child_and_descendant( # The best child leads to a viable head, but the child doesn't no_change elif child.weight == best_child.weight: - debugEcho "Reached tiebreak" - debugEcho " child.root 0x", child.root - debugEcho " best_child.root 0x", best_child.root - debugEcho " child.root.tiebreak(best_child.root): ", child.root.tiebreak(best_child.root) # Tie-breaker of equal weights by root if child.root.tiebreak(best_child.root): change_to_child else: - debugEcho "----> no change" no_change else: # Choose winner by weight if child.weight >= best_child.weight: @@ -433,9 +413,6 @@ func maybe_update_best_child_and_descendant( # There is no current best-child but the child is not viable no_change - debugEcho " new_best_child = ", new_best_child - debugEcho " new_best_descendant = ", new_best_descendant - self.nodes[parent_index].best_child = new_best_child self.nodes[parent_index].best_descendant = new_best_descendant diff --git a/tests/fork_choice/interpreter.nim b/tests/fork_choice/interpreter.nim index 11b0fd17f6..ded338cd78 100644 --- a/tests/fork_choice/interpreter.nim +++ b/tests/fork_choice/interpreter.nim @@ -61,7 +61,6 @@ func apply(ctx: var ForkChoice, id: int, op: Operation) = ## Apply the specified operation to a ForkChoice context ## ``id`` is additional debugging info. It is the ## operation index. - debugEcho " ---------------------------------------------------------------------------------" case op.kind of FindHead, InvalidFindHead: let r = ctx.find_head( @@ -73,10 +72,10 @@ func apply(ctx: var ForkChoice, id: int, op: Operation) = if op.kind == FindHead: doAssert r.isOk(), &"find_head (op #{id}) returned an error: {r.error}" doAssert r.get() == op.expected_head, &"find_head (op #{id}) returned an incorrect result: {r.get()} (expected: {op.expected_head})" - debugEcho " Successfully found head: ", op.expected_head + debugEcho " Found expected head: 0x", op.expected_head else: doAssert r.isErr(), "find_head was unexpectedly successful" - debugEcho " Successfully detected an invalid head" + debugEcho " Detected an expected invalid head" of ProcessBlock: let r = ctx.process_block( slot = op.slot, @@ -87,7 +86,7 @@ func apply(ctx: var ForkChoice, id: int, op: Operation) = finalized_epoch = op.blk_finalized_epoch ) doAssert r.isOk(), &"process_block (op #{id}) returned an error: {r.error}" - debugEcho " Processed block 0x", op.root, " with parent 0x", op.parent_root, " and justified epoch ", op.blk_justified_epoch + debugEcho " Processed block 0x", op.root, " with parent 0x", op.parent_root, " and justified epoch ", op.blk_justified_epoch of ProcessAttestation: let r = ctx.process_attestation( validator_index = op.validator_index, @@ -95,7 +94,7 @@ func apply(ctx: var ForkChoice, id: int, op: Operation) = target_epoch = op.target_epoch ) doAssert r.isOk(), &"process_attestation (op #{id}) returned an error: {r.error}" - debugEcho " Processed attestation for validator ", op.validator_index, " targeting ", op.block_root, " at epoch ", op.target_epoch + debugEcho " Processed att target 0x", op.block_root, " from validator ", op.validator_index, " for epoch ", op.target_epoch of Prune: ctx.proto_array.prune_threshold = op.prune_threshold let r = ctx.maybe_prune(op.finalized_root) From 6e2e28b3e2b867312c1b356e28751c6dd795557e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mamy=20Andr=C3=A9-Ratsimbazafy?= Date: Mon, 6 Apr 2020 16:51:08 +0200 Subject: [PATCH 06/22] Complexify the vote test --- tests/fork_choice/interpreter.nim | 2 +- tests/fork_choice/scenarios/votes.nim | 462 +++++++++++++++++++++++++- 2 files changed, 446 insertions(+), 18 deletions(-) diff --git a/tests/fork_choice/interpreter.nim b/tests/fork_choice/interpreter.nim index ded338cd78..b59e77115f 100644 --- a/tests/fork_choice/interpreter.nim +++ b/tests/fork_choice/interpreter.nim @@ -72,7 +72,7 @@ func apply(ctx: var ForkChoice, id: int, op: Operation) = if op.kind == FindHead: doAssert r.isOk(), &"find_head (op #{id}) returned an error: {r.error}" doAssert r.get() == op.expected_head, &"find_head (op #{id}) returned an incorrect result: {r.get()} (expected: {op.expected_head})" - debugEcho " Found expected head: 0x", op.expected_head + debugEcho " Found expected head: 0x", op.expected_head, " from justified checkpoint(epoch: ", op.justified_epoch, ", root: 0x", op.justified_root, ")" else: doAssert r.isErr(), "find_head was unexpectedly successful" debugEcho " Detected an expected invalid head" diff --git a/tests/fork_choice/scenarios/votes.nim b/tests/fork_choice/scenarios/votes.nim index edc4d17685..e6bac8c69c 100644 --- a/tests/fork_choice/scenarios/votes.nim +++ b/tests/fork_choice/scenarios/votes.nim @@ -8,7 +8,7 @@ import ../interpreter proc setup_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = - let balances = @[Gwei(1), Gwei(1)] + var balances = @[Gwei(1), Gwei(1)] let GenesisRoot = fakeHash(0) # Initialize the fork choice context @@ -23,14 +23,14 @@ proc setup_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = # ---------------------------------- # Head should be genesis - # result.ops.add Operation( - # kind: FindHead, - # justified_epoch: Epoch(1), - # justified_root: GenesisRoot, - # finalized_epoch: Epoch(1), - # justified_state_balances: balances, - # expected_head: GenesisRoot - # ) + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(1), + justified_root: GenesisRoot, + finalized_epoch: Epoch(1), + justified_state_balances: balances, + expected_head: GenesisRoot + ) # Add block 2 # @@ -51,14 +51,14 @@ proc setup_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = # 0 # / # 2 <- head - # result.ops.add Operation( - # kind: FindHead, - # justified_epoch: Epoch(1), - # justified_root: GenesisRoot, - # finalized_epoch: Epoch(1), - # justified_state_balances: balances, - # expected_head: fakeHash(2) - # ) + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(1), + justified_root: GenesisRoot, + finalized_epoch: Epoch(1), + justified_state_balances: balances, + expected_head: fakeHash(2) + ) # Add block 1 as a fork # @@ -140,6 +140,434 @@ proc setup_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = expected_head: fakeHash(2) ) + # Add block 3 as on chain 1 + # + # 0 + # / \ + # 2 1 + # | + # 3 + result.ops.add Operation( + kind: ProcessBlock, + slot: Slot(0), + root: fakeHash(3), + parent_root: fakeHash(1), + blk_justified_epoch: Epoch(1), + blk_finalized_epoch: Epoch(1) + ) + + # Head is still 2 + # + # 0 + # / \ + # head-> 2 1 + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(1), + justified_root: GenesisRoot, + finalized_epoch: Epoch(1), + justified_state_balances: balances, + expected_head: fakeHash(2) + ) + + # Move validator #0 vote from 1 to 3 + # + # 0 + # / \ + # 2 1 <- -vote + # | + # 3 <- +vote + result.ops.add Operation( + kind: ProcessAttestation, + validator_index: ValidatorIndex(0), + block_root: fakeHash(3), + target_epoch: Epoch(3) + ) + + # Head is still 2 + # + # 0 + # / \ + # head-> 2 1 + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(1), + justified_root: GenesisRoot, + finalized_epoch: Epoch(1), + justified_state_balances: balances, + expected_head: fakeHash(2) + ) + + # Move validator #1 vote from 2 to 1 (this is an equivocation, but fork choice doesn't + # care) + # + # 0 + # / \ + # -vote-> 2 1 <- +vote + # | + # 3 + result.ops.add Operation( + kind: ProcessAttestation, + validator_index: ValidatorIndex(1), + block_root: fakeHash(1), + target_epoch: Epoch(3) + ) + + # Head is now 3 + # + # 0 + # / \ + # 2 1 + # | + # 3 <- head + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(1), + justified_root: GenesisRoot, + finalized_epoch: Epoch(1), + justified_state_balances: balances, + expected_head: fakeHash(3) + ) + + # Add block 4 on chain 1-3 + # + # 0 + # / \ + # 2 1 + # | + # 3 + result.ops.add Operation( + kind: ProcessBlock, + slot: Slot(0), + root: fakeHash(4), + parent_root: fakeHash(3), + blk_justified_epoch: Epoch(1), + blk_finalized_epoch: Epoch(1) + ) + + # Head is now 4 + # + # 0 + # / \ + # 2 1 + # | + # 3 + # | + # 4 <- head + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(1), + justified_root: GenesisRoot, + finalized_epoch: Epoch(1), + justified_state_balances: balances, + expected_head: fakeHash(4) + ) + + # Add block 5, which has a justified epoch of 2. + # + # 0 + # / \ + # 2 1 + # | + # 3 + # | + # 4 + # / + # 5 <- justified epoch = 2 + result.ops.add Operation( + kind: ProcessBlock, + slot: Slot(0), + root: fakeHash(5), + parent_root: fakeHash(4), + blk_justified_epoch: Epoch(2), + blk_finalized_epoch: Epoch(2) + ) + + # Ensure that 5 is filtered out and the head stays at 4. + # + # 0 + # / \ + # 2 1 + # | + # 3 + # | + # 4 <- head + # / + # 5 + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(1), + justified_root: GenesisRoot, + finalized_epoch: Epoch(1), + justified_state_balances: balances, + expected_head: fakeHash(4) + ) + + # Add block 6, which has a justified epoch of 0. + # + # 0 + # / \ + # 2 1 + # | + # 3 + # | + # 4 + # / \ + # 5 6 <- justified epoch = 0 + result.ops.add Operation( + kind: ProcessBlock, + slot: Slot(0), + root: fakeHash(6), + parent_root: fakeHash(4), + blk_justified_epoch: Epoch(1), + blk_finalized_epoch: Epoch(1) + ) + + # Move both votes to 5. + # + # 0 + # / \ + # 2 1 + # | + # 3 + # | + # 4 + # / \ + # +2 vote-> 5 6 + result.ops.add Operation( + kind: ProcessAttestation, + validator_index: ValidatorIndex(0), + block_root: fakeHash(5), + target_epoch: Epoch(4) + ) + result.ops.add Operation( + kind: ProcessAttestation, + validator_index: ValidatorIndex(1), + block_root: fakeHash(5), + target_epoch: Epoch(4) + ) + + # Add blocks 7, 8 and 9. Adding these blocks helps test the `best_descendant` + # functionality. + # + # 0 + # / \ + # 2 1 + # | + # 3 + # | + # 4 + # / \ + # 5 6 + # | + # 7 + # | + # 8 + # / + # 9 + result.ops.add Operation( + kind: ProcessBlock, + slot: Slot(0), + root: fakeHash(7), + parent_root: fakeHash(5), + blk_justified_epoch: Epoch(2), + blk_finalized_epoch: Epoch(2) + ) + result.ops.add Operation( + kind: ProcessBlock, + slot: Slot(0), + root: fakeHash(8), + parent_root: fakeHash(7), + blk_justified_epoch: Epoch(2), + blk_finalized_epoch: Epoch(2) + ) + result.ops.add Operation( + kind: ProcessBlock, + slot: Slot(0), + root: fakeHash(9), + parent_root: fakeHash(8), + blk_justified_epoch: Epoch(2), + blk_finalized_epoch: Epoch(2) + ) + + # Ensure that 6 is the head, even though 5 has all the votes. This is testing to ensure + # that 5 is filtered out due to a differing justified epoch. + # + # 0 + # / \ + # 2 1 + # | + # 3 + # | + # 4 + # / \ + # 5 6 <- head + # | + # 7 + # | + # 8 + # / + # 9 + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(1), + justified_root: GenesisRoot, + finalized_epoch: Epoch(1), + justified_state_balances: balances, + expected_head: fakeHash(6) + ) + + # Change fork-choice justified epoch to 1, and the start block to 5 and ensure that 9 is + # the head. + # + # << Change justified epoch to 1 >> + # + # 0 + # / \ + # 2 1 + # | + # 3 + # | + # 4 + # / \ + # 5 6 + # | + # 7 + # | + # 8 + # / + # head-> 9 + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(2), + justified_root: fakeHash(5), + finalized_epoch: Epoch(2), + justified_state_balances: balances, + expected_head: fakeHash(9) + ) + + # Update votes to block 9 + # 0 + # / \ + # 2 1 + # | + # 3 + # | + # 4 + # / \ + # 5 6 + # | + # 7 + # | + # 8 + # / + # 9 <- +2 votes + result.ops.add Operation( + kind: ProcessAttestation, + validator_index: ValidatorIndex(0), + block_root: fakeHash(9), + target_epoch: Epoch(5) + ) + result.ops.add Operation( + kind: ProcessAttestation, + validator_index: ValidatorIndex(1), + block_root: fakeHash(9), + target_epoch: Epoch(5) + ) + + # Add block 10 + # 0 + # / \ + # 2 1 + # | + # 3 + # | + # 4 + # / \ + # 5 6 + # | + # 7 + # | + # 8 + # / \ + # 9 10 + result.ops.add Operation( + kind: ProcessBlock, + slot: Slot(0), + root: fakeHash(10), + parent_root: fakeHash(8), + blk_justified_epoch: Epoch(2), + blk_finalized_epoch: Epoch(2) + ) + + # Head should still be 9 + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(2), + justified_root: fakeHash(5), + finalized_epoch: Epoch(2), + justified_state_balances: balances, + expected_head: fakeHash(9) + ) + + # Introduce 2 new validators + balances = @[Gwei(1), Gwei(1), Gwei(1), Gwei(1)] + + # Have them vote for block 10 + # 0 + # / \ + # 2 1 + # | + # 3 + # | + # 4 + # / \ + # 5 6 + # | + # 7 + # | + # 8 + # / \ + # 9 10 <- +2 votes + result.ops.add Operation( + kind: ProcessAttestation, + validator_index: ValidatorIndex(2), + block_root: fakeHash(9), + target_epoch: Epoch(5) + ) + result.ops.add Operation( + kind: ProcessAttestation, + validator_index: ValidatorIndex(3), + block_root: fakeHash(9), + target_epoch: Epoch(5) + ) + + # # Check that the head is now 10. + # # + # # 0 + # # / \ + # # 2 1 + # # | + # # 3 + # # | + # # 4 + # # / \ + # # 5 6 + # # | + # # 7 + # # | + # # 8 + # # / \ + # # 9 10 <- head + # result.ops.add Operation( + # kind: FindHead, + # justified_epoch: Epoch(2), + # justified_root: fakeHash(5), + # finalized_epoch: Epoch(2), + # justified_state_balances: balances, + # expected_head: fakeHash(10) + # ) + proc test_votes() = echo " fork_choice - testing with votes" for i in 0 ..< 11: From a2e41b905066d724fa1c361cc2111a15074a5a31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mamy=20Andr=C3=A9-Ratsimbazafy?= Date: Mon, 6 Apr 2020 18:12:42 +0200 Subject: [PATCH 07/22] fakeHash use the bigEndian repr of number + fix tiebreak for good --- beacon_chain/fork_choice/fork_choice.nim | 14 ++++-- beacon_chain/fork_choice/proto_array.nim | 29 +++++++++++-- tests/fork_choice/interpreter.nim | 11 +++-- tests/fork_choice/scenarios/votes.nim | 54 ++++++++++++------------ 4 files changed, 70 insertions(+), 38 deletions(-) diff --git a/beacon_chain/fork_choice/fork_choice.nim b/beacon_chain/fork_choice/fork_choice.nim index 3d266e4fe3..a7152e841b 100644 --- a/beacon_chain/fork_choice/fork_choice.nim +++ b/beacon_chain/fork_choice/fork_choice.nim @@ -244,10 +244,16 @@ func compute_deltas( # Sanity checks on internal private procedures when isMainModule: - import nimcrypto/[sha2, utils] - - func fakeHash(index: int): Eth2Digest = - sha256.digest(cast[array[sizeof(int), byte]](index)) + import stew/endians2 + + # TODO: if the value added is 1 or 16, we error out. Why? Nim table bug with not enough spaced hashes? + func fakeHash*(index: SomeInteger): Eth2Digest = + ## Create fake hashes + ## Those are just the value serialized in big-endian + ## We add 16x16 to avoid having a zero hash are those are special cased + ## We store them in the first 8 bytes + ## as those are the one used in hash tables Table[Eth2Digest, T] + result.data[0 ..< 8] = (16*16+index).uint64.toBytesBE() proc tZeroHash() = echo " fork_choice compute_deltas - test zero votes" diff --git a/beacon_chain/fork_choice/proto_array.nim b/beacon_chain/fork_choice/proto_array.nim index 469527a2de..154f62ac5a 100644 --- a/beacon_chain/fork_choice/proto_array.nim +++ b/beacon_chain/fork_choice/proto_array.nim @@ -30,8 +30,11 @@ func tiebreak(a, b: Eth2Digest): bool = ## on the binary representation for i in 0 ..< a.data.len: if a.data[i] < b.data[i]: + return false + elif a.data[i] > b.data[i]: return true - return false + # else we have equality so far + return true # Forward declarations # ---------------------------------------------------------------------- @@ -467,7 +470,25 @@ when isMainModule: import nimcrypto/[hash, utils] - let a = Eth2Digest.fromHex("0xD86E8112F3C4C4442126F8E9F44F16867DA487F29052BF91B810457DB34209A4") # sha256(2) - let b = Eth2Digest.fromHex("0x7C9FA136D4413FA6173637E883B6998D32E1D675F88CDDFF9DCBCF331820F4B8") # sha256(1) + block: + let a = Eth2Digest.fromHex("0x0000000000000001000000000000000000000000000000000000000000000000") + let b = Eth2Digest.fromHex("0x0000000000000000000000000000000000000000000000000000000000000000") # sha256(1) - doAssert tiebreak(a, b) + doAssert tiebreak(a, b) + + + block: + let a = Eth2Digest.fromHex("0x0000000000000002000000000000000000000000000000000000000000000000") + let b = Eth2Digest.fromHex("0x0000000000000001000000000000000000000000000000000000000000000000") # sha256(1) + + doAssert tiebreak(a, b) + + + block: + let a = Eth2Digest.fromHex("0xD86E8112F3C4C4442126F8E9F44F16867DA487F29052BF91B810457DB34209A4") # sha256(2) + let b = Eth2Digest.fromHex("0x7C9FA136D4413FA6173637E883B6998D32E1D675F88CDDFF9DCBCF331820F4B8") # sha256(1) + + echo a.data + echo b.data + + doAssert tiebreak(a, b) diff --git a/tests/fork_choice/interpreter.nim b/tests/fork_choice/interpreter.nim index b59e77115f..135fedd0ab 100644 --- a/tests/fork_choice/interpreter.nim +++ b/tests/fork_choice/interpreter.nim @@ -9,16 +9,21 @@ import # Standard library std/strformat, std/tables, std/options, # Status libraries - stew/result, nimcrypto/[sha2, utils], + stew/[result, endians2], # Internals ../../beacon_chain/spec/[datatypes, digest], ../../beacon_chain/fork_choice/[fork_choice, fork_choice_types] export result, datatypes, digest, fork_choice, fork_choice_types, tables, options -func fakeHash*(index: int): Eth2Digest = +# TODO: if the value added is 1 or 16, we error out. Why? Nim table bug with not enough spaced hashes? +func fakeHash*(index: SomeInteger): Eth2Digest = ## Create fake hashes - sha256.digest(cast[array[sizeof(int), byte]](index)) + ## Those are just the value serialized in big-endian + ## We add 16x16 to avoid having a zero hash are those are special cased + ## We store them in the first 8 bytes + ## as those are the one used in hash tables Table[Eth2Digest, T] + result.data[0 ..< 8] = (16*16+index).uint64.toBytesBE() # The fork choice tests are quite complex. # For flexibility in block arrival, timers, operations sequencing, ... diff --git a/tests/fork_choice/scenarios/votes.nim b/tests/fork_choice/scenarios/votes.nim index e6bac8c69c..c4f1f6405d 100644 --- a/tests/fork_choice/scenarios/votes.nim +++ b/tests/fork_choice/scenarios/votes.nim @@ -532,41 +532,41 @@ proc setup_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = result.ops.add Operation( kind: ProcessAttestation, validator_index: ValidatorIndex(2), - block_root: fakeHash(9), + block_root: fakeHash(10), target_epoch: Epoch(5) ) result.ops.add Operation( kind: ProcessAttestation, validator_index: ValidatorIndex(3), - block_root: fakeHash(9), + block_root: fakeHash(10), target_epoch: Epoch(5) ) - # # Check that the head is now 10. - # # - # # 0 - # # / \ - # # 2 1 - # # | - # # 3 - # # | - # # 4 - # # / \ - # # 5 6 - # # | - # # 7 - # # | - # # 8 - # # / \ - # # 9 10 <- head - # result.ops.add Operation( - # kind: FindHead, - # justified_epoch: Epoch(2), - # justified_root: fakeHash(5), - # finalized_epoch: Epoch(2), - # justified_state_balances: balances, - # expected_head: fakeHash(10) - # ) + # Check that the head is now 10. + # + # 0 + # / \ + # 2 1 + # | + # 3 + # | + # 4 + # / \ + # 5 6 + # | + # 7 + # | + # 8 + # / \ + # 9 10 <- head + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(2), + justified_root: fakeHash(5), + finalized_epoch: Epoch(2), + justified_state_balances: balances, + expected_head: fakeHash(10) + ) proc test_votes() = echo " fork_choice - testing with votes" From 8af34408ba764453dedf5098d7ea4834adb2e946 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mamy=20Andr=C3=A9-Ratsimbazafy?= Date: Mon, 6 Apr 2020 19:42:22 +0200 Subject: [PATCH 08/22] Stash changes: found critical bug in nimcrypto `==` and var openarray --- beacon_chain/fork_choice/fork_choice.nim | 22 +++++++++++++++----- beacon_chain/fork_choice/proto_array.nim | 26 ++++++++++++++++++++---- tests/fork_choice/interpreter.nim | 4 +++- tests/fork_choice/scenarios/votes.nim | 12 +++++++++++ 4 files changed, 54 insertions(+), 10 deletions(-) diff --git a/beacon_chain/fork_choice/fork_choice.nim b/beacon_chain/fork_choice/fork_choice.nim index a7152e841b..32f55cf161 100644 --- a/beacon_chain/fork_choice/fork_choice.nim +++ b/beacon_chain/fork_choice/fork_choice.nim @@ -9,7 +9,7 @@ import # Standard library std/tables, std/options, std/typetraits, # Status libraries - nimcrypto/hash, stew/result, + stew/result, # Internal ../spec/[datatypes, digest], # Fork choice @@ -145,6 +145,8 @@ func find_head*( result.err("find_head compute_deltas failed: " & $delta_err) return + # debugEcho "find_head deltas: ", deltas + # Apply score changes let score_err = self.proto_array.apply_score_changes( deltas, justified_epoch, finalized_epoch @@ -190,6 +192,16 @@ func compute_deltas( ## - If a `Eth2Digest` in `votes` does not exist in `indices` ## except for the `default(Eth2Digest)` (i.e. zero hash) + # TODO: Displaying the votes for debugging will cause the tests to fail!!! + # if we do the precise sequence + # - tAll_voted_the_same() + # - tDifferent_votes() + # - tMoving_votes() + # - tChanging_balances() + # var openarray bug? + # + # debugEcho "compute_deltas votes: ", votes + for val_index, vote in votes.mpairs(): # No need to create a score change if the validator has never voted # or if votes are for the zero hash (alias to the genesis block) @@ -246,7 +258,7 @@ func compute_deltas( when isMainModule: import stew/endians2 - # TODO: if the value added is 1 or 16, we error out. Why? Nim table bug with not enough spaced hashes? + # TODO: Note - we don't want to import nimcrypto/hash as the `==` is buggy at the moment. func fakeHash*(index: SomeInteger): Eth2Digest = ## Create fake hashes ## Those are just the value serialized in big-endian @@ -570,10 +582,10 @@ when isMainModule: # ---------------------------------------------------------------------- echo "fork_choice internal tests for compute_deltas" - tZeroHash() + # tZeroHash() tAll_voted_the_same() tDifferent_votes() tMoving_votes() tChanging_balances() - tValidator_appears() - tValidator_disappears() + # tValidator_appears() + # tValidator_disappears() diff --git a/beacon_chain/fork_choice/proto_array.nim b/beacon_chain/fork_choice/proto_array.nim index 154f62ac5a..86e4e48ee8 100644 --- a/beacon_chain/fork_choice/proto_array.nim +++ b/beacon_chain/fork_choice/proto_array.nim @@ -8,8 +8,6 @@ import # Standard library std/tables, std/options, std/typetraits, - # Status libraries - nimcrypto/hash, # Internal ../spec/[datatypes, digest], # Fork choice @@ -107,8 +105,12 @@ func apply_score_changes*( ) node.weight = weight - # debugecho " deltas[", node_index, "] = ", deltas[node_index] - # debugecho " node.weight = ", node.weight + # var report: string + # # report.add "deltas[" & $node_index & "] = " & $deltas[node_index] + # report.add "node.weight = " & $node.weight + # report.add ", node root 0x" & $node.root + + # debugecho " ", report # If the node has a parent, try to update its best-child and best-descendant if node.parent.isSome(): @@ -337,6 +339,9 @@ func maybe_update_best_child_and_descendant( ## and the parent is updated with the new best descendant ## 3. The child is not the best child but becomes the best child ## 4. The child is not the best child and does not become the best child + + # debugEcho " self.maybe_update_best_child_and_descendant(parent = ", parent_index, ", child = ", child_index, ")" + if child_index notin {0..self.nodes.len-1}: return ForkChoiceError( kind: fcErrInvalidNodeIndex, @@ -352,6 +357,8 @@ func maybe_update_best_child_and_descendant( template child: untyped {.dirty.} = self.nodes[child_index] template parent: untyped {.dirty.} = self.nodes[parent_index] + # debugEcho " child: ", child + let (child_leads_to_viable_head, err) = self.node_leads_to_viable_head(child) if err.kind != fcSuccess: return err @@ -366,6 +373,10 @@ func maybe_update_best_child_and_descendant( ) no_change = (parent.best_child, parent.best_descendant) + # debugEcho " change_to_none = ", change_to_none + # debugEcho " change_to_child = ", change_to_child + # debugEcho " no_change = ", no_change + # TODO: state-machine? The control-flow is messy let (new_best_child, new_best_descendant) = block: @@ -398,6 +409,10 @@ func maybe_update_best_child_and_descendant( # The best child leads to a viable head, but the child doesn't no_change elif child.weight == best_child.weight: + # debugEcho "Reached tiebreak" + # debugEcho " child.root 0x", child.root + # debugEcho " best_child.root 0x", best_child.root + # debugEcho " child.root.tiebreak(best_child.root): ", child.root.tiebreak(best_child.root) # Tie-breaker of equal weights by root if child.root.tiebreak(best_child.root): change_to_child @@ -416,6 +431,9 @@ func maybe_update_best_child_and_descendant( # There is no current best-child but the child is not viable no_change + # debugEcho " new_best_child = ", new_best_child + # debugEcho " new_best_descendant = ", new_best_descendant + self.nodes[parent_index].best_child = new_best_child self.nodes[parent_index].best_descendant = new_best_descendant diff --git a/tests/fork_choice/interpreter.nim b/tests/fork_choice/interpreter.nim index 135fedd0ab..d6eeb67fd1 100644 --- a/tests/fork_choice/interpreter.nim +++ b/tests/fork_choice/interpreter.nim @@ -16,7 +16,8 @@ import export result, datatypes, digest, fork_choice, fork_choice_types, tables, options -# TODO: if the value added is 1 or 16, we error out. Why? Nim table bug with not enough spaced hashes? +# TODO: nimcrypto.hash.`==` is returns incorrect result with those fakeHash +# Don't import nimcrypto, let Nim do the `==` in the mean-time func fakeHash*(index: SomeInteger): Eth2Digest = ## Create fake hashes ## Those are just the value serialized in big-endian @@ -66,6 +67,7 @@ func apply(ctx: var ForkChoice, id: int, op: Operation) = ## Apply the specified operation to a ForkChoice context ## ``id`` is additional debugging info. It is the ## operation index. + # debugEcho " =========================================================================================" case op.kind of FindHead, InvalidFindHead: let r = ctx.find_head( diff --git a/tests/fork_choice/scenarios/votes.nim b/tests/fork_choice/scenarios/votes.nim index c4f1f6405d..a0713f57b9 100644 --- a/tests/fork_choice/scenarios/votes.nim +++ b/tests/fork_choice/scenarios/votes.nim @@ -236,6 +236,8 @@ proc setup_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = # 2 1 # | # 3 + # | + # 4 result.ops.add Operation( kind: ProcessBlock, slot: Slot(0), @@ -475,6 +477,16 @@ proc setup_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = target_epoch: Epoch(5) ) + # Head should still be 9 + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(2), + justified_root: fakeHash(5), + finalized_epoch: Epoch(2), + justified_state_balances: balances, + expected_head: fakeHash(9) + ) + # Add block 10 # 0 # / \ From 051b0e98afd7803dbfb1f26bd531a5f92b3780de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mamy=20Andr=C3=A9-Ratsimbazafy?= Date: Mon, 6 Apr 2020 21:34:20 +0200 Subject: [PATCH 09/22] Passing fork choice tests with varying votes --- beacon_chain/fork_choice/fork_choice.nim | 2 +- tests/fork_choice/scenarios/votes.nim | 137 +++++++++++++++++++++++ 2 files changed, 138 insertions(+), 1 deletion(-) diff --git a/beacon_chain/fork_choice/fork_choice.nim b/beacon_chain/fork_choice/fork_choice.nim index 32f55cf161..da761b8213 100644 --- a/beacon_chain/fork_choice/fork_choice.nim +++ b/beacon_chain/fork_choice/fork_choice.nim @@ -172,7 +172,7 @@ func maybe_prune*( let err = self.proto_array.maybe_prune(finalized_root) if err.kind != fcSuccess: result.err("find_head maybe_pruned failed: " & $err) - + result.ok() func compute_deltas( deltas: var openarray[Delta], diff --git a/tests/fork_choice/scenarios/votes.nim b/tests/fork_choice/scenarios/votes.nim index a0713f57b9..7192eeecbe 100644 --- a/tests/fork_choice/scenarios/votes.nim +++ b/tests/fork_choice/scenarios/votes.nim @@ -580,6 +580,143 @@ proc setup_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = expected_head: fakeHash(10) ) + # Set the last 2 validators balances to 0 + balances = @[Gwei(1), Gwei(1), Gwei(0), Gwei(0)] + + # head should be 9 again + # . + # | + # 8 + # / \ + # head -> 9 10 + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(2), + justified_root: fakeHash(5), + finalized_epoch: Epoch(2), + justified_state_balances: balances, + expected_head: fakeHash(9) + ) + + # Set the last 2 validators balances back to 1 + balances = @[Gwei(1), Gwei(1), Gwei(1), Gwei(1)] + + # head should be 10 again + # . + # | + # 8 + # / \ + # 9 10 <- head + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(2), + justified_root: fakeHash(5), + finalized_epoch: Epoch(2), + justified_state_balances: balances, + expected_head: fakeHash(10) + ) + + # Remove the validators + balances = @[Gwei(1), Gwei(1)] + + # head should be 9 again + # . + # | + # 8 + # / \ + # head -> 9 10 + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(2), + justified_root: fakeHash(5), + finalized_epoch: Epoch(2), + justified_state_balances: balances, + expected_head: fakeHash(9) + ) + + # Pruning below the prune threshold doesn't prune + result.ops.add Operation( + kind: Prune, + finalized_root: fakeHash(5), + prune_threshold: high(int), + expected_len: 11 + ) + + # Prune shouldn't have changed the head + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(2), + justified_root: fakeHash(5), + finalized_epoch: Epoch(2), + justified_state_balances: balances, + expected_head: fakeHash(9) + ) + + # Ensure that pruning above the prune threshold does prune. + # + # + # 0 + # / \ + # 2 1 + # | + # 3 + # | + # 4 + # -------pruned here ------ + # 5 6 + # | + # 7 + # | + # 8 + # / \ + # 9 10 + result.ops.add Operation( + kind: Prune, + finalized_root: fakeHash(5), + prune_threshold: 1, + expected_len: 6 + ) + + # Prune shouldn't have changed the head + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(2), + justified_root: fakeHash(5), + finalized_epoch: Epoch(2), + justified_state_balances: balances, + expected_head: fakeHash(9) + ) + + # Add block 11 + # + # 5 6 + # | + # 7 + # | + # 8 + # / \ + # 9 10 + # | + # 11 + result.ops.add Operation( + kind: ProcessBlock, + slot: Slot(0), + root: fakeHash(11), + parent_root: fakeHash(9), + blk_justified_epoch: Epoch(2), + blk_finalized_epoch: Epoch(2) + ) + + # Head is now 11 + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(2), + justified_root: fakeHash(5), + finalized_epoch: Epoch(2), + justified_state_balances: balances, + expected_head: fakeHash(11) + ) + proc test_votes() = echo " fork_choice - testing with votes" for i in 0 ..< 11: From 1d0b4089fa480a69811664fa4a9c4bb100d11340 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mamy=20Andr=C3=A9-Ratsimbazafy?= Date: Mon, 6 Apr 2020 23:47:16 +0200 Subject: [PATCH 10/22] Add FFG fork choice scenario + fork choice to the test suite --- beacon_chain/fork_choice/fork_choice.nim | 7 +- .../fork_choice/fork_choice_types.nim | 5 +- beacon_chain/fork_choice/proto_array.nim | 36 +- tests/all_tests.nim | 3 +- tests/fork_choice/interpreter.nim | 5 +- tests/fork_choice/scenarios/ffg_01.nim | 129 ++++++ tests/fork_choice/scenarios/ffg_02.nim | 391 ++++++++++++++++++ tests/fork_choice/scenarios/no_votes.nim | 10 +- tests/fork_choice/scenarios/votes.nim | 21 +- tests/fork_choice/tests_fork_choice.nim | 6 + 10 files changed, 579 insertions(+), 34 deletions(-) create mode 100644 tests/fork_choice/scenarios/ffg_01.nim create mode 100644 tests/fork_choice/scenarios/ffg_02.nim create mode 100644 tests/fork_choice/tests_fork_choice.nim diff --git a/beacon_chain/fork_choice/fork_choice.nim b/beacon_chain/fork_choice/fork_choice.nim index da761b8213..14f74cb54e 100644 --- a/beacon_chain/fork_choice/fork_choice.nim +++ b/beacon_chain/fork_choice/fork_choice.nim @@ -131,6 +131,8 @@ func find_head*( justified_state_balances: seq[Gwei] ): Result[Eth2Digest, string] {.raises: [UnpackError, KeyError].} = ## Returns the new blockchain head + # debugEcho "proto_array: ", self.proto_array + # debugEcho "-----------------------\n" # Compute deltas with previous call # we might want to reuse the `deltas` buffer across calls @@ -154,8 +156,11 @@ func find_head*( if score_err.kind != fcSuccess: result.err("find_head apply_score_changes failed: " & $score_err) - # Find the best block self.balances = justified_state_balances + # debugEcho "proto_array: ", self.proto_array + # debugEcho "-----------------------\n" + + # Find the best block var new_head{.noInit.}: Eth2Digest let ghost_err = self.proto_array.find_head(new_head, justified_root) if ghost_err.kind != fcSuccess: diff --git a/beacon_chain/fork_choice/fork_choice_types.nim b/beacon_chain/fork_choice/fork_choice_types.nim index 3a8857d65c..db5dd4afc9 100644 --- a/beacon_chain/fork_choice/fork_choice_types.nim +++ b/beacon_chain/fork_choice/fork_choice_types.nim @@ -91,8 +91,9 @@ type indices*: Table[Eth2Digest, Index] ProtoNode* = object - slot*: Slot - state_root*: Eth2Digest + # TODO: generic "Metadata" field for slot/state_root + slot*: Slot # This is unnecessary for fork choice but helps external components + state_root*: Eth2Digest # This is unnecessary for fork choice but helps external components root*: Eth2Digest parent*: Option[Index] justified_epoch*: Epoch diff --git a/beacon_chain/fork_choice/proto_array.nim b/beacon_chain/fork_choice/proto_array.nim index 86e4e48ee8..0358558ee3 100644 --- a/beacon_chain/fork_choice/proto_array.nim +++ b/beacon_chain/fork_choice/proto_array.nim @@ -64,6 +64,9 @@ func apply_score_changes*( ## updating if the current node should become the best-child ## 4. If required, update the parent's best-descendant with the current node or its best-descendant # TODO: remove spurious raised exceptions + + # debugEcho "apply_score_change" + if deltas.len != self.indices.len: return ForkChoiceError( kind: fcErrInvalidDeltaLen, @@ -195,12 +198,17 @@ func find_head*( ## The result may not be accurate if `on_new_block` ## is not followed by `apply_score_changes` as `on_new_block` does not ## update the whole tree. + + # debugEcho "proto_array: ", self + # debugEcho "-------------------------------------------------------------\n" + if justified_root notin self.indices: return ForkChoiceError( kind: fcErrJustifiedNodeUnknown, block_root: justified_root ) let justified_index = self.indices[justified_root] # TODO: this can't throw KeyError + # debugEcho "justified_index: ", justified_index if justified_index notin {0..self.nodes.len-1}: return ForkChoiceError( @@ -210,12 +218,16 @@ func find_head*( template justified_node: untyped {.dirty.} = self.nodes[justified_index] # Alias, TODO: no exceptions + # debugEcho "justified_node: ", justified_node + let best_descendant_index = block: if justified_node.best_descendant.isSome(): justified_node.best_descendant.unsafeGet() else: justified_index + # debugEcho "best_descendant_index: ", best_descendant_index + if best_descendant_index notin {0..self.nodes.len-1}: return ForkChoiceError( kind: fcErrInvalidBestDescendant, @@ -224,6 +236,8 @@ func find_head*( template best_node: untyped {.dirty.} = self.nodes[best_descendant_index] # Alias, TODO: no exceptions + # debugEcho "best_node: ", best_node + # Perform a sanity check to ensure the node can be head if not self.node_is_viable_for_head(best_node): return ForkChoiceError( @@ -340,7 +354,7 @@ func maybe_update_best_child_and_descendant( ## 3. The child is not the best child but becomes the best child ## 4. The child is not the best child and does not become the best child - # debugEcho " self.maybe_update_best_child_and_descendant(parent = ", parent_index, ", child = ", child_index, ")" + # debugEcho " self.maybe_update_best_child_and_descendant(parent = ", parent_index, ", child = ", child_index, ")" if child_index notin {0..self.nodes.len-1}: return ForkChoiceError( @@ -362,6 +376,7 @@ func maybe_update_best_child_and_descendant( let (child_leads_to_viable_head, err) = self.node_leads_to_viable_head(child) if err.kind != fcSuccess: return err + # debugEcho " child_leads_to_viable_head: ", child_leads_to_viable_head let # Aliases to the 3 possible (best_child, best_descendant) tuples change_to_none = (none(Index), none(Index)) @@ -385,10 +400,12 @@ func maybe_update_best_child_and_descendant( if best_child_index == child_index and not child_leads_to_viable_head: # The child is already the best-child of the parent # but it's not viable to be the head block => remove it + # debugEcho " branch 1 - invalid" change_to_none elif best_child_index == child_index: # If the child is the best-child already, set it again to ensure # that the best-descendant of the parent is up-to-date. + # debugEcho " branch 2 - child" change_to_child else: if best_child_index notin {0..self.nodes.len-1}: @@ -404,9 +421,11 @@ func maybe_update_best_child_and_descendant( if child_leads_to_viable_head and not best_child_leads_to_viable_head: # The child leads to a viable head, but the current best-child doesn't + # debugEcho " branch 3 - child" change_to_child elif not child_leads_to_viable_head and best_child_leads_to_viable_head: # The best child leads to a viable head, but the child doesn't + # debugEcho " branch 4 - no change" no_change elif child.weight == best_child.weight: # debugEcho "Reached tiebreak" @@ -415,20 +434,26 @@ func maybe_update_best_child_and_descendant( # debugEcho " child.root.tiebreak(best_child.root): ", child.root.tiebreak(best_child.root) # Tie-breaker of equal weights by root if child.root.tiebreak(best_child.root): + # debugEcho " branch 5 - child" change_to_child else: + # debugEcho " branch 6 - no change" no_change else: # Choose winner by weight if child.weight >= best_child.weight: + # debugEcho " branch 7 - child" change_to_child else: + # debugEcho " branch 8 - no change" no_change else: if child_leads_to_viable_head: # There is no current best-child and the child is viable + # debugEcho " branch 9 - child" change_to_child else: # There is no current best-child but the child is not viable + # debugEcho " branch 10 - no change" no_change # debugEcho " new_best_child = ", new_best_child @@ -472,12 +497,17 @@ func node_is_viable_for_head(self: ProtoArray, node: ProtoNode): bool {.raises: ## ## Any node that has a different finalized or justified epoch ## should not be viable for the head. + # debugEcho " viable node.justified_epoch = ", node.justified_epoch + # debugEcho " viable self.justified_epoch = ", self.justified_epoch + # debugEcho " viable node.finalized_epoch = ", node.finalized_epoch + # debugEcho " viable self.finalized_epoch = ", self.finalized_epoch + ( (node.justified_epoch == self.justified_epoch) or - (node.justified_epoch == Epoch(0)) + (self.justified_epoch == Epoch(0)) ) and ( (node.finalized_epoch == self.finalized_epoch) or - (node.finalized_epoch == Epoch(0)) + (self.finalized_epoch == Epoch(0)) ) # Sanity checks diff --git a/tests/all_tests.nim b/tests/all_tests.nim index 396b02f776..08996dd795 100644 --- a/tests/all_tests.nim +++ b/tests/all_tests.nim @@ -29,7 +29,8 @@ import # Unit test ./test_peer_pool, ./test_sync_manager, ./test_honest_validator, - ./test_interop + ./test_interop, + ./tests_fork_choice import # Refactor state transition unit tests # TODO re-enable when useful diff --git a/tests/fork_choice/interpreter.nim b/tests/fork_choice/interpreter.nim index d6eeb67fd1..b1052c9be7 100644 --- a/tests/fork_choice/interpreter.nim +++ b/tests/fork_choice/interpreter.nim @@ -49,7 +49,6 @@ type justified_state_balances*: seq[Gwei] expected_head*: Eth2Digest of ProcessBlock: - slot*: Slot root*: Eth2Digest parent_root*: Eth2Digest blk_justified_epoch*: Epoch @@ -85,10 +84,10 @@ func apply(ctx: var ForkChoice, id: int, op: Operation) = debugEcho " Detected an expected invalid head" of ProcessBlock: let r = ctx.process_block( - slot = op.slot, + slot = default(Slot), # unused in fork choice, only helpful for external components block_root = op.root, parent_root = op.parent_root, - state_root = default(Eth2Digest), + state_root = default(Eth2Digest), # unused in fork choice, only helpful for external components justified_epoch = op.blk_justified_epoch, finalized_epoch = op.blk_finalized_epoch ) diff --git a/tests/fork_choice/scenarios/ffg_01.nim b/tests/fork_choice/scenarios/ffg_01.nim new file mode 100644 index 0000000000..59524fa05e --- /dev/null +++ b/tests/fork_choice/scenarios/ffg_01.nim @@ -0,0 +1,129 @@ +# beacon_chain +# Copyright (c) 2018 Status Research & Development GmbH +# Licensed and distributed under either of +# * MIT license (license terms in the root directory or at https://opensource.org/licenses/MIT). +# * Apache v2 license (license terms in the root directory or at https://www.apache.org/licenses/LICENSE-2.0). +# at your option. This file may not be copied, modified, or distributed except according to those terms. + +import ../interpreter + +proc setup_finality_01(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = + var balances = @[Gwei(1), Gwei(1)] + let GenesisRoot = fakeHash(0) + + # Initialize the fork choice context + result.fork_choice = initForkChoice( + finalized_block_slot = Slot(0), # Metadata unused in fork choice + finalized_block_state_root = default(Eth2Digest), # Metadata unused in fork choice + justified_epoch = Epoch(1), + finalized_epoch = Epoch(1), + finalized_root = GenesisRoot + ).get() + + # ---------------------------------- + + # Head should be genesis + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(1), + justified_root: GenesisRoot, + finalized_epoch: Epoch(1), + justified_state_balances: balances, + expected_head: GenesisRoot + ) + + # Build the following chain + # + # 0 <- just: 0, fin: 0 + # | + # 1 <- just: 0, fin: 0 + # | + # 2 <- just: 1, fin: 0 + # | + # 3 <- just: 2, fin: 1 + result.ops.add Operation( + kind: ProcessBlock, + root: fakeHash(1), + parent_root: GenesisRoot, + blk_justified_epoch: Epoch(0), + blk_finalized_epoch: Epoch(0) + ) + result.ops.add Operation( + kind: ProcessBlock, + root: fakeHash(2), + parent_root: fakeHash(1), + blk_justified_epoch: Epoch(1), + blk_finalized_epoch: Epoch(0) + ) + result.ops.add Operation( + kind: ProcessBlock, + root: fakeHash(3), + parent_root: fakeHash(2), + blk_justified_epoch: Epoch(2), + blk_finalized_epoch: Epoch(1) + ) + + # Ensure that with justified epoch 0 we find 3 + # + # 0 <- start + # | + # 1 + # | + # 2 + # | + # 3 <- head + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(0), + justified_root: GenesisRoot, + finalized_epoch: Epoch(0), + justified_state_balances: balances, + expected_head: fakeHash(3) + ) + + # Ensure that with justified epoch 1 we find 2 + # + # 0 + # | + # 1 + # | + # 2 <- start + # | + # 3 <- head + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(1), + justified_root: fakeHash(2), + finalized_epoch: Epoch(0), + justified_state_balances: balances, + expected_head: fakeHash(2) + ) + + # Ensure that with justified epoch 2 we find 3 + # + # 0 + # | + # 1 + # | + # 2 + # | + # 3 <- start + head + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(2), + justified_root: fakeHash(3), + finalized_epoch: Epoch(1), + justified_state_balances: balances, + expected_head: fakeHash(3) + ) + +proc test_ffg01() = + echo " fork_choice - testing finality #01" + # for i in 0 ..< 4: + # echo " block (", i, ") hash: ", fakeHash(i) + # echo " ------------------------------------------------------" + + var (ctx, ops) = setup_finality_01() + ctx.run(ops) + +test_ffg01() diff --git a/tests/fork_choice/scenarios/ffg_02.nim b/tests/fork_choice/scenarios/ffg_02.nim new file mode 100644 index 0000000000..d446096907 --- /dev/null +++ b/tests/fork_choice/scenarios/ffg_02.nim @@ -0,0 +1,391 @@ +# beacon_chain +# Copyright (c) 2018 Status Research & Development GmbH +# Licensed and distributed under either of +# * MIT license (license terms in the root directory or at https://opensource.org/licenses/MIT). +# * Apache v2 license (license terms in the root directory or at https://www.apache.org/licenses/LICENSE-2.0). +# at your option. This file may not be copied, modified, or distributed except according to those terms. + +import ../interpreter + +proc setup_finality_02(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = + var balances = @[Gwei(1), Gwei(1)] + let GenesisRoot = fakeHash(0) + + # Initialize the fork choice context + result.fork_choice = initForkChoice( + finalized_block_slot = Slot(0), # Metadata unused in fork choice + finalized_block_state_root = default(Eth2Digest), # Metadata unused in fork choice + justified_epoch = Epoch(1), + finalized_epoch = Epoch(1), + finalized_root = GenesisRoot + ).get() + + # ---------------------------------- + + # Head should be genesis + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(1), + justified_root: GenesisRoot, + finalized_epoch: Epoch(1), + justified_state_balances: balances, + expected_head: GenesisRoot + ) + + # Build the following tree. + # + # 0 + # / \ + # just: 0, fin: 0 -> 1 2 <- just: 0, fin: 0 + # | | + # just: 1, fin: 0 -> 3 4 <- just: 0, fin: 0 + # | | + # just: 1, fin: 0 -> 5 6 <- just: 0, fin: 0 + # | | + # just: 1, fin: 0 -> 7 8 <- just: 1, fin: 0 + # | | + # just: 2, fin: 0 -> 9 10 <- just: 2, fin: 0 + + # Left branch + result.ops.add Operation( + kind: ProcessBlock, + root: fakeHash(1), + parent_root: GenesisRoot, + blk_justified_epoch: Epoch(0), + blk_finalized_epoch: Epoch(0) + ) + result.ops.add Operation( + kind: ProcessBlock, + root: fakeHash(3), + parent_root: fakeHash(1), + blk_justified_epoch: Epoch(1), + blk_finalized_epoch: Epoch(0) + ) + result.ops.add Operation( + kind: ProcessBlock, + root: fakeHash(5), + parent_root: fakeHash(3), + blk_justified_epoch: Epoch(1), + blk_finalized_epoch: Epoch(0) + ) + result.ops.add Operation( + kind: ProcessBlock, + root: fakeHash(7), + parent_root: fakeHash(5), + blk_justified_epoch: Epoch(1), + blk_finalized_epoch: Epoch(0) + ) + result.ops.add Operation( + kind: ProcessBlock, + root: fakeHash(9), + parent_root: fakeHash(7), + blk_justified_epoch: Epoch(2), + blk_finalized_epoch: Epoch(0) + ) + + # Build the following tree. + # + # 0 + # / \ + # just: 0, fin: 0 -> 1 2 <- just: 0, fin: 0 + # | | + # just: 1, fin: 0 -> 3 4 <- just: 0, fin: 0 + # | | + # just: 1, fin: 0 -> 5 6 <- just: 0, fin: 0 + # | | + # just: 1, fin: 0 -> 7 8 <- just: 1, fin: 0 + # | | + # just: 2, fin: 0 -> 9 10 <- just: 2, fin: 0 + + # Right branch + result.ops.add Operation( + kind: ProcessBlock, + root: fakeHash(2), + parent_root: GenesisRoot, + blk_justified_epoch: Epoch(0), + blk_finalized_epoch: Epoch(0) + ) + result.ops.add Operation( + kind: ProcessBlock, + root: fakeHash(4), + parent_root: fakeHash(2), + blk_justified_epoch: Epoch(0), + blk_finalized_epoch: Epoch(0) + ) + result.ops.add Operation( + kind: ProcessBlock, + root: fakeHash(6), + parent_root: fakeHash(4), + blk_justified_epoch: Epoch(0), + blk_finalized_epoch: Epoch(0) + ) + result.ops.add Operation( + kind: ProcessBlock, + root: fakeHash(8), + parent_root: fakeHash(6), + blk_justified_epoch: Epoch(1), + blk_finalized_epoch: Epoch(0) + ) + result.ops.add Operation( + kind: ProcessBlock, + root: fakeHash(10), + parent_root: fakeHash(8), + blk_justified_epoch: Epoch(2), + blk_finalized_epoch: Epoch(0) + ) + + # Ensure that if we start at 0 we find 10 (just: 0, fin: 0). + # + # 0 <-- start + # / \ + # 1 2 + # | | + # 3 4 + # | | + # 5 6 + # | | + # 7 8 + # | | + # 9 10 <-- head + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(0), + justified_root: GenesisRoot, + finalized_epoch: Epoch(0), + justified_state_balances: balances, + expected_head: fakeHash(10) + ) + + # Same with justified_epoch 2 + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(2), + justified_root: GenesisRoot, + finalized_epoch: Epoch(0), + justified_state_balances: balances, + expected_head: fakeHash(10) + ) + + # Justified epoch 3 is invalid + result.ops.add Operation( + kind: InvalidFindHead, + justified_epoch: Epoch(3), # <--- Wrong epoch + justified_root: GenesisRoot, + finalized_epoch: Epoch(0), + justified_state_balances: balances + ) + + # Add a vote to 1. + # + # 0 + # / \ + # +1 vote -> 1 2 + # | | + # 3 4 + # | | + # 5 6 + # | | + # 7 8 + # | | + # 9 10 + result.ops.add Operation( + kind: ProcessAttestation, + validator_index: ValidatorIndex(0), + block_root: fake_hash(1), + target_epoch: Epoch(0) + ) + + # Ensure that if we start at 0 we find 9 (just: 0, fin: 0). + # + # 0 <-- start + # / \ + # 1 2 + # | | + # 3 4 + # | | + # 5 6 + # | | + # 7 8 + # | | + # head -> 9 10 + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(0), + justified_root: GenesisRoot, + finalized_epoch: Epoch(0), + justified_state_balances: balances, + expected_head: fakeHash(9) + ) + + # Same with justified_epoch 2 + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(2), + justified_root: GenesisRoot, + finalized_epoch: Epoch(0), + justified_state_balances: balances, + expected_head: fakeHash(9) + ) + + # Justified epoch 3 is invalid + result.ops.add Operation( + kind: InvalidFindHead, + justified_epoch: Epoch(3), # <--- Wrong epoch + justified_root: GenesisRoot, + finalized_epoch: Epoch(0), + justified_state_balances: balances + ) + + # Add a vote to 2. + # + # 0 + # / \ + # 1 2 <- +1 vote + # | | + # 3 4 + # | | + # 5 6 + # | | + # 7 8 + # | | + # 9 10 + result.ops.add Operation( + kind: ProcessAttestation, + validator_index: ValidatorIndex(1), + block_root: fake_hash(2), + target_epoch: Epoch(0) + ) + + # Ensure that if we start at 0 we find 10 again (just: 0, fin: 0). + # + # 0 <-- start + # / \ + # 1 2 + # | | + # 3 4 + # | | + # 5 6 + # | | + # 7 8 + # | | + # 9 10 <-- head + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(0), + justified_root: GenesisRoot, + finalized_epoch: Epoch(0), + justified_state_balances: balances, + expected_head: fakeHash(10) + ) + + # Same with justified_epoch 2 + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(2), + justified_root: GenesisRoot, + finalized_epoch: Epoch(0), + justified_state_balances: balances, + expected_head: fakeHash(10) + ) + + # Justified epoch 3 is invalid + result.ops.add Operation( + kind: InvalidFindHead, + justified_epoch: Epoch(3), # <--- Wrong epoch + justified_root: GenesisRoot, + finalized_epoch: Epoch(0), + justified_state_balances: balances + ) + + # Ensure that if we start at 1 (instead of 0) we find 9 (just: 0, fin: 0). + # + # 0 + # / \ + # start-> 1 2 + # | | + # 3 4 + # | | + # 5 6 + # | | + # 7 8 + # | | + # head -> 9 10 + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(0), + justified_root: fakeHash(1), + finalized_epoch: Epoch(0), + justified_state_balances: balances, + expected_head: fakeHash(9) + ) + + # Same with justified_epoch 2 + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(2), + justified_root: fakeHash(1), + finalized_epoch: Epoch(0), + justified_state_balances: balances, + expected_head: fakeHash(9) + ) + + # Justified epoch 3 is invalid + result.ops.add Operation( + kind: InvalidFindHead, + justified_epoch: Epoch(3), # <--- Wrong epoch + justified_root: fakeHash(1), + finalized_epoch: Epoch(0), + justified_state_balances: balances + ) + + # Ensure that if we start at 2 (instead of 0) we find 10 (just: 0, fin: 0). + # + # 0 + # / \ + # 1 2 <- start + # | | + # 3 4 + # | | + # 5 6 + # | | + # 7 8 + # | | + # 9 10 <- head + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(0), + justified_root: fakeHash(2), + finalized_epoch: Epoch(0), + justified_state_balances: balances, + expected_head: fakeHash(10) + ) + + # Same with justified_epoch 2 + result.ops.add Operation( + kind: FindHead, + justified_epoch: Epoch(2), + justified_root: fakeHash(2), + finalized_epoch: Epoch(0), + justified_state_balances: balances, + expected_head: fakeHash(10) + ) + + # Justified epoch 3 is invalid + result.ops.add Operation( + kind: InvalidFindHead, + justified_epoch: Epoch(3), # <--- Wrong epoch + justified_root: fakeHash(2), + finalized_epoch: Epoch(0), + justified_state_balances: balances + ) + +proc test_ffg02() = + echo " fork_choice - testing finality #02" + # for i in 0 ..< 12: + # echo " block (", i, ") hash: ", fakeHash(i) + # echo " ------------------------------------------------------" + + var (ctx, ops) = setup_finality_02() + ctx.run(ops) + +test_ffg02() diff --git a/tests/fork_choice/scenarios/no_votes.nim b/tests/fork_choice/scenarios/no_votes.nim index a91ee9a0e4..cfd9d441d6 100644 --- a/tests/fork_choice/scenarios/no_votes.nim +++ b/tests/fork_choice/scenarios/no_votes.nim @@ -13,8 +13,8 @@ proc setup_no_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = # Initialize the fork choice context result.fork_choice = initForkChoice( - finalized_block_slot = Slot(0), - finalized_block_state_root = default(Eth2Digest), + finalized_block_slot = Slot(0), # Metadata unused in fork choice + finalized_block_state_root = default(Eth2Digest), # Metadata unused in fork choice justified_epoch = Epoch(1), finalized_epoch = Epoch(1), finalized_root = GenesisRoot @@ -39,7 +39,6 @@ proc setup_no_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = # 2 result.ops.add Operation( kind: ProcessBlock, - slot: Slot(0), root: fakeHash(2), parent_root: GenesisRoot, blk_justified_epoch: Epoch(1), @@ -67,7 +66,6 @@ proc setup_no_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = # 2 1 result.ops.add Operation( kind: ProcessBlock, - slot: Slot(0), root: fakeHash(1), parent_root: GenesisRoot, blk_justified_epoch: Epoch(1), @@ -97,7 +95,6 @@ proc setup_no_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = # 3 result.ops.add Operation( kind: ProcessBlock, - slot: Slot(0), root: fakeHash(3), parent_root: fakeHash(1), blk_justified_epoch: Epoch(1), @@ -129,7 +126,6 @@ proc setup_no_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = # 4 3 result.ops.add Operation( kind: ProcessBlock, - slot: Slot(0), root: fakeHash(4), parent_root: fakeHash(2), blk_justified_epoch: Epoch(1), @@ -163,7 +159,6 @@ proc setup_no_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = # 5 <- justified epoch = 2 result.ops.add Operation( kind: ProcessBlock, - slot: Slot(0), root: fakeHash(5), parent_root: fakeHash(4), blk_justified_epoch: Epoch(2), @@ -234,7 +229,6 @@ proc setup_no_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = # 6 result.ops.add Operation( kind: ProcessBlock, - slot: Slot(0), root: fakeHash(6), parent_root: fakeHash(5), blk_justified_epoch: Epoch(2), diff --git a/tests/fork_choice/scenarios/votes.nim b/tests/fork_choice/scenarios/votes.nim index 7192eeecbe..d551e61e26 100644 --- a/tests/fork_choice/scenarios/votes.nim +++ b/tests/fork_choice/scenarios/votes.nim @@ -13,8 +13,8 @@ proc setup_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = # Initialize the fork choice context result.fork_choice = initForkChoice( - finalized_block_slot = Slot(0), - finalized_block_state_root = default(Eth2Digest), + finalized_block_slot = Slot(0), # Metadata unused in fork choice + finalized_block_state_root = default(Eth2Digest), # Metadata unused in fork choice justified_epoch = Epoch(1), finalized_epoch = Epoch(1), finalized_root = GenesisRoot @@ -39,7 +39,6 @@ proc setup_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = # 2 result.ops.add Operation( kind: ProcessBlock, - slot: Slot(0), root: fakeHash(2), parent_root: GenesisRoot, blk_justified_epoch: Epoch(1), @@ -67,7 +66,6 @@ proc setup_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = # 2 1 result.ops.add Operation( kind: ProcessBlock, - slot: Slot(0), root: fakeHash(1), parent_root: GenesisRoot, blk_justified_epoch: Epoch(1), @@ -149,7 +147,6 @@ proc setup_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = # 3 result.ops.add Operation( kind: ProcessBlock, - slot: Slot(0), root: fakeHash(3), parent_root: fakeHash(1), blk_justified_epoch: Epoch(1), @@ -240,7 +237,6 @@ proc setup_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = # 4 result.ops.add Operation( kind: ProcessBlock, - slot: Slot(0), root: fakeHash(4), parent_root: fakeHash(3), blk_justified_epoch: Epoch(1), @@ -278,7 +274,6 @@ proc setup_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = # 5 <- justified epoch = 2 result.ops.add Operation( kind: ProcessBlock, - slot: Slot(0), root: fakeHash(5), parent_root: fakeHash(4), blk_justified_epoch: Epoch(2), @@ -318,7 +313,6 @@ proc setup_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = # 5 6 <- justified epoch = 0 result.ops.add Operation( kind: ProcessBlock, - slot: Slot(0), root: fakeHash(6), parent_root: fakeHash(4), blk_justified_epoch: Epoch(1), @@ -369,7 +363,6 @@ proc setup_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = # 9 result.ops.add Operation( kind: ProcessBlock, - slot: Slot(0), root: fakeHash(7), parent_root: fakeHash(5), blk_justified_epoch: Epoch(2), @@ -377,7 +370,6 @@ proc setup_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = ) result.ops.add Operation( kind: ProcessBlock, - slot: Slot(0), root: fakeHash(8), parent_root: fakeHash(7), blk_justified_epoch: Epoch(2), @@ -385,7 +377,6 @@ proc setup_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = ) result.ops.add Operation( kind: ProcessBlock, - slot: Slot(0), root: fakeHash(9), parent_root: fakeHash(8), blk_justified_epoch: Epoch(2), @@ -505,7 +496,6 @@ proc setup_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = # 9 10 result.ops.add Operation( kind: ProcessBlock, - slot: Slot(0), root: fakeHash(10), parent_root: fakeHash(8), blk_justified_epoch: Epoch(2), @@ -700,7 +690,6 @@ proc setup_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = # 11 result.ops.add Operation( kind: ProcessBlock, - slot: Slot(0), root: fakeHash(11), parent_root: fakeHash(9), blk_justified_epoch: Epoch(2), @@ -719,9 +708,9 @@ proc setup_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = proc test_votes() = echo " fork_choice - testing with votes" - for i in 0 ..< 11: - echo " block (", i, ") hash: ", fakeHash(i) - echo " ------------------------------------------------------" + # for i in 0 ..< 12: + # echo " block (", i, ") hash: ", fakeHash(i) + # echo " ------------------------------------------------------" var (ctx, ops) = setup_votes() ctx.run(ops) diff --git a/tests/fork_choice/tests_fork_choice.nim b/tests/fork_choice/tests_fork_choice.nim new file mode 100644 index 0000000000..83f221b9dc --- /dev/null +++ b/tests/fork_choice/tests_fork_choice.nim @@ -0,0 +1,6 @@ +# Don't forgot to run the following files as main modules: +# - beacon_chain/fork_choice/proto_array.nim (sanity checks for tiebreak) +# - beacon_chain/fork_choice/fork_choice.nim (sanity checks for compute_deltas) + +import + scenarios/[no_votes, votes, ffg_01, ffg_02] From 30bcbb1b60ec5fddd582b1dfc84f72806aff6657 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mamy=20Andr=C3=A9-Ratsimbazafy?= Date: Tue, 7 Apr 2020 00:04:50 +0200 Subject: [PATCH 11/22] Not sure why lmdb / rocksdb reappeared in rebase --- vendor/lmdb | 1 - vendor/nim-rocksdb | 1 - 2 files changed, 2 deletions(-) delete mode 160000 vendor/lmdb delete mode 160000 vendor/nim-rocksdb diff --git a/vendor/lmdb b/vendor/lmdb deleted file mode 160000 index c8ecc17b38..0000000000 --- a/vendor/lmdb +++ /dev/null @@ -1 +0,0 @@ -Subproject commit c8ecc17b38e164e6a728d66a9b1d05bc18dd3ace diff --git a/vendor/nim-rocksdb b/vendor/nim-rocksdb deleted file mode 160000 index 08fec021c0..0000000000 --- a/vendor/nim-rocksdb +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 08fec021c0f28f63d1221d40a655078b5b923d1b From 4947db0ae1e46548de44ef1a5aa2668c40733d07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mamy=20Andr=C3=A9-Ratsimbazafy?= Date: Tue, 7 Apr 2020 00:56:03 +0200 Subject: [PATCH 12/22] Add sanity checks to .nimble file + integrate fork choice tests to the test DB and test timing --- AllTests-mainnet.md | 10 +++++++++- AllTests-minimal.md | 10 +++++++++- beacon_chain.nimble | 5 ++++- beacon_chain/fork_choice/fork_choice.nim | 2 +- beacon_chain/fork_choice/proto_array.nim | 6 ++---- tests/all_tests.nim | 2 +- tests/fork_choice/scenarios/ffg_01.nim | 14 +++++++------- tests/fork_choice/scenarios/ffg_02.nim | 14 +++++++------- tests/fork_choice/scenarios/no_votes.nim | 14 +++++++------- tests/fork_choice/scenarios/votes.nim | 14 +++++++------- tests/fork_choice/tests_fork_choice.nim | 8 ++++++-- 11 files changed, 60 insertions(+), 39 deletions(-) diff --git a/AllTests-mainnet.md b/AllTests-mainnet.md index 360b04b233..ba4708a5e7 100644 --- a/AllTests-mainnet.md +++ b/AllTests-mainnet.md @@ -46,6 +46,14 @@ OK: 2/2 Fail: 0/2 Skip: 0/2 + Multiaddress to ENode OK ``` OK: 2/2 Fail: 0/2 Skip: 0/2 +## Fork Choice + Finality [Preset: mainnet] +```diff ++ fork_choice - testing finality #01 OK ++ fork_choice - testing finality #02 OK ++ fork_choice - testing no votes OK ++ fork_choice - testing with votes OK +``` +OK: 4/4 Fail: 0/4 Skip: 0/4 ## Honest validator ```diff + Attestation topics OK @@ -234,4 +242,4 @@ OK: 4/4 Fail: 0/4 Skip: 0/4 OK: 8/8 Fail: 0/8 Skip: 0/8 ---TOTAL--- -OK: 145/148 Fail: 3/148 Skip: 0/148 +OK: 149/152 Fail: 3/152 Skip: 0/152 diff --git a/AllTests-minimal.md b/AllTests-minimal.md index 8eb1599217..77498f7cfe 100644 --- a/AllTests-minimal.md +++ b/AllTests-minimal.md @@ -73,6 +73,14 @@ OK: 2/2 Fail: 0/2 Skip: 0/2 + Multiaddress to ENode OK ``` OK: 2/2 Fail: 0/2 Skip: 0/2 +## Fork Choice + Finality [Preset: minimal] +```diff ++ fork_choice - testing finality #01 OK ++ fork_choice - testing finality #02 OK ++ fork_choice - testing no votes OK ++ fork_choice - testing with votes OK +``` +OK: 4/4 Fail: 0/4 Skip: 0/4 ## Honest validator ```diff + Attestation topics OK @@ -261,4 +269,4 @@ OK: 4/4 Fail: 0/4 Skip: 0/4 OK: 8/8 Fail: 0/8 Skip: 0/8 ---TOTAL--- -OK: 160/163 Fail: 3/163 Skip: 0/163 +OK: 164/167 Fail: 3/167 Skip: 0/167 diff --git a/beacon_chain.nimble b/beacon_chain.nimble index af3435134a..fd545c5116 100644 --- a/beacon_chain.nimble +++ b/beacon_chain.nimble @@ -52,8 +52,12 @@ task test, "Run all tests": # price we pay for that. # Minimal config + buildBinary "proto_array", "beacon_chain/fork_choice/", "-d:const_preset=minimal" + buildBinary "fork_choice", "beacon_chain/fork_choice/", "-d:const_preset=minimal" buildBinary "all_tests", "tests/", "-d:chronicles_log_level=TRACE -d:const_preset=minimal" # Mainnet config + buildBinary "proto_array", "beacon_chain/fork_choice/", "-d:const_preset=mainnet" + buildBinary "fork_choice", "beacon_chain/fork_choice/", "-d:const_preset=mainnet" buildBinary "all_tests", "tests/", "-d:const_preset=mainnet" # Generic SSZ test, doesn't use consensus objects minimal/mainnet presets @@ -69,4 +73,3 @@ task test, "Run all tests": # State sim; getting into 4th epoch useful to trigger consensus checks buildBinary "state_sim", "research/", "", "--validators=1024 --slots=32" buildBinary "state_sim", "research/", "-d:const_preset=mainnet", "--validators=1024 --slots=128" - diff --git a/beacon_chain/fork_choice/fork_choice.nim b/beacon_chain/fork_choice/fork_choice.nim index 14f74cb54e..c8a4e78ab5 100644 --- a/beacon_chain/fork_choice/fork_choice.nim +++ b/beacon_chain/fork_choice/fork_choice.nim @@ -197,7 +197,7 @@ func compute_deltas( ## - If a `Eth2Digest` in `votes` does not exist in `indices` ## except for the `default(Eth2Digest)` (i.e. zero hash) - # TODO: Displaying the votes for debugging will cause the tests to fail!!! + # TODO: Displaying the votes for debugging will cause the tests in that very file to fail!!! # if we do the precise sequence # - tAll_voted_the_same() # - tDifferent_votes() diff --git a/beacon_chain/fork_choice/proto_array.nim b/beacon_chain/fork_choice/proto_array.nim index 0358558ee3..53fecd919c 100644 --- a/beacon_chain/fork_choice/proto_array.nim +++ b/beacon_chain/fork_choice/proto_array.nim @@ -515,9 +515,10 @@ func node_is_viable_for_head(self: ProtoArray, node: ProtoNode): bool {.raises: # Sanity checks on internal private procedures when isMainModule: - import nimcrypto/[hash, utils] + echo "Sanity checks on fork choice tiebreaks" + block: let a = Eth2Digest.fromHex("0x0000000000000001000000000000000000000000000000000000000000000000") let b = Eth2Digest.fromHex("0x0000000000000000000000000000000000000000000000000000000000000000") # sha256(1) @@ -536,7 +537,4 @@ when isMainModule: let a = Eth2Digest.fromHex("0xD86E8112F3C4C4442126F8E9F44F16867DA487F29052BF91B810457DB34209A4") # sha256(2) let b = Eth2Digest.fromHex("0x7C9FA136D4413FA6173637E883B6998D32E1D675F88CDDFF9DCBCF331820F4B8") # sha256(1) - echo a.data - echo b.data - doAssert tiebreak(a, b) diff --git a/tests/all_tests.nim b/tests/all_tests.nim index 08996dd795..7bc1442b3b 100644 --- a/tests/all_tests.nim +++ b/tests/all_tests.nim @@ -30,7 +30,7 @@ import # Unit test ./test_sync_manager, ./test_honest_validator, ./test_interop, - ./tests_fork_choice + ./fork_choice/tests_fork_choice import # Refactor state transition unit tests # TODO re-enable when useful diff --git a/tests/fork_choice/scenarios/ffg_01.nim b/tests/fork_choice/scenarios/ffg_01.nim index 59524fa05e..1ad98dd666 100644 --- a/tests/fork_choice/scenarios/ffg_01.nim +++ b/tests/fork_choice/scenarios/ffg_01.nim @@ -5,7 +5,7 @@ # * Apache v2 license (license terms in the root directory or at https://www.apache.org/licenses/LICENSE-2.0). # at your option. This file may not be copied, modified, or distributed except according to those terms. -import ../interpreter +# import ../interpreter # included to be able to use "suiteReport" proc setup_finality_01(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = var balances = @[Gwei(1), Gwei(1)] @@ -118,12 +118,12 @@ proc setup_finality_01(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = ) proc test_ffg01() = - echo " fork_choice - testing finality #01" - # for i in 0 ..< 4: - # echo " block (", i, ") hash: ", fakeHash(i) - # echo " ------------------------------------------------------" + timedTest "fork_choice - testing finality #01": + # for i in 0 ..< 4: + # echo " block (", i, ") hash: ", fakeHash(i) + # echo " ------------------------------------------------------" - var (ctx, ops) = setup_finality_01() - ctx.run(ops) + var (ctx, ops) = setup_finality_01() + ctx.run(ops) test_ffg01() diff --git a/tests/fork_choice/scenarios/ffg_02.nim b/tests/fork_choice/scenarios/ffg_02.nim index d446096907..7d77e7dbf0 100644 --- a/tests/fork_choice/scenarios/ffg_02.nim +++ b/tests/fork_choice/scenarios/ffg_02.nim @@ -5,7 +5,7 @@ # * Apache v2 license (license terms in the root directory or at https://www.apache.org/licenses/LICENSE-2.0). # at your option. This file may not be copied, modified, or distributed except according to those terms. -import ../interpreter +# import ../interpreter # included to be able to use "suiteReport" proc setup_finality_02(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = var balances = @[Gwei(1), Gwei(1)] @@ -380,12 +380,12 @@ proc setup_finality_02(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = ) proc test_ffg02() = - echo " fork_choice - testing finality #02" - # for i in 0 ..< 12: - # echo " block (", i, ") hash: ", fakeHash(i) - # echo " ------------------------------------------------------" + timedTest "fork_choice - testing finality #02": + # for i in 0 ..< 12: + # echo " block (", i, ") hash: ", fakeHash(i) + # echo " ------------------------------------------------------" - var (ctx, ops) = setup_finality_02() - ctx.run(ops) + var (ctx, ops) = setup_finality_02() + ctx.run(ops) test_ffg02() diff --git a/tests/fork_choice/scenarios/no_votes.nim b/tests/fork_choice/scenarios/no_votes.nim index cfd9d441d6..4ad23de7d0 100644 --- a/tests/fork_choice/scenarios/no_votes.nim +++ b/tests/fork_choice/scenarios/no_votes.nim @@ -5,7 +5,7 @@ # * Apache v2 license (license terms in the root directory or at https://www.apache.org/licenses/LICENSE-2.0). # at your option. This file may not be copied, modified, or distributed except according to those terms. -import ../interpreter +# import ../interpreter # included to be able to use "suiteReport" proc setup_no_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = let balances = newSeq[Gwei](16) @@ -255,12 +255,12 @@ proc setup_no_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = ) proc test_no_votes() = - echo " fork_choice - testing no votes" - # for i in 0 ..< 6: - # echo " block (", i, ") hash: ", fakeHash(i) - # echo " ------------------------------------------------------" + timedTest "fork_choice - testing no votes": + # for i in 0 ..< 6: + # echo " block (", i, ") hash: ", fakeHash(i) + # echo " ------------------------------------------------------" - var (ctx, ops) = setup_no_votes() - ctx.run(ops) + var (ctx, ops) = setup_no_votes() + ctx.run(ops) test_no_votes() diff --git a/tests/fork_choice/scenarios/votes.nim b/tests/fork_choice/scenarios/votes.nim index d551e61e26..176e7794a9 100644 --- a/tests/fork_choice/scenarios/votes.nim +++ b/tests/fork_choice/scenarios/votes.nim @@ -5,7 +5,7 @@ # * Apache v2 license (license terms in the root directory or at https://www.apache.org/licenses/LICENSE-2.0). # at your option. This file may not be copied, modified, or distributed except according to those terms. -import ../interpreter +# import ../interpreter # included to be able to use "suiteReport" proc setup_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = var balances = @[Gwei(1), Gwei(1)] @@ -707,12 +707,12 @@ proc setup_votes(): tuple[fork_choice: ForkChoice, ops: seq[Operation]] = ) proc test_votes() = - echo " fork_choice - testing with votes" - # for i in 0 ..< 12: - # echo " block (", i, ") hash: ", fakeHash(i) - # echo " ------------------------------------------------------" + timedTest "fork_choice - testing with votes": + # for i in 0 ..< 12: + # echo " block (", i, ") hash: ", fakeHash(i) + # echo " ------------------------------------------------------" - var (ctx, ops) = setup_votes() - ctx.run(ops) + var (ctx, ops) = setup_votes() + ctx.run(ops) test_votes() diff --git a/tests/fork_choice/tests_fork_choice.nim b/tests/fork_choice/tests_fork_choice.nim index 83f221b9dc..e9b15a41e1 100644 --- a/tests/fork_choice/tests_fork_choice.nim +++ b/tests/fork_choice/tests_fork_choice.nim @@ -2,5 +2,9 @@ # - beacon_chain/fork_choice/proto_array.nim (sanity checks for tiebreak) # - beacon_chain/fork_choice/fork_choice.nim (sanity checks for compute_deltas) -import - scenarios/[no_votes, votes, ffg_01, ffg_02] +import ../testutil, std/unittest + +# include to be able to use "suiteReport" +import ./interpreter +suiteReport "Fork Choice + Finality " & preset(): + include scenarios/[no_votes, votes, ffg_01, ffg_02] From 17d9ed3e595ae18b0274e8ef769a5972aa883ae9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mamy=20Andr=C3=A9-Ratsimbazafy?= Date: Tue, 7 Apr 2020 12:39:51 +0200 Subject: [PATCH 13/22] Cleanup debugging echos --- beacon_chain/fork_choice/fork_choice.nim | 26 ++------ .../fork_choice/fork_choice_types.nim | 7 +- beacon_chain/fork_choice/proto_array.nim | 64 +++---------------- 3 files changed, 22 insertions(+), 75 deletions(-) diff --git a/beacon_chain/fork_choice/fork_choice.nim b/beacon_chain/fork_choice/fork_choice.nim index c8a4e78ab5..290f469dcf 100644 --- a/beacon_chain/fork_choice/fork_choice.nim +++ b/beacon_chain/fork_choice/fork_choice.nim @@ -15,11 +15,13 @@ import # Fork choice ./fork_choice_types, ./proto_array -# https://github.com/ethereum/eth2.0-specs/blob/v0.10.1/specs/phase0/fork-choice.md +# https://github.com/ethereum/eth2.0-specs/blob/v0.11.1/specs/phase0/fork-choice.md # This is a port of https://github.com/sigp/lighthouse/pull/804 # which is a port of "Proto-Array": https://github.com/protolambda/lmd-ghost -# -# See also the port of the port: https://github.com/protolambda/eth2-py-hacks/blob/ec9395d371903d855e1488d04b8fe89fd5be0ad9/proto_array.py +# See also: +# - Protolambda port of Lighthouse: https://github.com/protolambda/eth2-py-hacks/blob/ae286567/proto_array.py +# - Prysmatic writeup: https://hackmd.io/bABJiht3Q9SyV3Ga4FT9lQ#High-level-concept +# - Gasper Whitepaper: https://arxiv.org/abs/2003.03052 const DefaultPruneThreshold = 256 @@ -104,6 +106,7 @@ func process_attestation*( result.ok() + func process_block*( self: var ForkChoice, slot: Slot, @@ -131,8 +134,6 @@ func find_head*( justified_state_balances: seq[Gwei] ): Result[Eth2Digest, string] {.raises: [UnpackError, KeyError].} = ## Returns the new blockchain head - # debugEcho "proto_array: ", self.proto_array - # debugEcho "-----------------------\n" # Compute deltas with previous call # we might want to reuse the `deltas` buffer across calls @@ -147,8 +148,6 @@ func find_head*( result.err("find_head compute_deltas failed: " & $delta_err) return - # debugEcho "find_head deltas: ", deltas - # Apply score changes let score_err = self.proto_array.apply_score_changes( deltas, justified_epoch, finalized_epoch @@ -157,8 +156,6 @@ func find_head*( result.err("find_head apply_score_changes failed: " & $score_err) self.balances = justified_state_balances - # debugEcho "proto_array: ", self.proto_array - # debugEcho "-----------------------\n" # Find the best block var new_head{.noInit.}: Eth2Digest @@ -179,6 +176,7 @@ func maybe_prune*( result.err("find_head maybe_pruned failed: " & $err) result.ok() + func compute_deltas( deltas: var openarray[Delta], indices: Table[Eth2Digest, Index], @@ -197,16 +195,6 @@ func compute_deltas( ## - If a `Eth2Digest` in `votes` does not exist in `indices` ## except for the `default(Eth2Digest)` (i.e. zero hash) - # TODO: Displaying the votes for debugging will cause the tests in that very file to fail!!! - # if we do the precise sequence - # - tAll_voted_the_same() - # - tDifferent_votes() - # - tMoving_votes() - # - tChanging_balances() - # var openarray bug? - # - # debugEcho "compute_deltas votes: ", votes - for val_index, vote in votes.mpairs(): # No need to create a score change if the validator has never voted # or if votes are for the zero hash (alias to the genesis block) diff --git a/beacon_chain/fork_choice/fork_choice_types.nim b/beacon_chain/fork_choice/fork_choice_types.nim index db5dd4afc9..437536312c 100644 --- a/beacon_chain/fork_choice/fork_choice_types.nim +++ b/beacon_chain/fork_choice/fork_choice_types.nim @@ -12,9 +12,13 @@ import # Internal ../spec/[datatypes, digest] -# https://github.com/ethereum/eth2.0-specs/blob/v0.10.1/specs/phase0/fork-choice.md +# https://github.com/ethereum/eth2.0-specs/blob/v0.11.1/specs/phase0/fork-choice.md # This is a port of https://github.com/sigp/lighthouse/pull/804 # which is a port of "Proto-Array": https://github.com/protolambda/lmd-ghost +# See also: +# - Protolambda port of Lighthouse: https://github.com/protolambda/eth2-py-hacks/blob/ae286567/proto_array.py +# - Prysmatic writeup: https://hackmd.io/bABJiht3Q9SyV3Ga4FT9lQ#High-level-concept +# - Gasper Whitepaper: https://arxiv.org/abs/2003.03052 # ProtoArray low-level types # ---------------------------------------------------------------------- @@ -94,6 +98,7 @@ type # TODO: generic "Metadata" field for slot/state_root slot*: Slot # This is unnecessary for fork choice but helps external components state_root*: Eth2Digest # This is unnecessary for fork choice but helps external components + # Fields used in fork choice root*: Eth2Digest parent*: Option[Index] justified_epoch*: Epoch diff --git a/beacon_chain/fork_choice/proto_array.nim b/beacon_chain/fork_choice/proto_array.nim index 53fecd919c..d5aba5b4d4 100644 --- a/beacon_chain/fork_choice/proto_array.nim +++ b/beacon_chain/fork_choice/proto_array.nim @@ -13,11 +13,13 @@ import # Fork choice ./fork_choice_types -# https://github.com/ethereum/eth2.0-specs/blob/v0.10.1/specs/phase0/fork-choice.md +# https://github.com/ethereum/eth2.0-specs/blob/v0.11.1/specs/phase0/fork-choice.md # This is a port of https://github.com/sigp/lighthouse/pull/804 # which is a port of "Proto-Array": https://github.com/protolambda/lmd-ghost -# -# See also the port of the port: https://github.com/protolambda/eth2-py-hacks/blob/ec9395d371903d855e1488d04b8fe89fd5be0ad9/proto_array.py +# See also: +# - Protolambda port of Lighthouse: https://github.com/protolambda/eth2-py-hacks/blob/ae286567/proto_array.py +# - Prysmatic writeup: https://hackmd.io/bABJiht3Q9SyV3Ga4FT9lQ#High-level-concept +# - Gasper Whitepaper: https://arxiv.org/abs/2003.03052 # Helper # ---------------------------------------------------------------------- @@ -65,8 +67,6 @@ func apply_score_changes*( ## 4. If required, update the parent's best-descendant with the current node or its best-descendant # TODO: remove spurious raised exceptions - # debugEcho "apply_score_change" - if deltas.len != self.indices.len: return ForkChoiceError( kind: fcErrInvalidDeltaLen, @@ -108,13 +108,6 @@ func apply_score_changes*( ) node.weight = weight - # var report: string - # # report.add "deltas[" & $node_index & "] = " & $deltas[node_index] - # report.add "node.weight = " & $node.weight - # report.add ", node root 0x" & $node.root - - # debugecho " ", report - # If the node has a parent, try to update its best-child and best-descendant if node.parent.isSome(): # TODO: Nim `options` module could use some {.inline.} @@ -199,16 +192,12 @@ func find_head*( ## is not followed by `apply_score_changes` as `on_new_block` does not ## update the whole tree. - # debugEcho "proto_array: ", self - # debugEcho "-------------------------------------------------------------\n" - if justified_root notin self.indices: return ForkChoiceError( kind: fcErrJustifiedNodeUnknown, block_root: justified_root ) let justified_index = self.indices[justified_root] # TODO: this can't throw KeyError - # debugEcho "justified_index: ", justified_index if justified_index notin {0..self.nodes.len-1}: return ForkChoiceError( @@ -218,16 +207,12 @@ func find_head*( template justified_node: untyped {.dirty.} = self.nodes[justified_index] # Alias, TODO: no exceptions - # debugEcho "justified_node: ", justified_node - let best_descendant_index = block: if justified_node.best_descendant.isSome(): justified_node.best_descendant.unsafeGet() else: justified_index - # debugEcho "best_descendant_index: ", best_descendant_index - if best_descendant_index notin {0..self.nodes.len-1}: return ForkChoiceError( kind: fcErrInvalidBestDescendant, @@ -236,8 +221,6 @@ func find_head*( template best_node: untyped {.dirty.} = self.nodes[best_descendant_index] # Alias, TODO: no exceptions - # debugEcho "best_node: ", best_node - # Perform a sanity check to ensure the node can be head if not self.node_is_viable_for_head(best_node): return ForkChoiceError( @@ -253,7 +236,10 @@ func find_head*( head = best_node.root return ForkChoiceSuccess - +# TODO: pruning can be made cheaper by keeping the new offset as a field +# in proto_array instead of scanning the table to substract the offset. +# In that case pruning can always be done and does not need a threshold for efficiency. +# https://github.com/protolambda/eth2-py-hacks/blob/ae286567/proto_array.py func maybe_prune*( self: var ProtoArray, finalized_root: Eth2Digest @@ -354,8 +340,6 @@ func maybe_update_best_child_and_descendant( ## 3. The child is not the best child but becomes the best child ## 4. The child is not the best child and does not become the best child - # debugEcho " self.maybe_update_best_child_and_descendant(parent = ", parent_index, ", child = ", child_index, ")" - if child_index notin {0..self.nodes.len-1}: return ForkChoiceError( kind: fcErrInvalidNodeIndex, @@ -371,12 +355,9 @@ func maybe_update_best_child_and_descendant( template child: untyped {.dirty.} = self.nodes[child_index] template parent: untyped {.dirty.} = self.nodes[parent_index] - # debugEcho " child: ", child - let (child_leads_to_viable_head, err) = self.node_leads_to_viable_head(child) if err.kind != fcSuccess: return err - # debugEcho " child_leads_to_viable_head: ", child_leads_to_viable_head let # Aliases to the 3 possible (best_child, best_descendant) tuples change_to_none = (none(Index), none(Index)) @@ -388,24 +369,17 @@ func maybe_update_best_child_and_descendant( ) no_change = (parent.best_child, parent.best_descendant) - # debugEcho " change_to_none = ", change_to_none - # debugEcho " change_to_child = ", change_to_child - # debugEcho " no_change = ", no_change - # TODO: state-machine? The control-flow is messy - let (new_best_child, new_best_descendant) = block: if parent.best_child.isSome: let best_child_index = parent.best_child.unsafeGet() if best_child_index == child_index and not child_leads_to_viable_head: # The child is already the best-child of the parent # but it's not viable to be the head block => remove it - # debugEcho " branch 1 - invalid" change_to_none elif best_child_index == child_index: # If the child is the best-child already, set it again to ensure # that the best-descendant of the parent is up-to-date. - # debugEcho " branch 2 - child" change_to_child else: if best_child_index notin {0..self.nodes.len-1}: @@ -421,44 +395,29 @@ func maybe_update_best_child_and_descendant( if child_leads_to_viable_head and not best_child_leads_to_viable_head: # The child leads to a viable head, but the current best-child doesn't - # debugEcho " branch 3 - child" change_to_child elif not child_leads_to_viable_head and best_child_leads_to_viable_head: # The best child leads to a viable head, but the child doesn't - # debugEcho " branch 4 - no change" no_change elif child.weight == best_child.weight: - # debugEcho "Reached tiebreak" - # debugEcho " child.root 0x", child.root - # debugEcho " best_child.root 0x", best_child.root - # debugEcho " child.root.tiebreak(best_child.root): ", child.root.tiebreak(best_child.root) # Tie-breaker of equal weights by root if child.root.tiebreak(best_child.root): - # debugEcho " branch 5 - child" change_to_child else: - # debugEcho " branch 6 - no change" no_change else: # Choose winner by weight if child.weight >= best_child.weight: - # debugEcho " branch 7 - child" change_to_child else: - # debugEcho " branch 8 - no change" no_change else: if child_leads_to_viable_head: # There is no current best-child and the child is viable - # debugEcho " branch 9 - child" change_to_child else: # There is no current best-child but the child is not viable - # debugEcho " branch 10 - no change" no_change - # debugEcho " new_best_child = ", new_best_child - # debugEcho " new_best_descendant = ", new_best_descendant - self.nodes[parent_index].best_child = new_best_child self.nodes[parent_index].best_descendant = new_best_descendant @@ -497,11 +456,6 @@ func node_is_viable_for_head(self: ProtoArray, node: ProtoNode): bool {.raises: ## ## Any node that has a different finalized or justified epoch ## should not be viable for the head. - # debugEcho " viable node.justified_epoch = ", node.justified_epoch - # debugEcho " viable self.justified_epoch = ", self.justified_epoch - # debugEcho " viable node.finalized_epoch = ", node.finalized_epoch - # debugEcho " viable self.finalized_epoch = ", self.finalized_epoch - ( (node.justified_epoch == self.justified_epoch) or (self.justified_epoch == Epoch(0)) From 133b6fd1c05d849e7a382b6d9d876e9b27cffda4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mamy=20Andr=C3=A9-Ratsimbazafy?= Date: Tue, 7 Apr 2020 12:59:18 +0200 Subject: [PATCH 14/22] nimcrypto fix https://github.com/status-im/nim-beacon-chain/pull/864 as been merged, remove TODO comment --- beacon_chain/fork_choice/fork_choice.nim | 1 - tests/fork_choice/interpreter.nim | 2 -- 2 files changed, 3 deletions(-) diff --git a/beacon_chain/fork_choice/fork_choice.nim b/beacon_chain/fork_choice/fork_choice.nim index 290f469dcf..706145913c 100644 --- a/beacon_chain/fork_choice/fork_choice.nim +++ b/beacon_chain/fork_choice/fork_choice.nim @@ -251,7 +251,6 @@ func compute_deltas( when isMainModule: import stew/endians2 - # TODO: Note - we don't want to import nimcrypto/hash as the `==` is buggy at the moment. func fakeHash*(index: SomeInteger): Eth2Digest = ## Create fake hashes ## Those are just the value serialized in big-endian diff --git a/tests/fork_choice/interpreter.nim b/tests/fork_choice/interpreter.nim index b1052c9be7..2d51fb8f16 100644 --- a/tests/fork_choice/interpreter.nim +++ b/tests/fork_choice/interpreter.nim @@ -16,8 +16,6 @@ import export result, datatypes, digest, fork_choice, fork_choice_types, tables, options -# TODO: nimcrypto.hash.`==` is returns incorrect result with those fakeHash -# Don't import nimcrypto, let Nim do the `==` in the mean-time func fakeHash*(index: SomeInteger): Eth2Digest = ## Create fake hashes ## Those are just the value serialized in big-endian From 10848b78191acd49b50e33bc7bb8fb00798986ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mamy=20Andr=C3=A9-Ratsimbazafy?= Date: Tue, 7 Apr 2020 14:24:59 +0200 Subject: [PATCH 15/22] Turn fork choice exception-free --- beacon_chain/fork_choice/fork_choice.nim | 18 +++++------ beacon_chain/fork_choice/proto_array.nim | 40 ++++++++++++++++-------- 2 files changed, 36 insertions(+), 22 deletions(-) diff --git a/beacon_chain/fork_choice/fork_choice.nim b/beacon_chain/fork_choice/fork_choice.nim index 706145913c..115a3744a8 100644 --- a/beacon_chain/fork_choice/fork_choice.nim +++ b/beacon_chain/fork_choice/fork_choice.nim @@ -34,7 +34,7 @@ func compute_deltas( votes: var openArray[VoteTracker], old_balances: openarray[Gwei], new_balances: openarray[Gwei] - ): ForkChoiceError {.raises: [KeyError].} + ): ForkChoiceError {.raises: [].} # Fork choice routines # ---------------------------------------------------------------------- @@ -45,7 +45,7 @@ func initForkChoice*( justified_epoch: Epoch, finalized_epoch: Epoch, finalized_root: Eth2Digest - ): Result[ForkChoice, string] {.raises: [KeyError].} = + ): Result[ForkChoice, string] {.raises: [].} = ## Initialize a fork choice context var proto_array = ProtoArray( prune_threshold: DefaultPruneThreshold, @@ -67,7 +67,7 @@ func initForkChoice*( result.ok(ForkChoice(proto_array: proto_array)) -func extend[T](s: var seq[T], minLen: int) = +func extend[T](s: var seq[T], minLen: int) {.raises: [].} = ## Extend a sequence so that it can contains at least `minLen` elements. ## If it's already bigger, the sequence is unmodified. ## The extension is zero-initialized @@ -115,7 +115,7 @@ func process_block*( state_root: Eth2Digest, justified_epoch: Epoch, finalized_epoch: Epoch - ): Result[void, string] {.raises: [KeyError].} = + ): Result[void, string] {.raises: [].} = ## Add a block to the fork choice context let err = self.proto_array.on_block( slot, block_root, some(parent_root), state_root, justified_epoch, finalized_epoch @@ -132,7 +132,7 @@ func find_head*( justified_root: Eth2Digest, finalized_epoch: Epoch, justified_state_balances: seq[Gwei] - ): Result[Eth2Digest, string] {.raises: [UnpackError, KeyError].} = + ): Result[Eth2Digest, string] {.raises: [].} = ## Returns the new blockchain head # Compute deltas with previous call @@ -169,7 +169,7 @@ func find_head*( func maybe_prune*( self: var ForkChoice, finalized_root: Eth2Digest - ): Result[void, string] {.raises: [KeyError].} = + ): Result[void, string] {.raises: [].} = ## Prune blocks preceding the finalized root as they are now unneeded. let err = self.proto_array.maybe_prune(finalized_root) if err.kind != fcSuccess: @@ -183,7 +183,7 @@ func compute_deltas( votes: var openArray[VoteTracker], old_balances: openarray[Gwei], new_balances: openarray[Gwei] - ): ForkChoiceError {.raises: [KeyError].} = + ): ForkChoiceError {.raises: [].} = ## Update `deltas` ## between old and new balances ## between votes @@ -220,7 +220,7 @@ func compute_deltas( # Ignore the current or next vote if it is not known in `indices`. # We assume that it is outside of our tree (i.e., pre-finalization) and therefore not interesting. if vote.current_root in indices: - let index = indices[vote.current_root] + let index = indices.unsafeGet(vote.current_root) if index >= deltas.len: return ForkChoiceError( kind: fcErrInvalidNodeDelta, @@ -231,7 +231,7 @@ func compute_deltas( # TODO: is int64 big enough? if vote.next_root in indices: - let index = indices[vote.next_root] + let index = indices.unsafeGet(vote.next_root) if index >= deltas.len: return ForkChoiceError( kind: fcErrInvalidNodeDelta, diff --git a/beacon_chain/fork_choice/proto_array.nim b/beacon_chain/fork_choice/proto_array.nim index d5aba5b4d4..6f4efbe6d8 100644 --- a/beacon_chain/fork_choice/proto_array.nim +++ b/beacon_chain/fork_choice/proto_array.nim @@ -36,6 +36,23 @@ func tiebreak(a, b: Eth2Digest): bool = # else we have equality so far return true +template getOrFailcase*[K, V](table: Table[K, V], key: K, failcase: untyped): V = + ## Get a value from a Nim Table, turning KeyError into + ## the "failcase" + block: + var value: V + try: + value = table[key] + except: + failcase + value + +template unsafeGet*[K, V](table: Table[K, V], key: K): V = + ## Get a value from a Nim Table, turning KeyError into + ## an AssertionError defect + getOrFailcase(table, key): + doAssert false, "The " & astToStr(table) & " table shouldn't miss a key" + # Forward declarations # ---------------------------------------------------------------------- @@ -51,7 +68,7 @@ func apply_score_changes*( deltas: var openarray[Delta], justified_epoch: Epoch, finalized_epoch: Epoch - ): ForkChoiceError {.raises: [UnpackError].}= + ): ForkChoiceError {.raises: [].}= ## Iterate backwards through the array, touching all nodes and their parents ## and potentially the best-child of each parent. ## @@ -138,7 +155,7 @@ func on_block*( state_root: Eth2Digest, justified_epoch: Epoch, finalized_epoch: Epoch - ): ForkChoiceError {.raises: [KeyError].} = + ): ForkChoiceError {.raises: [].} = ## Register a block with the fork choice ## A `none` parent is only valid for Genesis # TODO: fix exceptions raised @@ -156,7 +173,7 @@ func on_block*( # Is this possible? none(int) else: # TODO: How to tell the compiler not to raise KeyError - some(self.indices[parent.unsafeGet()]) + some(self.indices.unsafeGet(parent.unsafeGet())) let node = ProtoNode( slot: slot, @@ -184,7 +201,7 @@ func find_head*( self: var ProtoArray, head: var Eth2Digest, justified_root: Eth2Digest - ): ForkChoiceError {.raises: [KeyError].} = + ): ForkChoiceError {.raises: [].} = ## Follows the best-descendant links to find the best-block (i.e. head-block) ## ## ⚠️ Warning @@ -192,20 +209,20 @@ func find_head*( ## is not followed by `apply_score_changes` as `on_new_block` does not ## update the whole tree. - if justified_root notin self.indices: + let justified_index = self.indices.getOrFailcase(justified_root): return ForkChoiceError( kind: fcErrJustifiedNodeUnknown, block_root: justified_root ) - let justified_index = self.indices[justified_root] # TODO: this can't throw KeyError if justified_index notin {0..self.nodes.len-1}: return ForkChoiceError( kind: fcErrInvalidJustifiedIndex, index: justified_index ) + template justified_node: untyped {.dirty.} = self.nodes[justified_index] - # Alias, TODO: no exceptions + # Alias, IndexError are defects let best_descendant_index = block: if justified_node.best_descendant.isSome(): @@ -219,7 +236,7 @@ func find_head*( index: best_descendant_index ) template best_node: untyped {.dirty.} = self.nodes[best_descendant_index] - # Alias, TODO: no exceptions + # Alias, IndexError are defects # Perform a sanity check to ensure the node can be head if not self.node_is_viable_for_head(best_node): @@ -243,7 +260,7 @@ func find_head*( func maybe_prune*( self: var ProtoArray, finalized_root: Eth2Digest - ): ForkChoiceError {.raises: [KeyError].} = + ): ForkChoiceError {.raises: [].} = ## Update the tree with new finalization information. ## The tree is pruned if and only if: ## - The `finalized_root` and finalized epoch are different from current @@ -253,14 +270,11 @@ func maybe_prune*( ## - The finalized epoch is less than the current one ## - The finalized epoch matches the current one but the finalized root is different ## - Internal error due to invalid indices in `self` - # TODO: Exceptions - - if finalized_root notin self.indices: + let finalized_index = self.indices.getOrFailcase(finalized_root): return ForkChoiceError( kind: fcErrFinalizedNodeUnknown, block_root: finalized_root ) - let finalized_index = self.indices[finalized_root] if finalized_index < self.prune_threshold: # Pruning small numbers of nodes incurs more overhead than leaving them as is From 211901a9670a5a9dde7857d269cdd32892b275ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mamy=20Andr=C3=A9-Ratsimbazafy?= Date: Tue, 7 Apr 2020 14:31:48 +0200 Subject: [PATCH 16/22] Cleanup "result" to ensure early return is properly used --- beacon_chain/fork_choice/fork_choice.nim | 31 +++++++++--------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/beacon_chain/fork_choice/fork_choice.nim b/beacon_chain/fork_choice/fork_choice.nim index 115a3744a8..601eaf72e8 100644 --- a/beacon_chain/fork_choice/fork_choice.nim +++ b/beacon_chain/fork_choice/fork_choice.nim @@ -61,11 +61,10 @@ func initForkChoice*( justified_epoch, finalized_epoch ) - if err.kind != fcSuccess: - result.err("Failed to add finalized block to proto_array: " & $err) - return - result.ok(ForkChoice(proto_array: proto_array)) + if err.kind != fcSuccess: + return err("Failed to add finalized block to proto_array: " & $err) + return ok(ForkChoice(proto_array: proto_array)) func extend[T](s: var seq[T], minLen: int) {.raises: [].} = ## Extend a sequence so that it can contains at least `minLen` elements. @@ -103,9 +102,7 @@ func process_attestation*( # TODO: the "default" condition is probably unneeded vote.next_root = block_root vote.next_epoch = target_epoch - - result.ok() - + ok() func process_block*( self: var ForkChoice, @@ -121,9 +118,8 @@ func process_block*( slot, block_root, some(parent_root), state_root, justified_epoch, finalized_epoch ) if err.kind != fcSuccess: - result.err("process_block_error: " & $err) - return - result.ok() + return err("process_block_error: " & $err) + return ok() func find_head*( @@ -145,15 +141,14 @@ func find_head*( new_balances = justified_state_balances ) if delta_err.kind != fcSuccess: - result.err("find_head compute_deltas failed: " & $delta_err) - return + return err("find_head compute_deltas failed: " & $delta_err) # Apply score changes let score_err = self.proto_array.apply_score_changes( deltas, justified_epoch, finalized_epoch ) if score_err.kind != fcSuccess: - result.err("find_head apply_score_changes failed: " & $score_err) + return err("find_head apply_score_changes failed: " & $score_err) self.balances = justified_state_balances @@ -161,10 +156,9 @@ func find_head*( var new_head{.noInit.}: Eth2Digest let ghost_err = self.proto_array.find_head(new_head, justified_root) if ghost_err.kind != fcSuccess: - result.err("find_head failed: " & $ghost_err) - return + return err("find_head failed: " & $ghost_err) - result.ok(new_head) + return ok(new_head) func maybe_prune*( @@ -173,9 +167,8 @@ func maybe_prune*( ## Prune blocks preceding the finalized root as they are now unneeded. let err = self.proto_array.maybe_prune(finalized_root) if err.kind != fcSuccess: - result.err("find_head maybe_pruned failed: " & $err) - result.ok() - + return err("find_head maybe_pruned failed: " & $err) + return ok() func compute_deltas( deltas: var openarray[Delta], From d3227b0a37f3fc80c61b20cdb6ff863847c3f4d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mamy=20Andr=C3=A9-Ratsimbazafy?= Date: Tue, 7 Apr 2020 14:34:59 +0200 Subject: [PATCH 17/22] Add a comment on private/public error code vs Result --- beacon_chain/fork_choice/fork_choice.nim | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/beacon_chain/fork_choice/fork_choice.nim b/beacon_chain/fork_choice/fork_choice.nim index 601eaf72e8..ca0bbd6e7a 100644 --- a/beacon_chain/fork_choice/fork_choice.nim +++ b/beacon_chain/fork_choice/fork_choice.nim @@ -39,6 +39,10 @@ func compute_deltas( # Fork choice routines # ---------------------------------------------------------------------- +# API: +# - The private procs uses the ForkChoiceError error code +# - The public procs use Result + func initForkChoice*( finalized_block_slot: Slot, finalized_block_state_root: Eth2Digest, From 925ab9faa3774328cf10899b454105a22d377608 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mamy=20Andr=C3=A9-Ratsimbazafy?= Date: Tue, 7 Apr 2020 14:41:50 +0200 Subject: [PATCH 18/22] result -> results following https://github.com/status-im/nim-beacon-chain/pull/866 --- beacon_chain/fork_choice/fork_choice.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beacon_chain/fork_choice/fork_choice.nim b/beacon_chain/fork_choice/fork_choice.nim index ca0bbd6e7a..679a193a1a 100644 --- a/beacon_chain/fork_choice/fork_choice.nim +++ b/beacon_chain/fork_choice/fork_choice.nim @@ -9,7 +9,7 @@ import # Standard library std/tables, std/options, std/typetraits, # Status libraries - stew/result, + stew/results, # Internal ../spec/[datatypes, digest], # Fork choice From 22eebc83c351899c34c7a9d6a2da7ccb6a33b6bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mamy=20Andr=C3=A9-Ratsimbazafy?= Date: Tue, 7 Apr 2020 16:43:51 +0200 Subject: [PATCH 19/22] Address comments: - raises: [Defect] doesn't work -> TODO - process_attestation cannot fail - try/except as expression pending Nim v1.2.0 - cleanup TODOs --- beacon_chain/fork_choice/fork_choice.nim | 6 ++++-- beacon_chain/fork_choice/proto_array.nim | 9 ++++----- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/beacon_chain/fork_choice/fork_choice.nim b/beacon_chain/fork_choice/fork_choice.nim index 679a193a1a..44261aa4b6 100644 --- a/beacon_chain/fork_choice/fork_choice.nim +++ b/beacon_chain/fork_choice/fork_choice.nim @@ -35,6 +35,8 @@ func compute_deltas( old_balances: openarray[Gwei], new_balances: openarray[Gwei] ): ForkChoiceError {.raises: [].} +# TODO: raises [Defect] - once https://github.com/nim-lang/Nim/issues/12862 is fixed +# https://github.com/status-im/nim-beacon-chain/pull/865#pullrequestreview-389117232 # Fork choice routines # ---------------------------------------------------------------------- @@ -95,7 +97,7 @@ func process_attestation*( validator_index: ValidatorIndex, block_root: Eth2Digest, target_epoch: Epoch - ): Result[void, string] {.raises: [].} = + ) = ## Add an attestation to the fork choice context self.votes.extend(validator_index.int + 1) @@ -106,7 +108,7 @@ func process_attestation*( # TODO: the "default" condition is probably unneeded vote.next_root = block_root vote.next_epoch = target_epoch - ok() + func process_block*( self: var ForkChoice, diff --git a/beacon_chain/fork_choice/proto_array.nim b/beacon_chain/fork_choice/proto_array.nim index 6f4efbe6d8..ace9cac76d 100644 --- a/beacon_chain/fork_choice/proto_array.nim +++ b/beacon_chain/fork_choice/proto_array.nim @@ -40,10 +40,12 @@ template getOrFailcase*[K, V](table: Table[K, V], key: K, failcase: untyped): V ## Get a value from a Nim Table, turning KeyError into ## the "failcase" block: + # TODO: try/except expression with Nim v1.2.0: + # https://github.com/status-im/nim-beacon-chain/pull/865#discussion_r404856551 var value: V try: value = table[key] - except: + except KeyError: failcase value @@ -82,8 +84,6 @@ func apply_score_changes*( ## 3. Compare the current node with the parent's best-child, ## updating if the current node should become the best-child ## 4. If required, update the parent's best-descendant with the current node or its best-descendant - # TODO: remove spurious raised exceptions - if deltas.len != self.indices.len: return ForkChoiceError( kind: fcErrInvalidDeltaLen, @@ -158,7 +158,6 @@ func on_block*( ): ForkChoiceError {.raises: [].} = ## Register a block with the fork choice ## A `none` parent is only valid for Genesis - # TODO: fix exceptions raised # If the block is already known, ignore it if root in self.indices: @@ -172,7 +171,7 @@ func on_block*( elif parent.unsafeGet() notin self.indices: # Is this possible? none(int) - else: # TODO: How to tell the compiler not to raise KeyError + else: some(self.indices.unsafeGet(parent.unsafeGet())) let node = ProtoNode( From e12b89f0fed19d4076abe69d6bddc9defc98bac3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mamy=20Andr=C3=A9-Ratsimbazafy?= Date: Tue, 7 Apr 2020 16:46:40 +0200 Subject: [PATCH 20/22] re-enable all sanity checks --- beacon_chain/fork_choice/fork_choice.nim | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/beacon_chain/fork_choice/fork_choice.nim b/beacon_chain/fork_choice/fork_choice.nim index 44261aa4b6..6a49e9718e 100644 --- a/beacon_chain/fork_choice/fork_choice.nim +++ b/beacon_chain/fork_choice/fork_choice.nim @@ -573,10 +573,10 @@ when isMainModule: # ---------------------------------------------------------------------- echo "fork_choice internal tests for compute_deltas" - # tZeroHash() + tZeroHash() tAll_voted_the_same() tDifferent_votes() tMoving_votes() tChanging_balances() - # tValidator_appears() - # tValidator_disappears() + tValidator_appears() + tValidator_disappears() From b5355ca65f15d837819db4343d8bdbfdcc024c36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mamy=20Andr=C3=A9-Ratsimbazafy?= Date: Tue, 7 Apr 2020 16:47:55 +0200 Subject: [PATCH 21/22] tag no raise for process_attestation --- beacon_chain/fork_choice/fork_choice.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beacon_chain/fork_choice/fork_choice.nim b/beacon_chain/fork_choice/fork_choice.nim index 6a49e9718e..6064060078 100644 --- a/beacon_chain/fork_choice/fork_choice.nim +++ b/beacon_chain/fork_choice/fork_choice.nim @@ -97,7 +97,7 @@ func process_attestation*( validator_index: ValidatorIndex, block_root: Eth2Digest, target_epoch: Epoch - ) = + ) {.raises: [].} = ## Add an attestation to the fork choice context self.votes.extend(validator_index.int + 1) From 5aba3b57869007f1be1393fc1e6c47cebaae70e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mamy=20Andr=C3=A9-Ratsimbazafy?= Date: Tue, 7 Apr 2020 18:12:09 +0200 Subject: [PATCH 22/22] use raises defect everywhere in fork choice and fix process_attestation test --- beacon_chain/fork_choice/fork_choice.nim | 16 ++++++++-------- beacon_chain/fork_choice/proto_array.nim | 20 ++++++++++---------- tests/fork_choice/interpreter.nim | 3 +-- 3 files changed, 19 insertions(+), 20 deletions(-) diff --git a/beacon_chain/fork_choice/fork_choice.nim b/beacon_chain/fork_choice/fork_choice.nim index 6064060078..7dcbb87bdd 100644 --- a/beacon_chain/fork_choice/fork_choice.nim +++ b/beacon_chain/fork_choice/fork_choice.nim @@ -34,7 +34,7 @@ func compute_deltas( votes: var openArray[VoteTracker], old_balances: openarray[Gwei], new_balances: openarray[Gwei] - ): ForkChoiceError {.raises: [].} + ): ForkChoiceError {.raises: [Defect].} # TODO: raises [Defect] - once https://github.com/nim-lang/Nim/issues/12862 is fixed # https://github.com/status-im/nim-beacon-chain/pull/865#pullrequestreview-389117232 @@ -51,7 +51,7 @@ func initForkChoice*( justified_epoch: Epoch, finalized_epoch: Epoch, finalized_root: Eth2Digest - ): Result[ForkChoice, string] {.raises: [].} = + ): Result[ForkChoice, string] {.raises: [Defect].} = ## Initialize a fork choice context var proto_array = ProtoArray( prune_threshold: DefaultPruneThreshold, @@ -72,7 +72,7 @@ func initForkChoice*( return err("Failed to add finalized block to proto_array: " & $err) return ok(ForkChoice(proto_array: proto_array)) -func extend[T](s: var seq[T], minLen: int) {.raises: [].} = +func extend[T](s: var seq[T], minLen: int) {.raises: [Defect].} = ## Extend a sequence so that it can contains at least `minLen` elements. ## If it's already bigger, the sequence is unmodified. ## The extension is zero-initialized @@ -97,7 +97,7 @@ func process_attestation*( validator_index: ValidatorIndex, block_root: Eth2Digest, target_epoch: Epoch - ) {.raises: [].} = + ) {.raises: [Defect].} = ## Add an attestation to the fork choice context self.votes.extend(validator_index.int + 1) @@ -118,7 +118,7 @@ func process_block*( state_root: Eth2Digest, justified_epoch: Epoch, finalized_epoch: Epoch - ): Result[void, string] {.raises: [].} = + ): Result[void, string] {.raises: [Defect].} = ## Add a block to the fork choice context let err = self.proto_array.on_block( slot, block_root, some(parent_root), state_root, justified_epoch, finalized_epoch @@ -134,7 +134,7 @@ func find_head*( justified_root: Eth2Digest, finalized_epoch: Epoch, justified_state_balances: seq[Gwei] - ): Result[Eth2Digest, string] {.raises: [].} = + ): Result[Eth2Digest, string] {.raises: [Defect].} = ## Returns the new blockchain head # Compute deltas with previous call @@ -169,7 +169,7 @@ func find_head*( func maybe_prune*( self: var ForkChoice, finalized_root: Eth2Digest - ): Result[void, string] {.raises: [].} = + ): Result[void, string] {.raises: [Defect].} = ## Prune blocks preceding the finalized root as they are now unneeded. let err = self.proto_array.maybe_prune(finalized_root) if err.kind != fcSuccess: @@ -182,7 +182,7 @@ func compute_deltas( votes: var openArray[VoteTracker], old_balances: openarray[Gwei], new_balances: openarray[Gwei] - ): ForkChoiceError {.raises: [].} = + ): ForkChoiceError {.raises: [Defect].} = ## Update `deltas` ## between old and new balances ## between votes diff --git a/beacon_chain/fork_choice/proto_array.nim b/beacon_chain/fork_choice/proto_array.nim index ace9cac76d..8548a641d4 100644 --- a/beacon_chain/fork_choice/proto_array.nim +++ b/beacon_chain/fork_choice/proto_array.nim @@ -58,9 +58,9 @@ template unsafeGet*[K, V](table: Table[K, V], key: K): V = # Forward declarations # ---------------------------------------------------------------------- -func maybe_update_best_child_and_descendant(self: var ProtoArray, parent_index: Index, child_index: Index): ForkChoiceError {.raises: [].} -func node_is_viable_for_head(self: ProtoArray, node: ProtoNode): bool {.raises: [].} -func node_leads_to_viable_head(self: ProtoArray, node: ProtoNode): tuple[viable: bool, err: ForkChoiceError] {.raises: [].} +func maybe_update_best_child_and_descendant(self: var ProtoArray, parent_index: Index, child_index: Index): ForkChoiceError {.raises: [Defect].} +func node_is_viable_for_head(self: ProtoArray, node: ProtoNode): bool {.raises: [Defect].} +func node_leads_to_viable_head(self: ProtoArray, node: ProtoNode): tuple[viable: bool, err: ForkChoiceError] {.raises: [Defect].} # ProtoArray routines # ---------------------------------------------------------------------- @@ -70,7 +70,7 @@ func apply_score_changes*( deltas: var openarray[Delta], justified_epoch: Epoch, finalized_epoch: Epoch - ): ForkChoiceError {.raises: [].}= + ): ForkChoiceError {.raises: [Defect].}= ## Iterate backwards through the array, touching all nodes and their parents ## and potentially the best-child of each parent. ## @@ -155,7 +155,7 @@ func on_block*( state_root: Eth2Digest, justified_epoch: Epoch, finalized_epoch: Epoch - ): ForkChoiceError {.raises: [].} = + ): ForkChoiceError {.raises: [Defect].} = ## Register a block with the fork choice ## A `none` parent is only valid for Genesis @@ -200,7 +200,7 @@ func find_head*( self: var ProtoArray, head: var Eth2Digest, justified_root: Eth2Digest - ): ForkChoiceError {.raises: [].} = + ): ForkChoiceError {.raises: [Defect].} = ## Follows the best-descendant links to find the best-block (i.e. head-block) ## ## ⚠️ Warning @@ -259,7 +259,7 @@ func find_head*( func maybe_prune*( self: var ProtoArray, finalized_root: Eth2Digest - ): ForkChoiceError {.raises: [].} = + ): ForkChoiceError {.raises: [Defect].} = ## Update the tree with new finalization information. ## The tree is pruned if and only if: ## - The `finalized_root` and finalized epoch are different from current @@ -340,7 +340,7 @@ func maybe_prune*( func maybe_update_best_child_and_descendant( self: var ProtoArray, parent_index: Index, - child_index: Index): ForkChoiceError {.raises: [].} = + child_index: Index): ForkChoiceError {.raises: [Defect].} = ## Observe the parent at `parent_index` with respect to the child at `child_index` and ## potentiatlly modify the `parent.best_child` and `parent.best_descendant` values ## @@ -438,7 +438,7 @@ func maybe_update_best_child_and_descendant( func node_leads_to_viable_head( self: ProtoArray, node: ProtoNode - ): tuple[viable: bool, err: ForkChoiceError] {.raises: [].} = + ): tuple[viable: bool, err: ForkChoiceError] {.raises: [Defect].} = ## Indicates if the node itself or its best-descendant are viable ## for blockchain head let best_descendant_is_viable_for_head = block: @@ -463,7 +463,7 @@ func node_leads_to_viable_head( ForkChoiceSuccess ) -func node_is_viable_for_head(self: ProtoArray, node: ProtoNode): bool {.raises: [].} = +func node_is_viable_for_head(self: ProtoArray, node: ProtoNode): bool {.raises: [Defect].} = ## This is the equivalent of `filter_block_tree` function in eth2 spec ## https://github.com/ethereum/eth2.0-specs/blob/v0.10.0/specs/phase0/fork-choice.md#filter_block_tree ## diff --git a/tests/fork_choice/interpreter.nim b/tests/fork_choice/interpreter.nim index 2d51fb8f16..987ab36c99 100644 --- a/tests/fork_choice/interpreter.nim +++ b/tests/fork_choice/interpreter.nim @@ -92,12 +92,11 @@ func apply(ctx: var ForkChoice, id: int, op: Operation) = doAssert r.isOk(), &"process_block (op #{id}) returned an error: {r.error}" debugEcho " Processed block 0x", op.root, " with parent 0x", op.parent_root, " and justified epoch ", op.blk_justified_epoch of ProcessAttestation: - let r = ctx.process_attestation( + ctx.process_attestation( validator_index = op.validator_index, block_root = op.block_root, target_epoch = op.target_epoch ) - doAssert r.isOk(), &"process_attestation (op #{id}) returned an error: {r.error}" debugEcho " Processed att target 0x", op.block_root, " from validator ", op.validator_index, " for epoch ", op.target_epoch of Prune: ctx.proto_array.prune_threshold = op.prune_threshold