fix(lightning): enforce the fee cap, refuse sub-satoshi loss, guard the faucet
Reviewing alby.go against real money found two bugs and one deployment hazard. The fee cap was accepted and ignored: maxFeeMsat appeared only in the signature. The Service sizes that cap against what the house will lose on routing and relies on the node refusing anything above it, so ignoring it turned a bounded cost into an unbounded one. It is now requested from Alby and checked again on the result. Amounts were truncated from millisatoshis to satoshis. A 1500 msat withdrawal debited 1500 and sent 1000, and the missing 500 was unaccounted — drift that surfaces weeks later as a books-do-not-balance alarm. Amounts that are not whole satoshis are now refused. The dev faucet and a real node could both be enabled. The faucet mints balance backed by nothing, so a player could withdraw it as real satoshis and drain the node. The server now refuses to start with both set. Withdrawal processing is also gated on solvency: paying the front of a queue while short leaves the players behind it with nothing. 11 tests against a mock of the Alby REST API covering auth, unit conversion, the fee cap, unsettled payments, error propagation, and context cancellation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -89,11 +89,15 @@ type albyInvoice struct {
|
||||
}
|
||||
|
||||
// CreateInvoice creates a Lightning invoice via Alby Hub.
|
||||
// Alby Hub uses sats; we convert to millisats for the ledger.
|
||||
//
|
||||
// Alby Hub speaks satoshis. Amounts that do not divide into whole satoshis are
|
||||
// refused rather than truncated: the ledger works in millisatoshis, and
|
||||
// silently rounding here would mean the ledger and the node disagree about how
|
||||
// much moved, with the difference disappearing.
|
||||
func (a *AlbyNode) CreateInvoice(ctx context.Context, amountMsat int64, memo string) (Invoice, error) {
|
||||
sats := msatToSat(amountMsat)
|
||||
if sats < 1 {
|
||||
sats = 1
|
||||
sats, err := exactSats(amountMsat)
|
||||
if err != nil {
|
||||
return Invoice{}, err
|
||||
}
|
||||
raw, err := a.post(ctx, "invoices", map[string]any{
|
||||
"amount": sats,
|
||||
@@ -137,9 +141,19 @@ type albyPayment struct {
|
||||
}
|
||||
|
||||
// PayInvoice sends an outbound Lightning payment.
|
||||
//
|
||||
// The fee cap is passed to the node as a limit and checked again on the
|
||||
// result. Alby's REST API does not guarantee it will honour a requested cap,
|
||||
// and the caller sizes the cap against what the house is willing to lose on
|
||||
// routing — so an over-priced payment is reported as a failure, which makes
|
||||
// the Service refund the player rather than absorb an unbounded cost.
|
||||
func (a *AlbyNode) PayInvoice(ctx context.Context, bolt11 string, maxFeeMsat int64) (Payment, error) {
|
||||
raw, err := a.post(ctx, "payments", map[string]string{
|
||||
maxFeeSats := maxFeeMsat / 1000
|
||||
|
||||
raw, err := a.post(ctx, "payments", map[string]any{
|
||||
"invoice": bolt11,
|
||||
// Requested limit. Not all versions enforce it, hence the check below.
|
||||
"maxFeeSat": maxFeeSats,
|
||||
})
|
||||
if err != nil {
|
||||
return Payment{}, err
|
||||
@@ -149,8 +163,20 @@ func (a *AlbyNode) PayInvoice(ctx context.Context, bolt11 string, maxFeeMsat int
|
||||
return Payment{}, fmt.Errorf("parsing alby payment: %w", err)
|
||||
}
|
||||
if p.State != "settled" {
|
||||
return Payment{}, fmt.Errorf("payment %s: state=%s", p.PaymentHash, p.State)
|
||||
return Payment{}, fmt.Errorf("%w: payment %s state=%s",
|
||||
ErrPaymentFailed, p.PaymentHash, p.State)
|
||||
}
|
||||
|
||||
feeMsat := satToMsat(p.Fee)
|
||||
if maxFeeMsat > 0 && feeMsat > maxFeeMsat {
|
||||
// The payment has already gone out — Lightning cannot be recalled. Report
|
||||
// it so the operator sees the overrun rather than discovering it in the
|
||||
// books, and so the Service does not record it as a clean success.
|
||||
return Payment{}, fmt.Errorf(
|
||||
"%w: routing cost %d msat, above the %d msat cap (payment %s already sent)",
|
||||
ErrPaymentFailed, feeMsat, maxFeeMsat, p.PaymentHash)
|
||||
}
|
||||
|
||||
return Payment{
|
||||
PaymentHash: p.PaymentHash,
|
||||
Preimage: p.Preimage,
|
||||
@@ -180,5 +206,23 @@ func (a *AlbyNode) Balance(ctx context.Context) (int64, error) {
|
||||
|
||||
// ───────── sat ↔ msat conversion ─────────
|
||||
|
||||
func satToMsat(sats int64) int64 { return sats * 1000 }
|
||||
func msatToSat(msat int64) int64 { return msat / 1000 }
|
||||
func satToMsat(sats int64) int64 { return sats * 1000 }
|
||||
|
||||
// exactSats converts millisatoshis to satoshis, refusing any amount that would
|
||||
// lose precision.
|
||||
//
|
||||
// Lightning cannot carry sub-satoshi amounts. Truncating would mean the ledger
|
||||
// debits 1500 msat while 1000 msat actually leaves, and the missing 500 would
|
||||
// be unaccounted — the kind of drift that only shows up as a books-do-not-
|
||||
// balance alarm weeks later.
|
||||
func exactSats(msat int64) (int64, error) {
|
||||
if msat <= 0 {
|
||||
return 0, fmt.Errorf("%w: amount %d is not positive", ErrAmountOutOfRange, msat)
|
||||
}
|
||||
if msat%1000 != 0 {
|
||||
return 0, fmt.Errorf(
|
||||
"%w: %d msat is not a whole number of satoshis; Lightning cannot send sub-satoshi amounts",
|
||||
ErrAmountOutOfRange, msat)
|
||||
}
|
||||
return msat / 1000, nil
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user