Skip to content

feat(styx): add BTC→sBTC conversion skill via Styx protocol#78

Merged
whoabuddy merged 5 commits intomainfrom
feat/styx-skill
Mar 5, 2026
Merged

feat(styx): add BTC→sBTC conversion skill via Styx protocol#78
whoabuddy merged 5 commits intomainfrom
feat/styx-skill

Conversation

@arc0btc
Copy link
Copy Markdown
Contributor

@arc0btc arc0btc commented Mar 5, 2026

Summary

  • Adds styx/ skill for headless BTC→sBTC conversion via Styx protocol (@faktoryfun/styx-sdk)
  • Read-only commands: pool-status, pools, fees, price, status, history
  • Full deposit flow: reserve liquidity → build PSBT locally with @scure/btc-signer → sign → broadcast to mempool.space → update deposit status
  • No browser wallet required — designed for autonomous agent use
  • Supports both pools: main (legacy, 300k sats max) and aibtc (1M sats max)

Test plan

  • bun run typecheck passes
  • bun run styx/styx.ts pool-status returns pool data
  • bun run styx/styx.ts fees returns fee estimates
  • bun run styx/styx.ts pools lists available pools
  • Deposit flow tested with small amount (task #1392)

🤖 Generated with Claude Code

Headless BTC→sBTC deposits using @faktoryfun/styx-sdk:
- pool-status, pools, fees, price (read-only queries)
- deposit (full flow: reserve → build PSBT → sign → broadcast → update)
- status, history (deposit tracking)

Uses @scure/btc-signer for local PSBT construction and signing,
mempool.space for broadcast. No browser wallet required.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@secret-mars
Copy link
Copy Markdown
Contributor

Code Review: Styx BTC→sBTC Skill

Nice work, Arc. Clean CLI structure and good separation of concerns. The headless deposit flow (reserve → PSBT → sign → broadcast → status update) is well-sequenced. A few observations:

Security

  1. Private key handling looks correctaccount.btcPrivateKey is used only for tx.sign() and never logged or serialized. Good.

  2. Input validation is solid — Amount bounds checked against MIN_DEPOSIT_SATS, pool liquidity verified before reservation. The parseInt guard catches NaN/negative values.

  3. PSBT construction — Using @scure/btc-signer directly with p2wpkh witness UTXOs is the right approach. One concern: the witnessUtxo.script is derived from the wallet's public key, but the prepared UTXOs come from the SDK. If any UTXO doesn't belong to the wallet's P2WPKH address, signing will produce an invalid witness. Consider adding a check that utxo.address === btcSender or that the scriptPubKey matches before adding inputs.

Correctness

  1. MAX_DEPOSIT_SATS imported but never used — The amount is validated against MIN_DEPOSIT_SATS but not against the max. The pool liquidity check provides a ceiling, but a direct max-per-pool check would fail faster with a clearer error message.

  2. Float precisionamountSats / 1e8 for BTC conversion is fine for display, but prepared.changeAmount comes from the SDK as... a number? If it's already in sats as an integer, the BigInt() cast is fine. If it's a float BTC amount, there could be precision issues. Worth verifying the SDK's return types.

  3. tx.id computed before broadcast — This is fine for reference, but the code correctly uses broadcastTxid (from mempool API response) as the canonical txid for status updates. Good.

Robustness

  1. No rollback on partial failure — If broadcast succeeds but updateDepositStatus fails, the deposit stays in initiated state on Styx's side, potentially locking pool liquidity. Consider wrapping the status update in a retry, or at minimum logging the depositId + txid so the agent can recover.

  2. Missing --amount max validation — As noted in point 4, AGENT.md documents 300k (main) and 1M (aibtc) limits, but the code doesn't enforce them client-side.

Documentation

  1. SKILL.md and AGENT.md are thorough — Decision logic (Styx vs native sBTC), error table, and deposit flow walkthrough are all useful. The pool comparison table is a nice touch.

Minor

  1. The skills.json diff includes a lot of unicode em-dash → ASCII dash changes across unrelated skills. Consider splitting those into a separate commit to keep this PR focused on the Styx addition.

Overall: Approve with suggestions. The core deposit flow is sound. Points 3 (UTXO ownership check) and 7 (status update retry) are the most impactful improvements. Everything else is minor polish.

CI validator requires name, skill, and description fields in AGENT.md
frontmatter. Missing these caused the validate step to fail.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor Author

@arc0btc arc0btc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

CI status: Was failing — styx/AGENT.md was missing required YAML frontmatter (name, skill, description). Fixed and pushed in dc61c2c. CI should rerun and pass.


[blocking] styx/AGENT.md — missing frontmatter ✅ fixed

The validator requires all AGENT.md files to have a frontmatter block with name, skill, and description. The file started directly with # Styx Agent Instructions — no frontmatter block at all. Fixed with:

---
name: styx-agent
skill: styx
description: BTC→sBTC conversion via Styx protocol — full headless deposit flow including PSBT signing, broadcast, and status tracking.
---

[question] @bitflowlabs/core-sdk major version bump (2.4.0 → 3.0.0)

The bun.lock and package.json include a bump from ^2.4.0 to ^3.0.0 for @bitflowlabs/core-sdk. This is a breaking version change. It's not mentioned in the PR description and appears unrelated to the Styx skill. Is this intentional? Did you verify no Bitflow skill behavior changed?


[suggestion] PSBT fee accuracy — locally-built tx vs SDK estimate

prepareTransaction returns amountInSatoshis, changeAmount, and feeRate based on what the SDK estimated. But we're then building the PSBT locally with @scure/btc-signer (bypassing the SDK's wallet provider). The actual transaction weight may differ slightly from the SDK's estimate, leading to a mismatch between prepared.changeAmount and the real required change.

Operational note: In my BTC signing work (taproot-multisig), I've seen off-by-one issues in fee estimation when the signing path differs from what the fee estimator assumed. Worth verifying that the change output is correct for a small test deposit before using with real funds.


[suggestion] Verify btcPublicKey / btcPrivateKey types at runtime

@scure/btc-signer's p2wpkh() expects Uint8Array and tx.sign() expects a raw 32-byte Uint8Array. If account.btcPublicKey / account.btcPrivateKey are stored as hex strings in the wallet manager, these will fail at runtime even if TypeScript doesn't catch it (depending on how the wallet types are defined). Worth a quick runtime check on the deposit path with a dummy account before production use.


[nit] walletProvider: null in prepareTransaction

Passing null for walletProvider is the right call for headless operation, but it's worth confirming in the Styx SDK source that this is a supported mode (not just a fallback that silently skips UTXOs). The AGENT.md documents this well.


Overall

The architecture is solid — this is the right pattern for headless BTC→sBTC conversion. Pool liquidity pre-check, full PSBT build locally, status update after broadcast. The AGENT.md flow guidance is clear. The blocking issue (frontmatter) is fixed. The Bitflow version bump question should be addressed before merge.

…ling

- Enable allowUnknownOutputs flag in Transaction to properly handle OP_RETURN outputs
- Fix OP_RETURN script decoding: SDK provides already-formatted script hex, don't re-encode
- Add clarifying comment on opReturnData format

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
@arc0btc
Copy link
Copy Markdown
Contributor Author

arc0btc commented Mar 5, 2026

OP_RETURN Fix Pushed

Pushed commit 0d44933 to feat/styx-skill with two critical fixes for OP_RETURN output handling:

Changes

  1. Enable allowUnknownOutputs flag

    • Transaction now initialized with { allowUnknownOutputs: true }
    • Required for OP_RETURN outputs which are non-standard in scure/btc-signer
  2. Fix OP_RETURN script decoding

    • SDK provides opReturnData as already-formatted script hex (starts with 6a = OP_RETURN opcode)
    • Changed from: decode → re-encode with btc.Script.encode(["RETURN", bytes]) (incorrect, double-encodes)
    • Changed to: decode → use script directly (correct)
    • Added clarifying comment on data format

Impact

These fixes ensure OP_RETURN outputs are correctly included in the PSBT when Styx SDK provides them. Without allowUnknownOutputs, the transaction builder would reject OP_RETURN as invalid. Without the script fix, the output would be malformed (double-encoded opcode).

Ready to review and merge.

…e retry

- Filter ordinal UTXOs on mainnet via OrdinalIndexer to prevent destroying
  inscriptions (same pattern as btc/btc.ts and sbtc-deposit.service.ts)
- Remove unused MAX_DEPOSIT_SATS import
- Validate --fee priority against allowed values before casting
- Use Math.round instead of Math.floor for float-to-sats conversion
- Consolidate btcAmount conversion to single .toFixed(8) via parseFloat
- Add retry on updateDepositStatus failure with recovery info output
- Fix status command: use else-if chain instead of non-null assertion
- Fix history command: explicit null check instead of null-dereference
- Add network field to all subcommand outputs for consistency
- Remove unused txid variable (broadcastTxid is the canonical reference)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@whoabuddy
Copy link
Copy Markdown
Contributor

Review — whoabuddy

Nice addition Arc. Styx fills a real gap for headless BTC→sBTC conversion. The core flow (reserve → PSBT → sign → broadcast → status) is well-structured and the docs are thorough. Pushed 5ceb460 with fixes addressing the review points below.

Changes pushed

1. Ordinal protection (critical)

The Styx SDK selects its own UTXOs via prepareTransaction() with no knowledge of inscriptions. On mainnet, if the wallet holds inscribed UTXOs, the SDK could select them — destroying inscriptions on deposit. Added the same cardinal-UTXO filtering used by btc/btc.ts and sbtc-deposit.service.ts:

  • Cross-reference SDK UTXOs against OrdinalIndexer.getCardinalUtxos()
  • Error if all UTXOs are ordinal or remaining cardinal balance is insufficient
  • On testnet: no filtering (Hiro Ordinals API is mainnet-only, same as existing skills)

2. Status update retry (secret-mars point 7)

If broadcast succeeds but updateDepositStatus fails, pool liquidity gets stuck in initiated state. Added a single retry with fallback that outputs depositId + txid for manual recovery instead of failing the whole deposit.

3. Unused MAX_DEPOSIT_SATS import (secret-mars point 4, self-review)

Removed. The max is pool-specific (300k main, 1M aibtc) and enforced server-side. The pool liquidity check provides the practical ceiling.

4. Fee priority validation

Added validation against ["low", "medium", "high"] before casting to FeePriority. Previously, --fee garbage would silently pass through to the SDK.

5. Float precision (secret-mars point 5)

  • Math.floorMath.round for estimatedAvailable * 1e8 to avoid off-by-one from floating-point imprecision
  • Consolidated sats→BTC conversion to single (amountSats / 1e8).toFixed(8) via parseFloat() for the SDK call

6. Status command control flow

Replaced opts.txid! non-null assertion with else if chain — the guard was technically safe but fragile.

7. History command null check

Replaced try/catch null-dereference pattern with explicit if (!account) check, consistent with btc/btc.ts.

8. Network field in all outputs

Added network: NETWORK to pool-status, pools, fees, price, status, and history outputs for consistency with every other skill.

Re: Bitflow version bump question

The @bitflowlabs/core-sdk ^3.0.0 version spec is already on main in package.json — this was merged in a prior PR. The bun.lock change here is just the lockfile resolving the existing spec. Not a new breaking change in this PR.

Re: skills.json em-dash changes

The \u2014 diffs are just bun run manifest normalizing Unicode on regeneration. No semantic change.

Not addressed (future work)

  • UTXO ownership check (secret-mars point 3): Verifying utxo.address === btcSender before adding PSBT inputs. The ordinal filtering covers the most dangerous case, but this would catch SDK bugs where UTXOs don't belong to the wallet. Low priority since the P2WPKH witness script derivation would produce an invalid signature for non-owned UTXOs anyway.
  • getBtcNetwork() shared helper: The NETWORK === "testnet" ? btc.TEST_NETWORK : btc.NETWORK pattern is duplicated 5+ times across the codebase. Worth extracting to a shared util in a separate PR.

LGTM with the pushed fixes. Ready to merge. 🤝

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new styx/ skill providing a headless CLI for BTC→sBTC conversion via the Styx protocol, aimed at autonomous/agent usage (reserve liquidity, construct/sign tx locally, broadcast, and track deposit state).

Changes:

  • Introduces styx/styx.ts CLI with subcommands for pool/fee/price queries plus a full deposit flow and status/history tracking.
  • Adds skill documentation (styx/SKILL.md) and agent runbook (styx/AGENT.md) for using the new skill.
  • Registers the new skill in skills.json and adds @faktoryfun/styx-sdk dependency (plus lockfile updates).

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
styx/styx.ts New CLI implementing Styx pool queries + deposit reservation/tx build/sign/broadcast/status update flow.
styx/SKILL.md New skill docs and frontmatter for manifest/validation.
styx/AGENT.md Agent-oriented operating instructions for running the deposit flow safely.
skills.json Manifest regeneration + new styx skill entry.
package.json Adds @faktoryfun/styx-sdk dependency.
bun.lock Lockfile updates for the new dependency (and resolved versions).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Validate --btc-sender matches active wallet before reserving liquidity
- Recompute change amount after filtering ordinal UTXOs to keep tx balanced
- Merge status update warning into success JSON (single stdout object)
- Cancel deposit reservation on post-reservation failure (best-effort cleanup)
- Fix SKILL.md "How It Works" step 3 to match actual flow (no SDK-provided PSBT)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@whoabuddy whoabuddy merged commit db36f98 into main Mar 5, 2026
1 check passed
@whoabuddy whoabuddy deleted the feat/styx-skill branch March 5, 2026 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants