Skip to content

client-lib: fee support in RedeemNote#935

Open
louisinger wants to merge 2 commits intoarkade-os:masterfrom
louisinger:redeem-note-fee
Open

client-lib: fee support in RedeemNote#935
louisinger wants to merge 2 commits intoarkade-os:masterfrom
louisinger:redeem-note-fee

Conversation

@louisinger
Copy link
Collaborator

@louisinger louisinger commented Feb 26, 2026

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.

@louisinger louisinger requested a review from altafan February 26, 2026 22:45
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 26, 2026

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

🚥 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

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

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

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.

1 participant