Skip to content

Commit

Permalink
allow higher MIN_EPOCHS_FOR_BLOCK_REQUESTS than safe minimum (#5590)
Browse files Browse the repository at this point in the history
Gnosis uses `MIN_EPOCHS_FOR_BLOCK_REQUESTS` = 33024, but the computed
safe minimum (that Nimbus was using) is 2304. Relax the compatibility
check to allow `MIN_EPOCHS_FOR_BLOCK_REQUESTS` above the safe minimum
and honor `config.yaml` preferences for `MIN_EPOCHS_FOR_BLOCK_REQUESTS`.
  • Loading branch information
etan-status authored Nov 10, 2023
1 parent f48ce6c commit eb35039
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 15 deletions.
33 changes: 18 additions & 15 deletions beacon_chain/spec/presets.nim
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ type
# TODO GOSSIP_MAX_SIZE*: uint64
# TODO MAX_REQUEST_BLOCKS*: uint64
# TODO EPOCHS_PER_SUBNET_SUBSCRIPTION*: uint64
# TODO MIN_EPOCHS_FOR_BLOCK_REQUESTS*: uint64
MIN_EPOCHS_FOR_BLOCK_REQUESTS*: uint64
# TODO MAX_CHUNK_SIZE*: uint64
# TODO TTFB_TIMEOUT*: uint64
# TODO RESP_TIMEOUT*: uint64
Expand Down Expand Up @@ -240,7 +240,7 @@ when const_preset == "mainnet":
# `2**8` (= 256)
# TODO EPOCHS_PER_SUBNET_SUBSCRIPTION: 256,
# `MIN_VALIDATOR_WITHDRAWABILITY_DELAY + CHURN_LIMIT_QUOTIENT // 2` (= 33024, ~5 months)
# TODO MIN_EPOCHS_FOR_BLOCK_REQUESTS: 33024,
MIN_EPOCHS_FOR_BLOCK_REQUESTS: 33024,
# `10 * 2**20` (=10485760, 10 MiB)
# TODO MAX_CHUNK_SIZE: 10485760,
# 5s
Expand Down Expand Up @@ -385,7 +385,7 @@ elif const_preset == "gnosis":
# `2**8` (= 256)
# TODO EPOCHS_PER_SUBNET_SUBSCRIPTION: 256,
# `MIN_VALIDATOR_WITHDRAWABILITY_DELAY + CHURN_LIMIT_QUOTIENT // 2` (= 33024, ~5 months)
# TODO MIN_EPOCHS_FOR_BLOCK_REQUESTS: 33024,
MIN_EPOCHS_FOR_BLOCK_REQUESTS: 33024,
# `10 * 2**20` (=10485760, 10 MiB)
# TODO MAX_CHUNK_SIZE: 10485760,
# 5s
Expand Down Expand Up @@ -422,7 +422,7 @@ elif const_preset == "minimal":

const SECONDS_PER_SLOT* {.intdefine.}: uint64 = 6

# https://github.com/ethereum/consensus-specs/blob/v1.3.0/configs/minimal.yaml
# https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.4/configs/minimal.yaml
const defaultRuntimeConfig* = RuntimeConfig(
# Minimal config

Expand Down Expand Up @@ -527,7 +527,7 @@ elif const_preset == "minimal":
# `2**8` (= 256)
# TODO EPOCHS_PER_SUBNET_SUBSCRIPTION: 256,
# [customized] `MIN_VALIDATOR_WITHDRAWABILITY_DELAY + CHURN_LIMIT_QUOTIENT // 2` (= 272)
# TODO MIN_EPOCHS_FOR_BLOCK_REQUESTS: 272,
MIN_EPOCHS_FOR_BLOCK_REQUESTS: 272,
# `10 * 2**20` (=10485760, 10 MiB)
# TODO MAX_CHUNK_SIZE: 10485760,
# 5s
Expand Down Expand Up @@ -587,7 +587,7 @@ const SLOTS_PER_SYNC_COMMITTEE_PERIOD* =
SLOTS_PER_EPOCH * EPOCHS_PER_SYNC_COMMITTEE_PERIOD

# https://github.com/ethereum/consensus-specs/blob/v1.4.0-alpha.3/specs/phase0/p2p-interface.md#configuration
func MIN_EPOCHS_FOR_BLOCK_REQUESTS*(cfg: RuntimeConfig): uint64 =
func safeMinEpochsForBlockRequests*(cfg: RuntimeConfig): uint64 =
cfg.MIN_VALIDATOR_WITHDRAWABILITY_DELAY + cfg.CHURN_LIMIT_QUOTIENT div 2

func parse(T: type uint64, input: string): T {.raises: [ValueError].} =
Expand Down Expand Up @@ -668,30 +668,33 @@ proc readRuntimeConfig*(

# Certain config keys are baked into the binary at compile-time
# and cannot be overridden via config.
template checkCompatibility(constValue: untyped, name: string): untyped =
template checkCompatibility(
constValue: untyped, name: string, operator: untyped = `==`): untyped =
if values.hasKey(name):
const opDesc = astToStr(operator)
try:
let value = parse(typeof(constValue), values[name])
when constValue is distinct:
if distinctBase(value) != distinctBase(constValue):
if not operator(distinctBase(value), distinctBase(constValue)):
raise (ref PresetFileError)(msg:
"Cannot override config" &
" (compiled: " & name & "=" & $distinctBase(constValue) &
" (required: " & name & opDesc & $distinctBase(constValue) &
" - config: " & name & "=" & values[name] & ")")
else:
if value != constValue:
if not operator(value, constValue):
raise (ref PresetFileError)(msg:
"Cannot override config" &
" (compiled: " & name & "=" & $constValue &
" (required: " & name & opDesc & $constValue &
" - config: " & name & "=" & values[name] & ")")
values.del name
except ValueError:
raise (ref PresetFileError)(msg: "Unable to parse " & name)

template checkCompatibility(constValue: untyped): untyped =
template checkCompatibility(
constValue: untyped, operator: untyped = `==`): untyped =
block:
const name = astToStr(constValue)
checkCompatibility(constValue, name)
checkCompatibility(constValue, name, operator)

checkCompatibility SECONDS_PER_SLOT

Expand Down Expand Up @@ -785,8 +788,8 @@ proc readRuntimeConfig*(
msg: "Config not compatible with binary, compile with -d:const_preset=" & cfg.PRESET_BASE)

# Requires initialized `cfg`
checkCompatibility cfg.MIN_EPOCHS_FOR_BLOCK_REQUESTS,
"MIN_EPOCHS_FOR_BLOCK_REQUESTS"
checkCompatibility cfg.safeMinEpochsForBlockRequests(),
"MIN_EPOCHS_FOR_BLOCK_REQUESTS", `>=`

var unknowns: seq[string]
for name in values.keys:
Expand Down
1 change: 1 addition & 0 deletions tests/test_blockchain_dag.nim
Original file line number Diff line number Diff line change
Expand Up @@ -1145,6 +1145,7 @@ suite "Pruning":
var res = defaultRuntimeConfig
res.MIN_VALIDATOR_WITHDRAWABILITY_DELAY = 4
res.CHURN_LIMIT_QUOTIENT = 1
res.MIN_EPOCHS_FOR_BLOCK_REQUESTS = res.safeMinEpochsForBlockRequests()
doAssert res.MIN_EPOCHS_FOR_BLOCK_REQUESTS == 4
res
db = makeTestDB(SLOTS_PER_EPOCH)
Expand Down

0 comments on commit eb35039

Please sign in to comment.