fix: resolve discrepancy between balance and ln channels view

[ben-kaufman]

Fixes (at least the balance discrepancy part of) #615

[ovitrif]

utAck
LGTM 🙌🏻

[claude]

Code Review

Testing inline comment capability...

[claude]

Code Review

I've reviewed this PR and found several issues that need attention:

---

🚨 Critical Issue: Incorrect Reserve Used

Location: ChannelDetails.kt:20-22

This change appears to introduce a logic error that contradicts the project's own documentation. According to docs/balance.md (lines 164-172), the correct calculation for our balance on channel close is:

outboundCapacityMsat / 1000 + counterpartyUnspendablePunishmentReserve

The documentation explicitly warns against using unspendablePunishmentReserve, labeling it as a "Common Mistake":

Common Mistake: Adding unspendablePunishmentReserve instead of counterpartyUnspendablePunishmentReserve:

• unspendablePunishmentReserve is OUR reserve (already excluded from capacity)
• counterpartyUnspendablePunishmentReserve is THEIR reserve (we get it back on close)

Why this matters:

• counterpartyUnspendablePunishmentReserve (their reserve) is returned to us when the channel closes
• unspendablePunishmentReserve (our reserve) is already excluded from outboundCapacityMsat

This PR does the opposite of what the documentation recommends. Please clarify why this change is correct, or update the documentation if the understanding has changed.

---

CLAUDE.md Violations

1. Missing USat Wrapper for Overflow Protection

Location: ChannelDetails.kt:22

Issue: ULong arithmetic is performed without USat wrapper.

CLAUDE.md Rule: Line 207

ALWAYS wrap ULong numbers with USat in arithmetic operations, to guard against overflows

Current code:

return outboundCapacitySat + ourReserve

Should be:

return outboundCapacitySat.safe() + ourReserve.safe()

---

2. Commit Message Exceeds Character Limit

Issue: The commit message "fix: resolve discrepancy between balance and ln channels view" is 61 characters (exceeds the 50-character limit by 11).

CLAUDE.md Rule: Line 165

USE single-line commit messages under 50 chars

Suggested alternatives:

• fix: resolve balance discrepancy in ln channels (48 chars)
• fix: correct ln channel balance calculation (45 chars)
• fix: use correct reserve in channel balance (46 chars)

---

3. PR Description Missing Required Sections

Issue: The PR description only contains the issue reference and lacks required template sections.

CLAUDE.md Rule: Line 206

ALWAYS use template in .github/pull_request_template.md for PR descriptions

Missing sections:

• Description (extended summary of changes)
• Preview (screenshots/recordings if applicable)
• QA Notes (testing instructions)

---

Summary

Critical: 1 logic error (contradicts documentation)
CLAUDE.md violations: 3 (USat wrapper, commit message, PR template)

Please address the critical issue first, as it may affect the correctness of the balance calculation.

[ovitrif]

The review point about using USat (uLong.safe()) seems more relevant, not sure about the others…

[piotr-iohk]

🚨 Critical Issue: Incorrect Reserve Used

Guessing this is not relevant? It would be good to follow up then according to Claude suggestion:

Please clarify why this change is correct, or update the documentation if the understanding has changed.

[ovitrif]

Guessing this is not relevant? It would be good to follow up then according to Claude suggestion:

Please clarify why this change is correct, or update the documentation if the understanding has changed.

We discussed a bit vaguely in this thread.

It's definitely worth investigating and adjusting as needed.

I auto-merged by mistake tbh 🤦🏻 … well, I agree with the changes, but I also agree with CLAUDE that we need to clarify on these specs.

Currently exploring what it would take to expose the claimable_amount_satoshis directly from our ldk-node fork, which would outsource the calculations correctness upstream.