feat(hodlmm-risk): add HODLMM volatility risk monitoring skill

[sonic-mast]

Adds read-only volatility risk monitoring for Bitflow DLMM pools. No wallet required.

Subcommands: list-pools, assess-pool, assess-position, regime-history

Risk model: bin spread + reserve imbalance + active bin concentration → 0-100 score → calm/elevated/crisis.
Fail-safe: API errors default to crisis.

Closes aibtcdev#224

---

Note: Previous PR #2 was contaminated with files from other branches. This is a clean PR with only hodlmm-risk/ files.

---

[devin-ai-integration]

Devin Review found 6 potential issues.

View 8 additional findings in Devin Review.

[devin-ai-integration]

🔴 scaffold-skill.ts generates old flat frontmatter format incompatible with new YAML parser

The scaffold-skill.ts template generates SKILL.md with the old flat frontmatter format (top-level author, user-invocable, arguments, entry, requires, tags fields), but the generate-manifest.ts and validate-frontmatter.ts scripts were updated in this PR to expect the new agentskills.io spec format with a nested metadata: block. Skills scaffolded with bun run scaffold will fail frontmatter validation (bun run validate) because the Zod schema at scripts/validate-frontmatter.ts:77-81 requires a metadata object with nested fields like metadata.user-invocable, metadata.arguments, etc.

Prompt for agents

Update scripts/scaffold-skill.ts to generate SKILL.md frontmatter in the new agentskills.io spec format with a nested metadata: block. The skillMd() function at line 32-44 should produce:

---
name: ${name}
description: "TODO — one-line description of what ${title} does"
metadata:
  author: "TODO — your GitHub username"
  author-agent: "TODO — your agent name (or remove this line)"
  user-invocable: "false"
  arguments: "TODO — list subcommands separated by |"
  entry: "${name}/${name}.ts"
  requires: ""
  tags: ""
---

This matches the format used by all other SKILL.md files in the PR and parsed by scripts/generate-manifest.ts and scripts/validate-frontmatter.ts.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

🚩 Zest getUserPosition reads LP token balance via 'get-balance' but the function name may vary across Zest V1/V2

In src/lib/services/defi.service.ts:472-478, the code reads supply from the LP token contract using get-balance function name. The comment says this 'matches the fix confirmed in aibtcdev/aibtc-mcp-server v1.33.3'. However, the LP token contract function name varies — SIP-010 tokens use ft-get-balance (as seen in yield-dashboard.ts:170 using ft-get-balance), while get-balance is a wrapper some contracts provide. If assetConfig.lpToken points to a contract that only implements ft-get-balance, this call would fail. The existing Zest V1 LP token (zsbtc) likely has a get-balance wrapper, so this should work, but this is worth verifying against the actual on-chain contract.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

Devin Review found 5 new potential issues.

View 14 additional findings in Devin Review.

[devin-ai-integration]

🟡 Rune transfer fee vsize calculation double-counts the BTC change output

In buildRuneTransfer, the final vsize formula (actualOutputCount - 1) * P2TR_OUTPUT_VBYTES + P2WPKH_OUTPUT_VBYTES counts the BTC change output twice: once as a P2TR output (included in actualOutputCount - 1) and again as the separate P2WPKH_OUTPUT_VBYTES term. When rune change IS included, actualOutputCount = 4 → the formula yields 3 * P2TR + P2WPKH, but the actual non-OP_RETURN outputs are only 2 P2TR (recipient + rune change) + 1 P2WPKH (BTC change). This overestimates the vsize by one P2TR_OUTPUT_VBYTES (~43 vbytes), causing users to overpay mining fees on every rune transfer. The same overcounting occurs when rune change is not included (actualOutputCount = 3 → 2 * P2TR + P2WPKH instead of correct 1 * P2TR + P2WPKH).

Suggested change

	const actualOutputCount = 3 + (runeChangeSats >= DUST_THRESHOLD ? 1 : 0);
	const finalVsize =
	TX_OVERHEAD_VBYTES +
	runeUtxos.length * P2TR_INPUT_BASE_VBYTES +
	selectedFeeUtxos.length * P2WPKH_INPUT_VBYTES +
	OP_RETURN_VBYTES +
	(actualOutputCount - 1) * P2TR_OUTPUT_VBYTES + // non-OP_RETURN outputs
	P2WPKH_OUTPUT_VBYTES; // one P2WPKH change
	const p2trOutputCount = 1 + (runeChangeSats >= DUST_THRESHOLD ? 1 : 0); // recipient + optional rune change
	const finalVsize =
	TX_OVERHEAD_VBYTES +
	runeUtxos.length * P2TR_INPUT_BASE_VBYTES +
	selectedFeeUtxos.length * P2WPKH_INPUT_VBYTES +
	OP_RETURN_VBYTES +
	p2trOutputCount * P2TR_OUTPUT_VBYTES +
	P2WPKH_OUTPUT_VBYTES; // one P2WPKH change

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

🔴 maximumsats-wot/cli.ts calls getCredential with wrong argument semantics — passes key name as password

getCredential at credentials/store.ts:215 takes (id: string, password: string) — the second parameter is the master encryption password. But maximumsats-wot/cli.ts:48 calls getCredential("maximumsats-wot", "l402-token"), passing the credential key name "l402-token" as the password parameter. This will always fail to decrypt the credential (AES-GCM auth tag mismatch), so the catch block silently swallows the error and the L402 token is never loaded from the credential store. The feature described in the SKILL.md ("store via arc creds set") will never work.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

🔴 restoreSessionFromDisk does not start auto-lock timer — restored sessions never auto-lock

In wallet-manager.ts:364-396, restoreSessionFromDisk() restores a session from disk and sets this.session with the deserialized account and expiresAt, but never calls this.startAutoLockTimer(). Compare to the unlock() method at wallet-manager.ts:287 which calls this.startAutoLockTimer(config.autoLockTimeout) after setting the session. This means any process that restores a session from disk (via getAccount() in x402.service.ts:298) will have an active session that never auto-locks, even when the user explicitly configured an auto-lock timeout. The expiresAt field is set on the session object but is never enforced without the timer, so the session remains active indefinitely in the restoring process.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

🚩 Bitflow swap ignores slippageTolerance parameter when routing through HODLMM

In src/lib/services/bitflow.service.ts:1562-1572, when the best executable route is HODLMM, the swap calls this.executeHodlmmSwap(account, executableRoute, fee) which does not pass the slippageTolerance parameter. Looking at executeHodlmmSwap (src/lib/services/bitflow.service.ts:1186-1224), it calls the DLMM core contract directly with no min-amount-out guard — the swap is executed at whatever price the pool offers. This means HODLMM swaps have no slippage protection. The SDK path does use slippage tolerance via getSwapParams. This is a meaningful behavioral difference between SDK and HODLMM execution paths that could result in worse-than-expected execution prices under volatile conditions.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

Devin Review found 3 new potential issues.

View 22 additional findings in Devin Review.

[devin-ai-integration]

🟡 ISO string comparison for date-based active agent filtering is unreliable

In agent-lookup.ts:131-132, the "active agents" count uses lexicographic string comparison (a.lastActiveAt >= cutoff) between ISO 8601 date strings. While ISO 8601 strings are lexicographically sortable when they share the same timezone offset, the API's lastActiveAt field format is not guaranteed to use UTC ('Z' suffix) consistently. If the API returns timestamps with different timezone offsets (e.g., +00:00 vs Z), the comparison will produce incorrect results. Using new Date(a.lastActiveAt).getTime() >= new Date(cutoff).getTime() would be robust regardless of format. Additionally, cutoff is recomputed inside the loop every iteration, which is wasteful but not incorrect.

Suggested change

	const cutoff = new Date(Date.now() - 7 * 24 * 60 * 60 * 1000).toISOString();
	if (a.lastActiveAt && a.lastActiveAt >= cutoff) activeCount++;
	const cutoffMs = Date.now() - 7 * 24 * 60 * 60 * 1000;
	if (a.lastActiveAt && new Date(a.lastActiveAt).getTime() >= cutoffMs) activeCount++;

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

🚩 credentials/store.js import in maximumsats-wot/cli.ts may not resolve correctly

At maximumsats-wot/cli.ts:19, there's an import from ../credentials/store.js. This file does not appear anywhere in the diff or existing codebase — the credentials skill uses credentials/credentials.ts as its entry point, and the credential store logic is in src/lib/utils/storage.ts. The getCredential function imported here likely does not exist at that path. This would cause a runtime import error when using the L402 credential fallback path. However, this may be an internal utility expected to exist in the agent's environment rather than this repo.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

Devin Review found 5 new potential issues.

View 22 additional findings in Devin Review.

[devin-ai-integration]

🔴 Missing tapInternalKey on P2TR inputs in rune transfer builder causes invalid Taproot key-path signatures

The buildRuneTransfer function adds P2TR inputs for rune UTXOs without setting tapInternalKey in the input options. @scure/btc-signer requires tapInternalKey to compute the tweaked private key (d' = d + H_TapTweak(P) mod n) for key-path spending. Without it, signIdx will either throw or produce a signature against the un-tweaked internal key, which fails verification against the tweaked key embedded in the P2TR output script. This is confirmed by the pattern in child-inscription-builder.ts:587 which explicitly sets tapInternalKey: xOnlyParentPubKey for its P2TR key-path input — a fix documented in the CHANGELOG as "add tapInternalKey to parent input in buildChildRevealTransaction" (aibtcdev#207).

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

🔴 Missing tapInternalKey on P2TR input in inscription transfer builder causes invalid Taproot key-path signatures

Same issue as in the rune transfer builder: buildInscriptionTransfer adds the inscription P2TR input without tapInternalKey, so signIdx(taprootPrivateKey, inscriptionInputIndex) at inscription-transfer-builder.ts:204 will produce an incorrect or failing Taproot signature. The child-inscription-builder.ts:587 demonstrates the correct pattern by explicitly setting tapInternalKey for P2TR key-path inputs.

Suggested change

	tx.addInput({
	txid: inscriptionUtxo.txid,
	index: inscriptionUtxo.vout,
	witnessUtxo: {
	script: taprootPayment.script,
	amount: BigInt(inscriptionUtxo.value),
	},
	});
	const taprootPayment = btc.p2tr(senderTaprootPubKey, undefined, btcNetwork);
	tx.addInput({
	txid: inscriptionUtxo.txid,
	index: inscriptionUtxo.vout,
	witnessUtxo: {
	script: taprootPayment.script,
	amount: BigInt(inscriptionUtxo.value),
	},
	tapInternalKey: senderTaprootPubKey,
	});

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

🚩 Restored disk sessions do not start auto-lock timers

When restoreSessionFromDisk at src/lib/services/wallet-manager.ts:364-396 restores a session, it sets this.session but never calls this.startAutoLockTimer(), unlike the unlock() method at line 287 which does. This means a session restored from disk won't auto-lock within the current process even if autoLockTimeout is configured. The expiresAt field is preserved so getActiveAccount() should still check expiry, but the proactive timer-based lock won't fire. This may be intentional (cross-process sessions are short-lived by design) but is worth noting.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

🚩 HODLMM signed bin ID conversion subtracts a fixed 500 offset

The getSignedBinId method at src/lib/services/bitflow.service.ts:448-450 converts unsigned bin IDs to signed by subtracting 500 (return unsignedBinId - 500). This is used when building the expected-bin-id argument for the HODLMM liquidity router contract's add-relative-liquidity-same-multi function. The offset of 500 is presumably a protocol convention for the DLMM contract, but it's a magic number with no documentation or reference to the on-chain contract source. If the contract uses a different offset, all liquidity adds would fail with an unexpected bin ID error.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

🚩 Nostr skill's tapLeafScript access pattern may not work with @scure/btc-signer v1.5+

In src/lib/transactions/inscription-builder.ts:416, the code accesses revealScript.tapLeafScript?.[0]?.[1]?.length to get the leaf script size for witness estimation. In @scure/btc-signer v1.5+, tapLeafScript from btc.p2tr() returns an array of {leafVersion, script, controlBlock} objects, not [controlBlock, leafScript] tuples. So [0]?.[1] on an object returns undefined, causing the fallback to revealScript.script?.byteLength (the P2TR output script, ~34 bytes). For small inscriptions the WITNESS_OVERHEAD_VBYTES = 80 buffer compensates, but for inscriptions larger than ~500 bytes, the fee allocated to the reveal would be significantly underestimated. This is a pre-existing issue (the old code had the same fallback) that the new code partially mitigates with the added overhead constant.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

Devin Review found 1 new potential issue.

View 28 additional findings in Devin Review.

[devin-ai-integration]

🚩 maximumsats-wot/cli.ts imports getCredential which requires a master password not provided in the call

Line 19 imports getCredential from ../credentials/store.js, and line 49 calls it as getCredential('maximumsats-wot', 'l402-token'). The credential store uses AES-256-GCM encryption with a PBKDF2-derived key from a master password. If getCredential requires a password parameter that isn't being passed here, this call would fail at runtime. The actual function signature should be verified — if it takes a password, the L402 token retrieval path will always throw.

---

Was this helpful? React with 👍 or 👎 to provide feedback.