From f9536fd14b765fa8c2c1411425d27452e77636ae Mon Sep 17 00:00:00 2001 From: Etan Kissling Date: Sun, 10 Jul 2022 08:26:29 -0700 Subject: [PATCH] fork choice cleanups (#3849) Removes a few extra-ambitious templates to make `self` updates explicit, and moves the `FinalityCheckpoints` type from `base` to `helpers` as it is an additional Nimbus specific type not defined by spec. --- beacon_chain/fork_choice/fork_choice.nim | 28 +++++++++---------- .../fork_choice/fork_choice_types.nim | 3 +- beacon_chain/fork_choice/proto_array.nim | 1 + beacon_chain/spec/datatypes/base.nim | 11 -------- beacon_chain/spec/helpers.nim | 13 +++++++++ tests/fork_choice/interpreter.nim | 3 +- 6 files changed, 32 insertions(+), 27 deletions(-) diff --git a/beacon_chain/fork_choice/fork_choice.nim b/beacon_chain/fork_choice/fork_choice.nim index da617bbb9..b067a7ee8 100644 --- a/beacon_chain/fork_choice/fork_choice.nim +++ b/beacon_chain/fork_choice/fork_choice.nim @@ -159,15 +159,12 @@ proc update_checkpoints( proc on_tick( self: var ForkChoice, dag: ChainDAGRef, time: BeaconTime): FcResult[void] = ## Must be called at least once per slot. - template backend(): auto = self.backend - template checkpoints(): auto = self.checkpoints - - let previous_time = checkpoints.time + let previous_time = self.checkpoints.time # update store time if time < previous_time: return err ForkChoiceError(kind: fcInconsistentTick) - checkpoints.time = time + self.checkpoints.time = time let current_slot = time.slotOrZero @@ -175,7 +172,7 @@ proc on_tick( # Reset store.proposer_boost_root if this is a new slot if current_slot > previous_slot: - checkpoints.proposer_boost_root.reset() + self.checkpoints.proposer_boost_root = ZERO_HASH # Not a new epoch, return if not (current_slot > previous_slot and current_slot.is_epoch): @@ -183,19 +180,22 @@ proc on_tick( # Update store.justified_checkpoint if a better checkpoint on the # store.finalized_checkpoint chain - if checkpoints.best_justified.epoch > checkpoints.justified.checkpoint.epoch: + let + best_justified_epoch = self.checkpoints.best_justified.epoch + store_justified_epoch = self.checkpoints.justified.checkpoint.epoch + if best_justified_epoch > store_justified_epoch: let - blck = dag.getBlockRef(checkpoints.best_justified.root).valueOr: + blck = dag.getBlockRef(self.checkpoints.best_justified.root).valueOr: return err ForkChoiceError( kind: fcJustifiedNodeUnknown, - blockRoot: checkpoints.best_justified.root) - finalized_ancestor = blck.atEpochStart(checkpoints.finalized.epoch) - if finalized_ancestor.blck.root == checkpoints.finalized.root: - checkpoints.update_justified(dag, blck, checkpoints.best_justified.epoch) + blockRoot: self.checkpoints.best_justified.root) + finalized_ancestor = blck.atEpochStart(self.checkpoints.finalized.epoch) + if finalized_ancestor.blck.root == self.checkpoints.finalized.root: + self.checkpoints.update_justified(dag, blck, best_justified_epoch) # Pull-up chain tips from previous epoch - for realized in backend.proto_array.realizePendingCheckpoints(): - ? checkpoints.update_checkpoints(dag, realized) + for realized in self.backend.proto_array.realizePendingCheckpoints(): + ? self.checkpoints.update_checkpoints(dag, realized) ok() diff --git a/beacon_chain/fork_choice/fork_choice_types.nim b/beacon_chain/fork_choice/fork_choice_types.nim index 4302b0570..585f42fad 100644 --- a/beacon_chain/fork_choice/fork_choice_types.nim +++ b/beacon_chain/fork_choice/fork_choice_types.nim @@ -14,7 +14,8 @@ import stew/results, chronicles, # Internal - ../spec/datatypes/base + ../spec/datatypes/base, + ../spec/helpers # https://github.com/ethereum/consensus-specs/blob/v0.11.1/specs/phase0/fork-choice.md # This is a port of https://github.com/sigp/lighthouse/pull/804 diff --git a/beacon_chain/fork_choice/proto_array.nim b/beacon_chain/fork_choice/proto_array.nim index d6c01437d..72dcbf96b 100644 --- a/beacon_chain/fork_choice/proto_array.nim +++ b/beacon_chain/fork_choice/proto_array.nim @@ -15,6 +15,7 @@ import stew/results, # Internal ../spec/datatypes/base, + ../spec/helpers, # Fork choice ./fork_choice_types diff --git a/beacon_chain/spec/datatypes/base.nim b/beacon_chain/spec/datatypes/base.nim index f85c5318f..a1bfe16a7 100644 --- a/beacon_chain/spec/datatypes/base.nim +++ b/beacon_chain/spec/datatypes/base.nim @@ -246,10 +246,6 @@ type epoch*: Epoch root*: Eth2Digest - FinalityCheckpoints* = object - justified*: Checkpoint - finalized*: Checkpoint - # https://github.com/ethereum/consensus-specs/blob/v1.2.0-rc.1/specs/phase0/beacon-chain.md#AttestationData AttestationData* = object slot*: Slot @@ -752,12 +748,6 @@ func shortLog*(v: Checkpoint): auto = # epoch:root when logging epoch, root:slot when logging slot! $shortLog(v.epoch) & ":" & shortLog(v.root) -func shortLog*(v: FinalityCheckpoints): auto = - ( - justified: shortLog(v.justified), - finalized: shortLog(v.finalized) - ) - func shortLog*(v: AttestationData): auto = ( slot: shortLog(v.slot), @@ -829,7 +819,6 @@ func shortLog*(v: SomeSignedVoluntaryExit): auto = chronicles.formatIt AttestationData: it.shortLog chronicles.formatIt Attestation: it.shortLog chronicles.formatIt Checkpoint: it.shortLog -chronicles.formatIt FinalityCheckpoints: it.shortLog const # http://facweb.cs.depaul.edu/sjost/it212/documents/ascii-pr.htm diff --git a/beacon_chain/spec/helpers.nim b/beacon_chain/spec/helpers.nim index 4f33d82ab..ce79b21ae 100644 --- a/beacon_chain/spec/helpers.nim +++ b/beacon_chain/spec/helpers.nim @@ -18,6 +18,7 @@ import std/[algorithm, math, sets, tables], # Status libraries stew/[bitops2, byteutils, endians2, objects], + chronicles, # Internal ./datatypes/[phase0, altair, bellatrix], "."/[eth2_merkleization, forks, ssz_codec] @@ -27,6 +28,18 @@ import export forks, eth2_merkleization, ssz_codec +type FinalityCheckpoints* = object + justified*: Checkpoint + finalized*: Checkpoint + +func shortLog*(v: FinalityCheckpoints): auto = + ( + justified: shortLog(v.justified), + finalized: shortLog(v.finalized) + ) + +chronicles.formatIt FinalityCheckpoints: it.shortLog + # https://github.com/ethereum/consensus-specs/blob/v1.2.0-rc.1/specs/phase0/beacon-chain.md#integer_squareroot func integer_squareroot*(n: SomeInteger): SomeInteger = ## Return the largest integer ``x`` such that ``x**2 <= n``. diff --git a/tests/fork_choice/interpreter.nim b/tests/fork_choice/interpreter.nim index de0fc182e..4713e73e3 100644 --- a/tests/fork_choice/interpreter.nim +++ b/tests/fork_choice/interpreter.nim @@ -12,9 +12,10 @@ import stew/[results, endians2], # Internals ../../beacon_chain/spec/datatypes/base, + ../../beacon_chain/spec/helpers, ../../beacon_chain/fork_choice/[fork_choice, fork_choice_types] -export results, base, fork_choice, fork_choice_types, tables, options +export results, base, helpers, fork_choice, fork_choice_types, tables, options func fakeHash*(index: SomeInteger): Eth2Digest = ## Create fake hashes