Skip to content
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

Delay GetEpoch request until needed #2064

Merged
merged 4 commits into from
Oct 22, 2024
Merged

Delay GetEpoch request until needed #2064

merged 4 commits into from
Oct 22, 2024

Conversation

buberdds
Copy link
Contributor

@buberdds buberdds commented Sep 26, 2024

The second part of #1991

  • For some reason changes triggered ticker updates in snapshots
  • FromMnemonic not related to changes. PR hits timeouts in CI because component calls getAccountBalanceWithFallback saga for each account in modal. It is not needed in this unit tests.

Copy link

github-actions bot commented Sep 26, 2024

Deployed to Cloudflare Pages

Latest commit: 8415f2c00e5eac8747eae918f9941719a1c1c7aa
Status:✅ Deploy successful!
Preview URL: https://ef3ddf8a.oasis-wallet.pages.dev
Alias: https://pr-2064.oasis-wallet.pages.dev

@buberdds buberdds force-pushed the mz/epoch branch 2 times, most recently from a6bc87b to 76f90de Compare September 26, 2024 11:36
@buberdds buberdds marked this pull request as ready for review September 26, 2024 14:56
@buberdds buberdds requested a review from lukaw3d September 26, 2024 14:57
Comment on lines -60 to -63
const nic = yield* call(getOasisNic, network)
const { epoch } = yield* all({
epoch: call([nic, nic.beaconGetEpoch], oasis.consensus.HEIGHT_LATEST),
})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, one thing I didn't consider before:
if grpc is down, users now won't know until they try to make a transaction

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Future issue #2071

const currentEpoch = useSelector(selectEpoch)
const remainingHours = (props.epoch - currentEpoch) / estimatedEpochsPerHour
if (!props.currentEpoch) {
return null
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Epoch: ${props.epoch}" would be a very good fallback

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will keep as is. default value would require handling isLoading for epoch in redux too. Without it, user would see "blinking" value / switching from default to target value.

Added commit to refetch epoch after network switch.

@buberdds buberdds force-pushed the mz/epoch branch 2 times, most recently from 8bbbf7e to 6f1d3f4 Compare October 22, 2024 09:52
@buberdds buberdds merged commit e937375 into master Oct 22, 2024
13 checks passed
@buberdds buberdds deleted the mz/epoch branch October 22, 2024 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants