[gringokiwi]

Refs #119. Follow-up to #113 and #125.

Problem

Since #125, every application-layer failure maps to code 13 INTERNAL with the fixed string internal error. Callers cannot tell a deterministic rejection of their own transaction from a server fault. The detail only exists in the server log.

Two reported instances:

• A covenant clause evaluating false returns {"code":13,"message":"internal error","details":[]} (Make errors more specific for covenant clauses #119).
• arkd refusing the forwarded tx with InvalidArgument: ASSET_INPUT_INVALID (35): ... amount mismatch: 10000 != 10 reaches the client as the same internal error (Make errors more specific for covenant clauses #119 comment).

Fix

Classify the failure and relay what the caller can act on:

1. Emulator verdicts (*emulator.Error) are deterministic checks on the caller's own transaction. They now map to FailedPrecondition with the full message.
2. arkd refusals with a client-fault status (InvalidArgument, FailedPrecondition, NotFound, AlreadyExists, OutOfRange) are relayed with arkd's code and message through the new application.ArkdError, on both the submit and finalize paths. Other arkd codes keep the generic wrap.
3. Unexpected faults keep the Harden request validation, VM bounds, and signing checks #125 behavior: generic internal error, detail logged server-side. TestInternalErrorsAreNotLeaked still passes unchanged.

The classification covers all four signing endpoints: SubmitTx, SubmitIntent, SubmitFinalization, SubmitOnchainTx.

Error text fixes

• The double failed to execute arkade script: failed to execute arkade script: wrap is gone. Outer layers add the input index only.
• A script that runs to completion but leaves false on the stack now fails with an ExecutionError naming the last executed opcode.

Before:

{"code":13,"message":"internal error","details":[]}

After, covenant rejection:

code=FailedPrecondition
message=emulate offchain tx ark_txid=...: input 0: failed to execute arkade script:
script=1 opcode=OP_EQUAL position=8: final script validation failed:
false stack entry at end of script execution

After, arkd refusal:

code=InvalidArgument
message=arkd rejected the tx: ASSET_INPUT_INVALID (35): input 0 owns asset ...
but amount mismatch: 10000 != 10

Not covered

• Expected-vs-actual values in the message (expected 1000, got 500 from the issue example). The VM does not retain the compared values at the final stack check. The opcode context above names the failing clause instead.
• retryFinalize still spends its retry budget on a deterministic arkd refusal before the error surfaces.
• Plain fmt.Errorf validation failures in internal/application (checkpoint output checks, signature checks) still map to internal error. Classifying them needs one sentinel per check, left for a follow-up.

Tests

• New handler tests: classified rejections are relayed with code and message. Internal faults stay generic.
• New application tests: arkdRejection classification and the SubmitTx path with a status-error arkd.
• New engine test: false-eval scripts carry ExecutionError opcode context.
• Two pkg/emulator/testdata/intent.json vectors updated for the deduplicated wrap wording.
• go test passes in all modules except ./test, which needs a nigiri or docker regtest stack, unavailable locally.

Summary by CodeRabbit

• Bug Fixes
  • Transaction rejection responses now preserve relevant error codes and messages, making failures more informative.
  • Script execution errors now include the input and, when available, the script location and opcode that caused the failure.

[coderabbitai]

Caution

Review failed

The pull request is closed.

Walkthrough

The application layer now preserves selected Arkd rejection statuses for gRPC responses. Script execution errors also include opcode location details for final stack validation failures, and emulator error messages use an input-index prefix.

Changes

Arkd Rejection Propagation

Layer / File(s)	Summary
Classify Arkd rejection statuses internal/application/errors.go, internal/application/service.go, internal/application/errors_test.go	The application layer wraps selected Arkd status errors in ArkdError. SubmitTx returns recognized rejections directly and retains existing handling for unrecognized errors. Tests cover recognized and unrecognized statuses.
Map service errors to gRPC responses internal/interface/grpc/handlers/emulator_handler.go, internal/interface/grpc/handlers/emulator_handler_test.go	The handlers map emulator errors to FailedPrecondition, ArkdError values to their stored code and message, and other errors to Internal. Tests cover rejection responses from four endpoints.

Script Execution Error Context

Layer / File(s)	Summary
Attach opcode location to final validation errors pkg/arkade/engine.go, pkg/arkade/engine_test.go	Execute records the last successful opcode location and includes it in final stack validation errors when the location is valid. A regression test checks the reported script, position, and opcode.
Report script errors with input index pkg/emulator/service.go, pkg/emulator/testdata/intent.json, test/utils_test.go	Emulator execution errors use an input <index>: prefix. The fixture and utility test expect the updated error text.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: louisinger, kukks

Merge Risk: Minimal · up to 95536

This change returns actionable transaction rejections to clients with their real status codes instead of a generic internal error, and adds opcode and input-index context to script failures. No merge-blocking defect was found. One thing to confirm after merge: the production arkd client must keep the gRPC status on errors from SubmitTx and FinalizeTx. If it does not, clients will keep receiving the generic internal error for arkd rejections.

- [ ] Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

[gringokiwi]

Superseded by three smaller PRs, one per part of #119:

• Drop the duplicated "failed to execute arkade script" wrap #187 drops the duplicated error wrap (point 1)
• Name the failing opcode when the final stack check fails #188 names the failing opcode on a false stack result (point 2)
• Relay deterministic tx rejections instead of generic internal errors #189 relays emulator verdicts and arkd refusals with proper status codes (point 3)

#189 also drops the application-layer ArkdError this PR introduced: the handler now reads the gRPC status straight from the error chain, so no new types or changes outside the handler were needed.

[arkana-ai-bot]

Arkana code review — #186

Overall: clean, well-scoped change. No protocol-critical paths are touched (no VTXO handling, signing, forfeit construction, round lifecycle, or unilateral exit). Test coverage is solid. A handful of observations follow.

---

pkg/arkade/engine.go — opcode tracking in Execute()

Correct. opcodeArray is declared [256]opcode (opcode.go:333), so indexing with a byte is always in-bounds. lastOffset is captured from vm.tokenizer.ByteIndex() before Step(), so it correctly identifies the byte position of the opcode that was just executed, not the next one. The bounds guard before opcodeArray[op].name is explicit and correct.

TestExecuteFalseEvalCarriesOpcodeContext pins the invariant and the ScriptIndex: 1 / Position: 2 / Opcode: "OP_EQUAL" assertions are correct for the OP_1 OP_2 OP_EQUAL case in the Taproot execution model (script 0 = witness stack, script 1 = tapscript leaf).

---

internal/application/errors.go — ArkdError / arkdRejection

Correct. errors.As(err, &gs) traverses the full error chain, so a status wrapped with fmt.Errorf("retry exhausted: %w", statusErr) is handled — confirmed by TestArkdRejection/"wrapped status keeps the inner arkd message".

The relayableCodes set (InvalidArgument, FailedPrecondition, NotFound, AlreadyExists, OutOfRange) is a reasonable client-fault boundary. Unavailable, Internal, and Unknown correctly stay hidden.

Observation — trust boundary on arkd messages (errors.go:30-36): arkdRejection forwards arkd's raw st.Message() verbatim. The design assumption is that arkd's relayableCodes payloads never embed server-internal detail (paths, DB state, config). That holds for the production arkd versions in use, but it's worth a comment so future callers of this function understand the invariant they're preserving.

---

internal/application/service.go — arkdRejection hooks

First hook (line 245, s.arkd.SubmitTx): correct placement; fires before the error is wrapped in a generic fmt.Errorf.

Second hook (line 319, after retryFinalize): known limitation acknowledged by the author — retryFinalize calls s.arkd.FinalizeTx via retryWithBackoff, which retries on all errors. If arkd returns a deterministic FailedPrecondition on attempt 1, the full retry budget (N RPCs) is spent before the error surfaces and the arkdRejection check at line 319 fires. The caller eventually gets the right code, but the latency cost and the spurious finalization attempts on arkd are real. Worth a follow-up to short-circuit retries in retryWithBackoff on relayableCodes. Not a blocker for this PR since the author already called it out.

---

internal/interface/grpc/handlers/emulator_handler.go — toRPCError

Correct. The dispatch order (*emulator.Error → FailedPrecondition, *application.ArkdError → arkd's own code, default → Internal) is right. Classified rejections log at Warn, unexpected faults at Error — appropriate severity split.

One small thing: toRPCError returns status.Error(arkdErr.Code, arkdErr.Error()). arkdErr.Error() = "arkd rejected the tx: <msg>". The prefix travels to the caller, which is probably fine, but callers parsing the message string should know to strip the prefix. No other currently-deployed client does that (the ts-sdk just surfaces the full text as an Error), so not a problem in practice.

---

pkg/emulator/service.go — error message deduplication

Good deduplication. The old "failed to execute arkade script: %w vin=%d" double-wrap and the inconsistency between EmulateBatchRegistration ("at input %d") and the other two ("vin=%d" after the %w) are both fixed. The new "input %d: %w" is uniform and lets the wrapped ExecutionError carry its own opcode context.

---

test/utils_test.go

The executeArkadeScripts error format is updated consistently with the service.go changes. The ./test suite requires a live regtest stack (can't be verified locally per the PR description), but the two testdata/intent.json vector updates ("errorContains": "input 1: failed to execute arkade script") look consistent with the new prefix. Any integration-test ErrorContains checks in that suite that still use "failed to execute arkade script at input" will fail — worth a quick scan once the stack is available.

---

Cross-repo impact

Checked go-sdk, ts-sdk, rust-sdk, dotnet-sdk. None import pkg/emulator or internal/application directly. The ts-sdk RestEmulatorProvider (packages/sdk/src/providers/emulator.ts) uses the REST gateway and checks only response.ok; the richer error body it now receives on !response.ok is surfaced verbatim to callers. No breaking change, strictly better.

---

Summary

Protocol correctness	✅ No VTXO/signing/forfeit/round/exit changes
Security	✅ No new information leakage beyond caller's own tx/script
Cross-repo	✅ No consumer breakage
Test coverage	✅ Classification, relay, internal-error leak, opcode context all pinned
Known gaps (acknowledged)	retryFinalize wastes retry budget on deterministic rejections; plain fmt.Errorf validation paths still map to internal error

Good to merge once CI passes.