rm debugRaiseAssert; clean up several debugComments (#6308)

tersec · web-flow · commit c7bf6fb5421a · 2024-05-23T23:51:09.000+02:00
* rm debugRaiseAssert; clean up several debugComments

* exception linting
diff --git a/beacon_chain/consensus_object_pools/spec_cache.nim b/beacon_chain/consensus_object_pools/spec_cache.nim
@@ -16,8 +16,7 @@ import
   ../spec/datatypes/base,
   ./block_pools_types, blockchain_dag
 
-debugComment "probably just need Electra shortLog here"
-import ../spec/forks # prune this, it's for electra attestation logging
+from ../spec/datatypes/electra import shortLog
 
 export
   base, extras, block_pools_types, results
diff --git a/beacon_chain/light_client.nim b/beacon_chain/light_client.nim
@@ -366,8 +366,6 @@ proc updateGossipStatus*(
     lightClient: LightClient, slot: Slot, dagIsBehind = default(Option[bool])) =
   template cfg(): auto = lightClient.cfg
 
-  debugComment "when LC on electra works, add cfg.ELECTRA_FORK_EPOCH"
-
   let
     epoch = slot.epoch
 
diff --git a/beacon_chain/spec/datatypes/base.nim b/beacon_chain/spec/datatypes/base.nim
@@ -986,4 +986,3 @@ func ofLen*[T, N](ListType: type List[T, N], n: int): ListType =
     raise newException(SszSizeMismatchError)
 
 template debugComment*(s: string) = discard
-template debugRaiseAssert*(s: string) = discard
diff --git a/beacon_chain/spec/state_transition_epoch.nim b/beacon_chain/spec/state_transition_epoch.nim
@@ -1221,7 +1221,7 @@ func process_historical_summaries_update*(
 # https://github.com/ethereum/consensus-specs/blob/v1.5.0-alpha.0/specs/electra/beacon-chain.md#new-process_pending_balance_deposits
 func process_pending_balance_deposits*(
     cfg: RuntimeConfig, state: var electra.BeaconState,
-    cache: var StateCache) =
+    cache: var StateCache): Result[void, cstring] =
   let
     available_for_processing = state.deposit_balance_to_consume +
       get_activation_exit_churn_limit(cfg, state, cache)
@@ -1232,8 +1232,9 @@ func process_pending_balance_deposits*(
   for deposit in state.pending_balance_deposits:
     if processed_amount + deposit.amount > available_for_processing:
       break
-    debugComment "do this validatorindex check properly (it truncates)"
-    increase_balance(state, deposit.index.ValidatorIndex, deposit.amount)
+    let deposit_validator_index = ValidatorIndex.init(deposit.index).valueOr:
+      return err("process_pending_balance_deposits: deposit index out of range")
+    increase_balance(state, deposit_validator_index, deposit.amount)
     processed_amount += deposit.amount
     inc next_deposit_index
 
@@ -1247,8 +1248,12 @@ func process_pending_balance_deposits*(
     state.deposit_balance_to_consume =
       available_for_processing - processed_amount
 
+  ok()
+
 # https://github.com/ethereum/consensus-specs/blob/v1.5.0-alpha.0/specs/electra/beacon-chain.md#new-process_pending_consolidations
-func process_pending_consolidations*(cfg: RuntimeConfig, state: var electra.BeaconState) =
+func process_pending_consolidations*(
+    cfg: RuntimeConfig, state: var electra.BeaconState):
+    Result[void, cstring] =
   var next_pending_consolidation = 0
   for pending_consolidation in state.pending_consolidations:
     let source_validator =
@@ -1259,25 +1264,29 @@ func process_pending_consolidations*(cfg: RuntimeConfig, state: var electra.Beac
     if source_validator.withdrawable_epoch > get_current_epoch(state):
       break
 
+    let
+      source_validator_index = ValidatorIndex.init(
+          pending_consolidation.source_index).valueOr:
+        return err("process_pending_consolidations: source index out of range")
+      target_validator_index = ValidatorIndex.init(
+          pending_consolidation.target_index).valueOr:
+        return err("process_pending_consolidations: target index out of range")
+
     # Churn any target excess active balance of target and raise its max
-    debugComment "truncating integer conversion"
-    switch_to_compounding_validator(
-      state, pending_consolidation.target_index.ValidatorIndex)
+    switch_to_compounding_validator(state, target_validator_index)
 
     # Move active balance to target. Excess balance is withdrawable.
-    debugComment "Truncating"
-    let active_balance = get_active_balance(
-      state, pending_consolidation.source_index.ValidatorIndex)
-    decrease_balance(
-      state, pending_consolidation.source_index.ValidatorIndex, active_balance)
-    increase_balance(
-      state, pending_consolidation.target_index.ValidatorIndex, active_balance)
+    let active_balance = get_active_balance(state, source_validator_index)
+    decrease_balance(state, source_validator_index, active_balance)
+    increase_balance(state, target_validator_index, active_balance)
     inc next_pending_consolidation
 
   state.pending_consolidations =
     HashList[PendingConsolidation, Limit PENDING_CONSOLIDATIONS_LIMIT].init(
       state.pending_consolidations.asSeq[next_pending_consolidation..^1])
 
+  ok()
+
 # https://github.com/ethereum/consensus-specs/blob/v1.4.0/specs/phase0/beacon-chain.md#epoch-processing
 proc process_epoch*(
     cfg: RuntimeConfig, state: var phase0.BeaconState, flags: UpdateFlags,
@@ -1464,8 +1473,8 @@ proc process_epoch*(
   process_slashings(state, info.balances.current_epoch)
 
   process_eth1_data_reset(state)
-  process_pending_balance_deposits(cfg, state, cache)  # [New in Electra:EIP7251]
-  process_pending_consolidations(cfg, state)  # [New in Electra:EIP7251]
+  ? process_pending_balance_deposits(cfg, state, cache)  # [New in Electra:EIP7251]
+  ? process_pending_consolidations(cfg, state)  # [New in Electra:EIP7251]
   process_effective_balance_updates(state)  # [Modified in Electra:EIP7251]
   process_slashings_reset(state)
   process_randao_mixes_reset(state)
diff --git a/tests/consensus_spec/altair/test_fixture_state_transition_epoch.nim b/tests/consensus_spec/altair/test_fixture_state_transition_epoch.nim
@@ -11,7 +11,7 @@
 import
   # Beacon chain internals
   chronicles,
-  ../../../beacon_chain/spec/[beaconstate, presets, state_transition_epoch],
+  ../../../beacon_chain/spec/[presets, state_transition_epoch],
   ../../../beacon_chain/spec/datatypes/altair,
   # Test utilities
   ../../testutil,
diff --git a/tests/consensus_spec/bellatrix/test_fixture_state_transition_epoch.nim b/tests/consensus_spec/bellatrix/test_fixture_state_transition_epoch.nim
@@ -10,7 +10,7 @@
 
 import
   # Beacon chain internals
-  ../../../beacon_chain/spec/[beaconstate, presets, state_transition_epoch],
+  ../../../beacon_chain/spec/[presets, state_transition_epoch],
   ../../../beacon_chain/spec/datatypes/[altair, bellatrix],
   # Test utilities
   ../../testutil,
diff --git a/tests/consensus_spec/capella/test_fixture_state_transition_epoch.nim b/tests/consensus_spec/capella/test_fixture_state_transition_epoch.nim
@@ -5,13 +5,14 @@
 #   * 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.
 
+{.push raises: [].}
 {.used.}
 
 import
   # Status internals
   chronicles,
   # Beacon chain internals
-  ../../../beacon_chain/spec/[beaconstate, presets, state_transition_epoch],
+  ../../../beacon_chain/spec/[presets, state_transition_epoch],
   ../../../beacon_chain/spec/datatypes/[altair, capella],
   # Test utilities
   ../../testutil,
diff --git a/tests/consensus_spec/deneb/test_fixture_state_transition_epoch.nim b/tests/consensus_spec/deneb/test_fixture_state_transition_epoch.nim
@@ -12,7 +12,7 @@ import
   # Status internals
   chronicles,
   # Beacon chain internals
-  ../../../beacon_chain/spec/[beaconstate, presets, state_transition_epoch],
+  ../../../beacon_chain/spec/[presets, state_transition_epoch],
   ../../../beacon_chain/spec/datatypes/[altair, deneb],
   # Test utilities
   ../../testutil,
diff --git a/tests/consensus_spec/electra/test_fixture_state_transition_epoch.nim b/tests/consensus_spec/electra/test_fixture_state_transition_epoch.nim
@@ -12,7 +12,7 @@ import
   # Status internals
   chronicles,
   # Beacon chain internals
-  ../../../beacon_chain/spec/[beaconstate, presets, state_transition_epoch],
+  ../../../beacon_chain/spec/[presets, state_transition_epoch],
   ../../../beacon_chain/spec/datatypes/[altair, electra],
   # Test utilities
   ../../testutil,
@@ -146,13 +146,11 @@ runSuite(ParticipationFlagDir, "Participation flag updates"):
 # ---------------------------------------------------------------
 runSuite(PendingBalanceDepositsDir, "Pending balance deposits"):
   process_pending_balance_deposits(cfg, state, cache)
-  Result[void, cstring].ok()
 
 # Pending consolidations
 # ---------------------------------------------------------------
 runSuite(PendingConsolidationsDir, "Pending consolidations"):
   process_pending_consolidations(cfg, state)
-  Result[void, cstring].ok()
 
 # Sync committee updates
 # ---------------------------------------------------------------
diff --git a/tests/test_signing_node.nim b/tests/test_signing_node.nim
@@ -92,8 +92,7 @@ func init(
   of ConsensusFork.Deneb:
     return ForkedBeaconBlock.init(contents.denebData.signed_block.message)
   of ConsensusFork.Electra:
-    debugComment "probably like the deneb case"
-    return default(T)
+    return ForkedBeaconBlock.init(contents.electraData.signed_block.message)
 
 proc getBlock(
     fork: ConsensusFork,

Original file line number	Diff line number	Diff line change
`@@ -986,4 +986,3 @@ func ofLen*[T, N](ListType: type List[T, N], n: int): ListType =`
`986`	`986`	`raise newException(SszSizeMismatchError)`
`987`	`987`
`988`	`988`	`template debugComment*(s: string) = discard`
`989`		`-template debugRaiseAssert*(s: string) = discard`