fix(main): x402/MPP server-SDK safety (php/lua/go/ruby) + operability-caveats docs#144
Closed
EfeDurmaz16 wants to merge 27 commits into
Closed
Conversation
EfeDurmaz16
force-pushed
the
docs/pr142-operability-caveats-skill
branch
from
c0954fc to
639537e
Compare
EfeDurmaz16
changed the title
EfeDurmaz16
force-pushed
the
docs/pr142-operability-caveats-skill
branch
from
b6e720b to
a704ea3
Compare
…tokenProgram
The Rust spine x402 exact verifier gates the transferChecked program-id
against the fixed {Token, Token-2022} set by the actual instruction
program and never derives the gate from extra.tokenProgram
(rust/crates/x402/src/protocol/schemes/exact/verify.rs:371-375). Ruby
required extra.tokenProgram and built its allowlist from it, rejecting a
credential whose offer omitted the field even when the on-chain transfer
used the canonical Token program. Switch to the canonical set; the
destination ATA is still re-derived from the actual instruction program.
The Rust spine aliases testnet stablecoin mints to their devnet addresses (rust/crates/mpp/src/protocol/solana.rs:19-26, :45-60: USDC_TESTNET == USDC_DEVNET, USDG_TESTNET == USDG_DEVNET, PYUSD_TESTNET == PYUSD_DEVNET). Ruby's mint table had only devnet and mainnet keys, so resolve(currency, "testnet") fell back to the mainnet mint. A testnet-configured server then verified SPL transferChecked against the mainnet mint while a rust client built against the devnet mint, so matching failed. Add explicit testnet entries that alias the devnet mints.
The Rust spine stores MPP charge amounts as base-unit u64 and surfaces an explicit invalid-amount error on overflow (rust/crates/mpp/src/protocol/intents/charge.rs:53-58, parse_amount -> u64). Ruby parsed amounts as arbitrary-precision Integers with no upper bound, so a value above u64::MAX passed through construction and split arithmetic and was only rejected later as a downstream no-matching transfer. Cap amount_i and the split-amount parser at u64::MAX so overflow surfaces as the spine's invalid-amount error. The construction-time acceptance of "0"/leading-zero amounts is left unchanged (see report).
Match the canonical Rust x402 verifier on two points: - Raise the compute-unit-price cap from 50000 to 5000000 micro-lamports to match MAX_COMPUTE_UNIT_PRICE_MICROLAMPORTS in rust/crates/x402/src/protocol/schemes/exact/verify.rs. - Derive the transfer program-id gate from the actual instruction program (Token or Token-2022) instead of requiring a seller-pinned extra.tokenProgram. An offer that omits extra.tokenProgram now verifies a real Token-program transfer, matching verify_transfer_instruction. Regression tests assert the canonical cap and that an offer without extra.tokenProgram still verifies.
The Rust mpp charge server runs verify_pinned_fields on every credential, comparing the decoded request currency and recipient against the values fixed at server construction (rust/crates/mpp/src/server/charge.rs:457-468). The PHP ChargeServer only compared fields when the caller passed an expectedRequest. Add optional pinnedCurrency and pinnedRecipient constructor params and compare them unconditionally inside verifyAuthorizationHeader, so a credential that drifts from the pinned configuration is rejected even without an expectedRequest. Regression tests cover reject-on-drift and accept-on-match.
Two mpp charge verifier divergences from the Rust spine: - Default the expected ATA-creation payer to the transaction fee payer when no route fee payer is configured (client-pays-fees mode), mirroring expected_ata_payer = fee_payer.unwrap_or(tx_fee_payer) in rust/crates/mpp/src/server/charge.rs. The PHP path passed a null expected payer in client-pays mode and skipped the ATA-payer binding entirely. - Skip the compute-budget unit-limit/price caps on the push (on-chain) verification path. The Rust parsed-instruction allowlist does a bare continue on compute-budget instructions for settled transactions (charge.rs:1873-1876); only the pull-mode pre-broadcast path enforces the caps. The push path threads an onChain flag through verification. Regression tests assert the client-pays ATA-payer binding (reject on stranger payer, accept on fee-payer) and the pull-rejects / push-accepts split for above-cap compute budget.
The Rust spine caps SetComputeUnitPrice at 5_000_000 micro-lamports (rust/crates/x402/src/protocol/schemes/exact/verify.rs:17). The Lua verifier used 50_000, rejecting canonical wallet transactions whose compute-unit price sits above 50k but under the protocol cap.
Match the Rust spine (verify.rs:373): the transferChecked program id is accepted iff it is TOKEN_PROGRAM or TOKEN_2022_PROGRAM, derived from the actual instruction. The Lua verifier required extra.tokenProgram and bound the program id to it, rejecting spec-valid canonical offers that omit extra.tokenProgram.
The Lua verifier reconstructed u64 values via float multiplication, losing precision above 2^53. The Rust spine uses u64::from_le_bytes (verify.rs:405-409, :350-354), so a high-range amount or compute-unit price could collide with a different value through rounding. Decode byte-wise into an arbitrary-precision decimal string and compare via the uint helper so the full u64 range is exact.
Asserts the rust-matching behaviour for the three x402 verifier fixes: compute-price cap at 5M, transfer program bound to the canonical token set without extra.tokenProgram, and exact u64 amount decoding above 2^53. Existing positive/negative specs updated for the exact-decimal transfer descriptor amount.
Port the canonical-wire precedence from rust (protocol/schemes/exact/types.rs Deserialize, client/exact/payment.rs build_payment) onto the Go AcceptsEntry parse path: - amount falls back to maxAmountRequired, payTo to recipient, asset to currency (top-level canonical fields win over extra mirrors) - decimals defaults to 6 when absent (requirements.decimals.unwrap_or(6)) - tokenProgram defaults to the per-currency default when omitted - feePayer is a boolean toggle plus key, so an explicit false opts out - native SOL is detected when the currency resolves to no mint, so a literal SOL asset routes to a system transfer - currency matching resolves both offer and preference sides - preferred network defaults to mainnet and cluster slugs normalize to CAIP-2 before comparison - seller-pinned extra.memo over 256 bytes is rejected at build time - maxTimeoutSeconds defaults to 300 and the parsed offer retains its verbatim bytes for accepted echo
Match the rust x402 server spine on two server-side surfaces: - verify.go now returns the specific invalid_exact_svm_payload_* reason codes from verify.rs (instructions_length, compute_limit/price, no_transfer_instruction, fee_payer_transferring_funds, mint/recipient/ amount mismatch, positional unknown optional instruction, memo count/mismatch) and VerifyAndSettle surfaces them verbatim instead of collapsing every structural failure to charge_request_mismatch - VerifyAndSettle enforces the credential's echoed accepted offer against the route requirements before any transaction processing, mirroring verify_envelope_payload: targeted network/amount/recipient/currency checks plus a canonical structural backstop, rejecting a credential that lies about what it is paying for
Match the Rust spine (charge.rs:1623-1624): a transferChecked whose decimals byte disagrees with the route's expected decimals is not a valid match. The Lua verifier ignored the decimals byte, so a transfer of the right amount with the wrong decimals could satisfy the charge.
Match the Rust spine (charge.rs:1649-1657): when a route configures a fee payer, a transferChecked that sources funds from the fee-payer's own associated token account must be rejected, even when the transfer authority is a different key. The Lua verifier only guarded the authority slot, leaving the source-ATA drain shape open.
Match the Rust spine extract_parsed_instructions (charge.rs:2218-2230): concatenate meta.innerInstructions[*].instructions with the top-level message instructions before running the transfer matchers and allowlist. A settled transaction whose payment was emitted through a CPI previously failed to match because only top-level instructions were inspected.
Asserts the rust-matching behaviour for the three MPP verifier fixes: transferChecked decimals pinning, fee-payer source-ATA drain rejection, and inner-CPI instruction matching on the confirmed path.
Port the rust mpp charge verifier guards onto the Go server path (rust/crates/mpp/src/server/charge.rs): - SPL transferChecked rejects the configured fee payer as the transfer authority and rejects the fee payer's token account as the source ATA (verify_spl_transfer_instructions, both hard rejects) - SPL transferChecked decimals byte must match the challenge-pinned decimals - SOL transfer rejects the configured fee payer as the funding source (verify_sol_transfer_instructions) - ataCreationRequired splits require a raw SPL mint-address currency, enforced both at challenge issuance (validate_charge_options) and in the verify path when the currency is a symbol - push mode verifies on-chain before consuming the replay marker and never deletes it on failure (verify_push -> consume_signature) - v0 transactions carrying address lookup tables are rejected with a structured error (reject_address_lookup_tables)
…ssignment golangci-lint flagged the parse-alias raw field as unused (the public AcceptsEntry.raw is the one wired for the verbatim accepted echo); remove the dead alias field. luacheck flagged a remainder value overwritten before use in the x402 rust-parity spec; declare it at the assignment site.
Collaborator
Author
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
Originally the operability-caveats skill reference. Now also folds the validated main-branch non-Rust/TS server-SDK safety fixes into a single PR (per request), all aligned to the official x402
exactSVM spec (coinbase/x402scheme_exact_svm) and the rust reference contract.Docs
skills/pay-sdk-implementation/references/operability-caveats.md+ wire into SKILL.md.Go
validate_instruction_allowlist.SendTransactionsucceeds, so a confirmation/verify timeout never deletes the consumed marker (the broadcast tx can still land) — closes a double-pay window.extra.memo, else a random 16-byte hex nonce) per spec; verifier allows only the Lighthouse + Memo optional slots.Lua
methodDetails(stripping onlyrecentBlockhash) andexternalIdagainst the route-expected request, and settle from the expected — closes replay-to-a-different-route / fee-split misrouting (mirrors PHP/Ruby).mpp.expires_ininto challenge issuance (challenges had no TTL).Cache-Control: no-store; default replay store warns dev-only.PHP
getSignatureStatusesuntil confirmed/finalized, throw on on-chain failure or timeout before returning settlement success (mirrors the MPP charge path).MppConfig.expiresIninto challenge issuance; 402 responses setCache-Control: no-store.Ruby
Cache-Control: no-store; the default in-memory replay store warns dev-only.Verification
go build/vet/test ./...green (local). php: phpunit 329 + phpstan + cs-fixer clean. lua: 543 specs pass. ruby: 393 pass. Each fix carries a fail-before/pass-after regression test.Reviewing this PR
This PR ports the same set of safety fixes across four SDKs, each aligned to the rust spine (
rust/crates/mpp,rust/crates/x402). Reviewing one language's logic transfers to the others; the notes below point at the file that matters per SDK and the shared invariant it enforces.Read in this order
rust/crates/mpp/src/server/charge.rs(validate_instruction_allowlist,validate_create_ata_idempotent_instruction) andrust/crates/x402/src/protocol/schemes/exact/. Everything else mirrors these.Per-SDK reading guide
go/protocols/mpp/server/server.go: strict post-match instruction allowlist (validateInstructionAllowlist,validateCreateATAIdempotentInstruction,feePayerKey) and the replay-marker pin moved to immediately afterSendTransaction.go/protocols/x402/client/client.go: always-append Memo (sellerextra.memoor random hex nonce;nonceSourceis swappable for golden tests).go/protocols/x402/verify.go+x402.go: trailing-slot allowlist (Lighthouse + Memo) andexpectedMemobinding.php/src/Protocols/X402/Adapter.php: reserve signature between broadcast and confirmation, thenawaitConfirmationbefore returning success.php/src/Protocols/X402/Exact/Verifier.php: corrected Lighthouse program id, ATA-create dropped from optional allowlist.php/src/Protocols/Mpp/MppConfig.php(resolveExpiresIn) +Mpp/Adapter.php(challengeExpires): TTL wiring and the malformed-expires_inguard.php/src/Middleware/RequirePayment.php:Cache-Control: no-storeon 402.lua/pay_kit/protocols/mpp/server/init.lua: fullmethodDetails+externalIdroute binding (comparable_method_details, canonical compare), settle-from-expected.lua/pay_kit/protocols/mpp/init.lua: TTL wiring into issuance, default volatile-replay-store warning.lua/pay_kit/internal/config.lua: localnet RPC default, MPP secret resolution,expires_in = falseopt-out.lua/pay_kit/preflight.lua(new): boot checks, secret resolution,.envpersistence.ruby/lib/mpp.rb: dev-only replay-store sentinel + warning.ruby/lib/mpp/server/decorator.rb,ruby/lib/pay_kit/rack/payment_required.rb:Cache-Control: no-storeon 402.Risk areas
SendTransaction, so a confirmation/verify timeout no longer deletes the consumed marker. Confirm the marker is never reopened on the timeout path.awaitConfirmationpolling loop (40 attempts x 250ms). Verify timeout and on-chainerrboth throw before any success header is returned.verify_credential_with_expectednow binds the fullmethodDetails. The minimal{amount, currency, recipient}form intentionally skips method-detail binding; confirm callers that need shape pinning passmethodDetails.ruby/lib/x402/protocol/schemes/exact/verify.rb) is out of scope for this PR and still permits ATA-create; that divergence is intentional here and tracked as a follow-up.Test entry points
cd go && go test ./...; coverage gatego test -coverprofile=build/coverage.out ./... && go tool cover -func=build/coverage.out | tail -1(>= 91.0%). Key suites:protocols/mpp/server/server_allowlist_test.go,server_replay_durability_test.go,protocols/x402/{client,verify}_test.go.cd php && composer test && composer run lint. Key suites:tests/Protocols/X402/{ConfirmationTest,Exact/AtaCreateRejectTest}.php,tests/MppConfigTest.php,tests/Protocols/Mpp/AdapterTest.php.cd lua && eval "$(luarocks --lua-version=5.1 --tree lua_modules path)" && luajit tests/run.lua. Key specs:tests/cross_route_replay_spec.lua,tests/pay_kit/{main_fixes,preflight,x402_verify_negative}_spec.lua.cd ruby && bundle exec standardrb && bundle exec ruby -Itest test/run.rb. Key suites:test/mpp/dev_store_warning_test.rb,test/pay_kit/middleware_test.rb.