update to emulator v0.0.7-rc.1

[louisinger]

No description provided.

[coderabbitai]

Important

Review available on request

• 🔍 Trigger review

Reviews should be triggered manually for repositories with fewer than 10 stars. Select Trigger review above or comment @coderabbitai review to review the latest changes. For a full review, comment @coderabbitai full review.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2f94403a-b301-49cd-93e8-d657993f86d3

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[arkana-ai-bot]

Arkana review — PR #16: update to emulator v0.0.7-rc.1

Scope: go.mod dependency bump (emulator pkg/arkade + pkg/client → 20260810202659, x/sync v0.21.0, x/text v0.39.0) + new pkg/swap/contract/taker.go + new test/docker-compose.yml. The go.sum pruning (removal of ~143 cloud.google.com indirect entries) is consistent with the emulator dropping a GCP transitive dep — no concerns there.

---

Findings

taker.go:87 — first-VTXO selection is unguarded against multiple matches

swapVtxo := vtxosResp.Vtxos[0]

If the indexer returns multiple spendable VTXOs at the swap pkscript (e.g., a maker sends a second deposit to the same address, or a reorg replays an output), only the first is processed and the rest are silently ignored. The indexer response order isn't guaranteed by the API contract visible here. At minimum, consider logging a warning when len(vtxosResp.Vtxos) > 1 so the condition is observable.

---

taker.go:216 — positional Closures[0] access without bounds check

fulfillClosure := swapVtxoScript.Closures[0] // first closure is the fulfill leaf

There is no guard against an empty Closures slice. If offer.VtxoScript() somehow returns a script with no closures (e.g., a future refactor or a malformed offer that passes the pkscript equality check at line 72), this panics. A check analogous to the one at line 270 (if len(forfeitClosures) == 0) would make the failure deterministic and debuggable. Same pattern applies here.

---

taker.go:274 — forfeitClosures[0] may not be the round-appropriate closure

forfeitClosure := forfeitClosures[0]

Only the first forfeit closure is used. If a VTXO has multiple forfeit paths (e.g., one for the current round and one for a previous round pending expiry), silently picking [0] may produce a witness that the server rejects. Is the ordering of ForfeitClosures() specified in the contract interface? If not, this is a latent interoperability risk.

---

taker.go:385 — checkpoint assumed to have exactly one input

parentTxid := cp.UnsignedTx.TxIn[0].PreviousOutPoint.Hash.String()

No length check before indexing TxIn[0]. If offchain.BuildTxs ever returns a checkpoint with zero inputs (protocol violation, but defensible to check), this panics. A len(cp.UnsignedTx.TxIn) == 0 guard before the loop body would cost nothing and avoid a hard crash that could be induced by a malformed indexer response upstream.

---

taker.go — implicit full-wallet VTXO consolidation

All non-recoverable taker VTXOs that have contracts are swept into a single fulfillment transaction (loop at line 253). There is no coin-selection: a taker with 50 VTXOs will include all 50 in every swap, bloating transaction size and fees. The TODO comment at line 102 notes batch fulfillment is deferred, but the consolidation side-effect is implicit. Worth a comment or a future tracking issue.

---

taker.go:354 — signed checkpoints from SubmitTx are discarded

finalArkTxB64, _, err := emulatorClient.SubmitTx(ctx, signedArkTxB64, checkpointB64s)

The returned finalized checkpoints are intentionally discarded (per the comment block at lines 349–353: the emulator is the finalizer and submits to arkd internally). This is correct per protocol design, but the _ is easy to misread as an oversight. A named discard variable (finalizedCheckpoints) or a short inline comment would make the intent explicit.

---

test/docker-compose.yml — private key material committed to repo

Lines 54, 91, 125 embed raw hex private keys (ARKD_WALLET_SIGNER_KEY, EMULATOR_SECRET_KEY, SOLVER_WALLET_SEED). These appear to be regtest-only throwaway keys for local dev (network: nigiri), so this is not a production secret exposure. Confirm these keys are not reused in any non-regtest environment. A comment in the file asserting "regtest-only / do not reuse in production" would reduce future confusion.

---

What looks correct

• Swap pkscript consistency is verified before proceeding (line 72) — the taker cannot be tricked into building a tx for a mismatched contract.
• Checkpoints are pre-signed with taker keys before being handed to the emulator (line 337), satisfying verifyNonArkdCheckpointSignatures.
• The emulator packet is correctly bound to Vin: 0 (the swap VTXO input).
• buildFulfillAssetPacket correctly pins the wanted-asset group at index 0 for OP_INSPECTOUTASSETLOOKUP lookup (lines 504–512), with deterministic ordering enforced explicitly.
• Map iteration non-determinism for non-wanted asset groups (line 515) is benign: only the group at index 0 matters for script validation; the rest are informational.
• go.sum cleanup and x/text / x/sync bumps are routine and carry no protocol risk.
• E2E coverage in test/e2e/swap_test.go covers the happy path for asset↔BTC and BTC↔asset swaps.

---

Gap: no unit tests for taker.go

FulfillOffer and buildFulfillAssetPacket have no unit tests. Failure paths (insufficient BTC, missing forfeit closures, emulator rejection, zero VTXOs, empty asset inputs) are exercised only if the full E2E stack is running. Given this is money-moving code, unit tests with a mock emulator client would significantly reduce regression risk.