-
Notifications
You must be signed in to change notification settings - Fork 261
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
clean up exchange configuration handling #4126
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -136,7 +136,6 @@ type | |||
|
||||
exchangedConfiguration*: bool | ||||
terminalBlockHash*: Option[BlockHash] | ||||
terminalBlockNumber*: Option[Quantity] | ||||
|
||||
runFut: Future[void] | ||||
stopFut: Future[void] | ||||
|
@@ -547,7 +546,6 @@ type | |||
EtcStatus {.pure.} = enum | ||||
exchangeError | ||||
mismatch | ||||
localConfigurationUpdated | ||||
match | ||||
|
||||
proc exchangeTransitionConfiguration*(p: Eth1Monitor): Future[EtcStatus] {.async.} = | ||||
|
@@ -561,67 +559,46 @@ proc exchangeTransitionConfiguration*(p: Eth1Monitor): Future[EtcStatus] {.async | |||
if dataProvider.isNil: | ||||
return EtcStatus.exchangeError | ||||
|
||||
# https://github.com/ethereum/execution-apis/blob/main/src/engine/specification.md#engine_exchangetransitionconfigurationv1 | ||||
arnetheduck marked this conversation as resolved.
Show resolved
Hide resolved
|
||||
let consensusCfg = TransitionConfigurationV1( | ||||
terminalTotalDifficulty: p.depositsChain.cfg.TERMINAL_TOTAL_DIFFICULTY, | ||||
terminalBlockHash: | ||||
if p.terminalBlockHash.isSome: | ||||
p.terminalBlockHash.get | ||||
else: | ||||
(static default BlockHash), | ||||
terminalBlockNumber: | ||||
if p.terminalBlockNumber.isSome: | ||||
p.terminalBlockNumber.get | ||||
else: | ||||
(static default Quantity)) | ||||
terminalBlockHash: p.depositsChain.cfg.TERMINAL_BLOCK_HASH, | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The old code was not fully wrong here I think. We still need to consider the command-line override that the user may supply. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we consider the command line here: nimbus-eth2/beacon_chain/nimbus_beacon_node.nim Line 1830 in 667c3c9
|
||||
terminalBlockNumber: Quantity 0) | ||||
let executionCfg = | ||||
try: | ||||
awaitWithRetries( | ||||
dataProvider.web3.provider.engine_exchangeTransitionConfigurationV1( | ||||
consensusCfg), | ||||
timeout = 1.seconds) | ||||
except CatchableError as err: | ||||
error "Failed to exchange transition configuration", err = err.msg | ||||
warn "Failed to exchange transition configuration", err = err.msg | ||||
return EtcStatus.exchangeError | ||||
|
||||
if consensusCfg.terminalTotalDifficulty != executionCfg.terminalTotalDifficulty: | ||||
warn "Engine API configured with different terminal total difficulty", | ||||
engineAPI_value = executionCfg.terminalTotalDifficulty, | ||||
localValue = consensusCfg.terminalTotalDifficulty | ||||
return EtcStatus.mismatch | ||||
|
||||
if not p.exchangedConfiguration: | ||||
# Log successful engine configuration exchange once at startup | ||||
p.exchangedConfiguration = true | ||||
info "Exchanged engine configuration", | ||||
ttd = executionCfg.terminalTotalDifficulty, | ||||
terminalBlockHash = executionCfg.terminalBlockHash, | ||||
terminalBlockNumber = executionCfg.terminalBlockNumber.uint64 | ||||
|
||||
if p.terminalBlockNumber.isSome and p.terminalBlockHash.isSome: | ||||
var res = EtcStatus.match | ||||
|
||||
if consensusCfg.terminalBlockNumber != executionCfg.terminalBlockNumber: | ||||
return | ||||
if consensusCfg.terminalTotalDifficulty != executionCfg.terminalTotalDifficulty: | ||||
error "Engine API configured with different terminal total difficulty", | ||||
engineAPI_value = executionCfg.terminalTotalDifficulty, | ||||
localValue = consensusCfg.terminalTotalDifficulty | ||||
EtcStatus.mismatch | ||||
elif consensusCfg.terminalBlockNumber != executionCfg.terminalBlockNumber: | ||||
warn "Engine API reporting different terminal block number", | ||||
engineAPI_value = executionCfg.terminalBlockNumber.uint64, | ||||
localValue = consensusCfg.terminalBlockNumber.uint64 | ||||
res = EtcStatus.mismatch | ||||
if consensusCfg.terminalBlockHash != executionCfg.terminalBlockHash: | ||||
EtcStatus.mismatch | ||||
elif consensusCfg.terminalBlockHash != executionCfg.terminalBlockHash: | ||||
warn "Engine API reporting different terminal block hash", | ||||
engineAPI_value = executionCfg.terminalBlockHash, | ||||
localValue = consensusCfg.terminalBlockHash | ||||
res = EtcStatus.mismatch | ||||
return res | ||||
else: | ||||
if executionCfg.terminalBlockHash == default BlockHash: | ||||
# If TERMINAL_BLOCK_HASH is stubbed with | ||||
# 0x0000000000000000000000000000000000000000000000000000000000000000 then | ||||
# TERMINAL_BLOCK_HASH and TERMINAL_BLOCK_NUMBER parameters MUST NOT take | ||||
# an effect. | ||||
return EtcStatus.match | ||||
|
||||
p.terminalBlockNumber = some executionCfg.terminalBlockNumber | ||||
p.terminalBlockHash = some executionCfg.terminalBlockHash | ||||
return EtcStatus.localConfigurationUpdated | ||||
EtcStatus.mismatch | ||||
else: | ||||
if not p.exchangedConfiguration: | ||||
# Log successful engine configuration exchange once at startup | ||||
p.exchangedConfiguration = true | ||||
info "Exchanged engine configuration", | ||||
terminalTotalDifficulty = executionCfg.terminalTotalDifficulty, | ||||
terminalBlockHash = executionCfg.terminalBlockHash, | ||||
terminalBlockNumber = executionCfg.terminalBlockNumber.uint64 | ||||
EtcStatus.match | ||||
|
||||
template readJsonField(j: JsonNode, fieldName: string, ValueType: type): untyped = | ||||
var res: ValueType | ||||
|
@@ -1374,11 +1351,7 @@ proc startEth1Syncing(m: Eth1Monitor, delayBeforeStart: Duration) {.async.} = | |||
|
||||
if m.currentEpoch >= m.cfg.BELLATRIX_FORK_EPOCH: | ||||
let status = await m.exchangeTransitionConfiguration() | ||||
if status == EtcStatus.localConfigurationUpdated: | ||||
info "Obtained terminal block from Engine API", | ||||
terminalBlockNumber = m.terminalBlockNumber.get.uint64, | ||||
terminalBlockHash = m.terminalBlockHash.get | ||||
elif status != EtcStatus.match and isFirstRun and m.requireEngineAPI: | ||||
if status != EtcStatus.match and isFirstRun and m.requireEngineAPI: | ||||
fatal "The Bellatrix hard fork requires the beacon node to be connected to a properly configured Engine API end-point. " & | ||||
"See https://nimbus.guide/merge.html for more details. " & | ||||
"If you want to temporarily continue operating Nimbus without configuring an Engine API end-point, " & | ||||
|
@@ -1531,6 +1504,10 @@ proc startEth1Syncing(m: Eth1Monitor, delayBeforeStart: Duration) {.async.} = | |||
doAssert m.latestEth1Block.isSome | ||||
awaitWithRetries m.dataProvider.getBlockByHash(m.latestEth1Block.get.hash) | ||||
|
||||
# TODO when a terminal block has is configured in cfg.TERMINAL_BLOCK_HASH, | ||||
# we should try to fetch that block from the EL - this facility is not | ||||
# in use on any current network, but should be implemented for full | ||||
# compliance | ||||
if m.currentEpoch >= m.cfg.BELLATRIX_FORK_EPOCH and m.terminalBlockHash.isNone: | ||||
var terminalBlockCandidate = nextBlock | ||||
|
||||
|
@@ -1551,7 +1528,6 @@ proc startEth1Syncing(m: Eth1Monitor, delayBeforeStart: Duration) {.async.} = | |||
break | ||||
terminalBlockCandidate = parentBlock | ||||
m.terminalBlockHash = some terminalBlockCandidate.hash | ||||
m.terminalBlockNumber = some terminalBlockCandidate.number | ||||
|
||||
debug "startEth1Syncing: found merge terminal block", | ||||
currentEpoch = m.currentEpoch, | ||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -38,8 +38,7 @@ type | |||||
|
||||||
# Transition | ||||||
TERMINAL_TOTAL_DIFFICULTY*: UInt256 | ||||||
TERMINAL_BLOCK_HASH*: BlockHash | ||||||
# TODO TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH*: Epoch | ||||||
TERMINAL_BLOCK_HASH*: BlockHash # TODO use in eht1monitor | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
# Genesis | ||||||
MIN_GENESIS_ACTIVE_VALIDATOR_COUNT*: uint64 | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/ethereum/execution-apis/blob/v1.0.0-beta.1/src/engine/specification.md#engine_exchangetransitionconfigurationv1