`BorrowEngine.accrueLoan` throws away the sub-unit interest remainder every time it books, and `borrow` / `repay` / `writeOffLoan` throw it away when nothing booked, so frequently-touched BTC/ETH loans under-collect up to ~47% of their interest

オープン
#78 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る

まだ誰も着手していません。

評価

難易度
4/5
見積もり時間
3〜5日
初心者へのやさしさ
20/100
issue の種類
バグ
明瞭さ
明確に書かれている
活発さ
活発

調査の方向性

Start with accrueLoan in src/backend/lib/BorrowEngine.mo (around lines 69-76), where the floored interest is booked and lastAccrualNs is moved to now. Then check the Loan record in src/backend/lib/Types.mo, which has no field for a carried remainder, and the borrow, repay and writeOffLoan stamps. The reproduction script in the issue should show booked interest converging on the exact single-booking value. Done means that script's under-accrual falls to within rounding, with the existing BorrowEngine behaviour otherwise unchanged.

索引モデルが issue の本文から書いたものです。

説明

Severity (proposed): Low. Category: Solvency & conservation (vault LP interest under-collected).

Each accrual touch loses less than one base unit per loan; nothing is minted and balances stay conserved. But the error always favours the borrower, and since 1.75 the world scan touches every loan several times a minute with no user action. For BTC/ETH loans of roughly 0.05–2 tokens that is a 10–47% under-collection of interest owed to vault LPs (a 0.25 BTC loan at 8% APR pays about 4.2%). On #play this is play-money vault yield; on #production it would be a small, systematic leak of LP income.

Summary

accrueLoan books interest = floor(P·apr·elapsed / (SCALE·YEAR_NS)) and, whenever at least one unit books, sets lastAccrualNs := now. The remainder P·apr·elapsed mod (SCALE·YEAR_NS) is discarded rather than carried. The file header (BorrowEngine.mo:4-8) and the function comment (:54-57) state the opposite: the un-booked fraction "keeps accruing … so nothing is ever lost — it just books later". That only holds while the computed interest is 0. Separately, borrow, repay and writeOffLoan write lastAccrualNs = now after accruing, so a touch that booked 0 also discards the whole pending fraction.

Executed against the real lib/BorrowEngine.mo at 7bcdde6: over one simulated day, a 0.25 BTC loan touched every 30 s booked 47.43% less than single-booking interest, 1 BTC every 6 s 34.29% less, 1 BTC every 30 s 8.01% less; the once-a-day control lost 0%.

Affected surface

Canister: exchange backend (src/backend/main.mo), commit 7bcdde6, DEPLOY_MODE = #play (main.mo:173).

Mechanism

// BorrowEngine.mo:69-76
func accrueLoan(loan : Types.Loan, now : Int) : Types.Loan {
  if (now <= loan.lastAccrualNs) { return loan };
  let apr = switch (Types.borrowApr(loan.token)) { case (?r) { r }; case null { 0 } };
  let elapsed : Nat = Int.abs(now - loan.lastAccrualNs);
  let interest = Fixed.mulDiv(loan.principal * apr, elapsed, Fixed.SCALE * Types.YEAR_NS, false);
  if (interest == 0) { return loan };
  { token = loan.token; principal = loan.principal + interest; lastAccrualNs = now };
};

Fixed.mulDiv(..., false) is a plain floor (Fixed.mo:30-33). If an interval accrued k + f units (k ≥ 1, 0 ≤ f < 1), the call books k and moves the clock to now, so f is gone. The hold-the-clock rule covers only k = 0. With I the interest per touch interval, the fraction lost is (I − ⌊I⌋)/I: close to 50% when I sits just under 2 units, negligible once I is in the hundreds.

The three sibling stamps lose the pending fraction when nothing booked. repay (writeOffLoan is identical at :383 / :392):

case (?l) { accrueLoan(l, now) };   // :419 — < 1 unit accrued: loan returned unchanged
...
Map.add(loans, Text.compare, token, { token; principal = remaining; lastAccrualNs = now });   // :435

borrow (:367-368) writes { token; principal = priorPrincipal + amount; lastAccrualNs = now } the same way.

Touch cadence is set by the world. 1.75 (a225a74) added the accruing world scan: refreshVaultImpairment(true) has 0 call sites at f233d18 and 5 at a225a74 / 7bcdde6. With the liquidation sweep that is roughly 8 accruals per pool per minute at irregular phases. The defect itself predates 1.75 (the floor-and-stamp has been in place since the round-UP fix); 1.75 multiplied the touches and therefore the loss.

Reproduction

Executed: the real lib/BorrowEngine.mo at 7bcdde6 under the interpreter. From the repo root:

moc $(mops sources) --package mdex src/backend --implicit-package=core -r repro.mo

The script opens a margin account with $10M ICPUSD collateral and a vault holding 1,000 BTC, borrows P BTC at t = 0 through BorrowEngine.borrow, calls accrueAll every T seconds for one day, and compares the booked interest with one floor booking over the same day. The last block borrows 0.25 BTC, calls repay(1) at t = 15 s (0.95 unit accrued, nothing books, clock stamped to 15 s), then accrueAll at 45 s.

repro.mo (moc 1.9.0, commit 7bcdde6)
// BorrowEngine.accrueLoan drops the sub-unit remainder on every booking
// (lastAccrualNs := now). Real lib/BorrowEngine.mo, BTC at 8% APR.
import BE "mo:mdex/lib/BorrowEngine";
import ME "mo:mdex/lib/MarginEngine";
import Accounts "mo:mdex/lib/Accounts";
import Types "mo:mdex/lib/Types";
import Debug "mo:core/Debug";
import Runtime "mo:core/Runtime";
import Principal "mo:core/Principal";
import Nat "mo:core/Nat";

let vault = Principal.fromText("aaaaa-aa");
let pool  = Principal.fromText("tz2ag-zx777-77776-aaabq-cai");
func px(t : Types.TokenId) : ?Nat { switch (t) { case ("BTC") { ?10_000_000_000_000 }; case (_) { null } } }; // $100k
func noRes(_ : Principal, _ : Types.TokenId) : Nat { 0 };
let SEC : Nat = 1_000_000_000;
let DAY : Nat = 86_400 * SEC;
let APR : Nat = 8_000_000; // BTC 8%

func setup(p : Nat) : BE.LoanState {
  let st = BE.emptyState(); let mg = ME.emptyState(); let acc = Accounts.emptyState();
  Accounts.setBalance(acc, vault, "BTC", 100_000_000_000);
  switch (ME.open(mg, pool, 0)) { case (#ok(_)) {}; case (#err(e)) { Runtime.trap(e) } };
  Accounts.setBalance(acc, pool, Types.QUOTE_TOKEN, 1_000_000_000_000_000); // $10M collateral
  switch (BE.borrow(st, mg, acc, noRes, vault, pool, "BTC", p, 0, px)) { case (#ok(_)) {}; case (#err(e)) { Runtime.trap("borrow " # e) } };
  st
};

// exact linear interest over `ns` (floor, single booking)
func exact(p : Nat, ns : Nat) : Nat { p * APR * ns / (100_000_000 * Types.YEAR_NS) };

// Step trace: 0.25 BTC, touches at 30 s and 60 s
do {
  let p = 25_000_000;
  let st = setup(p);
  Debug.print("step trace, 0.25 BTC @8%:");
  Debug.print("  exact interest per 30 s (x1e4 units) = " # Nat.toText(p * APR * 30 * SEC * 10_000 / (100_000_000 * Types.YEAR_NS)));
  BE.accrueAll(st, pool, 30 * SEC);
  Debug.print("  t=30s  booked=" # Nat.toText(BE.loanOf(st, pool, "BTC") - p) # "  exact(0..30s)=" # Nat.toText(exact(p, 30 * SEC)));
  BE.accrueAll(st, pool, 60 * SEC);
  Debug.print("  t=60s  booked=" # Nat.toText(BE.loanOf(st, pool, "BTC") - p) # "  exact(0..60s)=" # Nat.toText(exact(p, 60 * SEC)));
  BE.accrueAll(st, pool, 90 * SEC);
  Debug.print("  t=90s  booked=" # Nat.toText(BE.loanOf(st, pool, "BTC") - p) # "  exact(0..90s)=" # Nat.toText(exact(p, 90 * SEC)));
  let st2 = setup(p);
  BE.accrueAll(st2, pool, 90 * SEC);
  Debug.print("  control: ONE touch at t=90s booked=" # Nat.toText(BE.loanOf(st2, pool, "BTC") - p));
};

func run(label_ : Text, p : Nat, periodS : Nat) {
  let st = setup(p);
  let period = periodS * SEC;
  var t = period;
  while (t <= DAY) { BE.accrueAll(st, pool, t); t += period };
  BE.accrueAll(st, pool, DAY);
  let booked : Nat = BE.loanOf(st, pool, "BTC") - p;
  let ex = exact(p, DAY);
  let lostBps = (ex - booked) * 10_000 / ex;
  Debug.print(label_ # ": exact/day=" # Nat.toText(ex) # " booked/day=" # Nat.toText(booked)
    # " under-accrued=" # Nat.toText(lostBps / 100) # "." # (if (lostBps % 100 < 10) "0" else "") # Nat.toText(lostBps % 100) # "%");
};

Debug.print("one simulated day, accrueAll every period:");
run("1 BTC  / 30s ", 100_000_000, 30);
run("1 BTC  / 6s  ", 100_000_000, 6);
run("0.25 BTC/30s ", 25_000_000, 30);
run("3.9 BTC/2s   ", 390_000_000, 2);
run("10 BTC / 1s  ", 1_000_000_000, 1);
run("1 BTC / 86400s (control)", 100_000_000, 86_400);

// repay path: accrue (interest < 1 unit -> no booking) then stamp `now`
do {
  let p = 25_000_000;
  let st = BE.emptyState(); let mg = ME.emptyState(); let acc = Accounts.emptyState();
  Accounts.setBalance(acc, vault, "BTC", 100_000_000_000);
  ignore ME.open(mg, pool, 0);
  Accounts.setBalance(acc, pool, Types.QUOTE_TOKEN, 1_000_000_000_000_000);
  ignore BE.borrow(st, mg, acc, noRes, vault, pool, "BTC", p, 0, px);
  Accounts.setBalance(acc, pool, "BTC", 1);
  // t=15s: exact interest 0.95 unit -> not booked; repay 1 unit stamps now=15s
  switch (BE.repay(st, acc, vault, pool, "BTC", 1, 15 * SEC, px)) { case (#ok(_)) {}; case (#err(e)) { Runtime.trap(e) } };
  BE.accrueAll(st, pool, 45 * SEC);
  Debug.print("repay path: principal after repay(1)@15s + accrue@45s = " # Nat.toText(BE.loanOf(st, pool, "BTC"))
    # " (one-booking floor of 0..45s on the same principal: " # Nat.toText(p - 1 + exact(p - 1, 45 * SEC)) # ")");
};
step trace, 0.25 BTC @8%:
  exact interest per 30 s (x1e4 units) = 19025
  t=30s  booked=1  exact(0..30s)=1
  t=60s  booked=2  exact(0..60s)=3
  t=90s  booked=3  exact(0..90s)=5
  control: ONE touch at t=90s booked=5
one simulated day, accrueAll every period:
1 BTC  / 30s : exact/day=21917 booked/day=20160 under-accrued=8.01%
1 BTC  / 6s  : exact/day=21917 booked/day=14400 under-accrued=34.29%
0.25 BTC/30s : exact/day=5479 booked/day=2880 under-accrued=47.43%
3.9 BTC/2s   : exact/day=85479 booked/day=43200 under-accrued=49.46%
10 BTC / 1s  : exact/day=219178 booked/day=172800 under-accrued=21.15%
1 BTC / 86400s (control): exact/day=21917 booked/day=21917 under-accrued=0.00%
repay path: principal after repay(1)@15s + accrue@45s = 25000000 (one-booking floor of 0..45s on the same principal: 25000001)

The step trace shows the mechanism directly: 1.9025 units accrue per 30 s, one books, 0.9025 is dropped, so three touches book 3 where one touch books 5. The repay line is one unit short of the single-booking figure.

Modelled, not executed on a canister: a BigInt model of accrueLoan on an approximation of the organic #play cadence (heat beat and liquidation sweep each every 30 s, four markets each re-marked about every 60 s with jitter; mean gap 7.5 s). Output: 0.05 BTC lost 12.4%, 0.25 BTC 47.5%, 1 BTC 21.2%, 2 BTC 10.8%, 5 BTC 3.9%, 50 BTC 0.4%. The model's cadence is an assumption, not logged beat timestamps from a venue.

Traced only (no replica run): a non-controller pool owner can add touches: createMarginPool (main.mo:14865), open a BTC short (borrowed at fill, main.mo:3568), then call repayPoolDebt(poolId, "BTC", 1) repeatedly while the pool holds no free BTC. Each call accrues at :15042 and returns #err("No free BTC …") at :15048 with the accrual committed. In the model each extra call saves at most about one base unit (0.27–0.90 sat per call).

Impact

  • Who loses: vault LPs, through interest never booked; borrowing pools gain the same amount. No value is minted; the debt is simply booked smaller, and vault NAV and the impairment provision consistently read the smaller figure.
  • Bound: under one base unit per accrual touch per (pool, token) loan; as a fraction of interest owed, (I − ⌊I⌋)/I < 50%.
  • Organic, no attacker: BTC/ETH (8% APR, 1e8 units) loans of roughly 0.05–2 tokens under-collect about 10–47% on the modelled #play cadence; 5+ tokens under 4%; large loans negligible. ICPUSD/SOL/ICP lose the same fraction only at sizes where it is dust.
  • Gaming: an owner cannot widen the gaps between world touches but can choose loan size; sizing (or splitting across pools, up to MAX_POOLS_PER_OWNER = 64, main.mo:5884) toward ~0.25 BTC/ETH roughly halves the effective borrow rate. Extra repayPoolDebt calls add little.
  • Magnitude: about 2,600 sat/day on a 0.25 BTC loan (~$2.60 at $100k/BTC). Play money on #play.
  • Not established: no measurement on a live venue; no leaderboard or fee effect traced.

Relationship to existing reports

  • #54 (OhShii Labs, round 18), informational comment line: noted that the header still said interest was "rounded UP" while the code "rounds DOWN with carry-forward, deliberately". 1.75 rewrote the header to "nothing is lost, it just books later". That note accepted the carry-forward design as working; this report shows the carry survives only while the booked interest is 0, so the stated invariant is false in code, not just in a comment.
  • The round-UP "phantom unit" fix described at BorrowEngine.mo:58-63 fixed the opposite error (over-charging dust debts). This is its incomplete half: the hold rule was applied to the interest == 0 branch but not to the booked branch or the three sibling stamps.
  • #62 (OhShii Labs, round 19), "interest charged from the refused slice": a borrow on a refused fill. Different root cause.
  • The accrue-before-close / settleFromQuote work (task 1788516128) covers interest booked between a close's call and its settlement message. Different mechanism.
  • No match found in the issues, advisories, PRs #57/#58, docs/issue-triage-2026-08.md, docs/hardening-plan-2026-09.md, docs/security-review.md, docs/pre-mainnet-checklist.md or docs/tasks/done/ for accrueLoan, lastAccrualNs, carry-forward, or interest rounding/remainder.
  • docs/integer-money-migration.md:74 still states the policy "interest up"; this code rounds every booking down and does not carry the difference.

Suggested fix

No schema change. Carry the remainder as a back-dated clock: book the floor, and set lastAccrualNs early enough that the next accrual re-earns the discarded fraction. This extends the existing hold rule at :74 from "hold the whole clock when nothing booked" to "hold back exactly the un-booked part". Round the back-date up (Fixed.mulDiv(..., true)), in line with the "interest up" policy.

// accrueLoan, replacing :73-75
let num = loan.principal * apr * elapsed;
let den = Fixed.SCALE * Types.YEAR_NS;
let interest = num / den;
if (interest == 0) { return loan };                       // existing hold
let p = loan.principal + interest;
{ token = loan.token; principal = p;
  lastAccrualNs = now - Fixed.mulDiv(num % den, 1, p * apr, true) };   // carry the remainder

At the three stamp sites (:368, :392, :435), keep the carry under the new principal instead of writing now: with carry = old.principal * apr * Int.abs(now - old.lastAccrualNs) (the un-booked numerator, below one unit after the accrual that just ran), stamp now - Fixed.mulDiv(carry, 1, newPrincipal * apr, true), or now when newPrincipal * apr == 0. Full closes that delete the entry forgive under one unit once per loan, which is fine. With lossless accrual, touch cadence stops mattering, so repayPoolDebt's accrue-before-validate needs no change.

Checked with a BigInt model of the sketch, one day, current / fixed / single-booking: 0.25 BTC every 30 s: 2,880 / 5,480 / 5,479; 1 BTC every 6 s: 14,400 / 21,920 / 21,917; 10 BTC every 1 s: 172,800 / 219,202 / 219,178. The small excess in the fixed column is the per-touch compounding the design already intends. Not compiled into the canister; tests/test_vault_lp.sh's interest_ns helper models a single-window floor and may need a one-unit tolerance.

Regression test: borrow 0.25 BTC, accrueAll every 30 s to 90 s, assert 5 units booked (today 3); then repay(1) at 15 s and accrueAll at 45 s, assert the single-booking figure (today one unit short).


Part 11 of 28 from an independent review of MULTI/DEX 1.80 (7bcdde6), carried out with Claude Opus 5.5. Each finding was checked against the existing issues, advisories and fix briefs before filing; the severity is a proposal under CONTRIBUTORS.md. Where a step says "traced", it was read from source and not executed.

主要言語
Shell
スター
19
フォーク
7
PR マージ指標
30日以内にマージされた PR はありません

環境構築

このプロジェクトには開発コンテナ、Dockerfile、コントリビューションガイドがありません。まず README を読み、一般的な手順ははじめてのコントリビューションガイドを参照してください。

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

dfinity/public-multidex のほかの issue

dfinity/public-multidex の issue をすべて見る

似ている issue

Shell/Bash の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。