fix(mpp): improve 402 handling, voucher races, and multi-challenge matching#381
Open
fix(mpp): improve 402 handling, voucher races, and multi-challenge matching#381
Conversation
grandizzy
force-pushed
the
fix/mpp-402-retry-key-authorization
branch
3 times, most recently
from
4cf5d0b to
2de6d32
Compare
grandizzy
changed the title
grandizzy
force-pushed
the
fix/mpp-402-retry-key-authorization
branch
from
2de6d32 to
d7522bc
Compare
…lready provisioned
Two fixes for the 'access key already exists' error on MPP charge payments:
1. pay_charge (session.rs): Strip key_authorization from the signing mode
when key_provisioned is true. Previously the charge path passed
self.signing_mode directly to TempoCharge::sign_with_options, which
always included key_authorization from keys.toml. The session path
(create_open_tx) already had this guard, but the charge path did not.
2. 402 retry (transport.rs): Only retry with key_authorization when the
error specifically indicates the key is not provisioned ('access key
does not exist'). Previously any 402 retry unconditionally called
mark_key_not_provisioned(), causing the second attempt to include
key_authorization even when the first failure was for a different
reason (e.g. wrong currency, insufficient balance).
Co-authored-by: Amp <amp@ampcode.com>
Amp-Thread-ID: https://ampcode.com/threads/T-019d4989-0b60-72ef-99a0-2d0d9f0bc62c
grandizzy
force-pushed
the
fix/mpp-402-retry-key-authorization
branch
from
d7522bc to
5e9ecd6
Compare
- Add is_key_provisioned() getter on SessionProvider - Consolidate mark_key_not_provisioned/mark_key_provisioned into set_key_provisioned(bool) on ResolveProvider trait - Add is_key_provisioned() to ResolveProvider trait - Mark key as provisioned after successful payment responses (204 voucher, 200 direct, and key-auth retry paths) - Expand 402 retry to also trigger on 'verification-failed' when key_provisioned is true — handles first-time key provisioning where the charge/session tx fails because key_auth was stripped but the key was not yet provisioned on-chain Amp-Thread-ID: https://ampcode.com/threads/T-019d4ca9-65f6-74bf-b1fe-ba79287a2b4b Co-authored-by: Amp <amp@ampcode.com> Amp-Thread-ID: https://ampcode.com/threads/T-019d4ca9-65f6-74bf-b1fe-ba79287a2b4b Co-authored-by: Amp <amp@ampcode.com>
grandizzy
force-pushed
the
fix/mpp-402-retry-key-authorization
branch
from
5e9ecd6 to
717968b
Compare
Amp-Thread-ID: https://ampcode.com/threads/T-019d4d92-8386-7309-94ce-3d878fe45ace Co-authored-by: Amp <amp@ampcode.com>
…aces Concurrent RPC requests (e.g. forge test with createSelectFork) could read the same cumulative_amount from the channel entry, producing duplicate vouchers that the server rejects with 'cumulativeAmount must be strictly greater than highest accepted voucher'. Fix by incrementing cumulative_amount in-place while holding the channels lock, so each concurrent request gets a unique value. Amp-Thread-ID: https://ampcode.com/threads/T-019d4d92-8386-7309-94ce-3d878fe45ace Co-authored-by: Amp <amp@ampcode.com>
Amp-Thread-ID: https://ampcode.com/threads/T-019d4d92-8386-7309-94ce-3d878fe45ace Co-authored-by: Amp <amp@ampcode.com>
Two fixes for the voucher cumulativeAmount race: 1. Share channel state globally: use a process-wide singleton keyed by origin URL so all SessionProvider instances for the same endpoint share the same in-memory channels map. This prevents multiple providers (e.g. forge script creating multiple transports) from reading stale cumulative_amount values from disk. 2. Retry stale vouchers: when the server rejects a voucher with 'cumulativeAmount must be strictly greater', re-pay with a fresh voucher instead of failing. This handles cross-process staleness (e.g. cast followed by forge script) where the disk state lags behind the server's accepted state. Amp-Thread-ID: https://ampcode.com/threads/T-019d4d92-8386-7309-94ce-3d878fe45ace Co-authored-by: Amp <amp@ampcode.com>
grandizzy
changed the title
Amp-Thread-ID: https://ampcode.com/threads/T-019d4d92-8386-7309-94ce-3d878fe45ace Co-authored-by: Amp <amp@ampcode.com>
…scope Amp-Thread-ID: https://ampcode.com/threads/T-019d4d92-8386-7309-94ce-3d878fe45ace Co-authored-by: Amp <amp@ampcode.com>
Amp-Thread-ID: https://ampcode.com/threads/T-019d4d92-8386-7309-94ce-3d878fe45ace Co-authored-by: Amp <amp@ampcode.com>
When a server offers multiple 402 challenges (e.g. different chains and currencies), iterate through them and pick the first one matching a key in keys.toml by both chain_id AND currency. Previously, the code only looked at the first challenge's chainId and committed to it, which could fail when the user only had keys for a different chain or currency. - Add DiscoverOptions with chain_id + currency filtering - Consolidate discover_mpp_config_for_chain/discover_mpp_config_with into a single discover_mpp_config(DiscoverOptions) - Use find_map to iterate challenges and probe key compatibility before caching the provider - Extract challenge parsing into extract_challenge_chain_and_currency() Amp-Thread-ID: https://ampcode.com/threads/T-019d51ba-2e7d-739a-a46c-21bc7827fd26 Co-authored-by: Amp <amp@ampcode.com>
grandizzy
changed the title
Amp-Thread-ID: https://ampcode.com/threads/T-019d51ba-2e7d-739a-a46c-21bc7827fd26 Co-authored-by: Amp <amp@ampcode.com>
Replace the split supports_challenge() + resolve_for_chain() with a single resolve_for(DiscoverOptions) that filters by both chain_id and currency when initializing the provider. This eliminates the split-brain issue where supports_challenge checked (chain_id, currency) but get_or_init only used chain_id, potentially caching the wrong key. Amp-Thread-ID: https://ampcode.com/threads/T-019d51ba-2e7d-739a-a46c-21bc7827fd26 Co-authored-by: Amp <amp@ampcode.com>
grandizzy
requested review from
0xrusowsky,
DaniPopes,
mattsse,
onbjerg and
zerosnacks
as code owners
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.
Fixes several MPP issues discovered when integrating with QuickNode's multi-challenge 402 responses.
Voucher race fixes
Multi-challenge matching
Key provisioning