Fix waitsync operation and performance #2008
Conversation
…ng on fragile substring checks
|
Read, tested, looks good. But needs one more pair of eyes on it. |
zancas
left a comment
zancas
left a comment
There was a problem hiding this comment.
Please don't use unnecessarily derived representations when the source quantity is directly observable.
zingo-cli/src/lib.rs
Outdated
| Ok(json_val) => { | ||
| // Extract both percentage fields as f64. | ||
| let blocks_pct_opt = json_val | ||
| .get("percentage_total_blocks_scanned") |
There was a problem hiding this comment.
Why use a percentage?
That's just a lossee re-representation of an underyling state, is it not?
Doesn't the percentage shift as a function of the denominator?
If it does not then isn't it just some fixed value... if it DOES... then isn't it reporting the same percentage for multiple different degrees of completion?!
Please don't use derived statistics when the actual observable underlying quantity is available.
If what we mean is 10 blocks from the tip, or 100 blocks unfinalized.. or whatever then lets use that!
zingo-cli/src/lib.rs
Outdated
| // Extract both percentage fields as f64. | ||
| let blocks_pct_opt = json_val | ||
| .get("percentage_total_blocks_scanned") | ||
| .and_then(|v| v.as_f64()); |
There was a problem hiding this comment.
Using the directly countable observables.. whatever they are will permit us to track values as e.g. u32.
The complexity introduced by floating point operations doesn't seem to paid for anywhere in this design.
|
I agree, thanks for feedback. |
|
any chance to review this again and possibly accept it? Thanks |
|
@zancas @fluidvanadium |
3 participants
The waitsync loop relied on brittle string matching against the human-readable poll output. When the wallet was already synced, poll did not necessarily return the exact expected strings, causing the loop to wait indefinitely. This patch replaces this with a check that queries sync status in JSON form and reads the numeric
percentage_total_blocks_scannedandpercentage_total_outputs_scannedfields. Sync completion is determined reliably by numeric threshold.