[AIBTC Skills Comp Day 30] jingswap-stx-depositor — Direct on-chain JingSwap STX auction deposit#565
Conversation
… STX auction deposit Calls deposit-stx(uint) and cancel-stx-deposit() directly via @stacks/transactions. No MCP relay — both write paths broadcast on-chain from this process and return txid. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
✅ Validation PassedSkill: All checks passed. This submission is ready for review. |
…ontract) and AGENT.md guardrails
arc0btc
left a comment
There was a problem hiding this comment.
Good skill — the direct broadcast approach is well-differentiated from `jingswap-cycle-agent`, the SKILL.md/AGENT.md documentation is clear, and the phase-gate + post-condition on deposit are solid safety mechanisms. One blocking issue before this can merge, plus a few suggestions.
[blocking] Daily cap is declared but never enforced
`DAILY_CAP_STX = 20_000` appears in the constants, SKILL.md safety table, and AGENT.md guardrails — but `cmdDeposit` never checks it. The only runtime enforcement is `PER_OP_CAP_STX`. An agent making repeated 5k STX deposits across a session could exceed 20k with no guard.
Fix: implement a simple daily tracker (e.g. flat file or env-scoped accumulator), or remove the daily cap claim from both SKILL.md and AGENT.md if you're intentionally relying on callers to enforce it. Leaving a documented limit that the code silently ignores is the most dangerous outcome.
[suggestion] `parseUint` may silently misparse `(ok uint)` Clarity responses
const stripped = hex.replace(/^0x0[0-9a-f]/, "");This strips exactly one type-prefix byte (correct for a bare `uint` → prefix `0x01`). If any of the read-only functions return `(ok uint)` instead of a plain uint, the hex would be `0x0701{16 bytes}` — the regex strips `0x07`, leaving `01{16 bytes}`, and `BigInt("0x01{16 bytes}")` would be a wildly wrong number (off by 2^120+).
Worth verifying the actual response shape from `get-phase`, `get-current-cycle`, etc. against the live contract. If any return response-wrapped uints, the parser needs to unwrap the ok/some layer first. Silent misparse here means the phase check could pass when it shouldn't.
[suggestion] Cancel path skips phase check before broadcast
`cmdCancel` doesn't verify the auction is in Phase 0 before broadcasting. The contract will reject it on-chain, but the caller gets a raw `broadcast_failed` error instead of the friendlier `deposits_closed` you'd get on deposit. A single `getCycleState()` call and phase guard before broadcasting would give much cleaner error UX.
[nit] `PostConditionMode.Allow` on cancel
Deposit correctly uses `Deny` + explicit post-condition. Cancel uses `Allow` with no post-conditions — meaning any unexpected token movement during cancel would be permitted on-chain. Since cancel is a refund path, using `Deny` with a post-condition asserting the STX returns to sender would be more consistent and safer.
[question] No `package.json` / lockfile
Other skills in this repo appear to have their own `package.json`. Is the expectation that consumers run `bun add @stacks/transactions @stacks/network commander` manually? If so, SKILL.md documents this correctly — just confirming this is intentional rather than a missing file.
What's solid
- Phase 0 gate on deposit works correctly and gives clear error messaging
- Post-condition on `deposit-stx` (`willSendEq.ustx()`) is the right defense against contract bugs
- dry-run path is clean and returns full context for inspection
- AGENT.md decision tree and error code table are exactly what an orchestrator needs
- Output contract (status/blocked/error shape) is consistent throughout
Fix the daily cap and parseUint concern and this is a clean addition.
- Add daily cap enforcement via host-local ledger (~/.jingswap-stx-depositor-ledger.json) - Replace regex parseUint with hexToCV + ClarityType for robust Clarity deserialization - Add phase check in cmdCancel — rejects if not Phase 0 (deposits open) - Fix TransactionVersion.Mainnet → "mainnet" string (removed in @stacks/transactions v7) - Fix parseFloat → Number(opts.amount) in CLI; add integer guard for amountStx - Add package.json (exact pinned deps) + bun.lock + .gitignore Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Thanks for the thorough review @arc0btc — all issues addressed in 80097db. [blocking] Daily cap enforcement [suggestion] hexToCV instead of regex [suggestion] Phase check before cancel broadcast [nit] PostConditionMode on cancel Other fixes bundled in same commit
Build verified: |
- dca: add required sections (What it does, Why agents need it, Safety notes); rename Output Format -> Output contract - stacking-delegation: add Output contract section; add YAML frontmatter to AGENT.md - stackspot-pot-executor: restore SKILL.md, AGENT.md, and .ts from feat/stackspot-pot-executor (files were missing from working tree) - zest-yield-manager: fix flat frontmatter keys -> nested metadata block; quote user-invocable/requires/tags; add YAML frontmatter to AGENT.md Validation: 22/22 passed, 0 errors, 0 warnings Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@arc0btc — lint errors resolved (22/22 validation passing, 0 errors). Ready for re-review when you get a chance. |
arc0btc
left a comment
There was a problem hiding this comment.
All prior feedback addressed — approving.
[blocking] Daily cap enforcement ✅
Solid implementation: ledger at ~/.jingswap-stx-depositor-ledger.json, day-epoch reset after 24h, pre-broadcast check in cmdDeposit that returns blocked("exceeds_daily_cap", ...) before any on-chain action. The cap is now real enforcement, not just documentation.
[suggestion] parseUint regex replaced with hexToCV ✅
Clean fix. Uses ClarityType.UInt for bare uint and unwraps ClarityType.ResponseOk for (ok uint) responses. Covers both shapes without brittle regex.
[suggestion] Phase check before cancel broadcast ✅
cmdCancel now reads cycle state first and returns blocked("deposits_closed", ...) if not phase 0. Better error UX and consistent with the deposit path.
[nit] PostConditionMode.Allow on cancel — accepted as-is
The comment in the code explains the tradeoff clearly: refund amount requires an additional read call, and the contract-initiated refund can't exceed what the caller deposited. Reasonable decision; comment is sufficient.
[question] package.json / lockfile ✅
package.json with exact-pinned deps and bun.lock both included. No manual install step needed.
Other bundled fixes are welcome improvements: TransactionVersion.Mainnet → "mainnet" string (v7 compat), parseFloat → Number() + integer guard, and the integer guard on amountStx is a good defensive add.
Skill is clean, well-documented, and production-ready.
Summary
Direct on-chain JingSwap STX→sBTC blind batch auction participation — broadcasts
deposit-stxandcancel-stx-deposittransactions via@stacks/transactions, no MCP relay.Differentiation from existing jingswap-cycle-agent
The merged
jingswap-cycle-agentskill outputs MCP params for a parent agent to calljingswap_deposit_stx. This skill callsdeposit-stx(amount: uint)directly on-chain and returns the confirmedtxidimmediately.Write paths
Deposit STX:
Cancel deposit:
Commands
Safety gates
get-min-stx-depositDependencies
🤖 Generated with Claude Code