client-lib: fee support in RedeemNote

[louisinger]

related to arkade-os/go-sdk#98

@altafan please review

Summary by CodeRabbit

• Bug Fixes
  • Improved redemption fee handling: fees are now assessed per note and validated so they never exceed note value; individual note contributions and the final redeemable amount are adjusted accordingly.
  • Added off-chain address validation and output-fee calculation to ensure accurate, fee-aware final transfers.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9ffbe74 and 575d784.

📒 Files selected for processing (1)

• pkg/client-lib/batch_session.go

🚧 Files skipped from review as they are similar to previous changes (1)

• pkg/client-lib/batch_session.go

---

Walkthrough

RedeemNotes now initializes a fee estimator from account info, computes and validates per-note input fees (rejecting notes where fees >= note value), aggregates adjusted note amounts, resolves offchain addresses, computes output fees for the transfer, and subtracts output fees from the receiver amount before forming final receivers.

Changes

Cohort / File(s)	Summary
Fee Estimation & Redemption Logic pkg/client-lib/batch_session.go	RedeemNotes updated to fetch account info, initialize a fee estimator, compute per-note input fees with validation and per-note amount adjustments, aggregate adjusted amounts, fetch offchain addresses, compute per-transfer output fees, subtract output fees from the receiver amount, and construct final receivers.

Sequence Diagram(s)

sequenceDiagram
    participant Caller
    participant RedeemNotes
    participant FeeEstimator
    participant OffchainResolver
    participant ReceiverCreator

    Caller->>RedeemNotes: request redeem(notes)
    RedeemNotes->>FeeEstimator: init from account info
    loop per note
        RedeemNotes->>FeeEstimator: estimate input fee(note)
        FeeEstimator-->>RedeemNotes: input fee
        RedeemNotes->>RedeemNotes: validate fee < note.value, adjust amount
    end
    RedeemNotes->>OffchainResolver: fetch offchain address(es)
    OffchainResolver-->>RedeemNotes: address(es)
    RedeemNotes->>FeeEstimator: estimate output fee(total amount)
    FeeEstimator-->>RedeemNotes: output fee
    RedeemNotes->>ReceiverCreator: create receiver(amount - output fee, address)
    ReceiverCreator-->>Caller: return receivers

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

• RedeemNotes missing fee estimation go-sdk#98: Addresses missing fee estimation by adding per-note and output fee calculations and validations in RedeemNotes.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'client-lib: fee support in RedeemNote' directly and clearly summarizes the main change: adding fee support functionality to the RedeemNote method in the client library.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pkg/client-lib/batch_session.go`:
- Around line 137-142: The check that validates fees vs amount should reject
cases where outputFeesSats is equal to amount as well as greater; update the
conditional in the function that contains the receiver/fees logic (the block
that currently returns "fees (%d) exceed amount (%d)") to use >= instead of > so
it returns an error for fee-equals-amount, preventing receiver.Amount from being
set to zero and avoiding creation of receiversOutput with a zero-valued
receiver.
- Line 116: Guard the uint64 accumulation to prevent silent wrap: in the loop
where amount is incremented using "amount += noteValue - feesSats", first ensure
noteValue >= feesSats (avoid underflow), compute delta := noteValue - feesSats,
then check overflow before adding (e.g., if amount > math.MaxUint64 - delta) and
return an error if it would overflow; update the code paths that use "amount",
"noteValue", and "feesSats" to handle and propagate this error instead of
allowing wrapping.

---

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0ed1abd and 9ffbe74.

📒 Files selected for processing (1)

• pkg/client-lib/batch_session.go

[IsaqueFranklin]

I validated the code using the E2E integration tests. Specifically, I ran TestBatchSession/redeem_notes and TestFee on this branch, and they both passed successfully.

The fee logic in RedeemNotes looks solid.

cACK.

[IsaqueFranklin]

Initially, I was concerned about using uint64 here for the value, because the feeEstimator returns a float64 and it would just truncate the number down, which would mean that the fee would be less than what the operator needs and the redeem would not work. But I checked the ToSatoshis() method in pkg/ark-lib/arkfee/types.go and it already uses math.Ceil, which rounds the number up, so it's fine.

[arkanaai]

Hey @louisinger — this was approved by @IsaqueFranklin on Mar 10 (21 days ago) and altafan is assigned as requested reviewer. Anything blocking merge?

[arkanaai]

🔍 Arkana PR Review — Fee support in RedeemNote (arkd)

Scope: 40 additions, 3 deletions in pkg/client-lib/batch_session.go — adds per-note fee calculation and output fee deduction to RedeemNotes.

✅ What looks good

1. Per-note fee validation — Each note's fee is calculated individually via feeEstimator.EvalOffchainInput() with VtxoTypeNote, and there's a check that fees don't exceed the note's value. This prevents negative-value notes from being submitted.

2. Output fee deduction — The receiver output amount is adjusted after deducting output fees via feeEstimator.EvalOffchainOutput(), ensuring the final transfer reflects actual costs.

3. Fee source from GetInfo — Using info.Fees.IntentFees from the server's GetInfo response ensures fee rates are current and server-authoritative.

⚠️ Concerns

1. Early abort on single unprofitable note — If any note has fees ≥ its value, the entire RedeemNotes call fails with an error. This means a batch of notes where one small-value note is unprofitable will fail entirely. Consider skipping unprofitable notes instead of aborting — log a warning, exclude that note from the batch, and continue with the rest. The current behavior forces users to manually filter notes before calling redeem.

2. Double fee deduction risk — Fees are deducted from each note (input side) and from the output. If the fee model charges both input and output fees, the total deduction is correct. But if IntentFees is meant to cover the full round-trip, this could over-deduct. Verify with the fee model that input fees + output fees = total expected fee (not 2x).

3. No dust check on final amount — After deducting output fees, receiver.Amount could be below the server's dust threshold. The joinBatchWithRetry call would presumably reject it, but an explicit check with a clear error message would be better UX.

📝 Nits

• feesSats and noteValue are both uint64 — the feesSats >= noteValue check correctly handles the edge case where they're equal (zero-value output), which is good.

🔗 Cross-repo impact

• This affects all SDK consumers that call RedeemNotes — the behavior change (fees now deducted) means the redeemed amount will be lower than before. SDKs and wallet apps should update any balance predictions / confirmation dialogs.
• Related to go-sdk#98 as noted in the PR description.

Clean, focused change. Main recommendation: skip unprofitable notes instead of failing the whole batch.