Conversation
Add a new `x402` feature to the Cargo.toml that enables the serde and serde_json dependencies, and conditionally compile the `tinywallet::x402` module behind this feature gate. This prepares the crate to support x402 machine-payment wire types without pulling in serialization dependencies for users who do not need that functionality. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a comprehensive test module covering payment requirement selection, chain identification, serialization behavior, and round-trip parsing for the x402 types. The tests verify that Solana and EVM requirements are correctly matched by network prefix, that Solana is preferred when both chains are offered, that amounts are preserved as strings through JSON serialization to avoid precision loss, and that unknown extension fields are preserved rather than rejected. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Co-authored-by: Medulla <medulla@tinyhumans.ai>
|
Warning Review limit reached
Next review available in: 27 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
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. Comment |
There was a problem hiding this comment.
tinysweeper found nothing blocking. Approving.
$0.0065 · 53,904 in / 23,045 out · 41,462 cached (77%) · z-ai/glm-5.2
critique: $0.0013 · 16,787 in / 4,240 out · 13,751 cached (82%) · z-ai/glm-5.2
security: $0.0009 · 15,609 in / 2,181 out · 11,592 cached (74%) · z-ai/glm-5.2
tests: $0.0026 · 10,409 in / 10,218 out · 7,423 cached (71%) · z-ai/glm-5.2
description: $0.0017 · 11,099 in / 6,406 out · 8,696 cached (78%) · z-ai/glm-5.2
| @@ -106,6 +106,7 @@ asset = ["btc", "evm", "solana", "tron"] | |||
| client = ["net", "asset", "tx", "serde/derive"] | |||
| # Transaction building and signing (`tinywallet::tx`). Needs secp256k1 | |||
There was a problem hiding this comment.
Move x402 below tx so the tx comment stays attached
The new x402 feature is inserted between the comment block that describes tx and the tx line itself, so the tx comment is now visually attached to x402:
# Transaction building and signing (`tinywallet::tx`). Needs secp256k1
# recoverable signing (via `bitcoin`) and Keccak-256 (via `keccak`).
x402 = ["dep:serde", "dep:serde_json", "serde/derive"]
tx = ["btc", "evm", "keccak", "solana", "tron", "key", "dep:ed25519-dalek", "dep:bs58", "dep:sha2", "dep:hex"]
The tx feature loses its explanatory comment and x402 gains a misleading one. The x402 feature should be placed elsewhere (e.g., below the tx line or in its own commented section).
[RULE] Comment why each crate is needed ·
| client = ["net", "asset", "tx", "serde/derive"] | ||
| # Transaction building and signing (`tinywallet::tx`). Needs secp256k1 | ||
| # recoverable signing (via `bitcoin`) and Keccak-256 (via `keccak`). | ||
| x402 = ["dep:serde", "dep:serde_json", "serde/derive"] |
There was a problem hiding this comment.
Add an explanatory comment for the x402 feature
The repository rules require commenting why each crate is needed, but the new x402 feature has no comment explaining its purpose or why it pulls in serde, serde_json, and serde/derive:
x402 = ["dep:serde", "dep:serde_json", "serde/derive"]
A descriptive comment should be added, consistent with the other feature definitions in this file.
[RULE] Comment why each crate is needed ·
| } | ||
| } | ||
|
|
||
| #[cfg(test)] |
There was a problem hiding this comment.
Move x402 unit tests into src/x402/test.rs
The PR's own branch rules say "Put module-local unit tests in src/<feature>/test.rs" and "Start every mod.rs and test.rs with a concise module-level //! description." The tests are instead inline in src/x402/types.rs as #[cfg(test)] mod test, and that inline module opens with #![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] rather than a //! description. The tests themselves are sound — they pin the Solana-before-EVM preference, prefix-matching, camelCase wire shape, string-amount round-trips, untagged serialization, and unknown-extension preservation — but they are in the wrong file.
[RULE] Put module-local unit tests in src/<feature>/test.rs ·
What this change touches4 files, +562 -1 across 3 components. The code graph knows nothing about these files yet — normal for newly added files, and a cold index otherwise. flowchart LR
n0["src/x402<br/>2 files +557 -0<br/>1 finding"]:::flagged
n1["root<br/>1 file +2 -1<br/>2 findings"]:::flagged
n2["src<br/>1 file +3 -0"]:::changed
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
Green: changed. Grey: untouched, reached through an import or a call. Orange: has findings. Red: has a finding that blocks the merge.
Changed files
|
Moves the x402 machine-payment protocol's wire types out of OpenHuman into the crate. Types only — the payment flow (
ops.rs, the payment store, the agent tools) stays in OpenHuman for now; see the bottom of this description.229 tests, clippy clean on every gate, rustdoc clean in both
--all-featuresand--no-default-features.A real bug surfaced on the way
best_exact_requirementcarried a doc comment reading "prefer EVM (Base), fall back to Solana" while the code checked Solana first.I preserved the code's behaviour and corrected the comment. Changing which chain a payer spends from is a behavioural change, not a documentation fix — so it is called out here rather than quietly "corrected" in either direction, and pinned by
solana_is_preferred_when_both_are_offeredso it cannot drift again. If Solana-first is wrong, that is a separate, deliberate change.What the tests are actually protecting
Each covers something that breaks a payment silently rather than loudly:
u64::MAX. JSON numbers are IEEE 754 doubles in most parsers, so au64amount through a JavaScript facilitator can come back a different number — a payment for the wrong sum. This is why the protocol uses decimal strings and why the type does too.payToandmaxTimeoutSecondsare asserted directly.{"transaction": …}or{"signature": …, "authorization": …}with no enum discriminant wrapping it.tinydocs'deny_unknown_fields. That crate's spec is a schema shown to a model, where a typo should fail loudly; this is a protocol other implementations extend, where an unknown key must not fail the parse.exactscheme is selected. A server may offer schemes this crate cannot pay; picking one would produce a proof the facilitator rejects.Why replay protection is in the type
EvmAuthorizationcarriesvalid_after,valid_beforeandnoncebecause in both supported schemes the payer never broadcasts — the facilitator does. A payment proof is therefore a capability someone else exercises later, and without those fields it would be replayable indefinitely. The module docs say so at the type rather than leaving it to be inferred.Not in this PR
x402's ~2,000 lines of logic — building the Solana partially-signed transaction, the EIP-3009 signing path, the payment store, and the facilitator round trip. Those depend on OpenHuman's quote store and agent-tool surface, so they need the same seam treatment the chain layer got rather than a lift-and-shift.
The types are the half that is genuinely host-agnostic and the half every other piece is expressed in terms of, so landing them first gives the logic port something stable to build against.