Improve progress representation in WalletSummary
#1541
Conversation
str4d
force-pushed
the
1070-zcb-wallet-summary-progress
branch
3 times, most recently
from
76670cf
to
711b4c8
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1541 +/- ##
==========================================
+ Coverage 60.46% 60.50% +0.04%
==========================================
Files 147 147
Lines 17115 17313 +198
==========================================
+ Hits 10348 10476 +128
- Misses 6767 6837 +70 ☔ View full report in Codecov by Sentry. |
str4d
force-pushed
the
1070-zcb-wallet-summary-progress
branch
from
711b4c8
to
d872202
Compare
| @@ -839,21 +839,23 @@ fn subtree_scan_progress( | |||
| fully_scanned_height: BlockHeight, | |||
| chain_tip_height: BlockHeight, | |||
| ) -> Result<Option<Ratio<u64>>, SqliteClientError> { | |||
| let mut stmt_scanned_count_from = conn.prepare_cached(&format!( | |||
There was a problem hiding this comment.
One of the things that I "left for later" when I was originally writing this code was the fact that this query seems potentially expensive/wasteful. It's probably not worth changing without benchmarking, but it does feel weird to do a sum over potentially millions of rows as a matter of course.
| let mut stmt_start_tree_size = conn.prepare_cached(&format!( | ||
| "SELECT MAX({table_prefix}_commitment_tree_size - {output_count_col}) | ||
| FROM blocks | ||
| WHERE height <= :start_height", |
There was a problem hiding this comment.
This query could potentially be inefficient; it would really be best if we could take advantage of our understanding that the note commitment tree size monotonically increases with block height instead of performing a million subtractions.
| // TODO: it would be nice to be able to reliably have the size of the | ||
| // commitment tree at the chain tip without having to have scanned that | ||
| // block. |
There was a problem hiding this comment.
I have wanted this before. It would be possible to use the GetLatestTreeState data for this, but really it should just be part of the GetLatestBlock results. That's a simple additive change to the protobuf; we should just do it.
str4d
force-pushed
the
1070-zcb-wallet-summary-progress
branch
from
d872202
to
8cc7566
Compare
|
Force-pushed to address review comments. |
str4d
force-pushed
the
1070-zcb-wallet-summary-progress
branch
from
8cc7566
to
d29a9fd
Compare
|
Force-pushed to address more comments and fix a compilation bug. |
After splitting recover progress out from scan progress, the overestimation of the chain tip tree size became much more noticeable. We now extrapolate from the most recent known "notes per block" rate.
…ests The wrong subtree was being completed, which didn't matter before but does now that scan progress is extrapolated.
str4d
force-pushed
the
1070-zcb-wallet-summary-progress
branch
from
d29a9fd
to
b4da98e
Compare
|
Force-pushed to address remaining comments. |
We now split progress into "scan progress" and "recover progress" based on the recover-until height (if set, otherwise recover progress is always 100%).
Part of #1070.