feat(options): add Arkade covered-call options web app#48
Open
tiero wants to merge 6 commits into
Open
Conversation
Standalone Bitcoin covered-call dApp (Rysk-style) under examples/options/web, consuming the compiler-sourced CoveredCall ABI. - pnpm + vite + react + typescript - @noble/@Scure for all crypto (BIP340 schnorr embedded wallet, persisted in the browser; key never leaves the tab) - dual-chain emulator sharing one Bitcoin block clock: virtual (Ark) = cooperative tapleaf, Operator co-sign, instant onchain (L1) = exit tapleaf, N-of-N + CSV, mempool confirmations - high-level Arkade-Script tx builder as the single SDK seam (spendVault -> addInput -> addOutput -> sign -> withOperator -> build) - full lifecycle: write / exercise / reclaim / transfer, Black-Scholes premium quoting, payoff chart, per-position tapleaf script inspector - Arkade-branded in-house UI component set - headless lifecycle smoke test (pnpm smoke) — all green Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EJC6mxR69rZp79iEmUCXXw
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (16)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (9)
WalkthroughAdds a complete standalone React/Vite browser application under ChangesArkade Covered-Call Options dApp
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Contributor
Playground PreviewA live preview of this PR's playground is available at:
|
- New workflow deploy-options-app.yml: builds examples/options/web on push to master (path-filtered) or workflow_dispatch, runs the smoke test, and publishes dist/ to the gh-pages branch under /options (https://arkade-os.github.io/compiler/options/). - Follows the repo's existing multi-target gh-pages pattern: deploy is scoped to the /options subfolder, and the playground root deploy now clean-excludes options/ so the two coexist without clobbering each other. - Pin packageManager (pnpm@10.33.0) for reproducible CI installs; document the deploy + live URL in the app README. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EJC6mxR69rZp79iEmUCXXw
Vercel-style preview deployments: every PR touching examples/options/web builds and publishes to gh-pages under /options-previews/pr-<N>/, comments the live URL on the PR, and tears the folder down on close (mirrors the playground's pr-preview workflow). Uses a dedicated previews root so all four gh-pages subtrees stay disjoint: / (playground), /pr-previews (playground PRs), /options (app prod), /options-previews (app PRs). The root playground deploy now also clean-excludes options-previews/. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EJC6mxR69rZp79iEmUCXXw
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
🧹 Nitpick comments (2)
examples/options/web/scripts/smoke.ts (1)
85-106: ⚡ Quick winAdd reverse-direction transfer coverage (
side: "buyer").Current smoke coverage checks buyer-leg transfer only when MM starts as buyer. Add one case where the user starts as buyer and
transferPosition(id, "buyer")must move buyer ownership away from user; this catches one-sided target-selection regressions.🤖 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/options/web/scripts/smoke.ts` around lines 85 - 106, Add a new scenario test case after the current Scenario 3 that covers the reverse direction of buyer leg transfer. In this new scenario, open a position with side set to "buyer" instead of "seller", then call S().transferPosition with "buyer" parameter to transfer the buyer leg away from the current user (who is the buyer). Add the same verification checks (checking that buyerPk changed, position status remains active, and new vault id is set) to ensure the transfer works correctly when initiated by the buyer themselves rather than the seller.examples/options/web/src/App.tsx (1)
11-11: Use selector-based Zustand subscriptions to avoid unnecessary re-renders.The
Toastcomponent and other UI components subscribe to the entire store without selectors, causing re-renders whenever any field changes (e.g.,snapshot,spot,vol). Use selectors with shallow comparison instead:const { toast, clearToast } = useStore((state) => ({ toast: state.toast, clearToast: state.clearToast }));This pattern applies across multiple components (
MarketBar,Header,Positions, etc.) to avoid redundant renders from unrelated state mutations.🤖 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/options/web/src/App.tsx` at line 11, The useStore hook in the App component and other components (MarketBar, Header, Positions, etc.) are subscribing to the entire Zustand store without selectors, causing unnecessary re-renders whenever any unrelated state field changes. Replace each useStore() call with a selector function that returns only the specific properties needed by that component, such as passing a function argument to useStore that returns an object containing only the toast and clearToast properties for the App component. Apply this same pattern consistently across all components that use useStore to ensure components only re-render when their specific subscribed state changes.
🤖 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/options/web/README.md`:
- Around line 65-78: The code fence in the Architecture section is missing a
language tag, which violates markdownlint rule MD040. Add a language tag to the
opening fence delimiter. Since this appears to be a plain text directory
structure listing, add the `text` language identifier to the opening triple
backticks (change ``` to ```text) to mark the fence as text content and satisfy
the linter requirement.
In `@examples/options/web/scripts/polyfill.ts`:
- Around line 8-9: The Storage shim implementation at key() and length
properties has hardcoded values that don't reflect actual storage state. Replace
the key function to dynamically retrieve actual keys from the storage object by
index (returning null for out-of-bounds indices), and replace the fixed length
value with a computed getter property that returns the actual count of items
currently stored. This ensures the shim correctly mirrors browser localStorage
behavior where both key() and length reflect the current contents of the
storage.
In `@examples/options/web/src/abi/cash_secured_put.json`:
- Around line 288-291: The witness pubkey encoding in the cash_secured_put.json
ABI definition uses compressed-33 format for the newSellerPk field, but this is
inconsistent with the x-only key format exported by the wallet layer, which can
cause transfer witness assembly to fail. Change the encoding from compressed-33
to x-only-32 (or the appropriate x-only format) for the newSellerPk field and
any other witness pubkey fields with the same encoding issue. Ensure all witness
pubkey encodings in the ABI match the key format that the wallet layer actually
exports to prevent conversion errors during transfer witness assembly.
- Around line 552-563: The cash_secured_put.json ABI file contains unresolved
placeholder warnings for functions reclaim, transferSeller, and transferBuyer
where placeholders like reclaimHeight, tx.time, newSellerPk, and newBuyerPk are
not bound in witnessSchema or constructorInputs. Resolve these warnings by
either adding the missing placeholders to the appropriate schema definitions in
the source contract, or update the artifact generation pipeline to properly bind
these placeholders before finalizing the ABI. Ensure all output-invariant
warnings are resolved before allowing this ABI to be released.
In `@examples/options/web/src/abi/covered_call.json`:
- Around line 288-291: The ABI definition in covered_call.json specifies pubkey
witness fields like newSellerPk with compressed-33 encoding, which conflicts
with the wallet surface that uses x-only (32-byte) pubkeys. Update all pubkey
field encodings from compressed-33 to the x-only format that matches the wallet
interface, ensuring you update both the occurrence around line 288-291 and the
other location mentioned at lines 423-426 to maintain consistency and prevent
transfer flow failures.
- Around line 546-557: The ABI artifact contains unresolved output-invariant
warnings indicating that placeholders like reclaimHeight, tx.time, newSellerPk,
and newBuyerPk in the functions exercise, transferSeller, transferBuyer, and
reclaim are not bound to witnessSchema or constructorInputs. To fix this, locate
these functions in the source smart contract code and ensure all placeholder
parameters are properly declared in either the witnessSchema or
constructorInputs sections, then recompile the contract to regenerate the ABI
file with the warnings resolved.
In `@examples/options/web/src/components/MarketBar.tsx`:
- Around line 15-24: The range input elements in the MarketBar component are
missing accessible labels that assistive technology users need to identify these
controls. Add an aria-label attribute to both range input elements (the first
one with value={spot} around line 15 and the second range input around line 28)
with descriptive text that clearly indicates what each slider controls. For
example, the spot slider should have an aria-label like "Spot price range" and
the other slider should have a descriptive label for whatever value it controls.
This ensures screen reader users can understand the purpose of each range
control.
In `@examples/options/web/src/components/Positions.tsx`:
- Around line 119-120: The canExercise variable only checks for a lower bound
(height >= expiryHeight) but lacks an upper bound check, allowing the exercise
action to remain available after reclaim height is reached. Modify the
canExercise condition to include an additional check that the current height is
less than reclaimH, creating a bounded window where exercise is only available
between expiryHeight and reclaimH. Apply this same fix to all locations that
define exercise availability (including the section mentioned at lines 176-180).
In `@examples/options/web/src/components/TradePanel.tsx`:
- Around line 35-47: In the submit function, add validation logic before calling
openPosition to ensure all numeric form fields (notionalBtc, strikeUsd,
premiumUsd, vol, expiryDays, graceBlocks, exitBlocks) are finite and
non-negative values. Check each field to verify it is a valid number and not NaN
or Infinity, and prevent the openPosition call from executing if any field fails
validation. Return early or display an error message if validation fails to
prevent invalid data from being persisted to the store.
In `@examples/options/web/src/lib/arkadeScript.ts`:
- Around line 137-147: The sighash field in the return object is incorrectly
assigned to this.inst.scriptId instead of the actual computed sighash value.
Locate the sighash variable that was computed earlier in the function (around
line 109) and update the return object to assign the sighash field to this
computed sighash variable instead of this.inst.scriptId. This ensures the return
value contains the correct sighash for downstream signing and verification
operations.
In `@examples/options/web/src/lib/contract.ts`:
- Around line 113-118: In the anonymous callback function passed to
fn.asm.map(), instead of silently returning unknown placeholder tokens when
bindings[inner] is undefined, add validation to fail fast. Create an explicit
allowlist of permitted structural placeholder tokens (like those containing
"VTXO" or "tx.time"), and only allow those to pass through unchanged. For any
other unbound placeholder that doesn't match the allowlist, throw an error that
clearly indicates which placeholder binding is missing, rather than returning
the token as-is.
In `@examples/options/web/src/lib/crypto.ts`:
- Around line 68-76: The u64le function does not validate that the input
parameter n is within the valid uint64 bounds (0 to 2^64-1) before encoding,
allowing negative numbers and values larger than 64-bits to silently wrap or
truncate. Add input validation at the start of the u64le function to check that
the converted BigInt value is greater than or equal to 0 and less than 2^64
(18446744073709551616n), and throw an error with a descriptive message if the
bounds are violated, before proceeding with the byte encoding loop.
In `@examples/options/web/src/lib/pricing.ts`:
- Around line 12-22: The normCdf function has a double-inversion bug in the
conditional logic. The approximation is being inverted with `if (x > 0) p = 1 -
p;` and then inverted again with the return statement `return 1 - p;`. This
causes positive inputs to produce incorrect tail probabilities instead of
cumulative distribution values. Fix this by removing the second inversion in the
return statement so that when x is positive, the value is inverted once and
returned directly, rather than being inverted twice. The corrected logic should
apply the conditional flip once and return the result without additional
transformation.
In `@examples/options/web/src/lib/wallet.ts`:
- Around line 39-46: The loadUserWallet function assumes the stored secret from
localStorage is always valid hex, but corrupted or malformed data will cause
fromHex to throw and crash initialization. Add explicit validation before using
the retrieved secret: when retrieving the secret with
localStorage.getItem(USER_KEY), validate it is valid hex before passing to
fromHex; if validation fails, treat it as missing and generate a new random
secret instead. Apply the same validation and recovery pattern to the other
similar load path mentioned at lines 68-75.
- Around line 42-44: The code is storing the private key (secret) in plaintext
in localStorage using localStorage.setItem with USER_KEY, which is a security
vulnerability as it exposes the key to XSS attacks and compromised scripts.
Remove the localStorage.setItem call that persists the secret from the wallet
creation logic, or alternatively, implement encryption of the secret before
storing it. Apply this fix to all occurrences where USER_KEY is set in
localStorage (the diff shows instances around the toHex(randomPrivateKey())
assignment and subsequent setItem calls at the mentioned line ranges). Consider
whether the private key should be persisted at all or if it should only exist in
memory during the session.
In `@examples/options/web/src/state/store.ts`:
- Around line 220-223: The target selection logic in the conditional statement
only handles the case where leg === "seller", leaving the buyer leg without
proper target switching. Expand the conditional logic to also check when leg ===
"buyer" and pos.buyerPk === user.pubkey. In that case, the target should also be
set to the maker object with pubkey and label, just as it is for the seller leg.
This ensures that transfers on both buyer and seller legs properly switch
targets when the current user is the respective party, preventing silent no-op
operations.
In `@examples/options/web/src/ui/index.tsx`:
- Around line 97-101: The onChange handler in the NumberInput component uses
parseFloat(e.target.value) which returns NaN when the input field is cleared
(since parseFloat("") evaluates to NaN). This NaN value then propagates to the
application state. Modify the onChange handler to check if the parsed float
value is NaN using Number.isNaN() and conditionally pass either an empty string
or a null/undefined value instead of the NaN to the onChange callback, ensuring
that invalid numeric states do not flow into downstream state management.
- Around line 145-149: The Modal component lacks essential accessibility
attributes and keyboard support. Add role="dialog" and aria-modal="true"
attributes to the inner div with className "ark-modal" to establish proper
dialog semantics. Additionally, implement keyboard event handling to dismiss the
modal when the Escape key is pressed by either adding an onKeyDown handler to
the modal container or using a useEffect hook to attach a global keydown
listener that calls the onClose function when event.key equals "Escape".
In `@examples/options/web/src/ui/theme.css`:
- Around line 33-34: The CSS variables `--ark-mono` and `--ark-sans` contain
font family declarations that violate the stylelint `value-keyword-case` rule on
lines 33-34 and 237-238. Review the font family values in these variables and
normalize the keyword casing and font family identifiers according to the
configured stylelint rules. Ensure that generic font keywords (such as
ui-monospace, system-ui, monospace, sans-serif) have consistent casing and that
any font identifiers that should be quoted are properly quoted. Apply the same
fixes to all four affected lines (33, 34, 237, and 238) to ensure lint passes
consistently across the entire file.
---
Nitpick comments:
In `@examples/options/web/scripts/smoke.ts`:
- Around line 85-106: Add a new scenario test case after the current Scenario 3
that covers the reverse direction of buyer leg transfer. In this new scenario,
open a position with side set to "buyer" instead of "seller", then call
S().transferPosition with "buyer" parameter to transfer the buyer leg away from
the current user (who is the buyer). Add the same verification checks (checking
that buyerPk changed, position status remains active, and new vault id is set)
to ensure the transfer works correctly when initiated by the buyer themselves
rather than the seller.
In `@examples/options/web/src/App.tsx`:
- Line 11: The useStore hook in the App component and other components
(MarketBar, Header, Positions, etc.) are subscribing to the entire Zustand store
without selectors, causing unnecessary re-renders whenever any unrelated state
field changes. Replace each useStore() call with a selector function that
returns only the specific properties needed by that component, such as passing a
function argument to useStore that returns an object containing only the toast
and clearToast properties for the App component. Apply this same pattern
consistently across all components that use useStore to ensure components only
re-render when their specific subscribed state changes.
🪄 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: 58e72fa6-49d9-42f1-ad05-5ec8d6f9aa7f
⛔ Files ignored due to path filters (2)
examples/options/web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlexamples/options/web/public/favicon.svgis excluded by!**/*.svg
📒 Files selected for processing (32)
examples/options/web/.gitignoreexamples/options/web/README.mdexamples/options/web/index.htmlexamples/options/web/package.jsonexamples/options/web/scripts/polyfill.tsexamples/options/web/scripts/run-smoke.mjsexamples/options/web/scripts/smoke.tsexamples/options/web/src/App.tsxexamples/options/web/src/abi/cash_secured_put.jsonexamples/options/web/src/abi/covered_call.jsonexamples/options/web/src/abi/index.tsexamples/options/web/src/components/ActivityLog.tsxexamples/options/web/src/components/Header.tsxexamples/options/web/src/components/InfoCards.tsxexamples/options/web/src/components/MarketBar.tsxexamples/options/web/src/components/PayoffChart.tsxexamples/options/web/src/components/Positions.tsxexamples/options/web/src/components/TradePanel.tsxexamples/options/web/src/lib/arkadeScript.tsexamples/options/web/src/lib/contract.tsexamples/options/web/src/lib/coveredCall.tsexamples/options/web/src/lib/crypto.tsexamples/options/web/src/lib/emulator.tsexamples/options/web/src/lib/format.tsexamples/options/web/src/lib/pricing.tsexamples/options/web/src/lib/wallet.tsexamples/options/web/src/main.tsxexamples/options/web/src/state/store.tsexamples/options/web/src/ui/index.tsxexamples/options/web/src/ui/theme.cssexamples/options/web/tsconfig.jsonexamples/options/web/vite.config.ts
Comment on lines
+288
to
+291
| "name": "newSellerPk", | ||
| "type": "pubkey", | ||
| "encoding": "compressed-33" | ||
| }, |
There was a problem hiding this comment.
pubkey witness encoding conflicts with x-only wallet keys.
This ABI requires compressed-33 for transfer witness pubkeys, while the wallet surface is x-only (32-byte) pubkeys. That contract mismatch is likely to break transfer flows unless explicit conversion is guaranteed at the call boundary.
Also applies to: 423-426
🤖 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/options/web/src/abi/covered_call.json` around lines 288 - 291, The
ABI definition in covered_call.json specifies pubkey witness fields like
newSellerPk with compressed-33 encoding, which conflicts with the wallet surface
that uses x-only (32-byte) pubkeys. Update all pubkey field encodings from
compressed-33 to the x-only format that matches the wallet interface, ensuring
you update both the occurrence around line 288-291 and the other location
mentioned at lines 423-426 to maintain consistency and prevent transfer flow
failures.
Comment on lines
+42
to
+44
| secret = toHex(randomPrivateKey()); | ||
| localStorage.setItem(USER_KEY, secret); | ||
| } |
There was a problem hiding this comment.
Do not persist private keys in cleartext localStorage.
Storing the user secret key as plaintext in localStorage materially weakens wallet security (easy exfiltration under XSS or compromised third-party scripts).
Also applies to: 49-51, 59-60
🧰 Tools
🪛 ast-grep (0.43.0)
[warning] 42-42: Do not store sensitive data to local storage
Context: localStorage.setItem(USER_KEY, secret)
Note: [CWE-312].
(local-storage-sensitive-data-typescript)
[warning] 42-42: Sensitive information detected in localStorage. Storing sensitive data like emails, usernames, or personal information in localStorage exposes it to malicious scripts and XSS attacks. Use secure storage alternatives or avoid storing sensitive data client-side.
Context: localStorage.setItem(USER_KEY, secret)
Note: [CWE-312] Cleartext Storage of Sensitive Information
(local-storage-sensitive-information)
🤖 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/options/web/src/lib/wallet.ts` around lines 42 - 44, The code is
storing the private key (secret) in plaintext in localStorage using
localStorage.setItem with USER_KEY, which is a security vulnerability as it
exposes the key to XSS attacks and compromised scripts. Remove the
localStorage.setItem call that persists the secret from the wallet creation
logic, or alternatively, implement encryption of the secret before storing it.
Apply this fix to all occurrences where USER_KEY is set in localStorage (the
diff shows instances around the toHex(randomPrivateKey()) assignment and
subsequent setItem calls at the mentioned line ranges). Consider whether the
private key should be persisted at all or if it should only exist in memory
during the session.
Source: Linters/SAST tools
Comment on lines
+33
to
+34
| --ark-mono: ui-monospace, "SF Mono", "JetBrains Mono", "Fira Code", Menlo, monospace; | ||
| --ark-sans: system-ui, -apple-system, "Segoe UI", Roboto, Inter, sans-serif; |
There was a problem hiding this comment.
Resolve Stylelint value-keyword-case violations before merge.
Line 33, Line 34, Line 237, and Line 238 currently fail the configured lint rule. Please normalize keyword casing (and/or quote font family identifiers) so lint passes consistently.
Suggested patch
- --ark-mono: ui-monospace, "SF Mono", "JetBrains Mono", "Fira Code", Menlo, monospace;
- --ark-sans: system-ui, -apple-system, "Segoe UI", Roboto, Inter, sans-serif;
+ --ark-mono: ui-monospace, "SF Mono", "JetBrains Mono", "Fira Code", "Menlo", monospace;
+ --ark-sans: system-ui, -apple-system, "Segoe UI", "Roboto", "Inter", sans-serif;
@@
- background: currentColor;
- box-shadow: 0 0 8px currentColor;
+ background: currentcolor;
+ box-shadow: 0 0 8px currentcolor;Also applies to: 237-238
🧰 Tools
🪛 Stylelint (17.13.0)
[error] 33-33: Expected "Menlo" to be "menlo" (value-keyword-case)
(value-keyword-case)
[error] 34-34: Expected "Roboto" to be "roboto" (value-keyword-case)
(value-keyword-case)
[error] 34-34: Expected "Inter" to be "inter" (value-keyword-case)
(value-keyword-case)
🤖 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/options/web/src/ui/theme.css` around lines 33 - 34, The CSS
variables `--ark-mono` and `--ark-sans` contain font family declarations that
violate the stylelint `value-keyword-case` rule on lines 33-34 and 237-238.
Review the font family values in these variables and normalize the keyword
casing and font family identifiers according to the configured stylelint rules.
Ensure that generic font keywords (such as ui-monospace, system-ui, monospace,
sans-serif) have consistent casing and that any font identifiers that should be
quoted are properly quoted. Apply the same fixes to all four affected lines (33,
34, 237, and 238) to ensure lint passes consistently across the entire file.
Source: Linters/SAST tools
- Collapse fixed-column layouts at breakpoints: trade-ticket form (2→1 col), position stat row (4→2 col), and the script-inspector ASM panes (2→1 col). - Reduce shell/card padding and wordmark size on small screens; let segmented controls wrap; cap toast width. - Fix a CSS-grid min-width:auto overflow where the non-wrapping <pre> ASM panes widened the page instead of scrolling internally (min-width:0 on grid children + pre); wrap long vault-address/key hex. Verified with Playwright at 320/375/414px: no horizontal overflow on load, with a position open, or with the inspector expanded. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EJC6mxR69rZp79iEmUCXXw
Contributor
Covered-Call App PreviewA live preview of this PR's covered-call dApp is available at:
|
Major UX simplification per feedback — drop the dual-chain/path/stablecoin complexity for a single, plain product: - BTC in, BTC out: deposit BTC, sell a call, settle in BTC. No stablecoin leg, no oracle. Outcome stated plainly: below strike you keep BTC + premium; above strike it's called away (capped) + premium. - One settlement model: Arkade treats UTXO and vUTXO identically, so no chain toggle and no cooperative/exit path choice. Settlement is a cooperative close co-signed by writer + maker + Operator; vault BTC split conserves value. - 5 strike options with a simulated market-maker RFQ (rfq.ts): five makers price off their own IV/edge; best quote highlighted; pick a strike, see the outcome, sell. - New lib/options.ts replaces coveredCall.ts/arkadeScript.ts; new TradeFlow, SpotBar, OutcomePanel components; Header/Positions/About simplified. - Fix inverted normCdf in Black-Scholes (premiums were clamping to 0) — caught by the rewritten smoke test (RFQ → open → settle kept → settle called, all green). - Mobile responsive verified with Playwright at 320/375/414px across the full RFQ→open flow (maker-quote rows and outcome rows now wrap; no overflow). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EJC6mxR69rZp79iEmUCXXw
There was a problem hiding this comment.
Actionable comments posted: 11
♻️ Duplicate comments (1)
examples/options/web/README.md (1)
71-71:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd the language tag to the Architecture code fence (MD040).
The code fence on line 71 is missing the required language identifier. Mark it as
textto satisfy markdownlint rule MD040.🏷️ Proposed fix
## Architecture -``` +```text src/ abi/ compiler-sourced ABIs (covered_call.json, cash_secured_put.json)🤖 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/options/web/README.md` at line 71, The code fence on line 71 in the Architecture section of README.md is missing a language identifier, which violates the MD040 markdownlint rule. Add the language tag "text" to the opening code fence by changing the three backticks to ```text to properly document that this fence contains plain text directory structure content.Source: Linters/SAST tools
🤖 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 @.github/workflows/deploy-options-app.yml:
- Around line 32-33: Replace all tag-based action references with their
corresponding commit SHAs to strengthen supply-chain security. Specifically,
update actions/checkout@v4, pnpm/action-setup@v4, actions/setup-node@v4, and
JamesIves/github-pages-deploy-action@v4 by replacing the `@v4` tag with the full
commit SHA of that version. Additionally, add persist-credentials: false to the
checkout step to prevent the GITHUB_TOKEN from being available for the entire
job duration.
In @.github/workflows/pr-preview-options.yml:
- Around line 35-36: In the deploy-preview job, the Checkout PR branch step
using actions/checkout@v4 needs to have credential persistence explicitly
disabled for security purposes. Add the `persist-credentials: false` input
parameter to the checkout action to prevent credentials from being persisted
after the checkout completes, since no git operations are performed after this
step.
- Line 36: Replace all semantic version tags in the `uses` statements with their
corresponding commit hash references to comply with the supply-chain security
pinning policy. Update the six action references: actions/checkout (lines 36 and
108) to use hash 34e114876b0b11c390a56381ad16ebd13914f8d5, pnpm/action-setup to
use hash f40ffcd9367d9f12939873eb1018b921a783ffaa, actions/setup-node to use
hash 49933ea5288caeca8642d1e84afbd3f7d6820020,
JamesIves/github-pages-deploy-action to use hash
d92aa235d04922e8f08b40ce78cc5442fcfbfa2f, and actions/github-script to use hash
f28e40c7f34bde8b3046d885e986cb6290c5673b. Change each `uses` line from the
format `uses: owner/action@vX` to `uses: owner/action@<commit-hash>` as
specified in the review comment.
In `@examples/options/web/scripts/smoke.ts`:
- Line 11: Replace the fixed sleep function and its usages throughout the file
with condition-based polling to reduce test flakiness. Instead of using await
sleep calls with hardcoded delays in the smoke flow (around lines 18-23 and
54-56), implement a polling mechanism that repeatedly checks for the desired
condition (such as RFQ-ready state) with a bounded timeout. This approach will
make the tests deterministic by waiting for the actual state change rather than
relying on arbitrary timing delays.
In `@examples/options/web/src/components/Positions.tsx`:
- Around line 43-47: The inline style with gridTemplateColumns on the div
element with className "stat-grid" is preventing CSS media query breakpoints
from working on small screens. Remove the inline gridTemplateColumns property
from the style prop on this div, and instead define the gridTemplateColumns in
the `.stat-grid` CSS class rules where it can be properly overridden by
responsive breakpoint media queries. Keep only the marginBottom value inline if
needed, or move that to CSS as well.
In `@examples/options/web/src/components/TradeFlow.tsx`:
- Around line 81-83: The onChange handler for the deposit input field is using
parseFloat directly without validation, which allows NaN to enter the store
state when the input is cleared or contains invalid values. This breaks
downstream logic. In the onChange callback for the deposit input in
TradeFlow.tsx, validate that the parsed value is a finite number using
Number.isFinite() before calling setDeposit. If the value is not finite (NaN or
Infinity), set the deposit to an appropriate default value like 0 instead of
allowing NaN to be stored in state.
In `@examples/options/web/src/lib/options.ts`:
- Around line 148-155: The premium payment in the emu.credit() call is minting
value directly to the user without debiting the maker's funds, breaking the
conservation of cashflows in the emulator. To fix this, add a corresponding
emu.debit() call before the emu.credit() call to debit the maker's account
(using userOwner with the maker reference, debiting a.premiumSats) and then
credit the user as currently done, ensuring the premium payment is transferred
from maker to user rather than minted.
- Around line 113-133: The writeCall function does not validate its input
arguments (WriteArgs) before processing, allowing invalid values such as
depositSats less than or equal to 0 or expiryDays less than or equal to 0 to
create invalid positions. Add validation at the beginning of the writeCall
function, before any operations like filtering utxos or calculating expiry
height, to ensure that a.depositSats and a.expiryDays are both greater than 0.
Throw descriptive errors immediately if either validation fails to prevent
downstream issues.
In `@examples/options/web/src/state/store.ts`:
- Around line 102-120: The requestQuotes method has a setTimeout that can create
stale responses overwriting newer state when multiple concurrent calls are made,
and the reset function leaves in-flight timers alive. Store the timer ID
returned from setTimeout in state, and before starting a new timer in
requestQuotes, clear any existing timer using clearTimeout. Also ensure the
reset function clears any pending timer to prevent stale async responses from
updating state after a reset.
- Line 98: The setExpiry setter in the store is accepting unbounded values for
expiryDays without validation, allowing zero or negative values that propagate
to quote/APY and position expiry calculations. Modify the setExpiry function to
clamp the input parameter d to a positive integer (minimum value of 1) before
passing it to the set function, ensuring invalid expiry values are prevented at
the setter boundary rather than downstream.
In `@examples/options/web/src/ui/theme.css`:
- Around line 552-555: The `.mono-wrap` class contains the deprecated CSS
property `word-break: break-word`, which is flagged by stylelint. Remove this
deprecated property from the class definition while keeping the `overflow-wrap:
anywhere` property, as it provides the same word-wrapping behavior and is the
modern standard approach.
---
Duplicate comments:
In `@examples/options/web/README.md`:
- Line 71: The code fence on line 71 in the Architecture section of README.md is
missing a language identifier, which violates the MD040 markdownlint rule. Add
the language tag "text" to the opening code fence by changing the three
backticks to ```text to properly document that this fence contains plain text
directory structure content.
🪄 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: a2b4079c-475b-47bc-bea4-37ec86b07107
📒 Files selected for processing (20)
.github/workflows/deploy-options-app.yml.github/workflows/deploy-playground.yml.github/workflows/pr-preview-options.ymlexamples/options/web/README.mdexamples/options/web/package.jsonexamples/options/web/scripts/smoke.tsexamples/options/web/src/App.tsxexamples/options/web/src/components/ActivityLog.tsxexamples/options/web/src/components/Header.tsxexamples/options/web/src/components/InfoCards.tsxexamples/options/web/src/components/OutcomePanel.tsxexamples/options/web/src/components/Positions.tsxexamples/options/web/src/components/SpotBar.tsxexamples/options/web/src/components/TradeFlow.tsxexamples/options/web/src/lib/options.tsexamples/options/web/src/lib/pricing.tsexamples/options/web/src/lib/rfq.tsexamples/options/web/src/state/store.tsexamples/options/web/src/ui/index.tsxexamples/options/web/src/ui/theme.css
🚧 Files skipped from review as they are similar to previous changes (4)
- examples/options/web/package.json
- examples/options/web/src/App.tsx
- examples/options/web/src/lib/pricing.ts
- examples/options/web/src/ui/index.tsx
Comment on lines
+32
to
+33
| - name: Checkout | ||
| uses: actions/checkout@v4 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file=".github/workflows/deploy-options-app.yml"
echo "== Unpinned actions (should be empty) =="
rg -nP '^\s*uses:\s*[^@]+@(?![0-9a-fA-F]{40}\b)' "$file" || true
echo
echo "== Checkout step config (should include persist-credentials: false) =="
rg -nA6 -B1 'uses:\s*actions/checkout@' "$file"Repository: arkade-os/compiler
Length of output: 527
🏁 Script executed:
cat .github/workflows/deploy-options-app.ymlRepository: arkade-os/compiler
Length of output: 1666
Pin all workflow actions to commit SHAs and add persist-credentials: false to checkout step.
This workflow uses tag-based action refs (@v4) instead of commit SHAs, which weakens supply-chain security by allowing tags to be retagged or hijacked. Additionally, the actions/checkout@v4 step does not include persist-credentials: false, leaving the GITHUB_TOKEN available throughout the entire job.
Affected actions:
- Line 33:
actions/checkout@v4 - Line 36:
pnpm/action-setup@v4 - Line 41:
actions/setup-node@v4 - Line 57:
JamesIves/github-pages-deploy-action@v4
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 32-33: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 33-33: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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 @.github/workflows/deploy-options-app.yml around lines 32 - 33, Replace all
tag-based action references with their corresponding commit SHAs to strengthen
supply-chain security. Specifically, update actions/checkout@v4,
pnpm/action-setup@v4, actions/setup-node@v4, and
JamesIves/github-pages-deploy-action@v4 by replacing the `@v4` tag with the full
commit SHA of that version. Additionally, add persist-credentials: false to the
checkout step to prevent the GITHUB_TOKEN from being available for the entire
job duration.
Source: Linters/SAST tools
| working-directory: examples/options/web | ||
| steps: | ||
| - name: Checkout PR branch | ||
| uses: actions/checkout@v4 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Resolve action versions to commit hashes for pinning
# Usage: run this to get the commit SHAs to pin in the workflow
echo "Fetching latest commit hashes for each action..."
# actions/checkout@v4
echo "actions/checkout@v4:"
gh api repos/actions/checkout/releases --jq '.[] | select(.tag_name == "v4") | .target_commitish' | head -1
# pnpm/action-setup@v4
echo "pnpm/action-setup@v4:"
gh api repos/pnpm/action-setup/releases --jq '.[] | select(.tag_name == "v4") | .target_commitish' | head -1
# actions/setup-node@v4
echo "actions/setup-node@v4:"
gh api repos/actions/setup-node/releases --jq '.[] | select(.tag_name == "v4") | .target_commitish' | head -1
# JamesIves/github-pages-deploy-action@v4
echo "JamesIves/github-pages-deploy-action@v4:"
gh api repos/JamesIves/github-pages-deploy-action/releases --jq '.[] | select(.tag_name == "v4") | .target_commitish' | head -1
# actions/github-script@v7
echo "actions/github-script@v7:"
gh api repos/actions/github-script/releases --jq '.[] | select(.tag_name == "v7") | .target_commitish' | head -1Repository: arkade-os/compiler
Length of output: 244
🏁 Script executed:
cat .github/workflows/pr-preview-options.ymlRepository: arkade-os/compiler
Length of output: 4066
🏁 Script executed:
#!/bin/bash
# Try alternative method: git ls-remote to get commit hashes
echo "Fetching commit hashes using git ls-remote..."
echo "actions/checkout v4:"
git ls-remote --refs https://github.com/actions/checkout.git refs/tags/v4 | awk '{print $1}'
echo "pnpm/action-setup v4:"
git ls-remote --refs https://github.com/pnpm/action-setup.git refs/tags/v4 | awk '{print $1}'
echo "actions/setup-node v4:"
git ls-remote --refs https://github.com/actions/setup-node.git refs/tags/v4 | awk '{print $1}'
echo "JamesIves/github-pages-deploy-action v4:"
git ls-remote --refs https://github.com/JamesIves/github-pages-deploy-action.git refs/tags/v4 | awk '{print $1}'
echo "actions/github-script v7:"
git ls-remote --refs https://github.com/actions/github-script.git refs/tags/v7 | awk '{print $1}'Repository: arkade-os/compiler
Length of output: 446
Pin all action references to commit hashes for supply-chain security.
The workflow uses semantic version tags (@v4, @v7) instead of commit hashes for all six action references. This violates the blanket pinning policy and creates supply-chain risk: tags can be retagged or compromised. Update each uses line to the format uses: owner/action@<hash>:
- Line 36:
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 - Line 39:
uses: pnpm/action-setup@f40ffcd9367d9f12939873eb1018b921a783ffaa - Line 44:
uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 - Line 57:
uses: JamesIves/github-pages-deploy-action@d92aa235d04922e8f08b40ce78cc5442fcfbfa2f - Line 65:
uses: actions/github-script@f28e40c7f34bde8b3046d885e986cb6290c5673b - Line 108:
uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 35-36: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 36-36: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 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 @.github/workflows/pr-preview-options.yml at line 36, Replace all semantic
version tags in the `uses` statements with their corresponding commit hash
references to comply with the supply-chain security pinning policy. Update the
six action references: actions/checkout (lines 36 and 108) to use hash
34e114876b0b11c390a56381ad16ebd13914f8d5, pnpm/action-setup to use hash
f40ffcd9367d9f12939873eb1018b921a783ffaa, actions/setup-node to use hash
49933ea5288caeca8642d1e84afbd3f7d6820020, JamesIves/github-pages-deploy-action
to use hash d92aa235d04922e8f08b40ce78cc5442fcfbfa2f, and actions/github-script
to use hash f28e40c7f34bde8b3046d885e986cb6290c5673b. Change each `uses` line
from the format `uses: owner/action@vX` to `uses: owner/action@<commit-hash>` as
specified in the review comment.
Source: Linters/SAST tools
Comment on lines
+43
to
+47
| <div className="stat-grid" style={{ gridTemplateColumns: "repeat(3, 1fr)", marginBottom: 12 }}> | ||
| <MiniStat k="Deposit" v={`${satsToBtc(p.depositSats).toFixed(4)} BTC`} /> | ||
| <MiniStat k="Premium earned" v={<span className="up">+{satsToBtc(p.premiumSats).toFixed(6)}</span>} /> | ||
| <MiniStat k="Strike" v={fmtUsd(p.strikeUsd)} /> | ||
| </div> |
There was a problem hiding this comment.
Inline 3-column override defeats .stat-grid small-screen responsiveness.
This inline gridTemplateColumns prevents the CSS breakpoint rule for .stat-grid from taking effect on narrow viewports.
Suggested patch
- <div className="stat-grid" style={{ gridTemplateColumns: "repeat(3, 1fr)", marginBottom: 12 }}>
+ <div className="stat-grid" style={{ gridTemplateColumns: "repeat(auto-fit, minmax(140px, 1fr))", marginBottom: 12 }}>🤖 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/options/web/src/components/Positions.tsx` around lines 43 - 47, The
inline style with gridTemplateColumns on the div element with className
"stat-grid" is preventing CSS media query breakpoints from working on small
screens. Remove the inline gridTemplateColumns property from the style prop on
this div, and instead define the gridTemplateColumns in the `.stat-grid` CSS
class rules where it can be properly overridden by responsive breakpoint media
queries. Keep only the marginBottom value inline if needed, or move that to CSS
as well.
- Remove the expiry selector; the call is a fixed 30-day tenor (EXPIRY_DAYS). - Trim unused surface: drop Modal/NumberInput/Segmented/Stat/Field UI primitives, renderAsm/getVariant, the unused CashSecuredPut ABI, and the expiry plumbing. CodeRabbit (PR #48): - Premium is now a real transfer from the maker's float, not minted. - Validate write args; clamp NaN deposit at the setter; guard u64le uint64 bounds; recover from a corrupted persisted wallet secret. - Guard RFQ against stale async responses (request token). - Smoke test polls instead of fixed sleeps; asserts premium-from-maker and the NaN clamp. - a11y: aria-label on the spot slider. - Storage shim: correct key()/length semantics. - CSS: drop deprecated word-break; lowercase currentcolor; real .stat-3 responsive class instead of an inline override. - README: language tag on the fence; trim stale refs. - Workflows: persist-credentials:false on the deploy checkouts. Build, smoke (all green), and Playwright responsive check (320/375/414) pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EJC6mxR69rZp79iEmUCXXw
Member
Author
|
Pushed Fixed
Already fixed (earlier commit) — the No longer applicable — resolved by the simplification rewrite, which deleted the code these targeted: transfer/exercise/reclaim flows, Intentionally not changed (happy to revisit):
Build, Generated by Claude Code |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Standalone Bitcoin covered-call dApp (Rysk-style) under examples/options/web,
consuming the compiler-sourced CoveredCall ABI.
the browser; key never leaves the tab)
virtual (Ark) = cooperative tapleaf, Operator co-sign, instant
onchain (L1) = exit tapleaf, N-of-N + CSV, mempool confirmations
(spendVault -> addInput -> addOutput -> sign -> withOperator -> build)
quoting, payoff chart, per-position tapleaf script inspector
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01EJC6mxR69rZp79iEmUCXXw
Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests