feat: propagate intent to getFee, carry settlementMode through payId hydration, and support preferredChains in payId mode - #62
Conversation
intent already reaches createPayment but not getFee, so fee quotes can diverge from actual settlement (e.g. stellar_direct 0-fee path). Also fixes connectkit's intent-common dep from pinned 0.1.26 to workspace:* so local pay-common changes are actually picked up.
getFee and createPayment hit the same endpoint with the same body (dryrun query param is the only difference), so getFee now takes CreateNewPaymentParams directly instead of a separate hand-built GetFeeParams shape. Removes the need for a duplicated stellar_direct resolution helper - call sites route through buildCreatePaymentPayload same as the real payment does.
…Id hydration getFee and createPayment now build the exact same request body (getFee just adds ?dryrun=true), so a consumer-set intent on the RozoPayButton reaches getFee the same way it already reached createPayment - fee quotes no longer diverge from what createPayment actually charges. formatPaymentResponseToHydratedOrder also now copies the backend's settlementMode onto the hydrated order's metadata, so the checkout-mode payId path (runSetPayIdEffects -> getPayment -> order_loaded) carries it through instead of silently dropping it.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| const paymentData = buildPaymentRequestBody(params); | ||
|
|
||
| const result = await apiClient.post<FeeResponseData | FeeErrorData>( | ||
| "payment-api/payments", | ||
| paymentData, | ||
| { params: { dryrun: "true" } }, |
There was a problem hiding this comment.
P1 — behavioral change to the fee request body, with no test guarding the invariant.
Previously getFee sent a minimal, purpose-built body (source/destination with tokenSymbol and a conditional amount, no display/metadata). It now sends the full createPayment body via buildPaymentRequestBody — display, metadata, amount on both source and destination unconditionally, and it resolves tokens through getKnownToken. Two consequences worth confirming before merge:
- This still posts to
payment-api/payments?dryrun=true(unchanged endpoint), butcreatePaymentposts to/payment-api. So the dryrun endpoint must accept the createPayment-shaped body. The PR's manual test checkboxes (Stellar direct/non-direct, explicitintent) are still unchecked — please verify the backend returns the same quote for this new shape, otherwise every fee quote breaks. - The whole PR rests on "getFee and createPayment build identical bodies," and the PR's own suggested-tests list calls for a unit test asserting exactly that — but no test was added.
buildPaymentRequestBody/getFee/createPaymenthave zero direct coverage (onlycreatePaymentBridgeConfigis tested intest/bridge.test.ts). This is fee-quote logic (what the user is told they'll be charged) shipping untested. Please add the invariant test.
| export function getCachedFee( | ||
| params: CreateNewPaymentParams, | ||
| ): Promise<FeeResult> { | ||
| const key = JSON.stringify(params); |
There was a problem hiding this comment.
P1 — unhandled rejection poisons the cache (now reachable). getCachedFee stores a {status:"pending"} entry (below) whose promise is only cleaned up inside .then (fulfillment). If getFee rejects, the pending entry is never deleted and has no TTL, so every subsequent call with the same key returns the stale rejected promise. This matters more now: getFee → buildPaymentRequestBody (packages/pay-common/src/api/payment.ts) throws "Source or destination token not found" on an unknown token — a synchronous throw the old getFee never had. In PayWith*Token, setFeeLoading(false) is also skipped on that path, so the spinner can stick. Add a .catch/.finally that cache.delete(key) on rejection.
Also: key = JSON.stringify(params) now hashes the entire CreateNewPaymentParams. It's stable as long as every call site builds the object with identical key ordering (they do today), but it's more fragile than the old explicit-field key — a metadata/title field slipping in would silently fragment the cache.
| setApiConfig({ version: params.apiVersion }); | ||
| } | ||
|
|
||
| const paymentData = buildPaymentRequestBody(params); |
There was a problem hiding this comment.
P2 (note). The createPayment refactor extracts the body construction into buildPaymentRequestBody verbatim, which is good — behavior looks preserved (setApiConfig still runs first, then body build). Just flagging that createPayment posts to /payment-api while getFee posts to payment-api/payments?dryrun=true — so the PR summary's "getFee just adds ?dryrun=true" is slightly inaccurate (different path too). Not a bug since this matches the pre-existing fee endpoint, just worth correcting in the description.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e2d5cc69d3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ...(paymentState.payParams?.intent | ||
| ? { intent: paymentState.payParams.intent } | ||
| : {}), |
There was a problem hiding this comment.
When the payer selects the Stellar token that matches a Stellar destination and the integrator did not explicitly pass payParams.intent, this fee request omits the intent: "stellar_direct" that buildCreatePaymentPayload adds for the actual payment. The dry-run therefore still uses the bridge/fee route, so the user can see/use a nonzero fee even though checkout/createPayment will settle direct; for ExactOut, that quoted fee is fed back into payment creation and can reduce the destination amount. Build the fee params through the same helper or duplicate the auto-detection before calling getCachedFee.
Useful? React with 👍 / 👎.
| preferredChain: selectedDepositAddressOption.token.chainId, | ||
| preferredTokenAddress: selectedDepositAddressOption.token.token, | ||
| toUnits: amount.toString(), | ||
| ...(payParams?.intent ? { intent: payParams.intent } : {}), |
There was a problem hiding this comment.
For the Stellar deposit-address option with a Stellar USDC/EURC destination and no explicit payParams.intent, this hand-built quote request also omits the auto stellar_direct intent that the later hydrate/create-payment path derives via buildCreatePaymentPayload. That means the dry-run can return a bridge fee for a direct settlement; the returned fee is passed into payWithDepositAddress, so ExactOut can create a lower payout than intended and the default flow displays the wrong fee. Reuse the same payload builder for the quote so the intent matches the payment.
Useful? React with 👍 / 👎.
PayWithToken, PayWithSolanaToken, PayWithStellarToken, and WaitingDepositAddress each hand-built an identical CreateNewPaymentParams object for fee quotes. Centralize that construction in feeCache.ts so a future field on CreateNewPaymentParams (or a change to appId/intent resolution) only needs to be wired in one place.
| return { | ||
| appId: resolveOrderAppId(order, payParams?.appId) ?? "", | ||
| feeType: payParams?.feeType ?? FeeType.ExactIn, | ||
| toChain: destChainId, | ||
| toToken: destTokenAddress, | ||
| toAddress: destAddress || payParams?.toAddress || "", | ||
| preferredChain: sourceChainId, | ||
| preferredTokenAddress: sourceTokenAddress, | ||
| toUnits, | ||
| ...(payParams?.intent ? { intent: payParams.intent } : {}), | ||
| }; |
There was a problem hiding this comment.
P2 — behavior change: appId is now always sent, even when empty.
When resolveOrderAppId(...) returns undefined, this emits appId: "", and buildPaymentRequestBody then copies it into metadata.appId too. The removed fee.ts#getFee did the opposite — ...(appId ? { appId } : {}) omitted it entirely when falsy.
For a dry-run quote this is likely harmless, but if the backend treats appId: "" differently from an absent appId (auth/attribution), the fee quote could diverge from what createPayment sends via the real payment payload. Worth confirming the backend ignores empty appId, or gate it the same way (...(resolvedAppId ? { appId: resolvedAppId } : {})).
| export function getCachedFee( | ||
| params: CreateNewPaymentParams, | ||
| ): Promise<FeeResult> { | ||
| const key = JSON.stringify(params); |
There was a problem hiding this comment.
P1 — rejected getFee now permanently poisons this cache.
This function's getFee (line 45, unchanged) is getFee(params).then(...) with no rejection handler, and the pending entry (line 60) has no TTL. That was safe before because the old fee.ts#getFee never threw — it only returned { error }.
The new getFee calls buildPaymentRequestBody → createPaymentBridgeConfig / getKnownToken, which throw for unsupported tokens, invalid addresses, or a getKnownToken miss ("Source or destination token not found"). If getFee rejects, the cache keeps { status: "pending", promise: <rejected> } forever, and every later call with the same JSON.stringify(params) key returns that rejected promise (line 41) — the payer can never get a quote for that token again this session, retries included.
Fix: add a .catch on the getFee(params) chain (line 45) that cache.delete(key) and rethrows, mirroring the existing result.error cleanup at line 55.
| * source/destination/type/intent resolution), with a dryrun flag, so the | ||
| * quote always matches what createPayment will actually charge. | ||
| * | ||
| * @param params - Same shape as createPayment's params |
There was a problem hiding this comment.
P1 — missing test for the invariant the whole change rests on.
The design doc (docs/.../2026-08-03-intent-propagation-getfee-design.md, "Testing") explicitly calls for a pay-common test asserting getFee and createPayment build identical request bodies for the same CreateNewPaymentParams (minus the dryrun query param) — "the invariant the whole fix rests on." No such test was added, and the PR's own test-plan checkbox for it is unchecked.
Since getFee now silently posts a different body shape than the old fee.ts version (symbol-based → the full buildPaymentRequestBody payload), and both branches feed a live pricing endpoint, this is exactly the kind of change that needs a regression test. A single assertEqual(buildPaymentRequestBody(p), <expected>) (or a getFee↔createPayment body-equality check) in packages/pay-common/test/ would lock it in.
Review summary — PR #62 (intent → getFee propagation)Solid, well-scoped refactor. Collapsing No P0 findings — no secrets, no auth/RLS changes, no edge-function deploys, no mirror-table writes, no blacklisted receiver, and the actual settlement path is unchanged. FindingsP1
P2
Not approving/merging — verdict left to the automated step. |
…ffect PayParamsData (the state persisted between preview and payment creation) had no `intent` field, so a top-level intent flag set via RozoPayButton's `intent` prop — e.g. "stellarsponsor" — was silently dropped before createPayment ever ran, for any flow that reaches hydrate_order (EVM PayWithToken). metadata.intent (the unrelated display title) survived, masking the loss: a request built this way carries a title but no sponsorship signal. PayWithSolanaToken and PayWithStellarToken were unaffected — they call paymentState.createPayment (full PayParams) directly, not this path. Adds `intent` to PayParamsData so a future regression here is a compile error, not silent data loss, and extracts the hydrate_order payload builder (buildHydratePayParamsPayload) so it's independently testable.
akbarsaputrait
changed the title
Review — no P0, but 5 × P1. Recommend not merging as-is.Verified against head No P0. No path was found where P1-1 — the headline bug is not actually fixed:
|
paddedChains.includes(chain) compares by reference. A consumer app passing its own chain object (e.g. base imported from a different resolved copy of viem/wagmi — realistic in pnpm monorepos with loose peer ranges) has the same chain.id but fails the identity check, so REQUIRED_CHAINS pushes a duplicate entry for that id. wagmi's createConfig then holds two distinct objects for one chain id, which can surface as viem's "chain: undefined (id: 8453)" during RPC/wallet client construction — reported on intents.rozo.ai as 11 failed Base USDC transfers from one user (2026-08-09 session). Fix is correct regardless of whether that's the confirmed mechanism for this specific report: dedupe should always be by id, not by reference. See 20260810-intent-pay-chain-undefined-base.md step 2a.
…n viem peer range Diagnosed from a production report on intents.rozo.ai: one user hit "An unknown RPC error occurred. chain: undefined (id: 8453)" 11 times in a row on Base USDC transfers, all source-chain, independent of destination. See scratch doc 20260810-intent-pay-chain-undefined-base.md (step 2a/2b/2c). - defaultConfig.ts: add resolveChainObject(chainId), exported so call sites can resolve the canonical Chain object instead of depending on wagmi's config.chains registry lookup at call time. - usePaymentState.ts: pass `chain` explicitly on the plain writeContractAsync ERC20 transfer path, alongside chainId. (The batched EIP-5792 writeContractsAsync and native sendTransactionAsync paths don't accept an explicit chain — wagmi's own types Omit it there, so this only applies to the one call site where it's possible.) - connectkit peerDependencies: viem "2.x" -> ">=2.52.0 <3". hyperEvm is only exported starting viem 2.52 (verified against unpkg); anything below that silently resolves undefined into REQUIRED_CHAINS. - pay-common: move viem from a regular dependency to peerDependency (+ devDependency for its own build/test), so it can't land a second, differently-resolved viem copy in a consumer's tree alongside connectkit's peer-resolved one — a realistic duplicate-object vector for the chain-identity dedupe bug fixed separately in defaultConfig.ts (paddedChains dedup by chain.id, already committed). - examples/nextjs-app: bump viem range to match the new connectkit floor. Also corrects two doc comments that claimed preferredTokens only affects sort order — useWalletPaymentOptions.ts's matchesPreferredTokens actually hard-filters wallet payment options to the preferred set. No behavior change; docs now describe what the code does. Verified: tsc --noEmit clean and full test suite green in both pay-common (35/35) and connectkit (22/22). pnpm install --frozen-lockfile still resolves cleanly (lockfile already pins viem@2.55.8 everywhere).
Asserts getFee and createPayment build identical request bodies for the same CreateNewPaymentParams input (same-chain, cross-chain with intent). The dryrun query param is the only difference between the two calls.
…gacy export, workspace:* dep, buttonProps cleanup
Review — 1 P0, please fix before mergeReviewed at head P01. Fee quote params ≠ createPayment params — the headline invariant doesn't hold at the call sites.
The new test P1
P2
Not approving while the P0 stands; happy to re-review after a fix. Reviewed with Claude Code (deep local review, read-only against head |
2 participants
Summary
getFeeandcreatePaymentnow build the exact same request body —getFeejust adds?dryrun=true— so a consumer-setintentonRozoPayButtonreachesgetFeethe same way it already reachedcreatePayment. Fee quotes no longer diverge from whatcreatePaymentactually charges.formatPaymentResponseToHydratedOrdernow copies the backend'ssettlementModeonto the hydrated order's metadata, so the checkout-mode payId path (runSetPayIdEffects→getPayment→order_loaded) carries it through instead of silently dropping it.intentremains entirely consumer-driven — no auto-detection (e.g. stellar_direct) added on the client; that decision stays with the backend viasettlementModein the response.packages/connectkit's@rozoai/intent-commondependency from a pinned0.1.26toworkspace:*so localpay-commonchanges are actually picked up during development.<RozoPayButton payId={...}>now honorspreferredChains/preferredTokens/preferredSymbol. Previously these props were silently dropped in payId mode — extracted nowhere inRozoPayButtonCustom's prop-destructure, andusePaymentState's payId-modestablePayParamsderived its token filter purely from the order's destination token, with no path for caller input to reach it.buttonProps/setButtonPropsalso existed but were never wired up (dead state) — nowRozoPayButtonCustompublishes props to it on every change, andstablePayParams,showSolanaPaymentMethod,showStellarPaymentMethod,solanaPaymentEligible, andstellarPaymentEligibleall read it as a payId-mode fallback.Changes
packages/pay-common/src/api/payment.ts: extractedbuildPaymentRequestBody(shared bycreatePaymentand the newgetFee);getFeenow takes the sameCreateNewPaymentParamsshape ascreatePayment.packages/pay-common/src/api/fee.ts: removed (superseded bypayment.ts).packages/pay-common/src/bridge-utils.ts:formatPaymentResponseToHydratedOrdercopiessettlementModeinto hydrated-order metadata.packages/pay-common/src/index.ts: dropped the./api/feere-export.packages/pay-common/test/payment.test.ts: new regression test assertinggetFeeandcreatePaymentbuild identical request bodies for the sameCreateNewPaymentParamsinput (same-chain, cross-chain with intent). Thedryrunquery param is the only difference.packages/connectkit/src/utils/feeCache.ts:getCachedFeetakesCreateNewPaymentParams; cache key covers the full payload (includingintent).packages/connectkit/src/components/Pages/{PayWithToken,Solana/PayWithSolanaToken,Stellar/PayWithStellarToken,WaitingDepositAddress}/index.tsx: fee-quote calls now build a fullCreateNewPaymentParams(carryingintent) instead of a separate hand-rolled shape.packages/connectkit/src/components/RozoPayButton/types.ts: payId branch ofPayButtonPaymentPropsgainspreferredChains/preferredTokens/preferredSymbol.packages/connectkit/src/components/RozoPayButton/index.tsx: new effect callspaymentState.setButtonProps(props)on every prop change (previously dead — nothing called it).packages/connectkit/src/hooks/usePaymentState.ts: payId-modestablePayParamsmergesbuttonProps'spreferredChains/preferredTokens/preferredSymbolover the destination-symbol-derived defaults (intersecting tokens by chain when both given, falling back to the full derived set if the intersection is empty);showSolanaPaymentMethod/showStellarPaymentMethod/solanaPaymentEligible/stellarPaymentEligiblenow read aneffective*value (currPayParams ?? buttonProps) and additionally gate onpreferredChains.Design doc:
docs/superpowers/specs/2026-08-03-intent-propagation-getfee-design.mdTest plan
pnpm build—pay-common(41 tests pass) andconnectkit(22 tests pass) both build cleanpnpm lint— no new warnings introducednpx tsc --noEmitclean onconnectkitafter the preferredChains changesgetFee/createPaymentbody-identity invariant — regression test covers same-chain and cross-chain-with-intent casesintentoverride and confirm the fee quote shown before payment matches whatcreatePaymentactually chargessettlementModeis available on the hydrated order afterrunSetPayIdEffects<RozoPayButton payId={...} preferredChains={[rozoStellar.chainId]}>narrows the modal to Stellar-only tiles/tokens (consumed by rozo-chat-ai's checkout.rozo.ai?preferredChain=integration)Additional fix (this update):
chain: undefined (id: 8453)on Base transfersDiagnosed from a production report on
intents.rozo.ai— one user hitAn unknown RPC error occurred. chain: undefined (id: 8453)11 times in a row on Base USDC transfers, source-chain always Base, independent of destination (Stellar/Solana/Ethereum all tried). Full trace:20260810-intent-pay-chain-undefined-base.md.Root cause (confirmed mechanism, defense-in-depth applied):
defaultConfig.ts's chain padding dedupedREQUIRED_CHAINSby object identity (Array.includes), notchain.id. A consumer-supplied chain object sourced from a differently-resolved viem copy has the sameidbut fails the identity check, landing two distinct8453entries inconfig.chains— wagmi's registry lookup can then resolvechain: undefinedfor a valid id.defaultConfig.ts: dedupeREQUIRED_CHAINSbychain.id, not reference identity. Also addsresolveChainObject(chainId), exported so call sites can resolve the canonicalChainobject directly instead of depending onconfig.chainsregistry lookup at call time.usePaymentState.ts: the plainwriteContractAsyncERC20 transfer path now passeschainexplicitly alongsidechainId(the batched EIP-5792writeContractsAsyncand nativesendTransactionAsyncpaths don't accept an explicitchain— wagmi's own typesOmitit there, so this applies only where possible).connectkitpeer range:viem: "2.x"→">=2.52.0 <3".hyperEvmis only exported starting viem 2.52 (verified against unpkg); anything below silently resolvesundefinedintoREQUIRED_CHAINS— a second, independent way this exact symptom can occur.pay-common: movedviemfrom a regulardependencytopeerDependency(+devDependencyfor its own build/test), removing a realistic vector for a second, differently-resolved viem copy landing in a consumer's tree alongside connectkit's peer-resolved one.examples/nextjs-app: bumpedviemrange to match the new connectkit floor.types.ts,useWalletPaymentOptions.ts) that claimedpreferredTokensonly affects sort order —matchesPreferredTokensactually hard-filters wallet payment options to the preferred set. No behavior change, docs now match the code.Verification:
tsc --noEmitclean, full test suites green (pay-common41/41,connectkit22/22),pnpm install --frozen-lockfileresolves cleanly (lockfile already pinsviem@2.55.8everywhere in this repo, so no local breakage). Not yet verified against the actual production failure — that requires the Step 1 repro from the trace doc (pnpm why viem/ runtime chain-count check onintents.rozo.aiitself), which needs deploy access this session doesn't have.Test plan addition:
config.chains.filter(c => c.id === 8453).length === 1after this change, in the actualintents.rozo.aibundleintents.rozo.aipayment_failedwithchain: undefined→ zero,payment_flow_startedflat or rising