sync: fresh and existing wallets skip trial decryption #164
Conversation
|
TalDerei
changed the title
| try { | ||
| const walletCreationBlockHeight = await fetchBlockHeight(randomEndpoint!); | ||
| if (walletCreationBlockHeight !== undefined) { | ||
| return walletCreationBlockHeight; | ||
| } else { | ||
| // Remove the current endpoint from the list and try again | ||
| const remainingEndpoints = endpoints.filter(endpoint => endpoint !== randomEndpoint); | ||
| return fetchBlockHeightWithFallback(remainingEndpoints); | ||
| } | ||
| } catch (error) { | ||
| const remainingEndpoints = endpoints.filter(endpoint => endpoint !== randomEndpoint); | ||
| return fetchBlockHeightWithFallback(remainingEndpoints); | ||
| } | ||
| }; |
There was a problem hiding this comment.
this fetches the block height by randomly querying one of the rpc endpoints in the chain registry. something to consider is whether this operation is possibly blocking or if the rpc call will eventually time out?
There was a problem hiding this comment.
I think we added timeouts to our promise clients (?). It's async, so not thread blocking, but we should have a think whether this should block the onboarding step.
There was a problem hiding this comment.
while this approach is non-blocking at the thread level, it could still delay onboarding if the rpc requests take too long. I noticed that we don't have timeouts set for our promise clients at either the transport level or at the request level (ie. getStatus). Consequently, should we implement a timeout at the transport level, request level within the async request in fetchBlockHeight, or both to prevent indefinite hanging? cc @turbocrime
There was a problem hiding this comment.
we construct a promise client with 2 second timeout, which should be sufficient for most rpc requests? However, do you think this timeout is short enough that it might lead to an increase in network request failures? Would it be better to extend the timeout to say 5 seconds?
I think this the only remaining consideration that's soft blocking @grod220
| try { | ||
| const walletCreationBlockHeight = await fetchBlockHeight(randomEndpoint!); | ||
| if (walletCreationBlockHeight !== undefined) { | ||
| return walletCreationBlockHeight; | ||
| } else { | ||
| // Remove the current endpoint from the list and try again | ||
| const remainingEndpoints = endpoints.filter(endpoint => endpoint !== randomEndpoint); | ||
| return fetchBlockHeightWithFallback(remainingEndpoints); | ||
| } | ||
| } catch (error) { | ||
| const remainingEndpoints = endpoints.filter(endpoint => endpoint !== randomEndpoint); | ||
| return fetchBlockHeightWithFallback(remainingEndpoints); | ||
| } | ||
| }; |
There was a problem hiding this comment.
I think we added timeouts to our promise clients (?). It's async, so not thread blocking, but we should have a think whether this should block the onboarding step.
|
suggestion: Did a quick draft, the idea is that copy to clipboard copies all (phrase + "\n blockheight: number") as most would probably do this |
TalDerei
changed the title
grod220
force-pushed
the
skip-trial-decrypt
branch
from
a12b8fa to
89a5502
Compare
grod220
force-pushed
the
skip-trial-decrypt
branch
from
c7dda19 to
c12a736
Compare
|
tested various workflows and error conditions and looks good 👍 |
references penumbra-zone/web#1278 and penumbra-zone/web#1707
Implements the necessary scaffolding for wallet birthdays in prax