feat: InventoryHedge — market-maker inventory-hedging vault#41
Conversation
Perpetual, BTC-settled, fully-collateralized market-maker inventory hedge generalizing stability_vault.ark: a desk converts residual BTC inventory into a delta-flat fiat claim (claim leg) while a treasury holds the BTC long leg and earns funding. Oracle-marked, no liquidation, one instance per BTC/<fiat> pair. - examples/hedging/inventory_hedge.ark (+ compiled .json): transfer, updateFunding, addCapital, removeCapital, claimExit, longExit (12 tapleaves). - tests/inventory_hedge_test.rs: roundtrip (both variants, non-empty witness schemas), oracle-path checks, claimExit clamp branches, <SERVER_KEY> injection, updatedAt-stripped determinism. - docs/mm-residual-hedge.md (design note) + docs/hedging/inventory_hedge_tx_flows.md (per-function tx layouts). - playground: Hedging project folder wired in main.js. https://claude.ai/code/session_01E3fpVJfLdM3ye3ZWy5VxGg
|
Warning Review limit reached
More reviews will be available in 49 minutes and 51 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughThis PR adds a BTC-settled InventoryHedge: a design doc, an Ark contract implementing six state transitions (transfer, updateFunding, addCapital, removeCapital, redeem, withdraw), compiled JSON artifact and playground registration, plus tests validating ASM/witness invariants and determinism. ChangesInventoryHedge Hedging Mechanism
Sequence DiagramsequenceDiagram
participant Desk as Market Desk
participant Treasury as Treasury/Provider
participant Vault as InventoryHedge
participant Claim as Claim Leg
participant Long as Long Leg
participant Oracle as Oracle
Desk->>Vault: transfer (new claim key)
Treasury->>Vault: updateFunding (roll accrual & set rate)
Treasury->>Vault: addCapital (deposit sats)
Treasury->>Vault: removeCapital (withdraw with oracle check)
Oracle->>Vault: signed price + timestamp
Desk->>Vault: redeem (claim payout, oracle-settled)
Treasury->>Vault: withdraw (long payout, oracle-settled)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
Dialectical reviewThesis — the case for this implementation
Antithesis — the case against (and an alternative draft)A separate one-shot draft (
So "compiles successfully" ≠ "correct": the draft compiles and leaves the clamp, the payout destination, and the state transition all unbound. The lesson cuts at this PR too — a green But the draft lands two fair hits worth keeping:
SynthesisPrefer the implementation whose guarantees are machine-checked against this compiler, not the one that reads best. This PR is the conservative generalization: same primitives as the reference, clamp/recursion/payout all genuinely enforced, gates green. Fold in the draft's insights (document the int64 ceiling; consider a Open questions for reviewers:
Draft — opened for review, not merge. Generated by Claude Code |
Playground PreviewA live preview of this PR's playground is available at:
|
Merge docs/hedging/inventory_hedge_tx_flows.md into docs/mm-residual-hedge.md as 'Part II — Transaction flows', cross-referencing the design sections (§2/§4/§6/ §9). Remove the standalone file and point the playground Hedging folder at the single doc. https://claude.ai/code/session_01E3fpVJfLdM3ye3ZWy5VxGg
Drop the Exit suffix: claimExit -> redeem (desk redeems its fiat claim for BTC), longExit -> withdraw (treasury-driven settlement). Update the contract, tests, and the Part II transaction-flow doc. https://claude.ai/code/session_01E3fpVJfLdM3ye3ZWy5VxGg
![arkanaai[bot]](https://avatars.githubusercontent.com/u/170820085?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 Arkana Code Review — InventoryHedge
Overall this is a clean generalization of StabilityVault with the exit fee removed and naming adapted for the hedging use case. The core settlement math, oracle pattern, anti-grief rule, and clamp logic are correctly carried over. The rename to redeem/withdraw in the latest commit is clean. No downstream consumers exist yet.
I found one substantive issue and a few items worth discussing before this leaves draft.
🔴 Issue — removeCapital missing newTargetFiat > 0 guard
inventory_hedge.ark:116 (the removeCapital function)
updateFunding correctly guards require(newTargetFiat > 0, "claim wiped by funding"), but removeCapital computes the same newTargetFiat = targetFiat + delta and feeds it into the collateral-ratio check without validating it's positive:
int newTargetFiat = targetFiat + delta;
int currentClaimBase = newTargetFiat * 100000000 / oraclePrice;
int minCollateral = currentClaimBase * (100 + collateralRatioPct) / 100;
If the initial fundingRatePerSec is negative (which the constructor does not prevent — the >= 0 constraint only applies to updateFunding's new rate), and enough time passes before the first updateFunding call, newTargetFiat can go negative → currentClaimBase negative → minCollateral negative → the ratio check newTotalCollateral >= minCollateral becomes trivially true → the long leg drains the vault.
Yes, this is the same pattern as StabilityVault. And yes, in practice the initial rate should be non-negative for a hedging vault. But defense-in-depth says: add require(newTargetFiat > 0, "claim wiped by funding") in removeCapital (and arguably in redeem/withdraw too, before the clamp — though the clamp handles it safely, an explicit guard documents intent).
Same applies to redeem and withdraw at lines ~651 and ~696. The clamp (if (claimRaw <= 0)) handles the case correctly, but an explicit require(newTargetFiat > 0) before the conversion makes the invariant visible and prevents subtle bugs if the clamp logic is ever refactored.
🟡 Observations (non-blocking, but worth considering)
1. Doc comment says "signed" but contract enforces >= 0 — inventory_hedge.ark:10 says fundingRatePerSec is a "signed fixed-point fraction" but updateFunding enforces newFundingRatePerSec >= 0. The initial rate at construction is the only place a negative value could enter. If negative initial rates are not intended for the hedging use case, consider calling this out explicitly in the doc (or adding a construction-time guard if the language supports it). Current wording could mislead an integrator into thinking negative rates are supported at runtime.
2. Doc §6 references split for resizing but the contract omits it — The design doc talks about "split → peel off part of the claim when D shrinks" and "merge → combine positions when D grows." These are deferred to "thinnest first build" per §10, but the doc still reads as if they exist. Consider either (a) noting in the doc that split/merge are deferred, or (b) adding a forward-reference to a future PR.
3. Compiler warnings in the JSON artifact — The compiled inventory_hedge.json carries 50+ output-invariant warnings (e.g., placeholder <tx.offchainTime> is not in witnessSchema). These are expected for computed intermediates and match StabilityVault's pattern, but shipping them in the committed JSON is noisy. If the compiler has a way to suppress expected warnings, it'd clean up the artifact.
4. Test coverage is compilation-level only — The test suite (tests/inventory_hedge_test.rs) pins structural properties (12 tapleaves, SERVER_KEY injection, OP_IF branches for the clamp). There are no behavioral/simulation tests for edge cases like: anti-grief rule triggering, collateral ratio boundary, dust threshold routing, zero-elapsed funding. This matches StabilityVault's test pattern, but for a protocol-critical contract, consider adding property-based or scenario tests that exercise the arithmetic with concrete values.
✅ What looks good
- Oracle verification: Fuji-pattern
sha256(ticker + price + time)+checkSigFromStack+ 600s freshness window — correct and consistent with StabilityVault. - Anti-grief rule in
updateFunding: correctly prevents zero-accrual clock advancement whenfundingRatePerSec != 0. - Settlement clamp in
redeem/withdraw: three-way branch (claim wiped / fully covered / split) with 330-sat dust guard — correct. - Self-replacement pattern: all non-terminal functions correctly assert
tx.outputs[0].scriptPubKey == new InventoryHedge(...)with the next state. - Exit variants: 2-of-2 (or 3-of-3 for transfer) multisig + CLTV — standard operator-liveness backstop.
removeCapitalcorrectly uses accrued funding for the ratio check without rolling it into the vault state (no clock advancement).- Playground integration is clean — just a new project entry.
- Rename to
redeem/withdraw(commit 3) was applied consistently across .ark, .json, .md, and .rs files.
Verdict: Request changes on the newTargetFiat > 0 defense-in-depth guard. Everything else is solid. Flagging for mandatory human review — this is a new settlement contract that handles real funds.
There was a problem hiding this comment.
🔍 Arkana Code Review — InventoryHedge
Overall this is a clean generalization of StabilityVault with the exit fee removed and naming adapted for the hedging use case. The core settlement math, oracle pattern, anti-grief rule, and clamp logic are correctly carried over. The rename to redeem/withdraw in the latest commit is clean. No downstream consumers exist yet.
I found one substantive issue and a few items worth discussing before this leaves draft.
🔴 Issue — `removeCapital` missing `newTargetFiat > 0` guard
`inventory_hedge.ark:116` (the `removeCapital` function)
`updateFunding` correctly guards `require(newTargetFiat > 0, "claim wiped by funding")`, but `removeCapital` computes the same `newTargetFiat = targetFiat + delta` and feeds it into the collateral-ratio check without validating it's positive:
```ark
int newTargetFiat = targetFiat + delta;
int currentClaimBase = newTargetFiat * 100000000 / oraclePrice;
int minCollateral = currentClaimBase * (100 + collateralRatioPct) / 100;
```
If the initial `fundingRatePerSec` is negative (which the constructor does not prevent — the `>= 0` constraint only applies to `updateFunding`'s new rate), and enough time passes before the first `updateFunding` call, `newTargetFiat` can go negative → `currentClaimBase` negative → `minCollateral` negative → the ratio check `newTotalCollateral >= minCollateral` becomes trivially true → the long leg drains the vault.
Yes, this is the same pattern as StabilityVault. And yes, in practice the initial rate should be non-negative for a hedging vault. But defense-in-depth says: add `require(newTargetFiat > 0, "claim wiped by funding")` in `removeCapital` (and arguably in `redeem`/`withdraw` too, before the clamp — though the clamp handles it safely, an explicit guard documents intent).
Same applies to `redeem` and `withdraw` at lines ~651 and ~696. The clamp (`if (claimRaw <= 0)`) handles the case correctly, but an explicit `require(newTargetFiat > 0)` before the conversion makes the invariant visible and prevents subtle bugs if the clamp logic is ever refactored.
🟡 Observations (non-blocking, but worth considering)
1. Doc comment says "signed" but contract enforces `>= 0` — `inventory_hedge.ark:10` says `fundingRatePerSec` is a "signed fixed-point fraction" but `updateFunding` enforces `newFundingRatePerSec >= 0`. The initial rate at construction is the only place a negative value could enter. If negative initial rates are not intended for the hedging use case, consider calling this out explicitly in the doc (or adding a construction-time guard if the language supports it). Current wording could mislead an integrator into thinking negative rates are supported at runtime.
2. Doc §6 references `split` for resizing but the contract omits it — The design doc talks about "split → peel off part of the claim when D shrinks" and "merge → combine positions when D grows." These are deferred to "thinnest first build" per §10, but the doc still reads as if they exist. Consider either (a) noting in the doc that split/merge are deferred, or (b) adding a forward-reference to a future PR.
3. Compiler warnings in the JSON artifact — The compiled `inventory_hedge.json` carries 50+ `output-invariant` warnings (e.g., `placeholder <tx.offchainTime> is not in witnessSchema`). These are expected for computed intermediates and match StabilityVault's pattern, but shipping them in the committed JSON is noisy. If the compiler has a way to suppress expected warnings, it'd clean up the artifact.
4. Test coverage is compilation-level only — The test suite (`tests/inventory_hedge_test.rs`) pins structural properties (12 tapleaves, SERVER_KEY injection, OP_IF branches for the clamp). There are no behavioral/simulation tests for edge cases like: anti-grief rule triggering, collateral ratio boundary, dust threshold routing, zero-elapsed funding. This matches StabilityVault's test pattern, but for a protocol-critical contract, consider adding property-based or scenario tests that exercise the arithmetic with concrete values.
✅ What looks good
- Oracle verification: Fuji-pattern `sha256(ticker + price + time)` + `checkSigFromStack` + 600s freshness window — correct and consistent with StabilityVault.
- Anti-grief rule in `updateFunding`: correctly prevents zero-accrual clock advancement when `fundingRatePerSec != 0`.
- Settlement clamp in `redeem`/`withdraw`: three-way branch (claim wiped / fully covered / split) with 330-sat dust guard — correct.
- Self-replacement pattern: all non-terminal functions correctly assert `tx.outputs[0].scriptPubKey == new InventoryHedge(...)` with the next state.
- Exit variants: 2-of-2 (or 3-of-3 for transfer) multisig + CLTV — standard operator-liveness backstop.
- `removeCapital` correctly uses accrued funding for the ratio check without rolling it into the vault state (no clock advancement).
- Playground integration is clean.
- Rename to `redeem`/`withdraw` (commit 3) was applied consistently across .ark, .json, .md, and .rs files.
Verdict: Request changes on the `newTargetFiat > 0` defense-in-depth guard. Everything else is solid. Flagging for mandatory human review — this is a new settlement contract that handles real funds.
Add require(newTargetFiat > 0) in removeCapital so a claim driven non-positive by a negative *initial* funding rate (not prevented at construction) can't make minCollateral <= 0 and trivially pass the ratio check — which would let the long leg drain the vault. Matches the existing updateFunding guard. Deliberately NOT added to redeem/withdraw: their claimRaw <= 0 clamp branch is the correct terminal settlement for a wiped claim (all collateral to the long leg); a hard require there would abort a legitimate withdraw. Documented inline. Also: clarify fundingRatePerSec doc (runtime >= 0; negative only possible at construction, unsupported), and note split/merge are deferred (design doc Part II). https://claude.ai/code/session_01E3fpVJfLdM3ye3ZWy5VxGg
|
Thanks — substantive review. Addressed in 🔴
|
There was a problem hiding this comment.
🔍 Arkana Code Review — follow-up on 9ed13de (removeCapital guard)
Previous review requested require(newTargetFiat > 0) in removeCapital. This commit delivers it correctly:
removeCapital(line ~170): Guard added afternewTargetFiatcomputation, before the collateral ratio check. Correct placement — blocks the negative-minCollateral drain vector.redeem/withdraw: Deliberately left without a hard guard. The reasoning in the added comments is sound: these are terminal settlement functions where the clamp (if (claimRaw <= 0)) is the correct invariant — it routes all collateral to the long leg, which is the right outcome for a wiped claim. Arequirewould block legitimate settlement. This is actually better than what I suggested.- Doc fixes:
fundingRatePerSeccomment now correctly documents the runtime vs. construction asymmetry. Split/merge deferred note added to the design doc. - JSON artifact: Recompiled cleanly —
OP_GREATERTHANopcode present inremoveCapitalscript, additionalcomparisonwitness schema entry matches.
All requested changes resolved. Approving.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
docs/mm-residual-hedge.md (1)
91-91: 💤 Low valueConsider adding language specifiers to fenced code blocks.
The markdown linter flags 12 fenced code blocks without language specifiers. While the content is correct, adding identifiers like
textorpseudocodewould improve rendering and markdown hygiene.Example fix for one block
-``` +```text fundingRate = clamp( premiumIndex + carryComponent , -cap, +cap ) premiumIndex ∝ open-interest skew between claim and long legsAlso applies to: 132-132, 287-287, 299-299, 313-313, 337-337, 355-355, 367-367, 379-379, 388-388, 411-411, 439-439
🤖 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/mm-residual-hedge.md` at line 91, Multiple fenced code blocks in docs/mm-residual-hedge.md are missing language specifiers; update each triple-backtick block (``` ... ```) noted in the review to include an appropriate language tag such as text or pseudocode (e.g., change ``` to ```text) so the markdown linter stops flagging them and rendering improves; apply this consistently to all flagged blocks (including the example block shown and the other listed blocks).Source: Linters/SAST tools
tests/inventory_hedge_test.rs (1)
149-164: 💤 Low valueOptional: Clarify test comment for clamp branch count.
The comment at line 151 states "three OP_IF branches plus the long-leg dust guard" which might be read as four total branches. Actually, the dust guard (
if (longPayout > 330)) is one of the three OP_IF statements, not additional. The test assertion>= 3is correct (the redeem function contains exactly 3 nested OP_IF statements).Consider rewording for clarity:
- // claimRaw is clamped into [0, totalCollateral]: claimRaw<=0 (all to long), - // claimRaw>=collateral (all to claim), else split. That is three OP_IF - // branches plus the long-leg dust guard. + // claimRaw is clamped into [0, totalCollateral] with three nested OP_IF branches: + // 1) claimRaw<=0 (all to long), 2) claimRaw>=collateral (all to claim), + // 3) else split with long-leg dust guard (longPayout > 330).🤖 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 `@tests/inventory_hedge_test.rs` around lines 149 - 164, The comment in test_redeem_has_clamp_branches is ambiguous about the OP_IF count; update the comment to state clearly that the redeem function contains exactly three OP_IF statements (one of which is the long-leg dust guard), not "three plus" a separate guard, so the assertion if_count >= 3 remains correct; locate the test_redeem_has_clamp_branches function and the references to HEDGE_CODE, redeem, f, and if_count and rewrite the inline comment to clarify that the dust guard is one of the three OP_IF branches.
🤖 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 `@examples/hedging/inventory_hedge.ark`:
- Line 219: The multiplication newTargetFiat * 100000000 can overflow int64;
replace the direct multiply/divide used to compute claimRaw in both redeem and
withdraw with a safe split-division to avoid exceeding INT64: compute the
quotient and remainder first and then scale, e.g. claimRaw = (newTargetFiat /
oraclePrice) * 100000000 + ((newTargetFiat % oraclePrice) * 100000000) /
oraclePrice, so the large scale factor is only applied to the smaller remainder;
update the calculations for the claimRaw assignment in both occurrences to this
safe form (refer to the claimRaw variable and the redeem/withdraw blocks).
---
Nitpick comments:
In `@docs/mm-residual-hedge.md`:
- Line 91: Multiple fenced code blocks in docs/mm-residual-hedge.md are missing
language specifiers; update each triple-backtick block (``` ... ```) noted in
the review to include an appropriate language tag such as text or pseudocode
(e.g., change ``` to ```text) so the markdown linter stops flagging them and
rendering improves; apply this consistently to all flagged blocks (including the
example block shown and the other listed blocks).
In `@tests/inventory_hedge_test.rs`:
- Around line 149-164: The comment in test_redeem_has_clamp_branches is
ambiguous about the OP_IF count; update the comment to state clearly that the
redeem function contains exactly three OP_IF statements (one of which is the
long-leg dust guard), not "three plus" a separate guard, so the assertion
if_count >= 3 remains correct; locate the test_redeem_has_clamp_branches
function and the references to HEDGE_CODE, redeem, f, and if_count and rewrite
the inline comment to clarify that the dust guard is one of the three OP_IF
branches.
🪄 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: 9cd49637-6142-4b08-834e-198dc2f5dd82
📒 Files selected for processing (5)
docs/mm-residual-hedge.mdexamples/hedging/inventory_hedge.arkexamples/hedging/inventory_hedge.jsonplayground/main.jstests/inventory_hedge_test.rs
| int rateElapsedScaled = fundingRatePerSec * elapsed / 1000000; | ||
| int delta = targetFiat * rateElapsedScaled / 1000000; | ||
| int newTargetFiat = targetFiat + delta; | ||
| int claimRaw = newTargetFiat * 100000000 / oraclePrice; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for overflow handling, max value checks, or documentation of limits
rg -nC3 --type=rust --type=md 'overflow|int64|9\.2e10|920.*million|targetFiat.*limit'Repository: arkade-os/compiler
Length of output: 23278
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and print the relevant sections around the referenced lines
for f in examples/hedging/inventory_hedge.ark; do
echo "===== $f ====="
wc -l "$f"
# print around line 180-260 (includes 219 and 269)
sed -n '180,260p' "$f" | nl -ba | sed 's/^/L:/' | head -n 200
# print around 255-310 (includes 269)
sed -n '255,310p' "$f" | nl -ba | sed 's/^/L:/' | head -n 120
done
# Search within examples/hedging for claimRaw / newTargetFiat / oraclePrice usages and any guards
rg -n --hidden --no-ignore -S 'claimRaw|newTargetFiat|oraclePrice|redeem\(|withdraw\(' examples/hedging/inventory_hedge.ark
# Search for documented overflow/ceiling patterns in docs and examples
rg -n --hidden --no-ignore -S 'OP_MUL64 aborts|int64 overflow|fail-closed|overflow flag|FOLLOW-UP|ceiling' docs examples | head -n 200Repository: arkade-os/compiler
Length of output: 213
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and print the relevant sections around the referenced lines
for f in examples/hedging/inventory_hedge.ark; do
echo "===== $f ====="
wc -l "$f"
# print around line 180-260 (includes 219 and 269)
sed -n '180,260p' "$f" | nl -ba | sed 's/^/L:/' | head -n 200
# print around 255-310 (includes 269)
sed -n '255,310p' "$f" | nl -ba | sed 's/^/L:/' | head -n 120
done
# Search within examples/hedging for claimRaw / newTargetFiat / oraclePrice usages and any guards
rg -n --hidden --no-ignore -S 'claimRaw|newTargetFiat|oraclePrice|redeem\(|withdraw\(' examples/hedging/inventory_hedge.ark
# Search for documented overflow/ceiling patterns in docs and examples
rg -n --hidden --no-ignore -S 'OP_MUL64 aborts|int64 overflow|fail-closed|overflow flag|FOLLOW-UP|ceiling' docs examples | head -n 200Repository: arkade-os/compiler
Length of output: 213
🏁 Script executed:
#!/bin/bash
set -euo pipefail
f="examples/hedging/inventory_hedge.ark"
echo "===== $f ====="
wc -l "$f"
echo "---- lines 190-245 ----"
awk 'NR>=190 && NR<=245 {printf "%d:%s\n", NR, $0}' "$f"
echo "---- lines 245-310 ----"
awk 'NR>=245 && NR<=310 {printf "%d:%s\n", NR, $0}' "$f"
echo "---- targeted usages in inventory_hedge.ark ----"
rg -n 'claimRaw|newTargetFiat|oraclePrice|redeem\(|withdraw\(' "$f"
echo "---- targeted overflow/ceiling documentation in docs/examples ----"
rg -n -S 'OP_MUL64 aborts|int64 overflow|fail-closed|overflow flag|FOLLOW-UP|ceiling|9\.2e10|920.*million|targetFiat.*limit' docs examples || trueRepository: arkade-os/compiler
Length of output: 11241
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect compiler arithmetic emission around the referenced comment
f="src/compiler/mod.rs"
echo "===== $f (around 2030-2120) ====="
sed -n '2030,2125p' "$f" | cat -n
echo "===== $f (around 2580-2690) ====="
sed -n '2580,2690p' "$f" | cat -n
echo "===== $f (around 2620-2685, arithmetic opcode selection) ====="
sed -n '2620,2685p' "$f" | cat -n
# Inspect repayment_pool overflow FOLLOW-UP notes
for rf in examples/bonds/repayment_pool.ark; do
echo "===== $rf (FOLLOW-UP overflow around first match) ====="
sed -n '140,190p' "$rf" | cat -n
echo "===== $rf (FOLLOW-UP overflow around second match) ====="
sed -n '540,590p' "$rf" | cat -n
echo "===== $rf (FOLLOW-UP overflow around third match) ====="
sed -n '600,660p' "$rf" | cat -n
doneRepository: arkade-os/compiler
Length of output: 22022
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find where OP_MUL / OP_MUL64 are emitted and how ArkType::Int vs ArkType::Uint64Le is handled.
rg -n "OP_MUL64|OP_MUL[^0-9]" src/compiler/mod.rs
rg -n "ArkType::Int|ArkType::Uint64Le" src/compiler/mod.rs | head -n 200
# Search for code paths that emit binary ops differently for 64-bit vs scriptnum
rg -n "emit_binary_op_asm|emit_.*binary.*op|BinaryOp" src/compiler/mod.rs | head -n 200
# Specifically inspect around where BinaryOp is compiled/emitted
# (find first occurrence near "Expression::BinaryOp" in emit_expression_asm)
rg -n "Expression::BinaryOp" src/compiler/mod.rsRepository: arkade-os/compiler
Length of output: 2408
🏁 Script executed:
#!/bin/bash
set -euo pipefail
f="src/compiler/mod.rs"
echo "===== OP_MUL64 at ~1147 ====="
sed -n '1125,1165p' "$f" | cat -n
echo "===== OP_MUL64 at ~2081 (emit_binary_op_asm) ====="
sed -n '2025,2105p' "$f" | cat -n
echo "===== BinaryOp compilation site around 1600-1705 ====="
sed -n '1580,1705p' "$f" | cat -n
echo "===== BinaryOp compilation site around 1660-1685 ====="
sed -n '1640,1695p' "$f" | cat -n
echo "===== Expression::BinaryOp earlier visitor around 70-120 (opcode planning) ====="
sed -n '70,130p' "$f" | cat -nRepository: arkade-os/compiler
Length of output: 16689
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --hidden --no-ignore -S 'stability_vault\.ark|9\.2e10|920.*million|newTargetFiat\s*\*\s*100000000|claimRaw\s*=.*100000000' examples docs || trueRepository: arkade-os/compiler
Length of output: 12580
🏁 Script executed:
#!/bin/bash
set -euo pipefail
f="examples/hedging/inventory_hedge.ark"
echo "===== inventory_hedge.ark header (1-120) ====="
awk 'NR>=1 && NR<=130 {print NR ":" $0}' "$f"
echo "===== inventory_hedge.ark around currentClaimBase/minCollateral (130-210) ====="
awk 'NR>=130 && NR<=210 {print NR ":" $0}' "$f"Repository: arkade-os/compiler
Length of output: 11205
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find stability_vault.ark (imported by examples)
fd -a -t f "stability_vault.ark" -d 6 . || true
# Search for the specific multiplication pattern / bound text in the repo
rg -n --hidden --no-ignore -S 'stability_vault\.ark|newTargetFiat\s*\*\s*100000000|claimRaw\s*=|92(\.|,)?0|9\.2e10|920.*million|100000000\s*/\s*oraclePrice' examples docs || true
# Search for the exact formula comment from stability_vault if present
rg -n --hidden --no-ignore -S 'claimRaw\s*=\s*newTargetFiat\s*×\s*1e8' . || trueRepository: arkade-os/compiler
Length of output: 25818
claimRaw int64 overflow ceiling in redeem/withdraw (fail-closed)
examples/hedging/inventory_hedge.ark computes claimRaw = newTargetFiat * 100000000 / oraclePrice in both redeem (line 219) and withdraw (line 269). The compiler emits OP_MUL64 followed by OP_VERIFY for *, so if newTargetFiat * 1e8 exceeds signed int64 range, the script aborts (liveness ceiling, not incorrect payouts). This happens when newTargetFiat > floor(INT64_MAX / 1e8) ≈ 9.22e10 minor units (≈ ~$922M if targetFiat is cents).
🤖 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 `@examples/hedging/inventory_hedge.ark` at line 219, The multiplication
newTargetFiat * 100000000 can overflow int64; replace the direct multiply/divide
used to compute claimRaw in both redeem and withdraw with a safe split-division
to avoid exceeding INT64: compute the quotient and remainder first and then
scale, e.g. claimRaw = (newTargetFiat / oraclePrice) * 100000000 +
((newTargetFiat % oraclePrice) * 100000000) / oraclePrice, so the large scale
factor is only applied to the smaller remainder; update the calculations for the
claimRaw assignment in both occurrences to this safe form (refer to the claimRaw
variable and the redeem/withdraw blocks).
- tests: assert the oracle message is RECONSTRUCTED on-chain (>=2 OP_CAT + OP_SHA256), matching stability_vault_test, so a compiler regression that drops cat/hash can't slip the replay guard; add addCapital-must-not-call-oracle negative test (code-review finding 5). - tests: reword redeem clamp-branch comment — the dust guard IS the third OP_IF, not a fourth branch (CodeRabbit nitpick). - docs: add 'text' language tags to fenced code blocks (markdown-lint nitpick). https://claude.ai/code/session_01E3fpVJfLdM3ye3ZWy5VxGg
There was a problem hiding this comment.
🔍 Arkana Code Review — follow-up on d93a3ef (oracle hardening + negative tests)
Diff reviewed: 9ed13de..d93a3ef (the only new commit since my last approval).
Changes
-
tests/inventory_hedge_test.rs— Three improvements:test_oracle_paths_verify_price_and_divide: now asserts>=2 OP_CATandOP_SHA256in every oracle-gated function, pinning the on-chain message reconstruction (sha256(ticker || price || time)). This mirrors thestability_vault_testpattern and closes the gap where a compiler regression could silently drop the concat/hash steps while still passing the old sig-only check. ✅- New
test_add_capital_does_no_oracle_call: negative test ensuringaddCapitalnever grows an oracle dependency. Directly addresses finding #5 from my first review. ✅ test_redeem_has_clamp_branchescomment: clarified that the dust guard IS the thirdOP_IF, not a fourth branch. Accurate. ✅
-
docs/mm-residual-hedge.md— All fenced code blocks now havetextlanguage tags. Pure markdown-lint fix, no content change. ✅
Verdict
Tests-only + docs-only diff. CI green (build + format + deploy-preview all pass). No protocol-critical code changes — the contract source (inventory_hedge.ark) is untouched.
LGTM. Approving.
Per design decision (leave logic as-is, document only): - updateFunding: note the known limitation that a negative *initial* rate bricks repricing (require(delta > 0) can't pass); funds aren't locked since redeem/withdraw still settle; a validating factory would seal it off. - Settlement math: note the fail-closed int64 ceiling (~9.2e10 minor units / ~$922M) on claimRaw, inherited verbatim from stability_vault.ark — overflow aborts via OP_MUL64+OP_VERIFY, never a wrong payout. Split-division deferred (no modulo operator; avoid diverging from the audited reference). https://claude.ai/code/session_01E3fpVJfLdM3ye3ZWy5VxGg
|
Resolution on the two deeper findings (high-effort review + the line-219/269 overflow comment). Decision: document, don't change the logic — both are inherited verbatim from the audited 1. 2. Test/oracle hardening + the nitpicks are in Generated by Claude Code |
…dback) A passive provider settling at a lagging oracle mark is adversely selected: the settlement initiator holds a free option on the oracle delay, which is why oracle-priced AMMs died on Ethereum (picked off when oracle error > fee, ignored when below). Stability-style lending tolerates the 0.2-0.3% enter/exit premium because settlement is rare; a hedging book may not. - design doc: new §9.b spelling out the problem, why §6 internalization is load-bearing (not an optimization), that the dropped exit fee is only sound desk<->own-treasury, that 'no perp-spot basis' needs qualification, and that the §8 order-book/RFQ layer is closer to required than optional for any public version. - contract header: trust-scope note on fee-free redeem/withdraw — reinstate seekerExitFee for any external counterparty. https://claude.ai/code/session_01E3fpVJfLdM3ye3ZWy5VxGg
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
examples/hedging/inventory_hedge.ark (1)
173-197:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReject negative collateral ratios before applying the withdrawal bound.
removeCapitaltrustscollateralRatioPctfrom state as-is. A negative ratio weakens the floor, and<= -100makesminCollateralnon-positive, so this path can authorize draining a live claim instead of enforcing the promised collateral buffer.Suggested fix
require(amount > 0, "zero amount"); require(oraclePrice > 0, "invalid oracle price"); + require(collateralRatioPct >= 0, "invalid collateral ratio");🤖 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 `@examples/hedging/inventory_hedge.ark` around lines 173 - 197, In removeCapital, reject negative collateral ratios before computing minCollateral/newTotalCollateral: add a precondition that collateralRatioPct >= 0 (matching updateFunding's invariant) at the start of the removal logic (before computing currentClaimBase/minCollateral), so a negative or <= -100 collateralRatioPct cannot weaken the floor and allow draining a live claim.
🤖 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 `@examples/hedging/inventory_hedge.ark`:
- Around line 115-124: The updateFunding function currently bails with
require(delta > 0, "no accrual; wait longer") when fundingRatePerSec != 0,
preventing recovery from a negative fundingRatePerSec; change the guard so
negative deltas are allowed to be applied (so the function can move
fundingRatePerSec toward zero) instead of always rejecting delta <= 0—i.e., in
updateFunding remove or relax the require(delta > 0) check and ensure subsequent
logic that adjusts fundingRatePerSec and claim/redeem accounting correctly
handles delta < 0 paths using the existing delta and fundingRatePerSec variables
to reduce (not increase) the negative rate back toward >= 0.
---
Outside diff comments:
In `@examples/hedging/inventory_hedge.ark`:
- Around line 173-197: In removeCapital, reject negative collateral ratios
before computing minCollateral/newTotalCollateral: add a precondition that
collateralRatioPct >= 0 (matching updateFunding's invariant) at the start of the
removal logic (before computing currentClaimBase/minCollateral), so a negative
or <= -100 collateralRatioPct cannot weaken the floor and allow draining a live
claim.
🪄 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: f1011f8f-85b2-4f35-a8fc-958391445a9f
📒 Files selected for processing (4)
docs/mm-residual-hedge.mdexamples/hedging/inventory_hedge.arkexamples/hedging/inventory_hedge.jsontests/inventory_hedge_test.rs
✅ Files skipped from review due to trivial changes (2)
- examples/hedging/inventory_hedge.json
- docs/mm-residual-hedge.md
| // KNOWN LIMITATION (by design): with a NEGATIVE current rate, delta < 0, so | ||
| // `require(delta > 0)` always fails and repricing via updateFunding is | ||
| // bricked — the rate can never be moved back to >= 0. This is only reachable | ||
| // when a vault is constructed with a negative initial fundingRatePerSec, | ||
| // which is unsupported (see the constructor comment). Funds are NOT locked: | ||
| // redeem/withdraw still settle (their clamp handles a wiped claim). A | ||
| // factory that validates the initial rate would seal this off; out of scope | ||
| // for the thinnest first build. | ||
| if (fundingRatePerSec != 0) { | ||
| require(delta > 0, "no accrual; wait longer"); |
There was a problem hiding this comment.
Allow updateFunding to recover from an invalid negative current rate.
The delta > 0 gate only protects against suppressing positive funding owed to the claim leg. With the current fundingRatePerSec != 0 check, any vault instantiated with a negative rate can never move back to a legal non-negative rate because delta is always <= 0.
Suggested fix
- if (fundingRatePerSec != 0) {
+ if (fundingRatePerSec > 0) {
require(delta > 0, "no accrual; wait longer");
}🤖 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 `@examples/hedging/inventory_hedge.ark` around lines 115 - 124, The
updateFunding function currently bails with require(delta > 0, "no accrual; wait
longer") when fundingRatePerSec != 0, preventing recovery from a negative
fundingRatePerSec; change the guard so negative deltas are allowed to be applied
(so the function can move fundingRatePerSec toward zero) instead of always
rejecting delta <= 0—i.e., in updateFunding remove or relax the require(delta >
0) check and ensure subsequent logic that adjusts fundingRatePerSec and
claim/redeem accounting correctly handles delta < 0 paths using the existing
delta and fundingRatePerSec variables to reduce (not increase) the negative rate
back toward >= 0.
CodeRabbit finding: the constructor cannot validate collateralRatioPct, and a negative ratio (<= -100) makes minCollateral non-positive so the ratio check trivially passes — the long leg could drain a live claim. Add require(collateralRatioPct >= 0), the same in-function defense-in-depth pattern as the existing newTargetFiat guard. StabilityOffer seals this at take() time; with no factory here the in-function guard is the equivalent. https://claude.ai/code/session_01E3fpVJfLdM3ye3ZWy5VxGg
|
Response to CodeRabbit's two actionable comments (pushed in ✅ Accepted — ❌ Declined (maintainer decision) — relax Generated by Claude Code |
2 participants
What
A new
.arkcontract —examples/hedging/inventory_hedge.ark— that sketches the on-chain core of the design notedocs/mm-residual-hedge.md. It is a perpetual, BTC-settled, fully-collateralized market-maker inventory hedge that generalizesexamples/stability/stability_vault.ark:claimPk, the desk): holds a fiat claimtargetFiat, delta-flat.longPk, the treasury): holds the residual BTC upside, posts the over-collateral, earns funding.[0, totalCollateral]), one instance perBTC/<fiat>pair viaticker.Functions (6 → 12 tapleaves)
transfer,updateFunding,addCapital,removeCapital,redeem(desk redeems its fiat claim for BTC),withdraw(treasury-driven settlement).Funding is rolled on-chain (
>= 0guard + accrual); the dynamic, imbalance-driven value is computed off-chain by the desk's risk engine (design note §4) and supplied toupdateFunding.Also in this PR
tests/inventory_hedge_test.rs— roundtrip (both variants emit, non-empty witness schemas), oracle-path checks (OP_CHECKSIGFROMSTACK+OP_DIV64),redeemclamp branches,<SERVER_KEY>injection, andupdatedAt-stripped determinism. 8 tests, green.docs/mm-residual-hedge.md— design note plus "Part II — Transaction flows" (per-function input/output layouts), now a single consolidated doc.playground/main.js— Hedging project folder wiringinventory_hedge.arkinto the WASM playground. (playground/contracts.jsis gitignored and regenerated by./playground/generate_contracts.sh.)Acceptance
cargo run -- examples/hedging/inventory_hedge.ark -o /tmp/ih.json✅ compiles; only the same benignoutput-invariantwarningsstability_vault.arkemits (computed spend-time locals used invalue >= Xchecks).cargo fmt --check✅ clean.cargo test✅ green (full suite, including the 8 new tests).Dialectical review
A self-review in thesis / antithesis / synthesis is in the first comment, including an empirical comparison against an alternative one-shot draft (compiled in this toolchain: its
min()clamp,new P2TR(pk)payouts, andbytes-bound state script all degrade to unbound placeholders — the clamp, payout destination, and state recursion go unenforced). Open questions for reviewers are flagged there.Summary by CodeRabbit
New Features
Documentation
Tests