chore(rln-relay): health check should account for window of roots #2664
Conversation
|
You can find the image built from this PR at Built from 6db6820 |
|
You can find the image built from this PR at Built from 6db6820 |
rymnc
requested review from
alrevuelta and
Ivansete-status
and removed request for
alrevuelta
| @@ -855,7 +855,8 @@ method isReady*(g: OnchainGroupManager): Future[bool] {.async.} = | |||
| g.retryWrapper(currentBlock, "Failed to get the current block number"): | |||
| cast[BlockNumber](await g.ethRpc.get().provider.eth_blockNumber()) | |||
|
|
|||
| if g.latestProcessedBlock < currentBlock: | |||
| # the node is still able to process messages if it is behind the latest block by a factor of the valid roots | |||
| if uint(g.latestProcessedBlock) < uint(currentBlock) - uint(g.validRoots.len): | |||
There was a problem hiding this comment.
I wonder if is safer to add parenthesis:
| if uint(g.latestProcessedBlock) < uint(currentBlock) - uint(g.validRoots.len): | |
| if uint(g.latestProcessedBlock) < ( uint(currentBlock) - uint(g.validRoots.len) ): |
There was a problem hiding this comment.
doesn't harm, i would add it as well :)
| @@ -855,7 +855,8 @@ method isReady*(g: OnchainGroupManager): Future[bool] {.async.} = | |||
| g.retryWrapper(currentBlock, "Failed to get the current block number"): | |||
| cast[BlockNumber](await g.ethRpc.get().provider.eth_blockNumber()) | |||
|
|
|||
| if g.latestProcessedBlock < currentBlock: | |||
| # the node is still able to process messages if it is behind the latest block by a factor of the valid roots | |||
| if uint(g.latestProcessedBlock) < uint(currentBlock) - uint(g.validRoots.len): | |||
There was a problem hiding this comment.
isn't it better widening rather than narrowing? i mean instead of casting everything to unit and operating with that, using the latestProcessedBlock type which has more bits and then "widening" validRoots to do the -?
wondering also if in the first run we can hit currentBlock-g.validRoots.len where currentBlock=0 and since we are using unit that will lead to a huge value, meaning that we are ready when we are not?
There was a problem hiding this comment.
currentBlock will never be 0, since that is fetched from the chain (and we can operate under the assumption that it will always be greater than 0). good point about widening vs narrowing
There was a problem hiding this comment.
casted to u256 - 442ca43
| @@ -855,7 +855,8 @@ method isReady*(g: OnchainGroupManager): Future[bool] {.async.} = | |||
| g.retryWrapper(currentBlock, "Failed to get the current block number"): | |||
| cast[BlockNumber](await g.ethRpc.get().provider.eth_blockNumber()) | |||
|
|
|||
| if g.latestProcessedBlock < currentBlock: | |||
| # the node is still able to process messages if it is behind the latest block by a factor of the valid roots | |||
| if uint(g.latestProcessedBlock) < uint(currentBlock) - uint(g.validRoots.len): | |||
There was a problem hiding this comment.
doesn't harm, i would add it as well :)
rymnc
changed the title
Description
This PR modifies the
readybehaviour of rln-relay's onchain mode. If the node is behind the tip of the blockchain bywindowblocks, it should still be marked as ready IMO.Changes
readyif the node is behind the tip of the blockchain bywindowblocks