Skip to content

Commit 3c9d17a

Browse files
etan-statuszah
authored andcommitted
fix crash when attaching to syncing EL (#5695)
In #5664, `nim-json-rpc` dependency got bumped which included a change in behaviour when processing `null` data for heap allocated objects. - status-im/nim-json-rpc#176 Old behaviour was to raise an exception, while new behaviour is to set the value to `nil` but treat it as a successful parse. Old exceptions were similar to "Parameter [result] expected JObject but got JNull". As part of the `nim-json-rpc` bump in #5664, `el_manager.nim` was not updated to match the new behaviour, leading to crash whenever its logic assumes that a successfully parsed web3 `BlockObject` (heap allocated) may be assumed to be non-`nil`. As a quick remedy, the `el_manager.nim` is updated to transform `nil` responses for `BlockObject` into `ValueError`, allowing reuse of the existing and tested exception based processing.
1 parent 47e035f commit 3c9d17a

File tree

1 file changed

+19
-13
lines changed

1 file changed

+19
-13
lines changed

beacon_chain/el/el_manager.nim

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,11 @@ template trackedRequestWithTimeout[T](connection: ELConnection,
390390
awaitWithTimeout(request, deadline):
391391
raise newException(DataProviderTimeout, "Timeout")
392392

393+
func raiseIfNil(web3block: BlockObject): BlockObject {.raises: [ValueError].} =
394+
if web3block == nil:
395+
raise newException(ValueError, "EL returned 'null' result for block")
396+
web3block
397+
393398
template cfg(m: ELManager): auto =
394399
m.eth1Chain.cfg
395400

@@ -983,7 +988,7 @@ proc waitELToSyncDeposits(connection: ELConnection,
983988

984989
while true:
985990
try:
986-
discard connection.trackedRequestWithTimeout(
991+
discard raiseIfNil connection.trackedRequestWithTimeout(
987992
"getBlockByHash",
988993
rpcClient.getBlockByHash(minimalRequiredBlock),
989994
web3RequestsTimeout,
@@ -1448,7 +1453,7 @@ proc fetchTimestamp(connection: ELConnection,
14481453
blk: Eth1Block) {.async.} =
14491454
debug "Fetching block timestamp", blockNum = blk.number
14501455

1451-
let web3block = connection.trackedRequestWithTimeout(
1456+
let web3block = raiseIfNil connection.trackedRequestWithTimeout(
14521457
"getBlockByHash",
14531458
rpcClient.getBlockByHash(blk.hash.asBlockHash),
14541459
web3RequestsTimeout)
@@ -1959,14 +1964,14 @@ proc syncBlockRange(m: ELManager,
19591964
let lastBlock = m.eth1Chain.blocks.peekLast
19601965
for n in max(lastBlock.number + 1, fullSyncFromBlock) ..< blk.number:
19611966
debug "Obtaining block without deposits", blockNum = n
1962-
let blockWithoutDeposits = connection.trackedRequestWithTimeout(
1967+
let noDepositsBlock = raiseIfNil connection.trackedRequestWithTimeout(
19631968
"getBlockByNumber",
19641969
rpcClient.getBlockByNumber(n),
19651970
web3RequestsTimeout)
19661971

19671972
m.eth1Chain.addBlock(
1968-
lastBlock.makeSuccessorWithoutDeposits(blockWithoutDeposits))
1969-
eth1_synced_head.set blockWithoutDeposits.number.toGaugeValue
1973+
lastBlock.makeSuccessorWithoutDeposits(noDepositsBlock))
1974+
eth1_synced_head.set noDepositsBlock.number.toGaugeValue
19701975

19711976
m.eth1Chain.addBlock blk
19721977
eth1_synced_head.set blk.number.toGaugeValue
@@ -2059,12 +2064,12 @@ proc syncEth1Chain(m: ELManager, connection: ELConnection) {.async.} =
20592064
let needsReset = m.eth1Chain.hasConsensusViolation or (block:
20602065
let
20612066
lastKnownBlock = m.eth1Chain.blocks.peekLast
2062-
matchingBlockAtNewProvider = connection.trackedRequestWithTimeout(
2067+
matchingBlockAtNewEl = raiseIfNil connection.trackedRequestWithTimeout(
20632068
"getBlockByNumber",
20642069
rpcClient.getBlockByNumber(lastKnownBlock.number),
20652070
web3RequestsTimeout)
20662071

2067-
lastKnownBlock.hash.asBlockHash != matchingBlockAtNewProvider.hash)
2072+
lastKnownBlock.hash.asBlockHash != matchingBlockAtNewEl.hash)
20682073

20692074
if needsReset:
20702075
trace "Resetting the Eth1 chain",
@@ -2075,11 +2080,10 @@ proc syncEth1Chain(m: ELManager, connection: ELConnection) {.async.} =
20752080
if shouldProcessDeposits:
20762081
if m.eth1Chain.blocks.len == 0:
20772082
let finalizedBlockHash = m.eth1Chain.finalizedBlockHash.asBlockHash
2078-
let startBlock =
2079-
connection.trackedRequestWithTimeout(
2080-
"getBlockByHash",
2081-
rpcClient.getBlockByHash(finalizedBlockHash),
2082-
web3RequestsTimeout)
2083+
let startBlock = raiseIfNil connection.trackedRequestWithTimeout(
2084+
"getBlockByHash",
2085+
rpcClient.getBlockByHash(finalizedBlockHash),
2086+
web3RequestsTimeout)
20832087

20842088
m.eth1Chain.addBlock Eth1Block(
20852089
hash: m.eth1Chain.finalizedBlockHash,
@@ -2106,7 +2110,7 @@ proc syncEth1Chain(m: ELManager, connection: ELConnection) {.async.} =
21062110
raise newException(CorruptDataProvider, "Eth1 chain contradicts Eth2 consensus")
21072111

21082112
let latestBlock = try:
2109-
connection.trackedRequestWithTimeout(
2113+
raiseIfNil connection.trackedRequestWithTimeout(
21102114
"getBlockByNumber",
21112115
rpcClient.eth_getBlockByNumber(blockId("latest"), false),
21122116
web3RequestsTimeout)
@@ -2205,6 +2209,8 @@ proc testWeb3Provider*(web3Url: Uri,
22052209
var res: typeof(read action)
22062210
try:
22072211
res = awaitOrRaiseOnTimeout(action, web3RequestsTimeout)
2212+
when res is BlockObject:
2213+
res = raiseIfNil res
22082214
stdout.write "\r" & actionDesc & ": " & $res
22092215
except CatchableError as err:
22102216
stdout.write "\r" & actionDesc & ": Error(" & err.msg & ")"

0 commit comments

Comments
 (0)