Skip to content

Commit 26d0281

Browse files
etan-statuszah
authored andcommitted
fix incorrect config validation regression from #5959 (#5966)
During refactoring of #5959, some implicit `return` were overlooked, resulting in spurious `err()` being returned without message. ``` {"lvl":"WRN","ts":"2024-02-26 10:12:20.469+00:00","msg":"Beacon nodes report different configuration values","reason":"","service":"fallback_service","node":"http://127.0.0.1:9303[Nimbus/v24.2.1-4e9bc7-stateofus]","node_index":0,"node_roles":"AGBSDT"} ``` Correcting the helpers to return explicit result in all exhaustive cases so that this cannot happen anymore by accident.
1 parent 4fcfed2 commit 26d0281

File tree

1 file changed

+14
-12
lines changed

1 file changed

+14
-12
lines changed

beacon_chain/validator_client/common.nim

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1440,23 +1440,24 @@ proc updateRuntimeConfig*(vc: ValidatorClientRef,
14401440
localForkEpoch: Epoch,
14411441
forkVersion: Opt[Version]): Result[void, string] =
14421442
if localForkVersion.isNone():
1443-
discard # Potentially discovered new fork, save it at end of function
1443+
ok() # Potentially discovered new fork, save it at end of function
14441444
else:
14451445
if forkVersion.isSome():
14461446
if forkVersion.get() == localForkVersion.get():
1447-
discard # Already known
1447+
ok() # Already known
14481448
else:
1449-
return err("Beacon node has conflicting " &
1450-
consensusFork.forkVersionConfigKey() & " value")
1449+
err("Beacon node has conflicting " &
1450+
consensusFork.forkVersionConfigKey() & " value")
14511451
else:
14521452
if wallEpoch < localForkEpoch:
14531453
debug "Beacon node must be updated before fork activates",
14541454
node = node,
14551455
consensusFork,
14561456
forkEpoch = localForkEpoch
1457+
ok()
14571458
else:
1458-
return err("Beacon node must be updated and report correct " &
1459-
$consensusFork & " config value")
1459+
err("Beacon node must be updated and report correct " &
1460+
$consensusFork & " config value")
14601461

14611462
? ConsensusFork.Capella.validateForkVersionCompatibility(
14621463
localForkConfig.capellaVersion,
@@ -1468,23 +1469,24 @@ proc updateRuntimeConfig*(vc: ValidatorClientRef,
14681469
localForkEpoch: Epoch,
14691470
forkEpoch: Epoch): Result[void, string] =
14701471
if localForkEpoch == FAR_FUTURE_EPOCH:
1471-
discard # Potentially discovered new fork, save it at end of function
1472+
ok() # Potentially discovered new fork, save it at end of function
14721473
else:
14731474
if forkEpoch != FAR_FUTURE_EPOCH:
14741475
if forkEpoch == localForkEpoch:
1475-
discard # Already known
1476+
ok() # Already known
14761477
else:
1477-
return err("Beacon node has conflicting " &
1478-
consensusFork.forkEpochConfigKey() & " value")
1478+
err("Beacon node has conflicting " &
1479+
consensusFork.forkEpochConfigKey() & " value")
14791480
else:
14801481
if wallEpoch < localForkEpoch:
14811482
debug "Beacon node must be updated before fork activates",
14821483
node = node,
14831484
consensusFork,
14841485
forkEpoch = localForkEpoch
1486+
ok()
14851487
else:
1486-
return err("Beacon node must be updated and report correct " &
1487-
$consensusFork & " config value")
1488+
err("Beacon node must be updated and report correct " &
1489+
$consensusFork & " config value")
14881490

14891491
? ConsensusFork.Altair.validateForkEpochCompatibility(
14901492
localForkConfig.altairEpoch, forkConfig.altairEpoch)

0 commit comments

Comments
 (0)