Skip to content

Commit e6f4aea

Browse files
committed
fix(bonds): address arkanaai review — deployment guards + dust-mint defence
Follow-up to the merged PR #37. Addresses the three actionable items from arkanaai's automated review (#pullrequestreview-4404098591), one P1 and two P2. P1 — auctionWindow > 0 deployment guard (issue + rollIn) Without it, `auctionWindow = 0` collapses the auction time gate `tx.time >= maturity AND tx.time < maturity` to the empty set, so no defaulted vault can EVER be settled via acceptAuction. mintedAmount stays in totalDebitOutstanding forever and credit holders absorb 100% of every default. Same class of deployment invariant as the existing initRatioBps > liqThresholdBps and liqThresholdBps > 0 checks; co-located with them at the top of issue() and rollIn(). P2 — auctionDiscountBps validation at deployment (issue + rollIn) Previously checked only at runtime in liquidate() and acceptAuction(). An out-of-range discount bricks both settlement paths, leaving the pool unsettleable — yet a vault could still be issued. Moved require(auctionDiscountBps >= 0 && < 10000) into the issuance deployment-safety block; runtime checks retained as defense-in-depth. P2 — Dust-issuance ceiling division on origination floor (issue + rollIn) required = amount * initRatioBps / 10000 floors to 1 for amount=1, initRatioBps=14999, letting 1 sat of collateral mint 1 credit + 1 debit. Attacker floods the auction window with thousands of dust defaults that are unprofitable for auctioneers to settle (gas > profit), leaving them as permanent deadweight in totalDebitOutstanding. Replaced with ceiling: required = (amount * initRatioBps + 9999) / 10000 Ceiling rounds UP so 1 * 14999 / 10000 → 2, breaking the unit-boundary mint hole. A configurable minAmount/minCollateral floor remains follow-up R1 (the ceiling alone is necessary but not sufficient for very-small-but-non-dust amounts). Tests - test_issue_enforces_liq_threshold_invariants renamed to test_issue_enforces_deployment_invariants. OP_GREATERTHAN count updated from 5 to 6 (added auctionWindow > 0). New OP_GREATERTHANOREQUAL64 >= 1 floor for the auctionDiscountBps deployment check. - test_roll_pair_enforces_both_threshold_invariants renamed to test_roll_pair_enforces_all_deployment_invariants; same updates. - New test_issue_uses_ceiling_division_on_required_collateral asserts the literal `9999` token appears in both issue and rollIn ASM — a regression to floor (or to a different bias) trips it. Docs - docs/bonds.md §Enforced deployment invariants expanded from two invariants to four (adds auctionWindow > 0 and auctionDiscountBps bounds) and now applies to rollIn as well as issue. - New paragraph on the ceiling-division origination floor. - Follow-up R2 marked WIRED with reference to the new regression test; R1 kept as a separate open item (configurable minimums). Out of scope here: the bot's P2.4 (loan_vault.ark still uses >= burn checks) is a separate contract and a separate PR. Tests: 255 -> 256 (one new ceiling-division regression test added). All suites green; cargo fmt --check clean.
1 parent 7dc8b7d commit e6f4aea

3 files changed

Lines changed: 145 additions & 37 deletions

File tree

docs/bonds.md

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -261,8 +261,9 @@ collateral and the health-call trigger (`liqThresholdBps`, e.g. 11000 =
261261
the more capital-efficient the loan but the more frequent the margin calls
262262
on volatile collateral.
263263

264-
**Enforced deployment invariants.** `issue` requires both
265-
`initRatioBps > liqThresholdBps` and `liqThresholdBps > 0`:
264+
**Enforced deployment invariants.** `issue` (and the alternate issuance
265+
entry `rollIn`) require FOUR checks on the pool's constructor params before
266+
any vault can be created:
266267

267268
- `initRatioBps > liqThresholdBps` — without it a deployer could set
268269
`liqThresholdBps >= initRatioBps`, in which case a vault minted at the
@@ -274,9 +275,33 @@ on volatile collateral.
274275
`collateralValue < healthFloor` can never hold and the margin call could
275276
never fire, silently disabling the health invariant that backs credit
276277
fungibility.
277-
278-
Both checks live in `issue` (one-time per issuance, not a hot path), so a
279-
misconfigured pool can never originate a vault at all.
278+
- `auctionWindow > 0` — a zero-length auction window collapses the
279+
`tx.time >= maturity AND tx.time < maturity` gate to the empty set, so no
280+
defaulted vault can ever be settled via `acceptAuction`. `mintedAmount`
281+
stays in `totalDebitOutstanding` forever and credit holders absorb 100% of
282+
every default. Same class of deployment invariant as the ratio checks.
283+
- `auctionDiscountBps ∈ [0, 10000)` — an out-of-range discount bricks every
284+
`liquidate` and `acceptAuction` at the runtime check, leaving the pool
285+
unsettleable. The settlement functions also revalidate the bound at
286+
runtime (defence-in-depth), but pool issuance MUST fail closed up-front
287+
rather than silently producing an unsettleable book.
288+
289+
All four checks live in both `issue` AND `rollIn` (one-time per issuance,
290+
not a hot path). A misconfigured pool can never originate a vault at all,
291+
through either issuance entry. Regression tests:
292+
`test_issue_enforces_deployment_invariants` and
293+
`test_roll_pair_enforces_all_deployment_invariants`.
294+
295+
**Ceiling division on the origination floor.** `issue` and `rollIn` use
296+
`required = (amount * initRatioBps + 9999) / 10000` (ceiling), not
297+
`amount * initRatioBps / 10000` (floor). Floor rounding lets dust-sized
298+
issuances bypass collateralisation entirely (`amount=1, initRatioBps=14999`
299+
floors to `required=1`, so 1 sat of collateral mints 1 credit + 1 debit).
300+
Ceiling rounds UP so `1 * 14999 / 10000` → 2, breaking the dust-mint shape.
301+
Regression test: `test_issue_uses_ceiling_division_on_required_collateral`.
302+
A configurable `minAmount` / `minCollateral` floor is follow-up R1 (the
303+
ceiling alone is not enough to make every very-small vault economically
304+
liquidatable, but it does close the unit-boundary hole).
280305

281306
---
282307

@@ -442,8 +467,8 @@ exact lines where each one would land.
442467

443468
| # | Item | Why | Sketch |
444469
|---|---|---|---|
445-
| **R1** | Add `minCollateral` + `minAmount` constructor params; reject dust-sized issuances. | `issue` today accepts `collateral=1, amount=1`. The `required = amount * initRatioBps / 10000` collateralisation check rounds DOWN, so for `amount=1, initRatioBps=9999` the required floor is silently zero. An attacker can mint 1 credit + 1 debit for 1 sat, polluting the auction window with unprofitable defaults. | Add two int constructor params; `require(amount >= minAmount && collateral >= minCollateral)` at top of `issue`. |
446-
| **R2** | Ceiling division on the origination collateral check. | Mitigates the rounding floor at the unit boundary even without R1's explicit minimums. | Replace `required = amount * initRatioBps / 10000` with `required = (amount * initRatioBps + 9999) / 10000`. |
470+
| **R1** | Add `minCollateral` + `minAmount` constructor params; reject dust-sized issuances. | Even with R2 ceiling division landed, very small but non-dust amounts can still produce vaults that are uneconomic to liquidate (auctioneer gas > spread). A configurable floor is the principled fix. | Add two int constructor params; `require(amount >= minAmount && collateral >= minCollateral)` at top of `issue`. |
471+
| **R2** | ~~Ceiling division on the origination collateral check~~**WIRED**. | `required = (amount * initRatioBps + 9999) / 10000` lands in both `issue` and `rollIn` (assert via `test_issue_uses_ceiling_division_on_required_collateral`). Closes the `amount=1, initRatioBps=14999` floor-rounding hole where 1 sat of collateral minted 1 credit + 1 debit. | |
447472
| **R3** | Final-redemption residue path. | `redeem`'s `require(payout > 0)` rejects dust redemptions; the last few USDT are stranded forever once they round below the per-unit rate. | Add a `redeemAll(holderPk, holderSig)` branch gated on `amount == totalCreditOutstanding` that pays the full residual `usdtBalance` regardless of payout rounding. |
448473
| **R4** | ASM-level assertion of the strict time gate on `redeem`. | `test_redeem_is_pro_rata_post_window` currently relies on the constructor-param surface, not the gate's actual ASM placement. A refactor removing the gate would not be caught by the test. | Pattern-match the comparison sequence in `redeem`'s ASM that proves `tx.time >= maturity + auctionWindow` is enforced. |
449474

examples/bonds/repayment_pool.ark

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -129,13 +129,27 @@ contract RepaymentPool(
129129
function issue(int amount, int collateral, pubkey borrowerPk, signature borrowerSig, int oraclePrice, int oracleTime, signature oracleSig) {
130130
require(amount > 0, "zero issuance");
131131
require(collateral > 0, "zero collateral");
132-
// Deployment-safety invariant: the health-call threshold MUST sit below
133-
// the origination ratio, otherwise a vault minted at the minimum
134-
// collateral is immediately liquidatable in the same block — a
135-
// guaranteed loss for the borrower. Checked here (one-time per issuance)
136-
// so a misconfigured pool can never originate a vault.
132+
// Deployment-safety invariants: collected here at issuance time so a
133+
// misconfigured pool can never originate a vault.
134+
// - initRatioBps > liqThresholdBps: a vault minted at the minimum
135+
// collateral must NOT be immediately liquidatable in the same block.
136+
// - liqThresholdBps > 0: a non-positive health threshold would invert
137+
// the liquidate gate (every vault liquidatable, or none).
138+
// - auctionWindow > 0: a zero-length auction window collapses the
139+
// `tx.time >= maturity && tx.time < maturity` gate to an empty set,
140+
// so no defaulted vault can ever be settled via acceptAuction —
141+
// mintedAmount stays in totalDebitOutstanding forever and credit
142+
// holders absorb the full default. Same class of deployment invariant
143+
// as the ratio checks above.
144+
// - auctionDiscountBps ∈ [0, 10000): an out-of-range discount bricks
145+
// every liquidate + acceptAuction (settlement-side runtime check
146+
// fails closed). Validate at deployment so pool issuance fails
147+
// rather than silently producing an unsettleable book.
137148
require(initRatioBps > liqThresholdBps, "liq threshold must be below init ratio");
138149
require(liqThresholdBps > 0, "liq threshold must be positive");
150+
require(auctionWindow > 0, "auction window must be positive");
151+
require(auctionDiscountBps >= 0, "negative discount");
152+
require(auctionDiscountBps < 10000, "discount >= 100%");
139153
require(tx.time < maturity, "pool matured");
140154
require(checkSig(borrowerSig, borrowerPk), "invalid borrower sig");
141155
require(oraclePrice > 0, "invalid oracle price");
@@ -150,7 +164,15 @@ contract RepaymentPool(
150164
// a $1M BTC price; OP_MUL64 fails closed. Chunk issuance or rescale the
151165
// price denominator (e.g. 1e4 vs 1e8) for very large vaults.
152166
int collateralValue = collateral * oraclePrice / 100000000;
153-
int required = amount * initRatioBps / 10000;
167+
// Ceiling division on the required collateral floor: floor division
168+
// lets dust-sized issuances bypass collateralisation entirely
169+
// (`amount=1, initRatioBps=14999` floors to `required=1`, so 1 sat of
170+
// collateral mints 1 credit + 1 debit). An attacker could flood the
171+
// auction window with thousands of dust defaults that are unprofitable
172+
// for auctioneers to settle (gas > profit), leaving them as permanent
173+
// deadweight in totalDebitOutstanding. Ceiling rounds UP, so
174+
// `1 * 14999 / 10000` → 2, breaking the dust-mint shape.
175+
int required = (amount * initRatioBps + 9999) / 10000;
154176
require(collateralValue >= required, "undercollateralized at issuance");
155177

156178
let creditGroup = tx.assetGroups.find(creditAssetId);
@@ -326,8 +348,13 @@ contract RepaymentPool(
326348
int outIdxPool, int outIdxVault, int outIdxCredit) {
327349
require(newMintedAmount > 0, "zero issuance");
328350
require(newCollateral > 0, "zero collateral");
351+
// Same deployment-safety invariants as issue() — rollIn is an alternate
352+
// issuance entry that must reject misconfigured pools identically.
329353
require(initRatioBps > liqThresholdBps, "liq threshold must be below init ratio");
330354
require(liqThresholdBps > 0, "liq threshold must be positive");
355+
require(auctionWindow > 0, "auction window must be positive");
356+
require(auctionDiscountBps >= 0, "negative discount");
357+
require(auctionDiscountBps < 10000, "discount >= 100%");
331358
require(tx.time < maturity, "new pool matured");
332359
require(checkSig(borrowerSig, borrowerPk), "invalid borrower sig");
333360
require(oraclePrice > 0, "invalid oracle price");
@@ -339,7 +366,9 @@ contract RepaymentPool(
339366
require(checkSigFromStack(oracleSig, oraclePk, oracleMsg), "invalid oracle signature");
340367

341368
int collateralValue = newCollateral * oraclePrice / 100000000;
342-
int required = newMintedAmount * initRatioBps / 10000;
369+
// Ceiling division — see explanation in issue(). Same dust-issuance
370+
// defence on the alternate issuance entry.
371+
int required = (newMintedAmount * initRatioBps + 9999) / 10000;
343372
require(collateralValue >= required, "undercollateralized at issuance");
344373

345374
let creditGroup = tx.assetGroups.find(creditAssetId);

tests/repayment_pool_test.rs

Lines changed: 77 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -237,28 +237,75 @@ fn test_issue_is_oracle_priced_and_dual_mints() {
237237
}
238238

239239
#[test]
240-
fn test_issue_enforces_liq_threshold_invariants() {
241-
// Deployment-safety invariants: issue must reject a pool where the
242-
// margin-call threshold is not strictly below the origination ratio, or is
243-
// non-positive — otherwise a freshly-minted vault at minimum collateral
244-
// would be immediately liquidatable (guaranteed borrower loss), or the
245-
// health gate would be inverted by a negative threshold.
240+
fn test_issue_enforces_deployment_invariants() {
241+
// Deployment-safety invariants: issue must reject a misconfigured pool at
242+
// origination, before any vault is created. Specifically:
243+
// - initRatioBps > liqThresholdBps: a vault minted at the minimum
244+
// collateral must not be immediately liquidatable.
245+
// - liqThresholdBps > 0: a non-positive threshold inverts the health
246+
// gate (every vault liquidatable, or none).
247+
// - auctionWindow > 0: a zero-length auction window means no defaulted
248+
// vault can ever be settled (`tx.time >= maturity && tx.time <
249+
// maturity` is empty), so totalDebitOutstanding accumulates forever.
250+
// - auctionDiscountBps ∈ [0, 10000): an out-of-range discount bricks
251+
// every liquidate + acceptAuction at the runtime check, leaving the
252+
// pool unsettleable.
246253
//
247254
// issue carries exactly five strict `>` comparisons that lower to
248-
// OP_GREATERTHAN: amount > 0, collateral > 0, oraclePrice > 0, the invariant
249-
// initRatioBps > liqThresholdBps, and liqThresholdBps > 0. The bare `> 0`
250-
// guards account for 3; the two threshold invariants add 2. Asserting the
251-
// exact count (5) means removing EITHER threshold invariant fails the test —
252-
// a `>= 4` lower bound would let one be silently deleted.
255+
// OP_GREATERTHAN sites: amount > 0, collateral > 0, oraclePrice > 0,
256+
// initRatioBps > liqThresholdBps, liqThresholdBps > 0, auctionWindow > 0.
257+
// Total: 3 `> 0` value guards + 3 deployment invariants = 6. Asserting
258+
// the EXACT count means removing any single invariant fails the test —
259+
// a `>= 5` lower bound would let one be silently deleted.
253260
// (OP_GREATERTHANOREQUAL / *_64 are distinct opcodes, not counted here.)
254261
let output = compile(CODE).expect("compilation failed");
255262
let gt = opcode_count(&output, "issue", "OP_GREATERTHAN");
256263
assert_eq!(
257-
gt, 5,
258-
"issue must carry BOTH initRatioBps > liqThresholdBps AND \
259-
liqThresholdBps > 0 (expected exactly 5 OP_GREATERTHAN incl. the 3 \
264+
gt, 6,
265+
"issue must carry initRatioBps > liqThresholdBps + liqThresholdBps > 0 \
266+
+ auctionWindow > 0 (expected exactly 6 OP_GREATERTHAN incl. the 3 \
260267
`> 0` value guards, found {gt})"
261268
);
269+
270+
// OP_GREATERTHANOREQUAL64 for auctionDiscountBps >= 0 — exactly one
271+
// site in the deployment-safety block. (asset-amount `>=` checks like
272+
// `output.assets.lookup(...) >= N` use the same opcode, but those fire
273+
// AFTER the deployment checks and aren't counted as deployment guards.
274+
// Floor at 1 captures the deployment check without coupling to the
275+
// exact count of asset-amount checks.)
276+
let gte = opcode_count(&output, "issue", "OP_GREATERTHANOREQUAL64");
277+
assert!(
278+
gte >= 1,
279+
"issue must carry auctionDiscountBps >= 0 (OP_GREATERTHANOREQUAL64, \
280+
found {gte} total — at least one must be the discount-bound check)"
281+
);
282+
}
283+
284+
#[test]
285+
fn test_issue_uses_ceiling_division_on_required_collateral() {
286+
// Dust-issuance defence: the required-collateral floor is computed via
287+
// CEILING division — `(amount * initRatioBps + 9999) / 10000` — not the
288+
// naive `amount * initRatioBps / 10000`. Without ceiling, an attacker
289+
// can mint 1 credit + 1 debit for 1 sat of collateral (amount=1,
290+
// initRatioBps=14999 → required floors to 1), then flood the auction
291+
// window with thousands of dust defaults that are unprofitable for
292+
// auctioneers to settle.
293+
//
294+
// The signature of the ceiling form in the emitted ASM is the literal
295+
// 9999 pushed before the OP_ADD64 that precedes the OP_DIV64. Asserting
296+
// on the presence of `9999` as a token in both issue + rollIn locks in
297+
// the fix at the ASM level — a regression to floor division (or to
298+
// a different bias like + 5000) trips this test.
299+
let output = compile(CODE).expect("compilation failed");
300+
for fn_name in ["issue", "rollIn"] {
301+
let tokens = asm_tokens(&output, fn_name);
302+
assert!(
303+
tokens.iter().any(|t| t == "9999"),
304+
"{fn_name} must use ceiling division on the required-collateral \
305+
floor (expected literal `9999` token in ASM; absent means \
306+
dust-issuance defence regressed). Tokens: {tokens:?}"
307+
);
308+
}
262309
}
263310

264311
#[test]
@@ -628,19 +675,26 @@ fn test_roll_in_oracle_priced_dual_mints_at_witness_indices() {
628675
}
629676

630677
#[test]
631-
fn test_roll_pair_enforces_both_threshold_invariants() {
632-
// The deployment-safety invariants (initRatioBps > liqThresholdBps AND
633-
// liqThresholdBps > 0) are re-checked in rollIn because rollIn is an
634-
// independent issuance path. Without re-checking, a misconfigured pool
635-
// that escaped issue could still mint vaults via rollIn.
678+
fn test_roll_pair_enforces_all_deployment_invariants() {
679+
// rollIn is an alternate issuance entry and must enforce the SAME
680+
// deployment-safety invariants as issue (initRatioBps > liqThresholdBps,
681+
// liqThresholdBps > 0, auctionWindow > 0, auctionDiscountBps in
682+
// [0, 10000)). Without these re-checks a misconfigured pool that
683+
// somehow escaped issue could still mint fresh vaults via rollIn.
636684
let output = compile(CODE).expect("compilation failed");
637685
let gt = opcode_count(&output, "rollIn", "OP_GREATERTHAN");
638686
// 3 `> 0` value guards (newMintedAmount, newCollateral, oraclePrice) +
639-
// 2 threshold invariants (initRatioBps > liqThresholdBps, liqThresholdBps > 0)
640-
// = 5 OP_GREATERTHAN, same as issue.
687+
// 3 deployment invariants (initRatioBps > liqThresholdBps,
688+
// liqThresholdBps > 0, auctionWindow > 0) = 6 OP_GREATERTHAN, matching
689+
// issue.
641690
assert_eq!(
642-
gt, 5,
643-
"rollIn must replicate issue's invariants (expected 5 OP_GREATERTHAN, found {gt})"
691+
gt, 6,
692+
"rollIn must replicate issue's invariants (expected 6 OP_GREATERTHAN, found {gt})"
693+
);
694+
let gte = opcode_count(&output, "rollIn", "OP_GREATERTHANOREQUAL64");
695+
assert!(
696+
gte >= 1,
697+
"rollIn must carry auctionDiscountBps >= 0 (found {gte} OP_GREATERTHANOREQUAL64)"
644698
);
645699
}
646700

0 commit comments

Comments
 (0)