fix(rln-relay): sync from deployed block number #1955
Conversation
|
You can find the image built from this PR at |
There was a problem hiding this comment.
lgtm! Thanks for this, a great improvement that will i) reduce rln tree sync time by a lot and ii) avoid choking eth nodes by requesting less blocks at once (something quite important when using your own non infura node)
left some comments, nothing critical but relevant imho.
|
🚀 thanks! |
| else: | ||
| let deployedBlockNumber = g.rlnContractDeployedBlockNumber.get() | ||
| let deployedBlockNumber = g.rlnContractDeployedBlockNumber |
There was a problem hiding this comment.
nitpick, but not needed?
| @@ -76,11 +76,11 @@ template initializedGuard(g: OnchainGroupManager): untyped = | |||
|
|
|||
|
|
|||
| proc setMetadata*(g: OnchainGroupManager): RlnRelayResult[void] = | |||
| if g.latestProcessedBlock.isNone(): | |||
| if g.latestProcessedBlock == 0: | |||
There was a problem hiding this comment.
mmm do we still need to error here? guess 0 could be an ok value? eg at startup?
There was a problem hiding this comment.
there shouldn't be a reason to save the metadata if the latest block number is 0, hence there were 2 options here, either a noop or an error, went for the latter
addressed in b78072c
rymnc
force-pushed
the
use-deployed-block-number
branch
from
110a8a8
to
b78072c
Compare
| @@ -39,6 +39,7 @@ contract(RlnStorage): | |||
| proc MEMBERSHIP_DEPOSIT(): Uint256 {.pure.} | |||
| proc members(idCommitment: Uint256): Uint256 {.view.} | |||
| proc idCommitmentIndex(): Uint256 {.view.} | |||
| proc deployedBlockNumber(): Uint256 {.view.} | |||
There was a problem hiding this comment.
Looks nice indeed! However, I wonder if we could add comments above each of these procs to elaborate more about their purposes.
There was a problem hiding this comment.
will address in a follow up pr :)
| info "reconnecting with the Ethereum client, and restarting group sync", fromBlock = fromBlock | ||
| try: | ||
| let newEthRpc = waitFor newWeb3(g.ethClientUrl) |
There was a problem hiding this comment.
I wonder if makes sense to perform some retrial mechanism?
There was a problem hiding this comment.
hi! #1958 tracks handling delays, this piece of code does handle the reconnections!
Description
Syncs from the deployed block number to prevent excess network calls
Changes
Issue
closes #1942