test(api_remote): value-bearing field coverage + lockstep error-string check - #122
Merged
TaprootFreak merged 8 commits intoMay 27, 2026
Merged
Conversation
…lue-bearing field assertions Adds five new tests in Section 4 that assert every wallet-app-facing response field by content, not just by presence. Mirrors the existing strong-assertion block in `send_commit_roundtrip_moves_balance` so a server bug returning a placeholder zero-hash or a truncated string fails CI at the API layer instead of in the wallet's integration loop. - mint_response_carries_state_hash_and_coins_root - commit_response_carries_state_hash_and_coins_root - balance_response_carries_username_after_claim - claim_response_carries_address - balance_response_has_no_username_for_unclaimed_wallet The mint and commit tests are written against the expected contract (hash fields populated as 32-byte non-zero hex); the current server sets them to `None`, so the two tests surface that lockstep gap until the server is updated to emit the fields.
Every 4xx the wallet app consumes MUST deserialise as
`{ success: false, error: <non-empty string> }` so the client can
branch on the failure reason without re-reading the body. This commit:
- extends `balance_invalid_hex_returns_422` and
`balance_wrong_length_returns_422` with body content assertions
(the balance handler uses a different envelope from
`handler_error_response`, so the assertion documents that today's
body is the bare `BalanceResponse { balance: 0 }` with no `error`
field — surfaces any future refactor that swaps shapes)
- adds `send_returns_structured_error_envelope` covering the
`handler_error_response` shape used by every `/api/send` 4xx path
…ssages.ts mapping
Adds a lockstep test against `app/src/lib/api/errorMessages.ts ::
KNOWN_SERVER_ERRORS` so a server-side error rename surfaces in CI
instead of degrading wallet UX to `Serverfehler <status>: <raw>`.
- new constant `APP_KNOWN_ERROR_STRINGS` mirrors the app's 19-entry
list (13 from `map_send_coins_error`, 6 from
`handler_error_response` call sites)
- new test `error_strings_match_known_app_mapping` provokes each
reachable string through a single amortised mint:
reachable: Unknown account address, Signature verification failed,
Request timestamp too old or in the future,
prev_commitment_pubkey required for account update,
Insufficient funds
mismatch (server emits more-specific text):
Invalid hex, Invalid address length
operator-only / internal-state-only (documented, not provoked):
In-coin not present in source's output_coins_root,
Source commitment not present in history MMR,
Coin is missing commitment,
Should provide an inclusion proof,
Coin should not exist in coin history tree,
Coin should not exist in tree yet,
Too many in-coins / out-coins for one transition,
prove failed, internal error,
Missing signature, Broadcast failed
- extends `mint_invalid_hex_address_returns_422`,
`mint_wrong_address_length_returns_422`,
`send_bad_address_hex_returns_422`,
`send_unknown_account_returns_404`,
`send_bad_signature_returns_401`,
`send_stale_timestamp_returns_401` with body content assertions so
each negative-path test is also a per-string contract anchor
TaprootFreak
marked this pull request as ready for review
May 26, 2026 22:36
TaprootFreak
added
the
ci:full
Resolves the four red tests from the field-coverage suite by
fixing the node side of each divergence (the app stays as-is for
these; the app-side family-matching is a separate PR).
N1 mint response: populate account_state_hash + output_coins_root
(hex-encoded 32-byte digests) so wallet clients have everything
needed to derive prev_commitment_pubkey for the next send
without a second GET /api/proof/:id round-trip.
N2 commit response: same pair populated in broadcast_commit_and_deliver,
so commit-side flows can also pin the resulting state directly.
N3 timestamp window: explicit check_timestamp_window helper runs
BEFORE verify_send_signature in send/commit/claim handlers,
emitting "Request timestamp too old or in the future" as its
own 401 instead of collapsing into "Signature verification
failed". Clock-skew misconfiguration now surfaces distinctly.
N4 missing signature: signed handlers now reject absent
signature/timestamp fields with 401 "Missing signature" /
"Missing timestamp" upstream of crypto verification. Defence-
in-depth Option-arms stay in verify_send_signature.
Unit tests in router_tests.rs updated to the new response shape
and to the dedicated timestamp string.
Closed
6 tasks
… is wired Replaces the inline "unreachable" comment with a live provocation: POST /api/send with signature deliberately omitted now returns 401 with "Missing signature". Mirrors the handler-level gate added in the same PR. Inventory comment updated to match.
send_bad_address_hex_returns_422: sign the request body so it passes the new "Missing signature"/timestamp gates that fire upstream of the per-field hex validator. The hex parser still rejects "0xZZZZZZ" with 422; the test now exercises the hex branch as intended. error_strings_match_known_app_mapping: the "prev_commitment_pubkey required" branch is the AccountUpdate transition, which is unreachable from a wallet that only received a mint (account.proof is still None → AccountCreation path). Move the string to the documented-only list with router_tests + account_node_tests references; the unit-level coverage is sufficient and saves a publisher-UTXO per CI run.
Adds a handler-level unit test asserting that POST /api/send with a stale (year-1970) timestamp returns 401 with "Request timestamp too old or in the future", covering router.rs:675-676 which the existing helper-level `check_timestamp_window_*` tests and the live `send_stale_timestamp_returns_401` api_remote test exercise but the coverage-gate nextest pass did not reach.
…overage
Resolve runtime.rs conflict: keep release-branch's
upsert_account_with_source("receive") and short-form digest_to_bytes
(matches the existing use statement at line 28).
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Tighten
node/tests/api_remote.rswith value-bearing field assertions on every endpoint response and a lockstep check against every server-error string the wallet app currently expects.The goal is not to make the node conform to the app — both are subject to redesign. The goal is to surface every divergence between what the node returns today and what the app reads today, so each gap can be resolved on its merits (sometimes server, sometimes app, sometimes both).
What's added
Field-coverage tests (5 new):
mint_response_carries_state_hash_and_coins_rootcommit_response_carries_state_hash_and_coins_rootbalance_response_carries_username_after_claimbalance_response_has_no_username_for_unclaimed_walletclaim_response_carries_addressError-contract tests (2 new + extended 8 existing):
APP_KNOWN_ERROR_STRINGSconstant — 19 strings, identical toapp/src/lib/api/errorMessages.ts::KNOWN_SERVER_ERRORS, anchored by a length-check so adding/removing an app string is a hard signalsend_returns_structured_error_envelope—{success, error}shape on 4xxerror_strings_match_known_app_mapping— provokes every reachable string, asserts exact matchDivergences surfaced
Each row below is an open design question, not a defect attribution. The test stays red until the divergence is resolved on the more sensible side.
account_state_hash+output_coins_rootshipped asNoneprev_commitment_pubkeyfor the next sendNonepair"Invalid hex"; node emits"account_address is not valid hex""Invalid address length"; node emits"account_address must be 32 bytes (64 hex chars)""Broadcast failed"; node emits"Failed to broadcast commitment inscription on-chain""Missing signature"; node skips signature verification entirely when signature field is absentCoverage matrix
/api/info/api/balance/api/mint/api/send/api/commit/api/username/claim/api/username/resolve/:uTest plan
cargo fmt --all -- --checkcleancargo clippy --workspace --all-features --no-deps -- -D warningscleancargo check --workspace --all-features --testscleanapi-e2ejob runs the suite againstdev-api.zkcoins.app(triggered viaworkflow_dispatchfrom this branch) — expected red on Release: develop -> main #1–Add CLAUDE.md with project instructions #6