Skip to content

chore(x402): reject bad recovery ids, exact chain labels, drop commit_reservation - #118

Merged
M3gA-Mind merged 5 commits into
tinyhumansai:mainfrom
M3gA-Mind:chore/x402-recovery-labels-ledger
Oct 23, 2026
Merged

M3gA-Mind merged 5 commits into
tinyhumansai:mainfrom
M3gA-Mind:chore/x402-recovery-labels-ledger

Conversation

@M3gA-Mind

@M3gA-Mind M3gA-Mind commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Four small x402 clean-ups, each with its test:

  1. Recovery id (fix). eip712_signature turned the wallet seam's r ‖ s ‖ recovery_id into r ‖ s ‖ v by adding 27 with only an overflow check, so any id from 2 to 228 became a v that no verifier accepts, and the payment failed later at the server instead of here. A secp256k1 recovery id is 0 or 1; anything else is now refused as recovery id out of range.
  2. Chain label (fix). The paid-response label used prefix matches: eip155:84532 (Base Sepolia) started with eip155:8453 and was shown as "Base", and every eip155:1… chain (Optimism eip155:10, Polygon eip155:137, …) was shown as "Ethereum". It now matches the CAIP-2 constants exactly: Base, Base Sepolia, Ethereum, then "EVM" / "Solana" by namespace.
  3. Dead commit path (deletion). Reservation::commit and PaymentLedger::commit_reservation had no production callers; both payment paths use commit_durable / try_commit_reservation, which keep the hold when the append fails. The removed path released the hold even when the record could not be written. Its tests are ported to the durable path, and the docs that pointed at it now point at commit_durable.
  4. Ledger paging (test only). The payments stream is read back 512 records at a time; nothing tested a ledger with more than one window. The code is correct; this pins it.

Related issue

None.

API or behavior changes

Breaking change, needs a release note: Reservation::commit and PaymentLedger::commit_reservation are removed from the public tinywallet-x402 API. Callers should use Reservation::commit_durable / PaymentLedger::try_commit_reservation, which keep the hold if the record cannot be written. No caller in this repository or in openhuman uses the removed methods.

  • A signature whose recovery id is not 0 or 1 is refused before the payment is sent.
  • eip155:84532 is labelled "Base Sepolia"; other non-Base, non-mainnet EVM chains are labelled "EVM".

Validation

  • cargo fmt --all -- --check (touched files, rustfmt edition 2024)
  • N/A: cargo clippy --all-targets --all-features -- -D warnings: left to CI
  • N/A: cargo build --all-targets --all-features: left to CI
  • N/A: cargo test --all-features: only the targeted tests below ran locally; CI runs the suite

Tests

cargo test -p tinywallet-x402 --features tools --lib -- <names>

  • crypto::test::the_recovery_id_is_offset_and_checked now also checks ids 2, 3 and 27. tools::test::networks_are_labelled_for_people adds Base Sepolia, Optimism and Polygon.
    • Before the fix: FAILED. 0 passed; 2 failed.
    • After: both pass.
  • New ledger::paging_test::records_past_one_read_window_survive_a_restart (in its own file, since ledger/mod_tests.rs is already past the line cap) (513 records, reopen, all reload).
    • With the reader deliberately stopped after its first window: FAILED (left: 512, right: 513).
    • Restored: passes.
  • Ported tests committing_a_reservation_records_the_payment_and_frees_the_hold, a_global_reservation_committed_is_recorded_once and the two hold-interleaving tests (now durable only) pass.
  • After all changes: ok. 7 passed; 0 failed.

Documentation

Module and type docs that referenced the removed methods now reference commit_durable.

Checklist

  • The change is focused on one logical change. It is four small x402 clean-ups from one audit item, grouped as planned.
  • No new #[allow(...)], #[ignore], or relaxed lints
  • No secrets, tokens, or .env contents in the diff or the description

@tinysweeper

tinysweeper Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Tiny Sweeper review

Reviewed head: 1535b7316935bc706c7a18a7efc2817e6ce2ba52

Verdict: Ready for maintainer review · Priority: none
Coverage: Reviewed 12 of 12 files.
Pull request purpose (title): chore(x402): reject…

Diagnostics and supporting details

Full summary

Pull request purpose (title): chore(x402): reject bad recovery ids, exact chain labels, drop commit_reservation. The PR makes four x402 clean-ups across 12 files (+103/−74): `eip712_signature` refuses a secp256k1 recovery id outside 0/1 instead of only overflow-checking the `+27` offset; the paid-response chain label matches exact CAIP-2 constants before falling back to namespace labels; the dead `Reservation::commit`/`PaymentLedger::commit_reservation` path is removed in favor of `commit_durable`/`try_commit_reservation`, with tests and docs repointed and the README updated; and a paging test file is added.

Review snapshot

Change surface Files Review signal Count
Production 6 Active findings 0
Tests 5 Noted findings 0
Documentation 1 Resolved findings 0
Configuration 0 Pending checks/questions 0

Completeness: Complete
Test assessment: Test coverage is assessed from changed tests and lane evidence; execution is not claimed without trusted check data.

What changed

Twelve files change (+103/−74). The recovery-id range check lands in `crates/tinywallet-x402/src/crypto/evm_payment.rs`; `crates/tinywallet-x402/src/tools/request.rs` swaps prefix matching for exact CAIP-2 constants; `crates/tinywallet-x402/src/ledger/reservation.rs` and `crates/tinywallet-x402/src/ledger/store.rs` delete the lossy commit path; doc references in `crates/tinywallet-x402/src/ledger/mod.rs` and `crates/tinywallet-x402/src/protocol/client.rs` move to `commit_durable`; `crates/tinywallet-x402/README.md` no longer advertises the removed `commit_reservation`; tests are updated in `crates/tinywallet-x402/src/crypto/mod_tests.rs`, `crates/tinywallet-x402/src/ledger/mod_tests.rs`, `crates/tinywallet-x402/src/ledger/mod_holds_tests.rs`, `crates/tinywallet-x402/src/tools/mod_tests.rs`, and the new `crates/tinywallet-x402/src/ledger/mod_paging_tests.rs`.

Features

  • Modified — Recovery-id range validation in eip712_signature: A signature whose recovery id is not 0 or 1 is rejected as `recovery id out of range` before the payment is sent, rather than producing a v value no verifier accepts and failing later at the server. (crates/tinywallet-x402/src/crypto/evm_payment.rs pub(super) fn eip712_signature(signature: &[u8]) -> Result<[u8; 65], X402Error>)
  • Modified — Exact CAIP-2 chain labels in paid-response formatting: Base Sepolia (`eip155:84532`) is labelled "Base Sepolia" instead of "Base", and non-Base, non-mainnet EVM chains such as Optimism and Polygon are labelled "EVM" rather than "Ethereum". (crates/tinywallet-x402/src/tools/request.rs pub(super) async fn format_response_with_payment(, crates/tinywallet-x402/src/tools/request.rs use crate::ledger::{self, PaymentRecord, PaymentStatus};)
  • Removed — Removal of Reservation::commit and PaymentLedger::commit_reservation: Breaking public API removal: callers must use `Reservation::commit_durable`/`PaymentLedger::try_commit_reservation`. The removed path pushed the record into memory and released the hold even when the append failed; the durable path keeps the hold on write failure. Docs, README, and tests are repointed. (crates/tinywallet-x402/src/ledger/reservation.rs impl Reservation {, crates/tinywallet-x402/src/ledger/store.rs impl PaymentLedger {, crates/tinywallet-x402/README.md configuration of its own.)
  • Added — Durable-path commit tests and hold-interleaving test refactor: Reservation commit tests now exercise only the durable path, including the interleaving helper that no longer branches on durability. (crates/tinywallet-x402/src/ledger/mod_tests.rs fn committing_a_reservation_records_the_payment_and_frees_the_hold() {, crates/tinywallet-x402/src/ledger/mod_tests.rs fn a_global_reservation_committed_is_recorded_once() {, crates/tinywallet-x402/src/ledger/mod_holds_tests.rs fn reserve_during_commit(durable: bool, committed_amount: u64, reopen: bool) {, crates/tinywallet-x402/src/ledger/mod_holds_tests.rs fn a_reservation_that_fits_is_retried_when_a_commit_makes_the_snapshot_stale() {)
  • Added — Ledger paging test file: Adds `mod_paging_tests.rs` registered via a `#[path]` module in the ledger mod. (crates/tinywallet-x402/src/ledger/mod_paging_tests.rs, crates/tinywallet-x402/src/ledger/mod.rs mod authorization_test;)

Tests

  • updated test — `the_recovery_id_is_offset_and_checked` now asserts ids 2, 3, 27, and 250 are all rejected with `recovery id out of range`.: Tests lane reports the recovery-id branch is exercised by a test that fails on regression. (crates/tinywallet-x402/src/crypto/mod_tests.rs fn the_recovery_id_is_offset_and_checked() {)
  • updated test — `networks_are_labelled_for_people` adds Base Sepolia (`eip155:84532`), Optimism (`eip155:10`), and Polygon (`eip155:137`) cases alongside the existing Base, Ethereum, Arbitrum, and Solana cases.: Tests lane reports the new chain labels are pinned for the added cases. (crates/tinywallet-x402/src/tools/mod_tests.rs async fn networks_are_labelled_for_people() {)
  • new test — `records_past_one_read_window_survive_a_restart` writes 513 records, reopens the ledger, and asserts all 513 are returned with newest first.: Tests lane reports pagination is covered by a test that fails on regression. (crates/tinywallet-x402/src/ledger/mod_paging_tests.rs)
  • ported test — `committing_a_reservation_records_the_payment_and_frees_the_hold` and `a_global_reservation_committed_is_recorded_once` now call `try_commit_reservation`/`commit_durable` instead of the removed methods.: Tests lane reports the commit-path tests were correctly reduced to the surviving API; no execution claims are made here. (crates/tinywallet-x402/src/ledger/mod_tests.rs fn committing_a_reservation_records_the_payment_and_frees_the_hold() {, crates/tinywallet-x402/src/ledger/mod_tests.rs fn a_global_reservation_committed_is_recorded_once() {)
  • updated test — The hold-interleaving helper drops its `durable` parameter and the two callers iterate only over `reopen`, so the concurrency scenarios run on the durable path.: Tests lane reports the durable-commit path is covered; no execution claims are made here. (crates/tinywallet-x402/src/ledger/mod_holds_tests.rs fn reserve_during_commit(durable: bool, committed_amount: u64, reopen: bool) {, crates/tinywallet-x402/src/ledger/mod_holds_tests.rs fn a_reservation_that_fits_is_retried_when_a_commit_makes_the_snapshot_stale() {)

Findings

No active actionable findings.

Before merge

None.

How this fits together

flowchart LR
  n0["...ed_when_a_commit_makes_the_snapshot_stale<br/>changed"]:::changed
  n1["reserve_during_commit<br/>changed"]:::changed
  n2["...al_reservation_committed_is_recorded_once<br/>changed"]:::changed
  n3["...on_records_the_payment_and_frees_the_hold<br/>changed"]:::changed
  n4["PaymentLedger<br/>changed"]:::changed
  n5["...he_session_total_with_or_without_a_thread"]:::impacted
  n6["...read_and_the_record_read_cannot_be_missed"]:::impacted
  n7["memory"]:::impacted
  n8["lock"]:::impacted
  n9["reserve_at"]:::impacted
  n10["record_payment"]:::impacted
  n0 -->|calls| n7
  n0 -->|tests| n7
  n0 -->|calls| n9
  n0 -->|tests| n9
  n0 -->|calls| n10
  n0 -->|tests| n10
  n1 -->|calls| n7
  n1 -->|calls| n8
  n1 -->|calls| n9
  n2 -->|calls| n7
  n2 -->|tests| n7
  n3 -->|calls| n7
  n3 -->|tests| n7
  n5 -->|uses| n4
  n5 -->|calls| n8
  n5 -->|tests| n8
  n6 -->|calls| n7
  n6 -->|tests| n7
  n6 -->|calls| n9
  n6 -->|tests| n9
  n6 -->|calls| n10
  n6 -->|tests| n10
  n8 -->|uses| n4
  classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
  classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
  classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
  classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Loading
Agent review details

critique

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: Critique lane answered with 0 findings across 12 of 12 files reviewed.
  • Lane summary: Reviewed 12 files; 0 findings. _The code index is behind this pull request (indexed at `5818d044d32b`), so retrieved context may be out of date._

security

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: Security lane answered with 0 findings across 11 of 11 security-reviewed files; README was not security-reviewed as prose.
  • Lane summary: Reviewed 11 files; 0 findings. 1 file was not security-reviewed: crates/tinywallet-x402/README.md (prose or tabular data). _The code index is behind this pull request (indexed at `5818d044d32b`), so retrieved context may be out of date._

tests

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: Tests lane answered reporting the changed branches are covered by regression-sensitive tests and the behaviour looks sound.
  • Lane summary: The change tightens the EVM recovery-id check, renames the infallible `commit`/`commit_reservation` to a fallible-only `try_commit_reservation`/`commit_durable`, replaces the prefix-matching chain labels with exact CAIP-2 constants, and adds a paging test. The recovery-id branch, the durable-commit path, the new chain labels and the pagination all get tests that fail on regression. The behaviour looks sound and is safe to merge. _The code index is behind this pull request (indexed at `5818d044d32b`), so retrieved context may be out of date._

commits

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: Commits lane answered reporting no sensitive content in committed files.
  • Lane summary: No sensitive content in committed files. _The code index is behind this pull request (indexed at `5818d044d32b`), so retrieved context may be out of date._

description

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: Description lane answered reporting the four stated clean-ups are present and the breaking-change note is called out; the only noted discrepancy (the `#![allow(...)]` header in the paging test file versus the no-relaxed-lints checklist item) is scoped to test files where repo rules permit it and is not a defect.
  • Lane summary: The description matches the diff: the four stated clean-ups are all present (recovery-id range check, exact CAIP-2 chain labels using constants that do exist in wire/types.rs, removal of `Reservation::commit`/`PaymentLedger::commit_reservation` with tests ported to the durable path, and the new 513-record paging test), and the breaking-change note for the removed public methods is called out. Only minor discrepancies: the linked test path (`ledger::paging_test`) differs from the actual module name (`tests` from `#[path = "mod_paging_tests.rs"] mod paging_test;` matches, so fine), and the description does not mention the test's `#![allow(clippy::unwrap_used, ...)]` header while the checklist claims no relaxed lints — that allowance is scoped to a test file, where the repo rules permit unwrap/expect, so it is not a defect in behaviour. No substantive mismatches found. _The code index is behind this pull request (indexed at `5818d044d32b`), so retrieved context may be out of date._
Evidence and run details
  • Models: ladder/vectors-oai3, deepseek-v4.1-flash
  • Spend: $0.002557
  • Tokens: 1028098 input · 12962 output · 208384 cached · 629 embedding
  • Updated: 2026-10-23T10:53:44Z
Head State Pass summary
e137eb25d37e ready for maintainer review 0 active finding(s), 0 resolved finding(s) (at 2026-10-23T03:31:37Z)
7940830910d9 ready for maintainer review 0 active finding(s), 0 resolved finding(s) (at 2026-10-23T08:22:51Z)
5818d044d32b ready for maintainer review 0 active finding(s), 0 resolved finding(s) (at 2026-10-23T09:24:02Z)
1535b7316935 ready for maintainer review 0 active finding(s), 0 resolved finding(s) (at 2026-10-23T10:53:44Z)

tinysweeper 0.1.2

@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b86c755e-70af-4e10-9bde-34206e6f2291
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tinysweeper found nothing blocking. Approving.

Diagnostics

Total spend: $0.0023

@tinysweeper tinysweeper Bot added the priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect. label Oct 23, 2026
…_reservation

- eip712_signature refuses a recovery id other than 0 or 1 instead of
  adding 27 to any byte up to 228.
- The paid-response label matches CAIP-2 ids exactly: eip155:84532 is
  Base Sepolia, not Base, and eip155:10/137/... are EVM, not Ethereum.
- Remove Reservation::commit and PaymentLedger::commit_reservation. They
  had no production callers and released the hold after a failed append;
  every path uses commit_durable / try_commit_reservation.
- Pin ledger reload past one 512-record read window.
ledger/mod_tests.rs is past the 750-line cap; put the new paging test in
a sibling mod_paging_tests.rs instead of growing it.
@M3gA-Mind
M3gA-Mind force-pushed the chore/x402-recovery-labels-ledger branch from e137eb2 to 7940830 Compare October 23, 2026 08:21
@tinysweeper tinysweeper Bot added priority: p2 Soon. Real but survivable — a rough edge, a gap, a thing that will bite later. and removed priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect. labels Oct 23, 2026
@tinysweeper tinysweeper Bot added priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect. and removed priority: p2 Soon. Real but survivable — a rough edge, a gap, a thing that will bite later. labels Oct 23, 2026
@M3gA-Mind
M3gA-Mind merged commit 7d12dbb into tinyhumansai:main Oct 23, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant