From 9c669cf57aded3c58cfc85d4de756c79f7ef61fa Mon Sep 17 00:00:00 2001 From: tersec Date: Tue, 14 May 2024 12:19:24 +0300 Subject: [PATCH] some debugRaiseAssert to debugComment; unblock some spec tests (#6284) --- beacon_chain/nimbus_beacon_node.nim | 1 - beacon_chain/spec/beaconstate.nim | 4 ++-- beacon_chain/spec/datatypes/electra.nim | 2 +- beacon_chain/spec/signatures_batch.nim | 2 +- beacon_chain/spec/state_transition.nim | 2 +- beacon_chain/spec/state_transition_block.nim | 6 +++--- beacon_chain/spec/state_transition_epoch.nim | 8 ++++---- tests/consensus_spec/test_fixture_sanity_blocks.nim | 8 -------- tests/test_beacon_chain_db.nim | 2 +- tests/test_signing_node.nim | 6 +++--- tests/test_toblindedblock.nim | 4 ++-- tests/testblockutil.nim | 2 +- 12 files changed, 19 insertions(+), 28 deletions(-) diff --git a/beacon_chain/nimbus_beacon_node.nim b/beacon_chain/nimbus_beacon_node.nim index faefb772c..fcc898adb 100644 --- a/beacon_chain/nimbus_beacon_node.nim +++ b/beacon_chain/nimbus_beacon_node.nim @@ -1372,7 +1372,6 @@ proc updateGossipStatus(node: BeaconNode, slot: Slot) {.async.} = for gossipFork in oldGossipForks: removeMessageHandlers[gossipFork](node, forkDigests[gossipFork]) - debugRaiseAssert "electra does have different gossip, add add/RemoveElectraFoo" const addMessageHandlers: array[ConsensusFork, auto] = [ addPhase0MessageHandlers, addAltairMessageHandlers, diff --git a/beacon_chain/spec/beaconstate.nim b/beacon_chain/spec/beaconstate.nim index 7ecffd73f..87d0db2d4 100644 --- a/beacon_chain/spec/beaconstate.nim +++ b/beacon_chain/spec/beaconstate.nim @@ -596,7 +596,7 @@ iterator get_attesting_indices_iter*( aggregation_bits: ElectraCommitteeValidatorsBits, committee_bits: auto, cache: var StateCache): ValidatorIndex = - debugRaiseAssert "replace this implementation with actual iterator, after checking on conditions re repeat vals, ordering, etc; this is almost direct transcription of spec link algorithm in one of the places it doesn't make sense" + debugComment "replace this implementation with actual iterator, after checking on conditions re repeat vals, ordering, etc; this is almost direct transcription of spec link algorithm in one of the places it doesn't make sense" ## Return the set of attesting indices corresponding to ``aggregation_bits`` ## and ``committee_bits``. var output: HashSet[ValidatorIndex] @@ -1221,7 +1221,7 @@ func queue_excess_active_balance( if balance > MIN_ACTIVATION_BALANCE.Gwei: let excess_balance = balance - MIN_ACTIVATION_BALANCE.Gwei state.balances.mitem(index) = MIN_ACTIVATION_BALANCE.Gwei - debugRaiseAssert "maybe check return value" + debugComment "maybe check return value" discard state.pending_balance_deposits.add( PendingBalanceDeposit(index: index.uint64, amount: excess_balance) ) diff --git a/beacon_chain/spec/datatypes/electra.nim b/beacon_chain/spec/datatypes/electra.nim index cefbf23fb..4d9429715 100644 --- a/beacon_chain/spec/datatypes/electra.nim +++ b/beacon_chain/spec/datatypes/electra.nim @@ -712,7 +712,7 @@ template asTrusted*( MsgTrustedSignedBeaconBlock): TrustedSignedBeaconBlock = isomorphicCast[TrustedSignedBeaconBlock](x) -debugRaiseAssert "this whole section with getValidatorIndices/shortLog needs refactoring and probably can be combined with identical implementations elsewhere" +debugComment "this whole section with getValidatorIndices/shortLog needs refactoring and probably can be combined with identical implementations elsewhere" from std/sets import toHashSet diff --git a/beacon_chain/spec/signatures_batch.nim b/beacon_chain/spec/signatures_batch.nim index 60ef3fd07..7e1700c49 100644 --- a/beacon_chain/spec/signatures_batch.nim +++ b/beacon_chain/spec/signatures_batch.nim @@ -464,7 +464,7 @@ proc collectSignatureSets*( block: # 9. Consolidations - debugRaiseAssert "check consolidations signatures" + debugComment "check consolidations signatures" ok() diff --git a/beacon_chain/spec/state_transition.nim b/beacon_chain/spec/state_transition.nim index e55e191c3..b533500ca 100644 --- a/beacon_chain/spec/state_transition.nim +++ b/beacon_chain/spec/state_transition.nim @@ -448,7 +448,7 @@ func partialBeaconBlock*( when consensusFork >= ConsensusFork.Deneb: res.body.blob_kzg_commitments = execution_payload.blobsBundle.commitments - debugRaiseAssert "either consolidate this within separate function or recombine, re when consensusFork >= foo and atts/attslashings; here to allow noninterference with pre-pectra" + debugComment "either consolidate this within separate function or recombine, re when consensusFork >= foo and atts/attslashings; here to allow noninterference with pre-pectra" res diff --git a/beacon_chain/spec/state_transition_block.nim b/beacon_chain/spec/state_transition_block.nim index d42f06fae..86108788e 100644 --- a/beacon_chain/spec/state_transition_block.nim +++ b/beacon_chain/spec/state_transition_block.nim @@ -323,7 +323,7 @@ proc apply_deposit( when typeof(state).kind < ConsensusFork.Electra: increase_balance(state, index.get(), amount) else: - debugRaiseAssert "check hashlist add return" + debugComment "check hashlist add return" discard state.pending_balance_deposits.add PendingBalanceDeposit( index: index.get.uint64, amount: amount) # [Modified in Electra:EIP-7251] @@ -359,7 +359,7 @@ proc apply_deposit( if not state.inactivity_scores.add(0'u64): return err("apply_deposit: too many validators (inactivity_scores)") when typeof(state).kind >= ConsensusFork.Electra: - debugRaiseAssert "check hashlist add return" + debugComment "check hashlist add return" # [New in Electra:EIP7251] discard state.pending_balance_deposits.add PendingBalanceDeposit( @@ -450,7 +450,7 @@ proc check_voluntary_exit*( when typeof(state).kind >= ConsensusFork.Electra: # Only exit validator if it has no pending withdrawals in the queue - debugRaiseAssert "truncating" + debugComment "truncating" if not (get_pending_balance_to_withdraw( state, voluntary_exit.validator_index.ValidatorIndex) == 0.Gwei): return err("Exit: still has pending withdrawals") diff --git a/beacon_chain/spec/state_transition_epoch.nim b/beacon_chain/spec/state_transition_epoch.nim index e23d1fd3e..ff83afb0c 100644 --- a/beacon_chain/spec/state_transition_epoch.nim +++ b/beacon_chain/spec/state_transition_epoch.nim @@ -1059,7 +1059,7 @@ func process_effective_balance_updates*(state: var electra.BeaconState) = balance = state.balances.item(vidx) effective_balance = state.validators.item(vidx).effective_balance if effective_balance_might_update(balance, effective_balance): - debugRaiseAssert "amortize validator read access" + debugComment "amortize validator read access" # Wrapping MAX_EFFECTIVE_BALANCE_ELECTRA.Gwei and # MIN_ACTIVATION_BALANCE.Gwei in static() results # in @@ -1232,7 +1232,7 @@ func process_pending_balance_deposits*( for deposit in state.pending_balance_deposits: if processed_amount + deposit.amount > available_for_processing: break - debugRaiseAssert "do this validatorindex check properly (it truncates)" + debugComment "do this validatorindex check properly (it truncates)" increase_balance(state, deposit.index.ValidatorIndex, deposit.amount) processed_amount += deposit.amount inc next_deposit_index @@ -1260,12 +1260,12 @@ func process_pending_consolidations*(cfg: RuntimeConfig, state: var electra.Beac break # Churn any target excess active balance of target and raise its max - debugRaiseAssert "truncating integer conversion" + debugComment "truncating integer conversion" switch_to_compounding_validator( state, pending_consolidation.target_index.ValidatorIndex) # Move active balance to target. Excess balance is withdrawable. - debugRaiseAssert "Truncating" + debugComment "Truncating" let active_balance = get_active_balance( state, pending_consolidation.source_index.ValidatorIndex) decrease_balance( diff --git a/tests/consensus_spec/test_fixture_sanity_blocks.nim b/tests/consensus_spec/test_fixture_sanity_blocks.nim index 27d5c4b17..22b2eb35d 100644 --- a/tests/consensus_spec/test_fixture_sanity_blocks.nim +++ b/tests/consensus_spec/test_fixture_sanity_blocks.nim @@ -79,14 +79,6 @@ template runForkBlockTests(consensusFork: static ConsensusFork) = suite "EF - " & forkHumanName & " - Sanity - Blocks " & preset(): for kind, path in walkDir(SanityBlocksDir, relative = true, checkDir = true): - debugRaiseAssert "this should be fixed in alpha.2; remove workaround" - if consensusFork == ConsensusFork.Electra and path in [ - "multiple_consolidations_above_churn", # no pre.ssz - "multiple_consolidations_below_churn", # assert block.parent_root == hash_tree_root(state.latest_block_header) - "multiple_consolidations_equal_churn", # assert block.parent_root == hash_tree_root(state.latest_block_header) - "multiple_consolidations_equal_twice_churn", # assert block.parent_root == hash_tree_root(state.latest_block_header) - ]: - continue consensusFork.runTest( "EF - " & forkHumanName & " - Sanity - Blocks", SanityBlocksDir, suiteName, path) diff --git a/tests/test_beacon_chain_db.nim b/tests/test_beacon_chain_db.nim index e9ce5f11c..c81bb95c9 100644 --- a/tests/test_beacon_chain_db.nim +++ b/tests/test_beacon_chain_db.nim @@ -113,7 +113,7 @@ proc getTestStates(consensusFork: ConsensusFork): auto = testStates -debugRaiseAssert "add some electra states, and test electra state/block loading/etc" +debugComment "add some electra states, and test electra state/block loading/etc" # Each set of states gets used twice, so scope them to module let diff --git a/tests/test_signing_node.nim b/tests/test_signing_node.nim index 6805aac89..325e29cf8 100644 --- a/tests/test_signing_node.nim +++ b/tests/test_signing_node.nim @@ -93,7 +93,7 @@ func init(T: type ForkedBeaconBlock, contents: ProduceBlockResponseV2): T = of ConsensusFork.Deneb: return ForkedBeaconBlock.init(contents.denebData.`block`) of ConsensusFork.Electra: - debugRaiseAssert "probably like the deneb case" + debugComment "probably like the deneb case" return default(T) proc getBlock( @@ -108,7 +108,7 @@ proc getBlock( of ConsensusFork.Capella: CapellaBlock % [feeRecipient] of ConsensusFork.Deneb: DenebBlockContents % [feeRecipient] of ConsensusFork.Electra: - debugRaiseAssert "electra test signing node getblock" + debugComment "electra test signing node getblock" raiseAssert "electra unsupported" except ValueError: # https://github.com/nim-lang/Nim/pull/23356 @@ -255,7 +255,7 @@ func getRemoteKeystoreData(data: string, basePort: int, pubkey: publicKey ) - debugRaiseAssert "check electraIndex" + debugComment "check electraIndex" ok case rt of RemoteSignerType.Web3Signer: KeystoreData( diff --git a/tests/test_toblindedblock.nim b/tests/test_toblindedblock.nim index 4d94fd334..b63400636 100644 --- a/tests/test_toblindedblock.nim +++ b/tests/test_toblindedblock.nim @@ -54,7 +54,7 @@ template bellatrix_steps() = check: b.message.body.proposer_slashings.add(default(ProposerSlashing)) do_check when false: - debugRaiseAssert "both Electra attestations and attestation slashings need to be done iff Electra" + debugComment "both Electra attestations and attestation slashings need to be done iff Electra" check: b.message.body.attester_slashings.add(default(phase0.AttesterSlashing)) do_check @@ -140,4 +140,4 @@ suite "Blinded block conversions": bellatrix_steps capella_steps deneb_steps - debugRaiseAssert "add electra_steps" \ No newline at end of file + debugComment "add electra_steps" \ No newline at end of file diff --git a/tests/testblockutil.nim b/tests/testblockutil.nim index d46402edc..da4f1a29a 100644 --- a/tests/testblockutil.nim +++ b/tests/testblockutil.nim @@ -207,7 +207,7 @@ proc addTestBlock*( else: default(bellatrix.ExecutionPayloadForSigning) - debugRaiseAssert "addTestBlock Electra attestation support" + debugComment "addTestBlock Electra attestation support" makeBeaconBlock( cfg,