Arkade script support#319
Conversation
Deprecate StorageAdapter (new)
* Contract Manager * Contract Manager * simplify * Refactor contract system to use URLSearchParams and improve initialization handling - Replace manual URL encoding/decoding in arkcontract with URLSearchParams - Add concurrent initialization guard for ContractManager to prevent race conditions - Make notifyIncomingFunds stop function async to properly handle cleanup - Update contract tests to use new contractHandlers registry instead of deprecated SpendingStrategyRegistry - Remove obsolete comments and simplify type definitions in ContractWatcher * Remove CSV generalization from DefaultContractHandler Contracts may have multiple unilateral exit paths with different CSV values, so this cannot be generalized at the handler level. Export the timelock utility functions for use by other handlers. * ContractManager: auto-watch on initialize, support multiple event callbacks - initialize() now automatically starts watching contracts - onContractEvent() added for registering event callbacks (returns unsubscribe) - startWatching() deprecated (still works but just registers callback) - Support multiple concurrent event callbacks - Add vtxo_spendable event type for sweeper notifications * Wallet: include pubKey and serverPubKey in default contract params The default contract registered with ContractManager now includes the actual pubKey and serverPubKey parameters instead of empty params. * ContractManager: validate params via handler on createContract Adds validation in createContract() that: 1. Verifies a handler exists for the contract type 2. Attempts to create the script from params (catches invalid/missing params) 3. Verifies the derived script matches the provided script Also updates contract tests to use valid params and scripts. * ContractWatcher: watch contracts with VTXOs regardless of state - Add getScriptsToWatch() that returns scripts for active contracts AND contracts with known VTXOs (regardless of state) - Update updateSubscription() to use getScriptsToWatch() - Update addContract() to poll first to discover VTXOs, then update subscription This ensures we continue monitoring contracts even after they're deactivated, as long as they have unspent VTXOs. * ContractSweeper: handler-defined sweep destinations - Add optional getSweepDestination method to ContractHandler interface - Update ContractSweeper to use handler-defined destinations - Destination priority: handler > contract.sweepDestination > default - Handler receives defaultDestination to use as fallback * Add delegation types and architecture hooks for future VTXO refresh Prepares the contract system for future delegation/refresh support: - Add DelegationConfig interface for contract delegation settings - Add DelegatedForfeit interface for pre-signed forfeit data - Add DelegationResult interface for delegation creation results - Add delegation field to Contract interface - Add supportsDelegation() method to ContractHandler interface - Implement supportsDelegation() on DefaultContractHandler (returns true) Delegation enables server-side VTXO refresh using pre-signed forfeit transactions (SIGHASH_ALL|ANYONECANPAY), eliminating the need for complex multi-party coordination in contracts like VHTLC. * ServiceWorker: add contract operation request/response types Adds request and response types for contract operations: - GET_CONTRACTS: List contracts with optional filter - GET_CONTRACT: Get single contract by ID - CREATE_CONTRACT: Create new contract - UPDATE_CONTRACT_STATE: Change contract state - GET_CONTRACT_VTXOS: Get VTXOs by contract - GET_CONTRACT_BALANCE: Get balance for a contract - CONTRACT_EVENT: Broadcast contract events This prepares the service worker API for ContractManager integration. Handler implementation can be added when the wallet's contract manager is properly initialized in the service worker context. * Add README documentation for contract system Documents the contract system architecture including: - Architecture overview with ASCII diagram - Core concepts (Contract, ContractHandler, ArkContract strings) - Usage examples (setup, creating, querying, lifecycle) - Event types reference - Watching and sweeping behavior - Delegation future support - Custom handler registration - File reference * Add comprehensive contract system tests Adds 23 new tests covering: - ArkContract encoding/decoding (7 tests) - Handler param validation (3 tests) - VTXO-based watching / getScriptsToWatch (3 tests) - Multiple event callbacks (2 tests) - DefaultContractHandler path selection (6 tests) - getSweepDestination (2 tests) Total contract tests: 44 (up from 21) * Address PR review comments - Add .claude/ to .gitignore and remove tracked settings file - Make ParsedArkContract generic with T extends Record<string, string> - Add ids filter to ContractFilter for bulk lookups - Add command injection warning in example fundAddress function * Remove sweeper and delegation code for separate PR - Delete ContractSweeper class and README - Remove SweeperConfig, SweepResult, and delegation types - Remove autoSweep/sweepDestination from Contract interface - Remove sweeper integration from ContractManager - Remove vtxo_swept event type from ContractWatcher - Remove supportsDelegation from ContractHandler - Update wallet to remove sweeper config and methods - Update service worker request types - Update tests to remove sweeper assertions This code will be reintroduced in a separate PR to keep the initial contract system PR focused and smaller. * Fix merge from next and merge ContractManagerRepository into ContractManager * Handle fresh DB in migration tool * Update example, modify signature of * Add test for VHTLC script * Move ContractManager to ReadonlyWallet * Fix docs, remove forcePoll to use config instead, fix test * Ensure an existing contract has same type when created * Refactor ContractManager and ContractWatcher, extract ContractCache * Continute refactoring, update tests * Implment contracts methods in SW * Example updated * Add tests for ContractVtxoCache * Address sequence comment * Remove/update comments * DRY - refactor handlers * Fix filters * Add deprecation notice for bolt-swap specific methods * remove Contract.data field and modify behavior of updateContract * Remove VTXO cache * Remove plurality from filter fields --------- Co-authored-by: Pietro Grandi <dev@pietro.uno>
…d methods (arkade-os#268) * Migrate commitmentTxid under WalletRepository, fixes first point of arkade-os#267 * Introduce new schema for contracts arkade-os#267 * Remove deprecated methods that are unused arkade-os#267 * Update tests * Explicit object names role (v1 - v2) and remove unused import * Remove deprecated methods entirely and delegate contracts migration to the package which created them * Remove last 'any' from handlers * Treat commitmentTx unique per txid * Remove ID from Contract * Use VTXO to determine spendable paths and introduce new method to calculate all paths * Add dedicated tests for CSV locks * Remove unnecessary deprecated method from IWallet - it's not released yet * Remove support for commitmentTxs * Uniform naming in repository classes
🔍 Arkana PR Review — #319 (Arkade script support)Scope: +5,677 / -217 across 48 files. Adds full client-side Arkade script support: opcodes, script codec, key tweaking, introspector provider, batch handler, banco swap maker/taker, PSBT field improvements, extension refactoring, and comprehensive tests. ✅ Strengths
|
![arkanaai[bot]](https://avatars.githubusercontent.com/u/170820085?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 Arkana PR Review — #319 (incremental: 8eb98de)
Scope of new changes (since last review 2026-04-03): 2 commits — merge from master + introspector v0.0.1 update. +1,552 / -76 across 16 files.
✅ Looks good
src/arkade/bignum.ts—encodeFixedsign-bit manipulation is correct; traces through positive, negative, and zero cases without data corruption.src/arkade/opcodes.ts—INSPECTPACKET(0xf4) /INSPECTINPUTPACKET(0xf5) correctly extend the opcode range.src/utils/unknownFields.ts—PrevArkTxFieldandPrevoutTxFieldencode/decode are symmetric and key-strings are distinct. Correct.src/extension/index.ts—getPackets()returnsreadonly, prevents mutation. Clean.src/providers/introspector.ts:221-245—submitOnchainTxhas proper error handling and response validation. Consistent with existing submit methods.- E2E tests — All negative test cases genuinely test what they claim (wrong dest, wrong amount, missing packet, wrong prevout, arkd-signer-in-tapscript).
enforcePayToandenforceSelfSendmatch the Go reference implementations opcode-for-opcode. - Test helpers (
addIntrospectorPacket,randomP2TR,waitForUtxo) — correctly handle extension merge/insert logic and P2A anchor positioning. - No cross-repo breakage — new APIs (
submitOnchainTx,PrevArkTxField,PrevoutTxField,BigNum, new opcodes) are additive. No downstream SDK consumers found.
⚠️ Minor findings (non-blocking)
-
src/arkade/script.ts:66-72— bigint MINIMALDATA concern:bigintvalues 1n–16n bypass the small-integer opcode optimization (OP_1–OP_16) and produce data pushes instead. If the arkade VM enforces MINIMALDATA, this would cause script failures. Currently not triggered by any code in the PR (all bigint usage is for amounts >> 16), but it's a latent footgun. Consider adding the small-integer check forbiginttoo:if (typeof o === "bigint" && o >= 1n && o <= 16n) { w.byte(Number(o) + 0x50); continue; }
-
Missing negative E2E tests (nice-to-have):
- HTLC: no "wrong preimage" test — would confirm
HASH160 <hash> EQUALenforcement - Delegate: no "version 3" test — would confirm the v2-only gate that blocks off-chain Ark txs
- Onchain: no "wrong destination, correct amount" test — would isolate the
INSPECTOUTPUTSCRIPTPUBKEYcheck path
- HTLC: no "wrong preimage" test — would confirm
-
Design note (codebase-wide, not new):
submitOnchainTx(and all submit* methods) trust the introspector's response without verifying outputs haven't been tampered with. Callers should verify the returned PSBT preserves their intended outputs. Not a regression — consistent with existing patterns.
Verdict
The introspector v0.0.1 update is clean. BigNum encoding is correct, new PSBT fields are symmetric, new opcodes are properly registered, and test coverage is solid with meaningful negative cases. The signing flow change (introspector auto-finalizes as last non-arkd signer) simplifies the client flow and the tests correctly adapt.
Approving — no protocol-correctness or security issues found. The MINIMALDATA concern is latent and not triggered by current code.
Review by Arkana 🔍 — [Agent: pr-reviewer]
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com
There was a problem hiding this comment.
🔍 Arkana PR Review — #319 (incremental: 8eb98de)
Scope of new changes (since last review 2026-04-03): 2 commits — merge from master + introspector v0.0.1 update. +1,552 / -76 across 16 files.
✅ Looks good
src/arkade/bignum.ts—encodeFixedsign-bit manipulation is correct; traces through positive, negative, and zero cases without data corruption.src/arkade/opcodes.ts—INSPECTPACKET(0xf4) /INSPECTINPUTPACKET(0xf5) correctly extend the opcode range.src/utils/unknownFields.ts—PrevArkTxFieldandPrevoutTxFieldencode/decode are symmetric and key-strings are distinct. Correct.src/extension/index.ts—getPackets()returnsreadonly, prevents mutation. Clean.src/providers/introspector.ts:221-245—submitOnchainTxhas proper error handling and response validation. Consistent with existing submit methods.- E2E tests — All negative test cases genuinely test what they claim (wrong dest, wrong amount, missing packet, wrong prevout, arkd-signer-in-tapscript).
enforcePayToandenforceSelfSendmatch the Go reference implementations opcode-for-opcode. - Test helpers (
addIntrospectorPacket,randomP2TR,waitForUtxo) — correctly handle extension merge/insert logic and P2A anchor positioning. - No cross-repo breakage — new APIs (
submitOnchainTx,PrevArkTxField,PrevoutTxField,BigNum, new opcodes) are additive. No downstream SDK consumers found.
⚠️ Minor findings (non-blocking)
-
src/arkade/script.ts:66-72— bigint MINIMALDATA concern:bigintvalues 1n–16n bypass the small-integer opcode optimization (OP_1–OP_16) and produce data pushes instead. If the arkade VM enforces MINIMALDATA, this would cause script failures. Currently not triggered by any code in the PR (all bigint usage is for amounts >> 16), but it's a latent footgun. Consider adding the small-integer check forbiginttoo:if (typeof o === "bigint" && o >= 1n && o <= 16n) { w.byte(Number(o) + 0x50); continue; }
-
Missing negative E2E tests (nice-to-have):
- HTLC: no "wrong preimage" test — would confirm
HASH160 <hash> EQUALenforcement - Delegate: no "version 3" test — would confirm the v2-only gate that blocks off-chain Ark txs
- Onchain: no "wrong destination, correct amount" test — would isolate the
INSPECTOUTPUTSCRIPTPUBKEYcheck path
- HTLC: no "wrong preimage" test — would confirm
-
Design note (codebase-wide, not new):
submitOnchainTx(and all submit* methods) trust the introspector's response without verifying outputs haven't been tampered with. Callers should verify the returned PSBT preserves their intended outputs. Not a regression — consistent with existing patterns.
Verdict
The introspector v0.0.1 update is clean. BigNum encoding is correct, new PSBT fields are symmetric, new opcodes are properly registered, and test coverage is solid with meaningful negative cases. The signing flow change (introspector auto-finalizes as last non-arkd signer) simplifies the client flow and the tests correctly adapt.
Approving — no protocol-correctness or security issues found. The MINIMALDATA concern is latent and not triggered by current code.
Review by Arkana 🔍 — [Agent: pr-reviewer]
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com
Continues the Arkade Script port from arkade-os/ts-sdk#319 / ArkLabsHQ/introspector. Builds on the opcode / scriptnum / tweak primitives committed earlier and adds: - ArkadeScript: encode/decode bytes <-> Op[], plus ASM helpers using the combined Bitcoin+Arkade opcode registry. Round-trip is a thin pass- through over NBitcoin's Script/Op since NBitcoin already treats the Arkade extension byte range (0xb3, 0xc4-0xf3) as opaque single-byte ops — no custom serializer needed. - IntrospectorPacket / IntrospectorEntry: TLV codec for the OP_RETURN payload that binds ArkadeScript to specific transaction inputs. Wire format is `compactSize(count) + [u16_le(vin) + compactSize(scriptLen) + script + compactSize(witnessLen) + witness]*`. Validates non-empty packet, non-empty script, unique vin (matching the introspector reference). Includes EncodePushList/DecodePushList for the inner list-of-pushes shape callers wrap into the witness slot. - IIntrospectorProvider + IntrospectorClient: REST client for the four endpoints (GET /v1/info, POST /v1/tx, POST /v1/intent, POST /v1/finalization), DI-friendly via the new IntrospectorServiceCollectionExtensions.AddIntrospectorClient. RegisterIntentMessage is JSON-stringified into the wire envelope to match ts-sdk parity. Tests: - ArkadeScriptNum: known sign-magnitude vectors + 32-byte EC scalar round-trip + non-minimal rejection. - ArkadeScriptCodec: opcode preservation, ASM mnemonic correctness, bare-name parsing parity with ts-sdk's fromASM, ALL 41 Arkade opcodes round-trip through ASM. - ArkadeScriptHash: deterministic compute, distinct scripts produce distinct digests, x-only key tweak round-trip. - IntrospectorPacket fixture-driven: every "valid" vector encodes to the exact bytes shipped in the introspector's testdata and parses back to the same entries. "Invalid" vectors split into Validate-failures (empty packet, empty script, duplicate vin) and Parse-failures (truncated, trailing bytes, length-fields exceeding the buffer). 302/302 unit tests green locally. Still pending on this branch: ArkadeVtxoScript, PSBT ArkadeScript/ArkadeScriptWitness fields, ArkadeBatchHandler, and the E2E docker test.
There was a problem hiding this comment.
🔍 Arkana PR Review — #319 (incremental: 1673ce4d)
Review by Arkana 🔍 — [Agent: pr-reviewer]
Scope since last review (8eb98de): 1 functional commit + 1 merge from master.
1673ce4d — fix minimal data number encoding (src/arkade/script.ts:62-73)
This directly addresses the MINIMALDATA concern I flagged in my previous review. The fix:
- Merges the
numberandbigintbranches into a singletypeof o === "number" || typeof o === "bigint"check - Adds OP_0 handling for
0n(o === 0 || o === 0n) — uses strict equality, correct since0n === 0isfalsein JS - Extends OP_1–OP_16 small-integer encoding to bigint values via
Number(o)coercion after the range check - JS comparison operators (
>=,<=) support mixed bigint/number operands, so1n >= 1and16n <= 16work correctly - Negative numbers (
-1,-1n) correctly fall through to BigNum encoding
The comment about Arkade VM enforcing MINIMALDATA is a good addition for future maintainers.
One note: No unit test was added for bigint MINIMALDATA encoding (e.g., ArkadeScript.encode([0n, 1n, 16n, 17n]) round-trip). Consider adding one to prevent regression.
Master merge (c1b01a86)
Two PR-touched files were modified by the merge:
src/extension/index.ts: import path.jsextension fix — trivial, correctsrc/utils/unknownFields.ts:bip68.decode→ centralizedsequenceToTimelockhelper — clean refactor from master, no semantic change
No conflicts. No issues.
Approving — the MINIMALDATA fix is correct and addresses the last open concern.
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com
There was a problem hiding this comment.
🔍 Arkana PR Review — #319 (incremental: bf176026)
Review by Arkana 🔍 — [Agent: pr-reviewer]
Scope since last review (1673ce4d): 1 merge from master + 1 fix commit.
bf176026 — fix runTask import (src/worker/expo/index.ts)
Fixes a real bug I flagged in my first review (concern #8). runTasks is an async function (value export), not a type — placing it inside export type { ... } caused TypeScript to erase it, making it undefined at runtime for any consumer importing from the barrel.
The fix:
- Removes
runTasksfrom theexport type { ... }block - Adds
export { runTasks, createTaskDependencies }as a value re-export createTaskDependencieswas also missing from the barrel — good catch including it
Verified: both runTasks (line 54) and createTaskDependencies (line 125) in taskRunner.ts are runtime function exports. The type-only re-exports (TaskProcessor, TaskDependencies, CreateTaskDependenciesOptions) remain correctly in the export type block.
No protocol-critical code touched. Approving.
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com
Vendors a npm-packed tarball of @arkade-os/sdk built from the unmerged arkade-script-final branch (PR arkade-os/ts-sdk#319 head 5a24cbf). This gives us the arkade.{ArkadeVtxoScript,ArkadeScript,...}, RestEmulatorProvider, EmulatorPacket, computeArkadeScriptPublicKey, and PrevArkTxField primitives required for the R1 forfeit covenant — and any future arkade-script work. - vendor/arkade-os-sdk-0.4.28-arkade-script-5a24cbf.tgz: the packed dist. - .gitignore: negate *.tgz inside /vendor so we can track the tarball. - All four package.json files: point @arkade-os/sdk at the file: path. - *-lock.json + yarn.lock: refreshed. Rebuilds clean (lib + server). Existing unit tests pass against the new SDK — arkade-forfeit.unit.test.ts (8 tests) + odds-math.unit.test.ts (4) both green. The 0.4.27/0.4.30 line worked too because the SDK's public API surface is a superset; pinning to 0.4.28-arkade-script lets us import { arkade } and reach ArkadeVtxoScript / RestEmulatorProvider. Drop the pin back to a tagged release once arkade-os/ts-sdk#319 merges.
|
Cleaned-up replay in #532 — same arkade/introspector content, replayed onto current master and sequenced by topic (committed by @louisinger via What's different from this PR:
Leaving this PR open for reference and so you can pick it up if there's anything in #532 that doesn't capture your intent. Happy to close once you've had a look. |
| // single empty push for output_index = 0 | ||
| witness: new Uint8Array([0x01, 0x00]), |
There was a problem hiding this comment.
can you explain? Seems to be good opportunity to abstract it in a way where we don't make developer aware of the packet and tweaking?
| const vtxoScript = new arkade.ArkadeVtxoScript([ | ||
| { | ||
| arkadeScript, | ||
| emulators: [emulatorPubkey], |
7 participants
Adds full client-side support for https://github.com/ArkLabsHQ/introspector and ArkadeScript
What's included
Tests
@Kukks @tiero please review
Summary by CodeRabbit
New Features
Repositories / Storage
Tests