feat(bond): Phase 3 — payout flow for slashed bonds

[grunch]

Summary

Implements Phase 3 of the Anti-Abuse Bond rollout (issue #711, spec
docs/ANTI_ABUSE_BOND.md §8). Picks up where #737 left off: bonds that
Phase 2 parked in BondState::PendingPayout now get drained through
to either Slashed, Forfeited, or Failed.

• New scheduler tick job_process_bond_payouts (mirror of the dev-fee
  job) that for each PendingPayout bond:
  • settles + transitions to Forfeited once payout_claim_window_days
    elapses with no counterparty invoice (long-stop forfeiture, §15.4);
  • asks the non-slashed counterparty for a payout bolt11 via
    Action::AddBondInvoice (cadence-gated on
    payout_invoice_window_seconds, with the forfeit deadline shipped
    as a Payload::TextMessage DM alongside);
  • on receipt, settles the bond HTLC and send_payments the
    counterparty share (amount_sats − node_share_sats), capped by
    MostroSettings::max_routing_fee;
  • retries send_payment failures up to payout_max_retries, then
    transitions to Failed with a loud error! for operator review;
  • skips the counterparty leg entirely when slash_node_share_pct = 1.0
    (settle → Slashed, no DMs, no send_payment).
• New action handler add_bond_invoice_action consuming
  Action::AddBondInvoice replies. CAS-guarded persistence (state = PendingPayout AND payout_invoice IS NULL) closes the late-invoice
  race against a concurrent Forfeited transition.
• Spec edits in docs/ANTI_ABUSE_BOND.md:
  • §8.1 / §8.3: removed the Nostr audit-event subsection (slash
    observability stays tracing-only via §14.5, which is left
    untouched);
  • §8.1 invoice-request DM and recipient resolution now reference
    Action::AddBondInvoice instead of AddInvoice — keeps the
    bond-payout invoice flow disjoint from the buyer-flow at the
    routing layer rather than relying on ambient DB lookups;
  • §14.3 documents the mostro-core 0.11.2 bump that ships
    Action::AddBondInvoice.
• Cargo.toml: bumped mostro-core from 0.11.1 to 0.11.2.
• Drive-by: added missing anti_abuse_bond: None to the
  config::util test helper make_settings, which had failed to
  compile in the test profile since Phase 0 landed the new field on
  Settings.

The split snapshot (node_share_sats) frozen by Phase 2 at the
Locked → PendingPayout transition is the sole source of truth for
the scheduler; slash_node_share_pct is never re-read from config.
Daemon restarts and operator config changes mid-payout cannot
rebalance an in-flight bond.

Test plan

• cargo test --bin mostrod — 298 tests pass (10 new tests under
  src/app/bond/payout.rs::tests).
• cargo clippy --all-targets --all-features -- -D warnings.
• cargo fmt --all.
• SQLX_OFFLINE=true cargo build --bin mostrod (no new compile-time
  query! macros, so the offline cache is unchanged).

How to test (manual regtest walkthrough)

These scenarios exercise Phase 3 end-to-end against polar/regtest LND.
The cast:

• User A — the maker. Posts a sell-order in scenarios S1–S5 and
  S7, a buy-order in S6.
• User B — the taker. Posts the anti-abuse bond (apply_to = "take").
• User C — the solver/admin with their pubkey in
  mostro.solvers. Resolves disputes via admin-settle / admin-cancel.

Common setup before every scenario:

1. In settings.toml enable the bond on the taker side:

   [anti_abuse_bond]
   enabled = true
   apply_to = "take"
   slash_node_share_pct = 0.5
   payout_invoice_window_seconds = 60   # shrink for faster manual runs
   payout_max_retries = 3
   payout_claim_window_days = 1         # shrink for faster manual runs

2. Start mostrod and tail logs with
   RUST_LOG=info,mostro=debug. Look for the
   bond payout: prefix — every transition logs through it.
3. sqlite3 mostro.db "select id, state, slashed_reason, node_share_sats, payout_attempts, invoice_request_attempts, last_invoice_request_at, payout_invoice from bonds;"
   is the source of truth for state per row.

S1 — Happy path: dispute slashes buyer, seller paid out

Tests the LostDispute → settle (at slash time) → AddBondInvoice → send_payment → Slashed flow.

1. A posts a sell-order on Mostro for, say, 100 000 sats.
2. B takes it. Order moves through WaitingTakerBond → bond
   bolt11 (Action::PayBondInvoice) is DMed to B. Confirm in the DB
   the bond row is requested, then locked once B pays the
   bond invoice.
3. Trade proceeds normally to WaitingPayment / Active. B
   (buyer) pays the hold invoice. Then A signals fiat-sent /
   release won't happen; one party opens a dispute.
4. C picks up the dispute (admin-take-dispute) and resolves
   with settle + slash buyer:

   { "order": { "version": 1, "id": "<order-id>",
     "action": "admin-settle",
     "payload": { "bond_resolution":
       { "slash_seller": false, "slash_buyer": true } } } }

5. Inspect the bond row — it must read
   state = pending-payout, slashed_reason = lost-dispute, node_share_sats = 500 (50% of a 1 000 sat bond at the spec
   defaults). slashed_at is set; payout_invoice is NULL.
   Also confirm in the log the line Bond HTLC settled and row transitioned to PendingPayout — the slash path settles the
   bond hold invoice immediately (Phase 2 §7.3), so Mostro's
   wallet is already up by 1 000 sats at this point. Phase 3
   only drives the recipient payout from sats already on hand.
6. Within ~60 s the scheduler tick fires. A (the non-slashed
   seller — the recipient under §3.1 mapping for a sell-order) must
   receive an Action::AddBondInvoice message with the order
   context (the amount field = 500 sats, the counterparty share).
   The message carries only the structured payload — no
   human-readable deadline text. Clients render the forfeit
   warning locally from slashed_at + bond_payout_claim_window_days × 86 400, with bond_payout_claim_window_days published on the
   kind-38385 info event (§13.1). DB-wise:
   invoice_request_attempts = 1, last_invoice_request_at is
   now-ish.
7. A replies with Action::AddBondInvoice carrying a bolt11 for
   500 sats. Verify the DB now shows payout_invoice = lnbc… and
   invoice_request_attempts reset to 0.
8. On the next scheduler tick (≤60 s) the daemon should:
   • send_payment to A's bolt11 for 500 sats, capped by
     max_routing_fee (the sats are drawn from the slash-time
     settle in step 4 — Phase 3 never calls settle_hold_invoice);
   • flip the row to state = slashed.
9. Confirm A actually received 500 sats on their LN wallet, the
   bond row is slashed, and payout_attempts is still 0 (no
   retries needed).

S2 — Forfeit window: counterparty never claims

Tests the long-stop Forfeited path (§15.4).

1. Repeat steps 1–4 of S1. Mostro's wallet is already up by 1 000
   sats from the slash-time settle.
2. Verify the bond is pending-payout and A received the first
   AddBondInvoice message. A does nothing — no reply.
3. Optional accelerator: shrink the deadline by tweaking
   payout_claim_window_days = 1 in settings.toml before
   starting the run (the scheduler reads the config on every tick,
   only the snapshot inside node_share_sats is frozen — so
   shrinking the window mid-flight also works).
4. After payout_claim_window_days × 86 400 seconds, the next
   scheduler tick must:
   • CAS-flip the row to state = forfeited — a pure SQL
     transition with the AND payout_invoice IS NULL race guard;
     no LND call (the HTLC was settled at slash time);
   • log bond forfeited: claim window elapsed, node retains full amount.
5. Confirm A receives no further messages and no Lightning
   payment. Mostro's wallet is still up by 1 000 sats (no change
   from step 1 — the forfeit transition just records that the
   counterparty share is also retained by the node now). bonds
   row is forfeited; payout_invoice is still NULL.

S3 — Late invoice on a forfeited bond is rejected

Companion to S2; covers the CAS-guarded add_bond_invoice_action.

1. From S2 step 5 (bond is now forfeited), A wakes up and
   sends an Action::AddBondInvoice reply with a valid bolt11.
2. Expected: the daemon returns
   CantDo(NotAllowedByStatus) to A and does not touch the
   bonds row. The row stays state = forfeited, payout_invoice IS NULL.

S4 — slash_node_share_pct = 1.0: full node retention

Tests the no-counterparty-leg shortcut.

1. Before starting: edit settings.toml to
   slash_node_share_pct = 1.0 and restart mostrod.
2. Run S1 steps 1–4.
3. Verify the bond row reads
   node_share_sats = 1000 (i.e. equal to amount_sats).
4. The HTLC was settled at slash time, so Mostro's wallet is
   already up by 1 000 sats by the end of step 2 (look for the
   Bond HTLC settled and row transitioned to PendingPayout log
   line). On the next scheduler tick:
   • the row CAS-flips directly to state = slashed — pure SQL,
     no LND call;
   • no AddBondInvoice message is sent to A, and no
     send_payment runs.
5. Confirm A receives nothing (no messages, no sats) and Mostro's
   wallet is up by 1 000 sats.

S5 — send_payment retries → Failed

Tests the technical-failure path that produces an operator-attention
state.

1. Set payout_max_retries = 3 in settings.toml.
2. Run S1 steps 1–6 normally.
3. At step 7, A submits a deliberately unroutable invoice —
   easiest way on polar: pay from a node with no channel path to A,
   or have A advertise an invoice on a node Mostro can't reach
   (force-close all channels between them before submission).
4. On each scheduler tick the daemon should:
   • try send_payment, fail;
   • log bond payout: send_payment failure (...); will retry on next tick;
   • bump payout_attempts by 1.
5. After three failures (payout_attempts = 3), the bond must
   transition state = failed and the log line must be the loud
   error! bond payout: send_payment exhausted retries — transitioning to Failed; node share retained, counterparty share stranded (operator review required).
6. Confirm Mostro's wallet retains the full 1 000 sats (the
   slash-time settle in step 2 deposited them; only the
   counterparty send_payment leg never landed). This is the
   known-limitation case the spec §8.2 calls out — operators
   manually pay A from logs.

S6 — Buy-order mirror of S1 (recipient resolution sanity)

Same flow, different §3.1 mapping. Confirms recipient resolution
isn't hard-coded for sell-orders.

1. A posts a buy-order (maker is buyer, taker will be
   seller).
2. B takes it (B is now the seller; their bond locks).
3. Drive the trade into dispute. C resolves with cancel +
   slash seller:

   { "order": { "version": 1, "id": "<order-id>",
     "action": "admin-cancel",
     "payload": { "bond_resolution":
       { "slash_seller": true, "slash_buyer": false } } } }

4. Expected recipient is now A (the buyer = the non-slashed
   side on a buy-order). Confirm A — not B — receives the
   AddBondInvoice message, and the rest of S1 plays out
   identically.

S7 — Daemon restart mid-payout preserves the split snapshot

Tests the "config changes can't rebalance an in-flight bond"
invariant.

1. Run S1 steps 1–5 (bond is pending-payout,
   node_share_sats = 500).
2. Before A submits an invoice, stop mostrod, edit
   slash_node_share_pct = 0.9 in settings.toml, restart.
3. On the next scheduler tick A still receives an AddBondInvoice
   message for 500 sats (not 100 sats — the 0.9 snapshot is
   ignored).
4. A submits an invoice for 500 sats → send_payment runs for
   500 sats (the bond HTLC was already settled at slash time
   in S1 step 4 of the original run, before the restart). Confirm
   the row's node_share_sats is still 500 and the bond ends in
   state = slashed.

Quick sanity grep before merging

sqlite3 mostro.db "select state, count(*) from bonds group by state;"

After running S1, S2, S4, S5, S6, S7 the row should be:
slashed | 3 (S1, S4, S6), forfeited | 1 (S2), failed | 1 (S5),
slashed | 1 (S7) — no row stuck in pending-payout other than the
late S3 case (which is forfeited).

Refs: #711.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Background bond payout processing: periodic job to advance pending payouts, request counterparty invoices, settle/pay or forfeit bonds, and handle retries; inbound invoice message handling.

• Documentation

  • Updated Anti‑Abuse Bond spec and runtime docs: Phase 3 payout flow, invoice-request rules, recipient resolution, message wording, and info‑tag bond-policy details.

• Tests

  • Added unit tests for recipient resolution, invoice-request cadence, forfeiture transitions, and payment retry behavior.

• Chores

  • Bumped core dependency to 0.11.2.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Adds Phase‑3 bond payout runtime and scheduler (run_bond_payout_cycle, payout state transitions, AddBondInvoice handler, tests), wires it into app and scheduler, bumps mostro-core to 0.11.2, fixes a test helper, and updates Phase 3 documentation and info-tags.

Changes

Bond payout runtime and scheduler

Layer / File(s)	Summary
Manifest and app wiring Cargo.toml, src/app.rs, src/app/bond/mod.rs, src/scheduler.rs, src/config/util.rs	Bump mostro-core to 0.11.2, import/re-route Action::AddBondInvoice to add_bond_invoice_action, expose bond::payout and re-exports, call the payout job at scheduler startup, and set anti_abuse_bond: None in test helper.
Scheduler bond job src/scheduler.rs	Background job_process_bond_payouts obtains an LND client with capped backoff and runs bond::run_bond_payout_cycle every 60s.
Payout module core src/app/bond/payout.rs	Adds run_bond_payout_cycle, process_one_bond, invoice-request cadence, recipient resolution, settle/pay flows, CAS state transitions (Forfeited, Slashed, Failed), add_bond_invoice_action handler, and unit tests.

Anti‑Abuse Bond Phase 3 Documentation and tags

Layer / File(s)	Summary
Phase 3 payout flow edits docs/ANTI_ABUSE_BOND.md	Replace AddInvoice with AddBondInvoice, require structured message payload for forfeit deadline, bound invoice-request counters to invoice_request_attempts/last_invoice_request_at, clarify success transition to Slashed, and document slash_node_share_pct = 1.0 shortcut.
NIP-33 info tags & startup docs src/nip33.rs, docs/STARTUP_AND_CONFIG.md, settings.tpl.toml, src/config/types.rs	Emit bond_enabled and bond policy tags (snake_case) in info events, map BondApplyTo to bond_apply_to, add tests, update MessageQueues wording to “order messages”, and change template create→make.
In-code wording updates multiple src/app/*, src/util.rs	Replace “DM” wording with “message” in comments and logs; adjust a send_dm log prefix.

Sequence Diagram(s)

sequenceDiagram
  participant Scheduler
  participant BondPayout as run_bond_payout_cycle
  participant DB
  participant LND as LndConnector
  participant Recipient

  Scheduler->>BondPayout: periodic tick -> enumerate PendingPayout
  BondPayout->>DB: query bond rows (PendingPayout)
  alt missing payout_invoice
    BondPayout->>Recipient: enqueue Action::AddBondInvoice (via MessageQueues)
    Recipient-->>BondPayout: AddBondInvoice reply (bolt11 invoice)
    BondPayout->>DB: CAS-update payout_invoice
  end
  BondPayout->>LND: settle hold invoice (preimage)
  BondPayout->>LND: send_payment for counterparty share
  LND-->>BondPayout: payment status (success/failure)
  BondPayout->>DB: CAS-transition to Slashed/Failed/Forfeited

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related issues

• Feature: Anti-Abuse Bond — Phased Implementation for Taker and Maker Flows #711: The PR implements Phase 3 payout mechanics described by this issue.

Possibly related PRs

• MostroP2P/mostro#725: Overlapping Phase 3 payout work and AddBondInvoice handling.
• MostroP2P/mostro#727: Adds BondState::Forfeited used by payout forfeiture transitions.
• MostroP2P/mostro#712: Phase‑0 bond data model that the payout runtime reads.

Suggested reviewers

• AndreaDiazCorreia
• arkanoider
• Catrya

Poem

A rabbit checks the payout trail,
Hops through docs and LND mail,
AddBondInvoice leads the dance,
Deadlines set, the bonds advance,
Hooray — payouts prance 🐇

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'feat(bond): Phase 3 — payout flow for slashed bonds' clearly and specifically summarizes the main change: implementing Phase 3 of the bond payout flow for slashed bonds.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/bond-phase-3-payout

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

• Generate code and open pull requests
• Plan features and break down work
• Investigate incidents and troubleshoot customer tickets together
• Automate recurring tasks and respond to alerts with triggers
• Summarize progress and report instantly

Built for teams:

• Shared memory across your entire org—no repeating context
• Per-thread sandboxes to safely plan and execute work
• Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

---

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.}

[grunch]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/ANTI_ABUSE_BOND.md`:
- Around line 995-1001: The doc currently instructs sending an
Action::AddBondInvoice asking for counterparty_share_sats -
estimated_routing_fee, which conflicts with the handler that validates the
invoice principal against the full counterparty_share_sats and will cause
InvalidInvoice errors; change the text so the DM (Action::AddBondInvoice with a
Payload::TextMessage body and the forfeit deadline) requests the full
counterparty_share_sats (you can mention estimated routing fee as guidance only,
not as a subtraction), and keep the instruction to bump invoice_request_attempts
so the behavior matches the handler's validation.

In `@src/app/bond/payout.rs`:
- Around line 152-153: The code currently defaults bond.node_share_sats to zero
with unwrap_or(0) in PendingPayout flows which hides missing data; instead make
node_share_sats mandatory by replacing unwrap_or(0) with an explicit
missing-value error path (e.g., use bond.node_share_sats.ok_or_else(||
PayoutError::MissingNodeShareSats)? or .expect with a clear message) so the
function returns an error rather than silently treating node share as 0 before
computing counterparty_share; apply the same change for the other occurrences of
node_share_sats handling (lines referenced 312-313, 401-402, 633-634) so all
PendingPayout code paths validate bond.node_share_sats before computing
counterparty_share.
- Around line 176-190: The code allows terminal transitions and progressing to
send_payment when bond.preimage is None; update the payout logic to
"fail-closed": before any terminal transition or before calling
ln_client.settle_hold_invoice or send_payment, assert the presence of
bond.preimage (use bond.preimage.as_deref()), and if missing return an Err (do
not mark PendingPayout as Forfeited nor proceed to send_payment). Specifically,
in the forfeit/settle branches around ln_client.settle_hold_invoice and in the
send_payment path, replace the current else-paths that log and continue with an
immediate error return (include contextual logging via bond.id and
bond.order_id) so accounting and wallet spending cannot proceed without a valid
preimage; apply the same change to the other similar blocks noted (the other two
places handling preimage at the payout transitions).
- Line 140: The code currently uses bond.slashed_at.unwrap_or(now) which
silently resets the forfeiture anchor; instead, guard on the Option and do not
default to now: replace that line with logic that checks bond.slashed_at and, if
None, either skip/return from processing this PendingPayout (or mark it as
already forfeited) rather than assigning now—use bond.slashed_at.unwrap() only
when Some and handle the None case explicitly (log a warning and bail/continue
or set forfeiture handling) in the function that computes slashed_at.

In `@src/scheduler.rs`:
- Around line 604-608: The bond payout worker currently returns early when
LndConnector::new() fails (the block that assigns ln_client), so transient LND
startup failures permanently stop draining PendingPayout bonds; change this to a
resilient retry loop: repeatedly attempt LndConnector::new() (reference
LndConnector::new()) with a capped backoff (e.g., tokio::time::sleep between
attempts), log each failure with context (use process/error logging instead of
return error!), and only proceed once you have a successful ln_client; keep the
rest of the bond payout job logic unchanged so PendingPayout processing resumes
when LND becomes available.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: dfb09e2b-016f-4066-86b9-149537bf9fc3

📥 Commits

Reviewing files that changed from the base of the PR and between 42943fd and 1102a47.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (7)

• Cargo.toml
• docs/ANTI_ABUSE_BOND.md
• src/app.rs
• src/app/bond/mod.rs
• src/app/bond/payout.rs
• src/config/util.rs
• src/scheduler.rs

[coderabbitai]

🧹 Nitpick comments (1)

docs/ANTI_ABUSE_BOND.md (1)

189-189: 💤 Low value

Use consistent action naming format.

The phase summary uses add-bond-invoice (lowercase, hyphenated), but the detailed sections reference this action as Action::AddBondInvoice (lines 995, 1041, 1075, 1479) or AddBondInvoice without the prefix (lines 1037, 1063). For searchability and consistency, consider using AddBondInvoice here as well.

📝 Suggested consistency fix

-| 3 | Payout flow: `add-bond-invoice` to winner, routing-fee estimation, retries | 2 | pending |
+| 3 | Payout flow: `AddBondInvoice` to winner, routing-fee estimation, retries | 2 | pending |

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/ANTI_ABUSE_BOND.md` at line 189, The phase summary line uses a lowercase
hyphenated action name; update it to match the rest of the document by replacing
`add-bond-invoice` with the canonical action name `AddBondInvoice` (consistent
with references like `Action::AddBondInvoice` and `AddBondInvoice` elsewhere) so
searchability and naming are consistent across the file.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@docs/ANTI_ABUSE_BOND.md`:
- Line 189: The phase summary line uses a lowercase hyphenated action name;
update it to match the rest of the document by replacing `add-bond-invoice` with
the canonical action name `AddBondInvoice` (consistent with references like
`Action::AddBondInvoice` and `AddBondInvoice` elsewhere) so searchability and
naming are consistent across the file.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e0b90150-5d28-4d09-9741-9009e610ef8e

📥 Commits

Reviewing files that changed from the base of the PR and between 1102a47 and 166b34f.

📒 Files selected for processing (3)

• docs/ANTI_ABUSE_BOND.md
• src/app/bond/payout.rs
• src/scheduler.rs

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

• src/scheduler.rs
• src/app/bond/payout.rs

[coderabbitai]

Actionable comments posted: 5

♻️ Duplicate comments (1)

docs/ANTI_ABUSE_BOND.md (1)

1129-1132: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Acceptance text still describes an incorrect payout-invoice principal.

This criterion says the winner is asked for floor(amount/2) − routing_fee, which contradicts the updated Phase 3 contract that requests the full counterparty share and treats routing fee separately.

Suggested doc patch

-- is asked for a bolt11 sized at `floor(amount/2) − routing_fee`,
+- is asked for a bolt11 sized at `floor(amount/2)`,

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/ANTI_ABUSE_BOND.md` around lines 1129 - 1132, Update the acceptance text
that describes the payout-invoice principal to match the Phase 3 contract:
replace the line that says the winner is asked for "floor(amount/2) −
routing_fee" with wording that the winner is asked for the full counterparty
share (based on slash_node_share_pct) and that routing_fee is treated and
deducted separately; ensure references to slash_node_share_pct and
"payout-invoice principal" are used so the text clearly distinguishes the
counterparty share calculation from routing fee handling.

🧹 Nitpick comments (1)

src/nip33.rs (1)

847-878: ⚡ Quick win

Add an enabled-path info-tag test to lock down the new mapping.

The new test only verifies bond_enabled=false behavior. Please add a companion test with bond enabled that asserts bond_apply_to and the other bond policy tags are present with expected values.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/nip33.rs` around lines 847 - 878, Add a companion test that mirrors
info_to_tags_emits_bond_disabled_marker_when_bond_off but sets up
settings/ln_status with the anti_abuse_bond feature enabled, calls info_to_tags,
asserts get_tag_value(&tags, "bond_enabled") == Some("true"), and verifies each
bond policy tag
("bond_amount_pct","bond_base_amount_sats","bond_apply_to","bond_slash_on_waiting_timeout","bond_slash_node_share_pct","bond_payout_claim_window_days")
is present and equals the expected value derived from the ln_status/bond
settings (use make_ln_status to build an LN status containing the bond policy
and get_tag_value to compare values); name the test similarly (e.g.,
info_to_tags_emits_bond_enabled_marker_when_bond_on) and reuse
init_test_settings/make_ln_status/get_tag_value to locate code paths.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/ANTI_ABUSE_BOND.md`:
- Around line 266-274: Update the legacy Action::AddInvoice references to the
new Action::AddBondInvoice throughout the Phase 3 documentation: replace
occurrences of "Action::AddInvoice" and "AddInvoice" in the explanatory text
surrounding the invoice_request_attempts column and the payout invoice timestamp
(which drives payout_invoice_window_seconds cadence) with
"Action::AddBondInvoice"; also make the same replacements for the other
occurrences noted (the ones around the payout-flow spec mentioned as also
applies to 1136-1137) so the doc consistently uses AddBondInvoice.

In `@src/app/bond/payout.rs`:
- Around line 214-220: The two branches that call ln_client.settle_hold_invoice
(the block starting with "if let Err(e) =
ln_client.settle_hold_invoice(preimage).await" and the similar branch at lines
250-256) must treat LND's idempotent "already settled" response as success:
instead of returning Err(e) unconditionally, detect the condition using
is_already_settled_error(e) and, when true, proceed as if settle succeeded
(i.e., continue to the CAS transition/path that updates SQL), otherwise log and
return the error; keep the existing warn log context including bond.id and
bond.order_id.
- Around line 223-227: The forfeiture CAS currently updates state to
BondState::Forfeited without proving an invoice is still missing; modify the
UPDATE in the payout logic (the sqlx::query that binds BondState::Forfeited,
bond.id, BondState::PendingPayout) to include "AND payout_invoice IS NULL" in
the WHERE clause, and after execute() treat rows_affected() == 0 as a lost
race/no-op (i.e., do not treat it as an error) to avoid discarding an invoice
that was inserted by add_bond_invoice_action between the scheduler snapshot and
this update.
- Around line 464-481: The receive loop on rx.recv() can block forever; wrap the
receive operations in a bounded timeout (e.g. tokio::time::timeout) and treat
both timeout and EOF (None) as payment failures routed through
on_send_payment_failure. Specifically, in the loop that checks
PaymentStatus::try_from(msg.payment.status) (around the rx.recv() usage),
replace the direct await with a timeout'd receive, and on Err(timeout) or
Ok(None) call on_send_payment_failure with a clear timeout/EOF message and set
failure (or return) as the existing PaymentStatus::Failed branch does; keep the
existing handling for Succeeded and Failed so succeeded and failure variables
are set the same way. Ensure the timeout duration is configurable/constant and
that the call to on_send_payment_failure uses the same signature/context as the
current code.
- Around line 182-189: The helper counterparty_share_sats currently subtracts
node_share_sats from bond.amount_sats without bounds checks which can produce a
negative value; update counterparty_share_sats to validate that
bond.node_share_sats is Some and lies within 0..=bond.amount_sats, returning a
MostroError (matching the existing
MostroInternalErr(ServiceError::UnexpectedError(...)) pattern) if the value is
out of range or missing, otherwise return the computed amount_sats -
node_share_sats; reference the Bond struct's node_share_sats field and the
counterparty_share_sats function when making the change.

---

Duplicate comments:
In `@docs/ANTI_ABUSE_BOND.md`:
- Around line 1129-1132: Update the acceptance text that describes the
payout-invoice principal to match the Phase 3 contract: replace the line that
says the winner is asked for "floor(amount/2) − routing_fee" with wording that
the winner is asked for the full counterparty share (based on
slash_node_share_pct) and that routing_fee is treated and deducted separately;
ensure references to slash_node_share_pct and "payout-invoice principal" are
used so the text clearly distinguishes the counterparty share calculation from
routing fee handling.

---

Nitpick comments:
In `@src/nip33.rs`:
- Around line 847-878: Add a companion test that mirrors
info_to_tags_emits_bond_disabled_marker_when_bond_off but sets up
settings/ln_status with the anti_abuse_bond feature enabled, calls info_to_tags,
asserts get_tag_value(&tags, "bond_enabled") == Some("true"), and verifies each
bond policy tag
("bond_amount_pct","bond_base_amount_sats","bond_apply_to","bond_slash_on_waiting_timeout","bond_slash_node_share_pct","bond_payout_claim_window_days")
is present and equals the expected value derived from the ln_status/bond
settings (use make_ln_status to build an LN status containing the bond policy
and get_tag_value to compare values); name the test similarly (e.g.,
info_to_tags_emits_bond_enabled_marker_when_bond_on) and reuse
init_test_settings/make_ln_status/get_tag_value to locate code paths.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 19c00ca9-3bb5-46f7-b421-9e66516d5496

📥 Commits

Reviewing files that changed from the base of the PR and between ebeaf5f and ad1f74a.

📒 Files selected for processing (12)

• docs/ANTI_ABUSE_BOND.md
• docs/STARTUP_AND_CONFIG.md
• src/app/bond/flow.rs
• src/app/bond/model.rs
• src/app/bond/payout.rs
• src/app/bond/slash.rs
• src/app/cancel.rs
• src/app/last_trade_index.rs
• src/app/take_buy.rs
• src/app/take_sell.rs
• src/nip33.rs
• src/util.rs

✅ Files skipped from review due to trivial changes (9)

• src/util.rs
• src/app/take_sell.rs
• src/app/last_trade_index.rs
• src/app/bond/slash.rs
• src/app/take_buy.rs
• src/app/cancel.rs
• src/app/bond/model.rs
• docs/STARTUP_AND_CONFIG.md
• src/app/bond/flow.rs

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/ANTI_ABUSE_BOND.md`:
- Around line 1109-1111: The doc statement that "winner is kept informed via
periodic messages" conflicts with §8.1 step 1 because Action::AddBondInvoice is
only scheduled when payout_invoice IS NULL; if the invoice already exists there
is no path shown to notify the winner on retries. Update the text to either (A)
explicitly reference and document the notification mechanism used when
payout_invoice IS NOT NULL (name the action or function that emits periodic
notifications to the winner), or (B) remove or reword the claim about periodic
messages so it only applies to the branch where Action::AddBondInvoice is
scheduled; ensure you reference Action::AddBondInvoice and the payout_invoice IS
NULL condition so readers can locate the relevant decision point.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6b2624b7-8a1f-48af-8309-412954172251

📥 Commits

Reviewing files that changed from the base of the PR and between ad1f74a and a0ea402.

📒 Files selected for processing (6)

• docs/ANTI_ABUSE_BOND.md
• settings.tpl.toml
• src/app/bond/model.rs
• src/app/bond/payout.rs
• src/config/types.rs
• src/nip33.rs

✅ Files skipped from review due to trivial changes (1)

• settings.tpl.toml

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

• src/app/bond/model.rs
• src/nip33.rs
• src/app/bond/payout.rs

[mostronatorcoder]

I did a strict pass on the current PR head and I can't approve this yet.

There is one conceptual mismatch between the current spec and the implementation that looks important enough to block:

1. The slash path still does not settle the slashed bond HTLC immediately.

Right now src/app/bond/slash.rs transitions slashed bonds from Locked -> PendingPayout and snapshots slashed_reason, slashed_at, and node_share_sats, but it never calls settle_hold_invoice there. The actual settle still happens later in the Phase 3 scheduler (src/app/bond/payout.rs) inside forfeit_bond, settle_node_only, or settle_and_pay.

That matches the current wording in the Phase 3 section, but it means the slashed user's hold invoice remains merely parked until a later scheduler tick. If the intended behavior is that a slash should immediately claim the slashed HTLC, then both the spec and the code still need another change: the settle must move into the slash path itself (and if both sides are slashed, both HTLCs must be settled there).

So before this can be approved, I think we need one of two things, but they must be made consistent:

• either explicitly keep the current delayed-settle design and document/defend it as the intended final behavior,
• or change the implementation so the slash path settles the slashed hold invoice(s) immediately and Phase 3 only handles the asynchronous payout / retry / forfeiture part afterwards.

As written today, the PR still implements the delayed-settle model.

[mostronatorcoder]

Re-reviewed the latest PR head (c65d8e12b190ade234d0bc9da678d14d0ac18b4c).

My previous blocking point is resolved in this head.

This commit stack now updates both the spec and the implementation so that the slashed bond HTLC is settled immediately in the slash path:

• src/app/bond/slash.rs now performs settle_hold_invoice(preimage) before the Locked -> PendingPayout CAS,
• the spec (docs/ANTI_ABUSE_BOND.md) now explicitly documents that ordering,
• and Phase 3 is reframed as handling only the asynchronous counterparty payout / retry / forfeiture flow after Mostro already holds the sats.

So the earlier objection about delayed settle no longer applies. Sorry, that earlier review was based on an older head and was stale.

[mostronatorcoder]

I re-reviewed the latest PR head (c65d8e12b190ade234d0bc9da678d14d0ac18b4c) and found one remaining issue.

The implementation now correctly settles the slashed HTLC at slash time, and the spec was updated accordingly. However, one user-visible/log-facing string in src/app/bond/payout.rs is now stale and misleading:

• add_bond_invoice_action still logs "bond payout: invoice accepted; awaiting scheduler tick for settle+pay"

That was accurate under the old delayed-settle model, but it is no longer true after commit 28929e4: by the time the user submits Action::AddBondInvoice, the slashed HTLC has already been settled in the slash path. The scheduler is now only waiting to do the counterparty payout leg.

So this should be reworded to avoid documenting the old behavior in logs/comments. Something like awaiting scheduler tick for payout would match the new model.

[Catrya]

When the payment to the user's invoice fails, the bond ends up as Failed in the DB and mostrod doesn't send another add-bond-invoice to the user.
If the user submits a new invoice anyway, mostrod responds with cant-do: not_allowed_by_status, so there's no way to recover from the user side

[ermeme]

Hermes Agent Review

Changes requested. I found one blocking issue in the payout request flow.

Blocking

• enqueue_order_msg() is called before the database write that persists invoice_request_attempts and last_invoice_request_at. If the process crashes, the DB write fails, or the worker restarts after the message has already been queued, the durable state still says no request was sent. The next scheduler tick will emit the same Action::AddBondInvoice again.
• That breaks the pacing guarantee and can produce duplicate payout requests / duplicate DMs.

Fix

• Make the durable state the source of truth: either persist the request first and then enqueue, or use a transactional outbox pattern so the DB update and the outgoing message are coupled.

Everything else in the reviewed bond payout path looks internally consistent with the current spec.