Walkthrough

This PR introduces anti-abuse bond slash support for admin dispute finalization in Mostrix. It bumps the mostro-core dependency to 0.11.3, adds a new BondSlashChoice type that maps user selections to protocol bond-resolution payloads, extends error handling for invalid payloads, and documents the feature across multiple developer and user guides.

Changes

Anti-abuse bond slash support for admin dispute finalization

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

• grunch
• mostronatorcoder

🐰 A slashing choice now blooms so bright,
Four options guide the admin's might,
Bonds are dashed with purpose true,
Disputes settled, disputes through,
Protocol speaks in JSON hue. ✨

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.

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

@coderabbitai review!

#!/bin/bash
# Read the new bond_resolution.rs file
cat src/util/order_utils/bond_resolution.rs

echo "---MOD---"
cat src/util/order_utils/mod.rs

echo "---TYPES---"
cat src/util/types.rs

echo "---CARGO---"
cat Cargo.toml

#!/bin/bash
echo "---EXECUTE_ADMIN_SETTLE---"
cat src/util/order_utils/execute_admin_settle.rs

echo "---EXECUTE_ADMIN_CANCEL---"
cat src/util/order_utils/execute_admin_cancel.rs

echo "---EXECUTE_FINALIZE---"
cat src/util/order_utils/execute_finalize_dispute.rs

echo "---DOCS_FINALIZE---"
cat docs/FINALIZE_DISPUTES.md

@arkanoider: The PR is well-scoped and cleanly implemented — the protocol layer foundation is solid, tests cover all four BondSlashChoice variants across both actions, and the documentation clearly communicates what's done vs. pending. A few things to address:

Review

src/util/order_utils/bond_resolution.rs

1. to_optional_payload always returns Some — misleading contract

The method name implies it may return None (e.g. for BondSlashChoice::None), but it unconditionally wraps. The docstring explains this, but anyone reading just the signature will be confused. Consider one of:

• Rename to into_some_payload() to make the unconditional wrapping explicit, or
• Make it actually optional for BondSlashChoice::None, returning None there and Some(_) for the three slash variants:

pub fn to_optional_payload(self) -> Option<Payload> {
    match self {
        Self::None => None,
        _ => Some(self.to_payload()),
    }
}

This would also let callers on legacy daemons fall back to payload: null naturally. Depends on whether you want to always send explicit {false,false} or only send the payload when there's an actual slash.

2. Missing Default implementation

BondSlashChoice::None is the natural default, and the upcoming TUI picker will almost certainly want one. Add:

impl Default for BondSlashChoice {
    fn default() -> Self {
        Self::None
    }
}

3. to_bond_resolution visibility

to_bond_resolution is only called by to_payload within the same file and by the unit tests in the same module. Consider pub(crate) unless external callers need direct access to BondResolution:

pub(crate) fn to_bond_resolution(self) -> BondResolution {

4. Missing doc comments on three methods

label(), slash_seller(), and slash_buyer() lack doc comments while to_payload() and to_optional_payload() have them. Minor inconsistency:

/// Human-readable label for TUI display.
pub fn label(self) -> &'static str { … }

/// Returns `true` if the seller's bond should be slashed.
pub fn slash_seller(self) -> bool { … }

/// Returns `true` if the buyer's bond should be slashed.
pub fn slash_buyer(self) -> bool { … }

5. No explicit JSON field assertion for SlashBoth

bond_resolution_slash_both_roundtrip verifies deserialized field values but doesn't assert the raw JSON string. Consider adding wire assertions parallel to the SlashBuyer test:

#[test]
fn bond_resolution_wire_format_slash_both() {
    let msg = dispute_message(Action::AdminSettle, BondSlashChoice::SlashBoth);
    let json = msg.as_json().expect("serialize");
    assert!(json.contains("\"slash_seller\":true"));
    assert!(json.contains("\"slash_buyer\":true"));
}

src/util/order_utils/execute_admin_settle.rs / execute_admin_cancel.rs

The None payload is intentional and documented. One small note: the inline comment says // No payload needed — this will be stale once the bond-slash step lands. Consider:

// TODO: pass BondSlashChoice payload once TUI wiring is complete (see FINALIZE_DISPUTES.md)

docs/FINALIZE_DISPUTES.md

The Rust snippet uses use mostrix::util::order_utils::BondSlashChoice; — that's valid for an external consumer but internal callers would write use crate::util::order_utils::BondSlashChoice;. Worth making consistent with how it will actually appear in the execute files.

Cargo.toml

mostro-core = "0.11.3" is interpreted by Cargo as ^0.11.3 (>=0.11.3, <0.12.0), which is fine and consistent with the pinning guidance in docs/CODING_STANDARDS.md.

Summary table:

@coderabbitai review!

@arkanoider, triggering a fresh review of the PR. The findings from my previous pass (summarized below) remain outstanding — please let me know if any have already been addressed and I'll re-check:

I think they are fixed @coderabbitai, please check latest commits

#!/bin/bash
echo "=== bond_resolution.rs ==="
cat src/util/order_utils/bond_resolution.rs

echo "=== execute_admin_settle.rs ==="
cat src/util/order_utils/execute_admin_settle.rs

echo "=== execute_admin_cancel.rs ==="
cat src/util/order_utils/execute_admin_cancel.rs

echo "=== FINALIZE_DISPUTES.md snippet ==="
grep -n "mostrix\|crate::" docs/FINALIZE_DISPUTES.md | head -30

@arkanoider: Great progress! Items 1–5 are all confirmed fixed:

Two small items are still outstanding:

#6 — execute_admin_settle.rs / execute_admin_cancel.rs — Both files still carry // No payload needed - just the order ID. Consider updating to a TODO so it doesn't become misleading when bond-slash wiring lands:

// TODO: pass BondSlashChoice::to_optional_payload() once TUI wiring is complete (see FINALIZE_DISPUTES.md)

#7 — docs/FINALIZE_DISPUTES.md line 143 — Still reads:

use mostrix::util::order_utils::BondSlashChoice;

Internal callers will write:

use crate::util::order_utils::BondSlashChoice;

(ᵔᴥᵔ) Almost there!

@coderabbitai look at new commit

#!/bin/bash
echo "=== execute_admin_settle.rs ==="
cat src/util/order_utils/execute_admin_settle.rs

echo "=== execute_admin_cancel.rs ==="
cat src/util/order_utils/execute_admin_cancel.rs

echo "=== FINALIZE_DISPUTES.md use path ==="
grep -n "mostrix\|crate::" docs/FINALIZE_DISPUTES.md | head -30

@arkanoider: Both remaining items are confirmed fixed in the latest commit:

All 7 findings from this review are resolved. The PR looks great — the protocol layer foundation is solid and well-tested. 🎉

(ᵔᴥᵔ)