mobilecoinofficial / full-service

A MobileCoin service for wallet implementations.
Other
46 stars 21 forks source link

Python CLI command `mob status` giving wrong results #1013

Closed christian-oudard closed 2 weeks ago

christian-oudard commented 4 weeks ago

When when syncing ledger from scratch, and there are only accounts created recently in the wallet, we should not use min_synced_block_index, but instead use local_block_height to communicate which block the sync status is on.

docs-page[bot] commented 4 weeks ago

To view this pull requests documentation preview, visit the following URL:

docs.page/mobilecoinofficial/full-service~1013

Documentation is deployed and generated using docs.page.

holtzman commented 2 weeks ago

Thanks @christian-oudard. Do you think that the core issue is with how the python CLI uses the result of get_wallet_status, or that its actually how get_wallet_status is computing the min_synced_block_index?

Perhaps here: https://github.com/mobilecoinofficial/full-service/blob/4137547fb287725c6bff0f245807ce2adf9c2416/full-service/src/service/balance.rs#L300 the starting point for min_synced_block_index should be the local_block_height instead of the network_block_height?

Then here: https://github.com/mobilecoinofficial/full-service/blob/4137547fb287725c6bff0f245807ce2adf9c2416/full-service/src/service/balance.rs#L334 it would never be set greater than the local_block_height, even if one imported an account with a next_block_index greater than than the local_block_height.

holtzman commented 2 weeks ago

please see pr #1018 which fixes this issue in an alternate way, as suggested above. Once that PR is approved and merged, we will close this PR without merging.

holtzman commented 2 weeks ago

closing this PR as the reported defect is addressed by PR #1018, which has been merged.