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

fix: cli: Only display warning if behind sync #11140

Merged
merged 3 commits into from
Aug 16, 2023
Merged

Conversation

rjan90
Copy link
Contributor

@rjan90 rjan90 commented Aug 8, 2023

Related Issues

Closes #11134

Proposed Changes

Only display the warning: may display 0 if chain sync in progress if behind sync and balance is 0 in the lotus wallet balance <f....>

Additional Info

Tested on the butterfly-network. Address with a balance of 1000 FIL at a later epoch, deleted the chain data, and synced from scratch:

When sync in progress:

lotus sync wait
Worker: 0; Base: 0; Target: 2104 (diff: 2104)
State: message sync; Current Epoch: 1983; Todo: 121
Validated 648 messages (115 per second)

Wallet balance displays the error:

lotus wallet balance t1yhrjgfj6lknqbdbe2px2vgljltgxbwyt6em3r6q
0 FIL (warning: may display 0 if chain sync in progress)

After syncing past the epoch where it got funds, not displaying the warning:

lotus wallet balance t1yhrjgfj6lknqbdbe2px2vgljltgxbwyt6em3r6q
1000 FIL

On a new wallet with a 0 balance and you are in sync, it does not display the warning:

 lotus wallet new
t1ntydo3kn3umlmghkknoijtrzx4jxidzjzk5nwbi
-----
lotus wallet balance t1ntydo3kn3umlmghkknoijtrzx4jxidzjzk5nwbi
0 FIL

Checklist

Before you mark the PR ready for review, please make sure that:

  • Commits have a clear commit message.
  • PR title is in the form of of <PR type>: <area>: <change being made>
    • example: fix: mempool: Introduce a cache for valid signatures
    • PR type: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, test
    • area, e.g. api, chain, state, market, mempool, multisig, networking, paych, proving, sealing, wallet, deps
  • New features have usage guidelines and / or documentation updates in
  • Tests exist for new functionality or change in behavior
  • CI is green

Only display `chain sync in progress` if behind sync
@rjan90 rjan90 requested a review from a team as a code owner August 8, 2023 08:15
Copy link
Contributor

@fridrik01 fridrik01 left a comment

Choose a reason for hiding this comment

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

LGTM, and nice test plan!

Copy link
Contributor

@arajasek arajasek left a comment

Choose a reason for hiding this comment

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

@rjan90 Can we unify the "almost synced" logic with the code we have here?

Also the test failures are benign, but not flaky -- we'll probably have to slightly tweak the test's expectation.

Unify IsSyncDone in cli/sync.go and cli/wallet.go
@rjan90
Copy link
Contributor Author

rjan90 commented Aug 9, 2023

@arajasek Unified the logic a bit in 18ae6bd, and re-ran the tests.

Deleted the chain data, and synced from scratch:

When sync in progress:

lotus sync wait
Worker: 0; Base: 0; Target: 5832 (diff: 5832)
State: message sync; Current Epoch: 1325; Todo: 4507
Validated 361 messages (105 per second)
lotus wallet list
Address                                                                                 Balance  Nonce  
t1j6r2g42hq6etvwq37tg7nakidor7a4ou3oxtddq                                               0 FIL    0      
t1ntydo3kn3umlmghkknoijtrzx4jxidzjzk5nwbi                                               0 FIL    0      
t1yhrjgfj6lknqbdbe2px2vgljltgxbwyt6em3r6q                                               0 FIL    0      
t3qboc75reueiufqj4dlx4kssrmr4zq6wlnxwfwh6rypeigydk443pidmtzkh36kioue2keiaa6djcj7tkjiiq  0 FIL    0      
t3tesfsqly7xgkoxtzwp3oywp22v6zqyypueqfxojn3b4255gjfi4lrvxkp2ujqck3j6xy2gduynzauagtefbq  0 FIL    0      

Wallet balance displays the error:

lotus wallet balance t3tesfsqly7xgkoxtzwp3oywp22v6zqyypueqfxojn3b4255gjfi4lrvxkp2ujqck3j6xy2gduynzauagtefbq
0 FIL (warning: may display 0 if chain sync in progress)

When getting past epoch where there are fund in the wallet, or in you are in sync:

lotus wallet balance t3tesfsqly7xgkoxtzwp3oywp22v6zqyypueqfxojn3b4255gjfi4lrvxkp2ujqck3j6xy2gduynzauagtefbq
119.144231741800188209 FIL

Also for wallets with 0-balance

lotus wallet balance t3qboc75reueiufqj4dlx4kssrmr4zq6wlnxwfwh6rypeigydk443pidmtzkh36kioue2keiaa6djcj7tkjiiq
0 FIL

I will look into the tests expectations now

Set up an expected call to ChainHead
@rjan90
Copy link
Contributor Author

rjan90 commented Aug 11, 2023

TestWalletBalance is now happy again! But looks like wdpost_no_miner_storage is potentially flaky?

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.

cli: wallet: Don't show the 'may display 0 if chain sync in progress' when synced
4 participants