Skip to content

swapwallet: Hide boarding sweep fee ledger rows#833

Open
ellemouton wants to merge 1 commit into
mainfrom
ellemouton/investigate-issue-826
Open

swapwallet: Hide boarding sweep fee ledger rows#833
ellemouton wants to merge 1 commit into
mainfrom
ellemouton/investigate-issue-826

Conversation

@ellemouton

@ellemouton ellemouton commented Jun 30, 2026

Copy link
Copy Markdown
Member

Fixes #826.

Root Cause

Confirmed boarding sweeps already have a canonical wallet activity row from
the boarding_sweeps table. The history projector also treated
ledger-sourced sweep rows as user-facing sweeps, even though those rows are
internal fee-clearing legs.

For the issue's reported transaction, that internal leg was 139 sats of
miner fee plus the 330 sat anchor, so it surfaced as a bogus second
EXIT PENDING -469 row.

The same projection bug also applies to unilateral-exit onchain_fee_paid
rows, which are ledger-sourced sweep fee legs with no user-facing chain txid
or confirmation lifecycle.

Changes

  • Filter ledger-sourced sweep rows out of wallet activity and onchain
    history.
  • Keep the canonical boarding_sweeps row as the visible exit entry.
  • Add a regression test for the reported 50_000 / 139 / 469 shape.
  • Add a regression test for unilateral-exit onchain_fee_paid rows.
  • Cover the onchain list view so these internal rows do not surface there
    with empty txids.

Validation

  • make unit-swapruntime pkg=./swapwallet case=TestHistoryHidesBoardingSweepFeeLedgerLeg
  • make unit-swapruntime pkg=./swapwallet case=TestHistoryHidesUnilateralExitFeeLedgerLeg
  • make unit-swapruntime pkg=./swapwallet case=TestListViewOnchainFlattensLedgerRows
  • make unit-swapruntime pkg=./swapwallet
  • make lint-changed-local
  • git diff --check

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request updates the ledger row classification in swapwallet/history.go to ignore internal boarding-sweep chain costs (ledger.EventBoardingSweepFeePaid) by classifying them as unspecified and unexported. It also adds a corresponding unit test TestHistoryHidesBoardingSweepFeeLedgerLeg in swapwallet/history_test.go to verify this behavior. There are no review comments, so I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@ellemouton
ellemouton force-pushed the ellemouton/investigate-issue-826 branch from 6470c25 to f967348 Compare June 30, 2026 20:06
@ellemouton

Copy link
Copy Markdown
Member Author

@claude review this with the context of the issue it references

@claude

claude Bot commented Jun 30, 2026

Copy link
Copy Markdown

Claude finished @ellemouton's task in 3m 19s —— View job


Review: PR #833 — Hide boarding sweep fee ledger rows

I traced the full ledger → ListTransactions → activity-projection path against the issue's reported 50_000 / 139 / 469 shape. The fix is correct, minimal, and well-targeted.


Why the fix is right

A confirmed boarding sweep books several ledger legs in handleBoardingSweepConfirmed (ledger/handlers.go:841-929):

Leg event_type transaction_type (per db/sqlc/queries/fee_accounting.sql:68-74) classifyLedgerRow result
Fee leg (miner + P2A anchor) boarding_sweep_fee_paid sweep was EXIT -1now hidden
Per-input clearing debit wallet_utxo_spent round already hidden (default branch)
External destination wallet_sweep_transfer round already hidden (default branch)
Wallet-return destination wallet_utxo_created boarding DEPOSIT +1 (correct — funds re-enter wallet)

So among all the sweep's ledger legs, only the fee leg mapped to transaction_type = 'sweep', which is exactly why it leaked through as the bogus EXIT PENDING -469 row. The other internal legs were already swallowed by the default branch. The new guard at swapwallet/history.go:1199 removes precisely the one spurious row — nothing more, nothing less.

The canonical row is safe from the new filter

The visible exit comes from the boarding_sweeps source row (fee_accounting.sql:241-258), whose subtype is the sweep status (confirmed/published/pending/…), never an event_type. It therefore can never equal ledger.EventBoardingSweepFeePaid ("boarding_sweep_fee_paid"), so the filter discriminates the internal leg from the canonical row cleanly. Good.

Filtering vs. de-duping was the correct choice

dedupeByID couldn't have collapsed these two rows: the fee leg sets no ChainTxid (ledger/handlers.go:850-859), so it falls back to the synthetic ledger-N id (history.go:1106-1108) while the canonical row keys off the sweep txid. Dedup keys never match → filtering is the right tool here.

Test

TestHistoryHidesBoardingSweepFeeLedgerLeg reproduces the exact reported shape and asserts the single surviving EXIT / COMPLETE / -50_000 / fee 139 row. Tight, focused regression coverage. ledger is already imported and the newHistoryFixture 3-value signature matches the existing tests. 👍


One observation (minor, likely out of scope)

listOnchain (swapwallet/history.go:280-283) maps every ListTransactions row to an OnchainTx without going through classifyLedgerRow, so the boarding_sweep_fee_paid leg still surfaces in the Onchain view (with an empty txid, since the leg carries no chain_txid). This is pre-existing behavior and the Onchain view is a more technical surface, so it's plausibly intentional — but since #826 is about a phantom row from this same internal leg, it's worth confirming the Onchain view shouldn't get the same treatment.

Fix this → (optional — confirm desired behavior first)

Verdict

LGTM. The root-cause analysis in the PR description matches the code, the fix is surgical, and the regression test pins the reported case.
· branch ellemouton/investigate-issue-826

@ellemouton
ellemouton force-pushed the ellemouton/investigate-issue-826 branch 2 times, most recently from 666cf64 to 7e9262d Compare June 30, 2026 21:45
@levmi levmi added activity Activity feed / wallet-entry projection bug Something isn't working P2 Priority 2 — medium wallet labels Jul 6, 2026
The boarding_sweeps table already provides the canonical user-facing
exit row for a confirmed boarding sweep. The ledger's
boarding_sweep_fee_paid event is an internal clearing leg. The same
projection issue applies to unilateral-exit onchain_fee_paid rows.

Filter ledger-sourced sweep rows from activity and onchain history
projection, and add regression tests covering both fee-leg shapes.
@ellemouton
ellemouton force-pushed the ellemouton/investigate-issue-826 branch from 7e9262d to 063b582 Compare July 10, 2026 21:41
@lightninglabs-deploy

Copy link
Copy Markdown
Collaborator

@ellemouton, remember to re-request review from reviewers when ready

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

activity Activity feed / wallet-entry projection bug Something isn't working P2 Priority 2 — medium wallet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

After sweep: Anchor forever pending with incorrect amount

3 participants