From e8543e9e67d2db24ca60694b383824dea604ee5f Mon Sep 17 00:00:00 2001 From: tersec Date: Fri, 20 Jan 2023 14:31:39 +0000 Subject: [PATCH 1/4] clean up some Nim 1.2 workarounds --- beacon_chain/beacon_chain_db.nim | 10 ---- .../attestation_pool.nim | 6 +-- beacon_chain/spec/beacon_time.nim | 48 +++++++++---------- beacon_chain/spec/helpers.nim | 4 -- ncli/ncli_common.nim | 10 +--- 5 files changed, 26 insertions(+), 52 deletions(-) diff --git a/beacon_chain/beacon_chain_db.nim b/beacon_chain/beacon_chain_db.nim index 21b8b87b43..20542c0c31 100644 --- a/beacon_chain/beacon_chain_db.nim +++ b/beacon_chain/beacon_chain_db.nim @@ -1115,8 +1115,6 @@ proc getStateOnlyMutableValidators( ## not found at all, rollback will not be called # TODO rollback is needed to deal with bug - use `noRollback` to ignore: # https://github.com/nim-lang/Nim/issues/14126 - # TODO RVO is inefficient for large objects: - # https://github.com/nim-lang/Nim/issues/13879 case store.getSnappySSZ(key, toBeaconStateNoImmutableValidators(output)) of GetResult.found: @@ -1156,8 +1154,6 @@ proc getStateOnlyMutableValidators( ## not found at all, rollback will not be called # TODO rollback is needed to deal with bug - use `noRollback` to ignore: # https://github.com/nim-lang/Nim/issues/14126 - # TODO RVO is inefficient for large objects: - # https://github.com/nim-lang/Nim/issues/13879 case store.getSZSSZ(key, toBeaconStateNoImmutableValidators(output)) of GetResult.found: @@ -1195,8 +1191,6 @@ proc getStateOnlyMutableValidators( ## not found at all, rollback will not be called # TODO rollback is needed to deal with bug - use `noRollback` to ignore: # https://github.com/nim-lang/Nim/issues/14126 - # TODO RVO is inefficient for large objects: - # https://github.com/nim-lang/Nim/issues/13879 case store.getSZSSZ(key, toBeaconStateNoImmutableValidators(output)) of GetResult.found: @@ -1257,8 +1251,6 @@ proc getState*( ## not found at all, rollback will not be called # TODO rollback is needed to deal with bug - use `noRollback` to ignore: # https://github.com/nim-lang/Nim/issues/14126 - # TODO RVO is inefficient for large objects: - # https://github.com/nim-lang/Nim/issues/13879 type T = type(output) if not getStateOnlyMutableValidators( @@ -1280,8 +1272,6 @@ proc getState*( ## not found at all, rollback will not be called # TODO rollback is needed to deal with bug - use `noRollback` to ignore: # https://github.com/nim-lang/Nim/issues/14126 - # TODO RVO is inefficient for large objects: - # https://github.com/nim-lang/Nim/issues/13879 type T = type(output) getStateOnlyMutableValidators( db.immutableValidators, db.statesNoVal[T.toFork], key.data, output, diff --git a/beacon_chain/consensus_object_pools/attestation_pool.nim b/beacon_chain/consensus_object_pools/attestation_pool.nim index 2f8ec8e6eb..ea3d59ba4e 100644 --- a/beacon_chain/consensus_object_pools/attestation_pool.nim +++ b/beacon_chain/consensus_object_pools/attestation_pool.nim @@ -664,11 +664,7 @@ proc getAttestationsForBlock*(pool: var AttestationPool, # Fast path for when all remaining candidates fit if candidates.lenu64 < MAX_ATTESTATIONS: candidates.len - 1 else: maxIndex(candidates) - # TODO slot not used; replace with _ when - # https://github.com/nim-lang/Nim/issues/15972 and - # https://github.com/nim-lang/Nim/issues/16217 are - # fixed in Status's Nim. - (_, slot, entry, j) = candidates[candidate] + (_, _, entry, j) = candidates[candidate] candidates.del(candidate) # careful, `del` reorders candidates diff --git a/beacon_chain/spec/beacon_time.nim b/beacon_chain/spec/beacon_time.nim index ecf980bbb4..d1314f2306 100644 --- a/beacon_chain/spec/beacon_time.nim +++ b/beacon_chain/spec/beacon_time.nim @@ -50,46 +50,44 @@ const NANOSECONDS_PER_SLOT = SECONDS_PER_SLOT * 1_000_000_000'u64 -# TODO when https://github.com/nim-lang/Nim/issues/14440 lands in Status's Nim, -# switch proc {.noSideEffect.} to func. template ethTimeUnit*(typ: type) {.dirty.} = - proc `+`*(x: typ, y: uint64): typ {.borrow, noSideEffect.} - proc `-`*(x: typ, y: uint64): typ {.borrow, noSideEffect.} - proc `-`*(x: uint64, y: typ): typ {.borrow, noSideEffect.} + func `+`*(x: typ, y: uint64): typ {.borrow.} + func `-`*(x: typ, y: uint64): typ {.borrow.} + func `-`*(x: uint64, y: typ): typ {.borrow.} # Not closed over type in question (Slot or Epoch) - proc `mod`*(x: typ, y: uint64): uint64 {.borrow, noSideEffect.} - proc `div`*(x: typ, y: uint64): uint64 {.borrow, noSideEffect.} - proc `div`*(x: uint64, y: typ): uint64 {.borrow, noSideEffect.} - proc `-`*(x: typ, y: typ): uint64 {.borrow, noSideEffect.} + func `mod`*(x: typ, y: uint64): uint64 {.borrow.} + func `div`*(x: typ, y: uint64): uint64 {.borrow.} + func `div`*(x: uint64, y: typ): uint64 {.borrow.} + func `-`*(x: typ, y: typ): uint64 {.borrow.} iterator countdown*(a, b: typ, step: Positive = 1): typ = # otherwise we use the signed version that breaks at the boundary for i in countdown(distinctBase(a), distinctBase(b), step): yield typ(i) - proc `*`*(x: typ, y: uint64): uint64 {.borrow, noSideEffect.} + func `*`*(x: typ, y: uint64): uint64 {.borrow.} - proc `+=`*(x: var typ, y: typ) {.borrow, noSideEffect.} - proc `+=`*(x: var typ, y: uint64) {.borrow, noSideEffect.} - proc `-=`*(x: var typ, y: typ) {.borrow, noSideEffect.} - proc `-=`*(x: var typ, y: uint64) {.borrow, noSideEffect.} + func `+=`*(x: var typ, y: typ) {.borrow.} + func `+=`*(x: var typ, y: uint64) {.borrow.} + func `-=`*(x: var typ, y: typ) {.borrow.} + func `-=`*(x: var typ, y: uint64) {.borrow.} # Comparison operators - proc `<`*(x: typ, y: typ): bool {.borrow, noSideEffect.} - proc `<`*(x: typ, y: uint64): bool {.borrow, noSideEffect.} - proc `<`*(x: uint64, y: typ): bool {.borrow, noSideEffect.} - proc `<=`*(x: typ, y: typ): bool {.borrow, noSideEffect.} - proc `<=`*(x: typ, y: uint64): bool {.borrow, noSideEffect.} - proc `<=`*(x: uint64, y: typ): bool {.borrow, noSideEffect.} + func `<`*(x: typ, y: typ): bool {.borrow.} + func `<`*(x: typ, y: uint64): bool {.borrow.} + func `<`*(x: uint64, y: typ): bool {.borrow.} + func `<=`*(x: typ, y: typ): bool {.borrow.} + func `<=`*(x: typ, y: uint64): bool {.borrow.} + func `<=`*(x: uint64, y: typ): bool {.borrow.} - proc `==`*(x: typ, y: typ): bool {.borrow, noSideEffect.} - proc `==`*(x: typ, y: uint64): bool {.borrow, noSideEffect.} - proc `==`*(x: uint64, y: typ): bool {.borrow, noSideEffect.} + func `==`*(x: typ, y: typ): bool {.borrow.} + func `==`*(x: typ, y: uint64): bool {.borrow.} + func `==`*(x: uint64, y: typ): bool {.borrow.} # Nim integration - proc `$`*(x: typ): string {.borrow, noSideEffect.} - proc hash*(x: typ): Hash {.borrow, noSideEffect.} + func `$`*(x: typ): string {.borrow.} + func hash*(x: typ): Hash {.borrow.} template asUInt64*(v: typ): uint64 = distinctBase(v) template shortLog*(v: typ): auto = distinctBase(v) diff --git a/beacon_chain/spec/helpers.nim b/beacon_chain/spec/helpers.nim index 8e4f875ab5..2619631307 100644 --- a/beacon_chain/spec/helpers.nim +++ b/beacon_chain/spec/helpers.nim @@ -135,10 +135,6 @@ func compute_domain*( fork_version: Version, genesis_validators_root: Eth2Digest = ZERO_HASH): Eth2Domain = ## Return the domain for the ``domain_type`` and ``fork_version``. - # - # TODO Can't be used as part of a const/static expression: - # https://github.com/nim-lang/Nim/issues/15952 - # https://github.com/nim-lang/Nim/issues/19969 let fork_data_root = compute_fork_data_root(fork_version, genesis_validators_root) result[0..3] = domain_type.data diff --git a/ncli/ncli_common.nim b/ncli/ncli_common.nim index a39ee85bf7..266685b4dc 100644 --- a/ncli/ncli_common.nim +++ b/ncli/ncli_common.nim @@ -69,10 +69,7 @@ func matchFilenameUnaggregatedFiles(filename: string): bool = # epochNumberRegexStr & epochFileNameExtension filename.len == epochInfoFileNameDigitsCount + epochFileNameExtension.len and filename.endsWith(epochFileNameExtension) and - # TODO should use toOpenArray, but - # https://github.com/nim-lang/Nim/issues/15952 - # https://github.com/nim-lang/Nim/issues/19969 - allIt(filename[0 ..< epochInfoFileNameDigitsCount], it.isDigit) + allIt(filename.toOpenArray(0, epochInfoFileNameDigitsCount - 1), it.isDigit) static: for filename in [ @@ -96,10 +93,7 @@ func matchFilenameAggregatedFiles(filename: string): bool = # epochNumberRegexStr & "_" & epochNumberRegexStr & epochFileNameExtension filename.len == epochInfoFileNameDigitsCount * 2 + "_".len + epochFileNameExtension.len and filename.endsWith(epochFileNameExtension) and - # TODO should use toOpenArray, but - # https://github.com/nim-lang/Nim/issues/15952 - # https://github.com/nim-lang/Nim/issues/19969 - allIt(filename[0 ..< epochInfoFileNameDigitsCount], it.isDigit) and + allIt(filename.toOpenArray(0, epochInfoFileNameDigitsCount - 1), it.isDigit) and filename[epochInfoFileNameDigitsCount] == '_' and allIt( filename[epochInfoFileNameDigitsCount + 1 ..< 2 * epochInfoFileNameDigitsCount + 1], From d513a1637850de12f802592b6ff21938c6d0a80c Mon Sep 17 00:00:00 2001 From: tersec Date: Fri, 20 Jan 2023 14:46:55 +0000 Subject: [PATCH 2/4] re-add notes about JS backend --- beacon_chain/spec/helpers.nim | 3 +++ ncli/ncli_common.nim | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/beacon_chain/spec/helpers.nim b/beacon_chain/spec/helpers.nim index 2619631307..0b8bd582f3 100644 --- a/beacon_chain/spec/helpers.nim +++ b/beacon_chain/spec/helpers.nim @@ -135,6 +135,9 @@ func compute_domain*( fork_version: Version, genesis_validators_root: Eth2Digest = ZERO_HASH): Eth2Domain = ## Return the domain for the ``domain_type`` and ``fork_version``. + # + # TODO toOpenArray can't be used from JavaScript backend + # https://github.com/nim-lang/Nim/issues/15952 let fork_data_root = compute_fork_data_root(fork_version, genesis_validators_root) result[0..3] = domain_type.data diff --git a/ncli/ncli_common.nim b/ncli/ncli_common.nim index 266685b4dc..e052ef95c2 100644 --- a/ncli/ncli_common.nim +++ b/ncli/ncli_common.nim @@ -69,6 +69,8 @@ func matchFilenameUnaggregatedFiles(filename: string): bool = # epochNumberRegexStr & epochFileNameExtension filename.len == epochInfoFileNameDigitsCount + epochFileNameExtension.len and filename.endsWith(epochFileNameExtension) and + # TODO can't be used with JS backend: + # https://github.com/nim-lang/Nim/issues/15952 allIt(filename.toOpenArray(0, epochInfoFileNameDigitsCount - 1), it.isDigit) static: @@ -93,6 +95,8 @@ func matchFilenameAggregatedFiles(filename: string): bool = # epochNumberRegexStr & "_" & epochNumberRegexStr & epochFileNameExtension filename.len == epochInfoFileNameDigitsCount * 2 + "_".len + epochFileNameExtension.len and filename.endsWith(epochFileNameExtension) and + # TODO can't be used with JS backend: + # https://github.com/nim-lang/Nim/issues/15952 allIt(filename.toOpenArray(0, epochInfoFileNameDigitsCount - 1), it.isDigit) and filename[epochInfoFileNameDigitsCount] == '_' and allIt( From bab22a770880da3660d883ce7374f3814a3068c3 Mon Sep 17 00:00:00 2001 From: tersec Date: Fri, 20 Jan 2023 14:52:06 +0000 Subject: [PATCH 3/4] another proc/noSideEffect -> func --- beacon_chain/eth1/eth1_monitor.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beacon_chain/eth1/eth1_monitor.nim b/beacon_chain/eth1/eth1_monitor.nim index 3de2da9ea3..2178de8184 100644 --- a/beacon_chain/eth1/eth1_monitor.nim +++ b/beacon_chain/eth1/eth1_monitor.nim @@ -620,7 +620,7 @@ proc forkchoiceUpdated*( withdrawals: mapIt(withdrawals.get, it.asEngineWithdrawal)))) # TODO can't be defined within exchangeTransitionConfiguration -proc `==`(x, y: Quantity): bool {.borrow, noSideEffect.} +func `==`(x, y: Quantity): bool {.borrow.} type EtcStatus {.pure.} = enum From 949a4d2cd96f7b0cc4f4388327c3184ca61244a0 Mon Sep 17 00:00:00 2001 From: tersec Date: Fri, 20 Jan 2023 15:15:24 +0000 Subject: [PATCH 4/4] revert ncli/ncli_common.nim changes; 19969 evidently wasn't backported to 1.6 --- ncli/ncli_common.nim | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/ncli/ncli_common.nim b/ncli/ncli_common.nim index e052ef95c2..a39ee85bf7 100644 --- a/ncli/ncli_common.nim +++ b/ncli/ncli_common.nim @@ -69,9 +69,10 @@ func matchFilenameUnaggregatedFiles(filename: string): bool = # epochNumberRegexStr & epochFileNameExtension filename.len == epochInfoFileNameDigitsCount + epochFileNameExtension.len and filename.endsWith(epochFileNameExtension) and - # TODO can't be used with JS backend: + # TODO should use toOpenArray, but # https://github.com/nim-lang/Nim/issues/15952 - allIt(filename.toOpenArray(0, epochInfoFileNameDigitsCount - 1), it.isDigit) + # https://github.com/nim-lang/Nim/issues/19969 + allIt(filename[0 ..< epochInfoFileNameDigitsCount], it.isDigit) static: for filename in [ @@ -95,9 +96,10 @@ func matchFilenameAggregatedFiles(filename: string): bool = # epochNumberRegexStr & "_" & epochNumberRegexStr & epochFileNameExtension filename.len == epochInfoFileNameDigitsCount * 2 + "_".len + epochFileNameExtension.len and filename.endsWith(epochFileNameExtension) and - # TODO can't be used with JS backend: + # TODO should use toOpenArray, but # https://github.com/nim-lang/Nim/issues/15952 - allIt(filename.toOpenArray(0, epochInfoFileNameDigitsCount - 1), it.isDigit) and + # https://github.com/nim-lang/Nim/issues/19969 + allIt(filename[0 ..< epochInfoFileNameDigitsCount], it.isDigit) and filename[epochInfoFileNameDigitsCount] == '_' and allIt( filename[epochInfoFileNameDigitsCount + 1 ..< 2 * epochInfoFileNameDigitsCount + 1],