Menese DeFi Team evaluation, Part 2/2: value, integrity, and the oracle input path

[KYounesMercatura]

Menese DeFi Team evaluation of MULTI/DEX, Part 2/2: value, integrity, and the oracle input path

Continuing our evaluation (Part 1 covered load-bounding). This group is about places
where a stated guarantee (conservation of value, ledger integrity, oracle robustness)
holds almost everywhere but has one seam. None require a malicious controller; all are
reproduced on the published tip (commit 0241cbafb779415e45dc56f0fabc1bbbb6ba9a5d). Each
item follows the report shape SECURITY.md asks for: affected surface, steps to reproduce,
and the impact we believe it has.

One note up front: your mulDiv is arbitrary-precision Nat, so the Solidity
mulDiv-overflow class (the one that forces Uniswap's FullMath) does not exist here.
The items below are narrower.

The two pure-function findings here (items 3 and 4) ship as runnable Motoko tests:
https://gist.github.com/KYounesMercatura/3bb91a214ed9419fd17423915cceb36f — drop
ReproR5.test.mo and ReproParser.test.mo into tests/ and run mops test ReproR5 /
mops test ReproParser.

---

1. The insurance tranche's share math is asymmetric; a JIT round-trip mints value from the fund (a ported ERC-4626 detail)

Where: stake mints with the virtual-share offset in stakeInsurance
(src/backend/main.mo:10625-10631), using the same OpenZeppelin pattern as
VaultMath.mintAmount. Unstake redeems at the raw ratio in unstakeInsurance
(main.mo:10668), with no offset. seedInsuranceFund (main.mo:10697) is also raw.

Why it matters: ERC-4626's virtual-share offset is designed to apply to both conversion
directions so that a mint-then-burn round-trip is value-neutral. Applied to mint only,
the round-trip is not the identity: redeem(mint(A)) > A whenever the share value exceeds
1.0. Share value exceeds 1.0 as the normal state, because liquidation penalties accrue to
the fund without minting shares (settleInsuranceArrears, main.mo:5786-5793). So a
staker can enter just before a penalty lands and exit just after, skimming a slice of the
penalty that is meant to reward the long-term stakers who bear the bad-debt risk. There is
no exit fee and no minimum stake duration on this tranche. The AMM LP tranche has the same
asymmetry, but its LP_EXIT_FEE_BPS = 40 dominates the sub-basis-point edge, so it is not
profitable there; the fix already exists in your codebase.

Reproduction (local replica, exact to the base unit):

1. Bootstrap the insurance pool and stake a seed so shareSupply > 0.

2. Move the share value off parity (in production this is the normal state via penalty
   accrual; on a fresh replica we credited the fund principal directly to reach
   shareValueUsd = 1.03333333).

3. Stake A = 1_000_000_000_000 and immediately redeem the shares minted:

   shareValue at mint : 1.03333333
   staked   A         : 1_000_000_000_000
   returned V         : 1_000_000_104_165     (+104_165 base units from nothing)
   pool buffer        : fell by exactly 104_165 (the existing stakers funded it)
   

4. Confirmed at four share values (1.033, 1.05, 1.083, 1.15), each matching the closed
   form gain = A*V*(P-S) / ((P+V)*(S+m)) to the base unit.

Why we flag it despite the small magnitude: it is permissionless, repeatable, and it
targets the one fund whose whole job is solvency; over time it pushes out the honest
long-term stakers the backstop depends on. It sits in your stated "mints, loses, or
double-counts value" category.

Suggested direction: apply the virtual-share offset to the redeem side too, or add an
insurance exit fee / minimum duration mirroring LP_EXIT_FEE_BPS. The simplest fix is
symmetry: offset on both conversions, or neither.

---

2. verifyChain skips one link per page and still reports ok = true

Where: src/backend/ArchiveCanister.mo:231-259.

Why it matters: the archive's promise is an append-only, offline-verifiable hash chain
under a certified head. verifyChain seeds var prev = null and only compares each
event's prevHash against the previous event in the same call. So the first event of
every page has its inbound link unchecked, and nextSeq resumes at an event whose link is
skipped again. A paged audit of N events at page size P leaves ceil(N/P) - 1 links
unverified, and returns ok = true throughout. Corruption sitting exactly on a page
boundary is invisible to the one function built to detect it. This is a detection gap, not
an insertion path; appendBatch still enforces the chain on ingest, so an attacker would
need pre-existing ability to alter stored bytes.

Reproduction (live archive on a local replica, 56 events / 55 links):

page size	pages	links verified	needed	silently unverified
5	12	44	55	11
10	6	50	55	5
20	3	53	55	2

The gap equals pages - 1 at every page size, with ok = true each time. At the 10,000
page cap a 10M-event archive leaves ~999 links unchecked while reporting success.

Suggested direction: one line; when start > chainStartSeq, seed prev from
loadAt(globalIndex, start - base - 1) so each page re-checks its boundary link.

---

3. Outlier rejection is inoperative at the minimum source count you accept

Where: PRICE_MIN_SOURCES = 2 (src/backend/main.mo:12901) versus trimOutliers, which
returns the sample set untrimmed below 3 (lib/PriceFeed.mo:199-200,
if (n < 3) { return xs }).

Why it matters: the aggregator will accept a mark computed from as few as 2 readings, but
the plus/minus 2-sigma outlier filter only engages at 3 or more. So at exactly the minimum
you allow, the outlier rejection does nothing and a single rogue or glitching source
carries 50% weight of a mark that drives collateral valuation and liquidations. The
exposure window is precisely when sources are scarce, the moment robustness matters most.

Reproduction (mops test, pure PriceFeed functions):

• n=2, one honest (100) plus one rogue (200): trimOutliers returns both untrimmed, and
  the robust median is 150; the rogue moved the mark 50%.
• n=3, two honest (100) plus one rogue (200): trimmed, mark holds at 100.
• The only difference between the two is the sample count.

Suggested direction: either raise PRICE_MIN_SOURCES to 3 so a published mark always
passes through the filter, or apply a 2-of-2 agreement bound (reject when two sources
disagree beyond a threshold) for the n=2 case.

---

4. The primary oracle parser drops scientific notation and first-matches keys

Where: parseLeadingFloat (lib/PriceFeed.mo:271-312) and numberAfterKey/findAfter
(:316-339), used by extractFromBody (:341) on the HTTPS-outcall bodies fetched in
main.mo:fetchFromSource (:11959). This is the primary mark path
(refreshMultiSourcePrice, main.mo:3039); the XRC canister is only a typed fallback
anchor and does not go through this parser.

Why it matters: parseLeadingFloat stops at the first non-digit, so a scientific-notation
value loses its exponent silently, and numberAfterKey returns the number after the first
occurrence of the key. Both feed the mark that drives liquidations and collateral value,
and they compound with item 3: at the 2-source minimum the outlier filter is off, so a
single misparsed source moves the median directly.

Reproduction (mops test, pure PriceFeed functions):

parseLeadingFloat("1.5e3")    => 1.5     (true value 1500, 1000x low)
parseLeadingFloat("1.2e-8")   => 1.2     (true value 0.000000012, ~1e8 high)
parseLeadingFloat("6.4E4")    => 6.4     (true value 64000)
parseLeadingFloat("64046.03") => 64046.03  (control, plain decimal, correct)
numberAfterKey("{\"a\":\"99.90\",\"ticker\":{\"a\":\"2.29\"}}", "\"a\"")  => 99.90  (intended 2.29)

This is latent for the currently-wired assets (BTC/ETH/SOL/ICP print plain decimals) and
becomes active if a low-priced asset is added, or a provider emits scientific notation or
reorders fields so the first key match is the wrong one.

Suggested direction: parse the exponent (or reject a value containing e/E explicitly
rather than truncating it), and anchor each extractor's key to its full path so a short
key cannot first-match an earlier field.

---

We are happy to open PRs; several of these are one- to few-line changes.

Filed by the Menese DeFi Team.

[rvnt9999]

We ran trimOutliers rather than reading it, and the output settles a disagreement that has now
run through three issues — including a wrong remedy of our own. Posting it here because item 3 is
where the narrative starts.

Your finding is correct. Outlier rejection is inoperative at PRICE_MIN_SOURCES = 2. The n=2
row of your reproduction is exactly right and is the real defect: the rogue moves the mark 50%.

One line of the reproduction is not. Item 3 reads:

• n=3, two honest (100) plus one rogue (200): trimmed, mark holds at 100.

Nothing is trimmed at n=3. Production calls trimOutliers(okPrices, 2.0) (lib/PriceFeed.mo:248,
inside aggregate), so we ran the repository's own function at that exact σ, one rogue at 200
against n−1 honest sources at 100:

n=2  kept=2/2  rogue rejected: NO   robustMedian=150
n=3  kept=3/3  rogue rejected: NO   robustMedian=100
n=4  kept=4/4  rogue rejected: NO   robustMedian=100
n=5  kept=4/5  rogue rejected: YES  robustMedian=100
n=6  kept=5/6  rogue rejected: YES  robustMedian=100

Reproduction, from the repo root — it compiles nothing but a five-line probe and modifies nothing:

// TrimProbe.mo
import PriceFeed "mo:mdlib/PriceFeed";
import Debug "mo:core/Debug";
let k = 2.0;
Debug.print(debug_show(PriceFeed.trimOutliers([100.0, 100.0, 200.0], k)));            // [100, 100, 200]
Debug.print(debug_show(PriceFeed.trimOutliers([100.0, 100.0, 100.0, 200.0], k)));     // [100, 100, 100, 200]
Debug.print(debug_show(PriceFeed.trimOutliers([100.0,100.0,100.0,100.0,200.0], k)));  // [100, 100, 100, 100]

MOC=$(mops toolchain bin moc)
read -r -a SRCS <<< "$(mops sources | tr '\n' ' ')"
"$MOC" "${SRCS[@]}" --package mdlib "$PWD/src/backend/lib" --implicit-package=core -r TrimProbe.mo

Why the mark holds at 100 at n=3 is the median, not the filter. With three samples the median
is the middle value, so a single outlier cannot move it whether or not it was trimmed. The
if (n < 3) { return xs } guard at :201 is real and you cite it correctly — but clearing that
guard is not the same as the band actually rejecting anything. At n=3 and n=4 the lone outlier
inflates the sample stddev enough that ±2σ still covers it. The band cannot reject a lone
outlier until n = 5.

Why this matters rather than being a nit. The n=3 row is what makes the remedy look
sufficient. Item 3 suggests:

raise PRICE_MIN_SOURCES to 3 so a published mark always passes through the filter

At n=3 the mark does pass through the filter, but the filter is a no-op — so the change buys
the appearance of outlier rejection and none of the substance. We proposed the same remedy, in
the same words, in our own #9 item 1, and it was wrong for the same reason. @andreij6 already
established this in #17 from the algebra (median ± 1.1547·d at n=3, exactly ±d at n=4, with an
inclusive keep test at PriceFeed.mo:209). The run above is independent confirmation of #17 from
the other direction, and the boundary he derived — n=5 — is the boundary the function exhibits.

So the correct reading of the three reports together: the defect is real and is yours and ours
jointly; the fix is neither of the two we proposed; #17's direction (a high-breakdown estimator —
MAD × 1.4826, or re-trimming on the kept set, with stddevBps computed after the trim) is the
one that actually closes it.

Your second suggested direction — a 2-of-2 agreement bound for the n=2 case — is untouched by
this and, on our reading, is the more valuable half of item 3: it addresses the row where the
mark genuinely moves.

— Ravenith, OhShii Labs

[dominicwilliams]

All four findings confirmed. Two are fully fixed, one is fixed at the site you named but not at the level a later report showed was needed, and one had its remedy superseded.

#3.1 — our own 31 July fix created the asymmetry. Confirmed and fixed. e66a27e added the virtual offset to stakeInsurance only; unstakeInsurance had never been touched since genesis, so redeem(mint(A)) > A whenever share value > 1.0 — which is the normal state, not an edge case. Verification reproduced your figure exactly: +104,165 base units. This is the cleanest finding in either wave: it identified that a fix was the defect, which is the hardest class of bug to see from outside.

Also fixed alongside it, and worth knowing because it is a distinct root cause rather than the same one: OhShii's #7.2 found that stakeInsurance's mint denominator omits insuranceOwedUsd. Fixing either alone would have left the other open.

#3.2 — verifyChain reseeds prev = null per call. Confirmed and fixed: one link per page went unverified while the function returned ok = true, in exactly the paged-audit flow its own docstring recommends. It now seeds from the predecessor and returns ok = false with brokenAt when that predecessor is unreadable.

But the function is not yet trustworthy, and a third reviewer caught the reason. andreij6's #18 found that verifyChain never anchors its recomputed tail to the certified chainHead, so corruption of the newest event — or any consistent rewrite of a tail suffix — still verifies clean. Their filing predicted precisely this: that a sibling fix landing next to a defect would make the function read as repaired while the deeper hole stayed open. They were right, and the coverage-auditing test your fix gained makes it look more finished than it is. That is open.

#3.3 — trimOutliers returns untrimmed below 3 samples while PRICE_MIN_SOURCES = 2. Confirmed, with two qualifications and a superseded remedy:

• Your contrast example for n=3 does not hold: trimOutliers doesn't exclude the outlier at n=3 either, and the median's breakdown point does the work you attribute to the trim. Your conclusion is right; that piece of evidence is not.
• The real-world exposure was bounded by a 50bps dispersion gate the report did not account for.
• The remedy you proposed — raising PRICE_MIN_SOURCES to 3 — was implemented and then withdrawn, and OhShii withdrew the same suggestion from their OhShii Labs review, 6/8 · 4 findings (#9.1–#9.4): the oracle input path and the archive shed #9.1. andreij6's The outlier-trim fix proposed in #3 and #9 leaves the defect in place #17 showed the band was computed over the whole set including the outlier, so raising the floor moves the failure from "the trim never runs" to "the trim runs and mathematically cannot reject". The shipped fix is a MAD-based band with a 50% breakdown point, a Croux-Rousseeuw finite-sample correction, and a 50bps width floor.

#3.4 — parseLeadingFloat silently truncates scientific notation. Confirmed, and the generalisation is the valuable part: findAfter is first-occurrence by construction, so every extractor built on it is fragile, not just the three you named. Latent for today's plain-decimal feeds, which is why it is recorded rather than urgent.

One residual on the #3.3 line, disclosed rather than quietly carried: _minSourcesOverride can still pin the source floor below the robustness floor. It is dev-gated and today's only caller raises it, so nothing is broken — but the hook can defeat the n≥3 guarantee and should clamp.

The code is in #30, the first push to this mirror since the July snapshot; your line numbers against 0241cba still resolve.