Skip to content

fix(walletapi): override wallet height with daemon tip for accurate sync indicator#27

Open
moralpriest wants to merge 1 commit into
DEROFDN:community-devfrom
moralpriest:fix/walletapi-sync-indicator-height
Open

fix(walletapi): override wallet height with daemon tip for accurate sync indicator#27
moralpriest wants to merge 1 commit into
DEROFDN:community-devfrom
moralpriest:fix/walletapi-sync-indicator-height

Conversation

@moralpriest

Copy link
Copy Markdown

Override the stored Height/Topoheight in the encrypted balance result with the daemon tip values before saving.

Problem

The daemon GetEncryptedBalance response contains both Height (stable height, ~8 blocks behind tip where balances are computed) and DHeight (actual daemon tip). The wallet stored Height as its wallet height but compared it against DHeight for daemon height. This caused a persistent ~8-block gap that prevented sync indicators from ever showing green.

Changes

  • Before calling setEncryptedBalanceresult, override result.Height = result.DHeight
  • Override result.Topoheight = result.DTopoheight
  • Both measurements now come from the same point in time

Notes

  • This is a display-layer fix only — balance math is unaffected
  • The actual balance computation uses the encrypted balance data, not the Height field

…ync indicator

The daemon GetEncryptedBalance response contains both Height (stable,
~8 blocks behind tip) and DHeight (actual tip). The wallet stored Height
as wallet height but compared against DHeight, causing a permanent gap
that prevented sync indicators from turning green. Override stored
Height/Topoheight with daemon tip values before saving.

@Dirtybird99 Dirtybird99 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verified the mechanism against the base: Get_Height() returns getEncryptedBalanceresult(zero).Height (wallet.go:388), so this does close the indicator gap. Two things the "display-layer fix only" note misses:

  1. The stored Topoheight is also SyncHistory's binary-search bound (end_topo = w.getEncryptedBalanceresult(scid).Topoheight, daemon_communication.go:614/620). After this change, history scans to the daemon tip instead of the stable height. Recent-block ordering in the DAG can still change there, so entries recorded near tip will periodically hit the "header mismatch" prune/re-sync path — recent-transaction flicker plus extra RPC churn. (Partially mitigated: synchistory_block skips SideBlocks.)
  2. w.account.TopoHeight (persisted) also becomes tip. As far as I can trace this is harmless — transaction building takes its heights from a fresh GetSelfEncryptedBalanceAtTopoHeight response (wallet_transfer.go:247–255), so it's unaffected.

An alternative that keeps the indicator accurate without touching SyncHistory's bounds: leave the stored result intact and expose the tip separately for display — e.g. store result.DHeight in a dedicated field and add a Get_Daemon_Tip_Height()-style accessor for the indicator comparison.

@Dirtybird99

Copy link
Copy Markdown

Reviewed against the community-dev base. The mechanism works — Get_Height() returns getEncryptedBalanceresult(zero).Height (wallet.go:388), so overriding it with DHeight does close the indicator gap.

The one thing to flag: "this is a display-layer fix only" isn't quite accurate. The Topoheight you also override is SyncHistory's binary-search upper bound (end_topo = w.getEncryptedBalanceresult(scid).Topoheight). After this change, history scans to the daemon tip instead of the stable height, and recent-block ordering in the DAG can still change there — so entries recorded near tip will periodically hit the "header mismatch" prune/re-sync path (recent-transaction flicker + extra RPC churn). It's partially mitigated because synchistory_block skips SideBlocks.

Transaction building is unaffected — I checked; it reads heights from a fresh GetSelfEncryptedBalanceAtTopoHeight response (wallet_transfer.go:247-255), not the stored result.

Lower-risk alternative: leave the stored result intact and expose the tip separately for the indicator — store result.DHeight in a dedicated field and add a Get_Daemon_Tip_Height()-style accessor for the display comparison, rather than rewriting the value SyncHistory consumes.

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