diff --git a/.skillshare/skills/1k-retrospective/references/case-studies.md b/.skillshare/skills/1k-retrospective/references/case-studies.md index 074ab875dd6a..e21765543dd5 100644 --- a/.skillshare/skills/1k-retrospective/references/case-studies.md +++ b/.skillshare/skills/1k-retrospective/references/case-studies.md @@ -60,3 +60,10 @@ Cases are appended by AI after each bug fix. Do NOT reorder or delete entries **Root Cause**: The parent treated a null quick bar during configuration loading and a null quick bar after `indicatorsEnabled: false` as the same state, so it could not reserve or release chart height at the correct lifecycle point. **Fix**: Added explicit loading, visible, and hidden quick bar states; reserve the slot only while loading or visible, and restore the chart height when the quick bar is hidden. **Catchable by**: Section 5: "Not loaded" versus intentionally unavailable state must be distinguished + +## Case: Private Send dropped the Gas Account quote when the server preferred Megafuel +**Date**: 2026-08-19 | **Platforms**: mobile, desktop, web, extension +**Symptom**: On BNB-chain Private Send, when the fee service returned `payer='megafuel'` together with an eligible Gas Account quote, the confirm flow suppressed Megafuel for display but kept `selectedPayer='user'`, so the sponsored quote was silently dropped and the tx broadcast user-paid (OK-59993 follow-up, PR #12916). +**Root Cause**: The display payer (`effectiveFeePayer`) and the submit wiring (`selectedPayer`) were derived in separate places from different inputs — display from the post-filtered sponsor state, submit from the raw backend `payer` — so scenario suppression (Private Send disables Megafuel) could update one without the other. Review also caught that the extracted eligibility check (`gasAccountEligible && gasAccountQuote`) omitted the non-empty `quoteId` guard every downstream consumer requires, which would have shown a sponsored UI while broadcasting user-paid. +**Fix**: Extracted `resolveSponsorPayerState()` to derive `effectiveFeePayer` and `selectedPayer` together from the post-filtered sponsor state, with the megafuel-suppressed preference falling through to an eligible Gas Account quote; hardened eligibility via `isGasAccountQuoteEligible()` requiring a non-empty `quoteId`; locked both invariants with unit tests. +**Catchable by**: Section 4: Data flow end-to-end: API -> state -> UI (display state and submit wiring must derive from the same filtered source); Section 5: runtime-validate network-response fields even when typed as required diff --git a/packages/kit/src/states/jotai/contexts/signatureConfirm/atoms.ts b/packages/kit/src/states/jotai/contexts/signatureConfirm/atoms.ts index b0d3e17d7548..28c2c450a19b 100644 --- a/packages/kit/src/states/jotai/contexts/signatureConfirm/atoms.ts +++ b/packages/kit/src/states/jotai/contexts/signatureConfirm/atoms.ts @@ -208,18 +208,22 @@ export const defaultEffectiveFeePayer = 'user' as IGasPayer; // `effectiveFeePayerAtom` is the authoritative "who pays the fee" signal the // UI renders from (sponsor badges, free copy, fee hiding). It mirrors the -// server's `payer` field with two narrow overrides to `'user'`: -// - when a custom RPC is active (all sponsors disabled), and -// - when the server indicates `'gasAccount'` while gas account is +// server's `payer` field with narrow overrides: +// - `'user'` when a custom RPC is active (all sponsors disabled), +// - `'user'` when the server indicates `'gasAccount'` while gas account is // temporarily disabled after a fallback (the gas-account path only; -// a concurrent `'megafuel'` payer still surfaces). +// a concurrent `'megafuel'` payer still surfaces), and +// - when megafuel is suppressed for Private Send, a server `'megafuel'` +// payer falls through to `'gasAccount'` if an eligible quote exists, +// otherwise `'user'`. // // This is intentionally separate from `gasAccountUiState.selectedPayer` below: // - `effectiveFeePayer` drives *display* (can be `'megafuel'` even when gas // account quote exists — megafuel wins UI-wise). // - `selectedPayer` drives *submit wiring* (whether to attach `quoteId` / // `idempotencyKey` to the broadcast request). -// Keep them aligned in TxFeeInfo's estimate handler. +// Both derive from `resolveSponsorPayerState` in TxFeeInfo's estimate handler +// so they stay aligned by construction. export const { atom: effectiveFeePayerAtom, use: useEffectiveFeePayerAtom } = contextAtom(defaultEffectiveFeePayer); diff --git a/packages/kit/src/views/SignatureConfirm/components/TxFee/TxFeeInfo.tsx b/packages/kit/src/views/SignatureConfirm/components/TxFee/TxFeeInfo.tsx index 7802e99178da..df68e86c9921 100644 --- a/packages/kit/src/views/SignatureConfirm/components/TxFee/TxFeeInfo.tsx +++ b/packages/kit/src/views/SignatureConfirm/components/TxFee/TxFeeInfo.tsx @@ -109,6 +109,10 @@ import { EGasAccountErrorStrategy, getGasAccountErrorEntry, } from '../../constants/gasAccountErrorCodes'; +import { + isGasAccountQuoteEligible, + resolveSponsorPayerState, +} from '../../utils/gasAccountPayerSelection'; import { buildPresetMultiTxsFee } from './presetFeeInfoUtils'; import { TxFeeEditor } from './TxFeeEditor'; @@ -560,15 +564,29 @@ function TxFeeInfo(props: IProps) { // Megafuel is an independent sponsor mechanism and should still be // honored when the server indicates `payer='megafuel'`, even when a // frontend scenario disables Gas Account. + // + // Display payer and submit wiring are derived together from the + // post-filtered sponsor state so they cannot drift: when megafuel is + // suppressed for Private Send, a server `payer='megafuel'` preference + // falls through to an eligible gas account quote instead of silently + // degrading to user-paid. const serverPayer: IGasPayer = r.payer ?? 'user'; - const nextEffectiveFeePayer: IGasPayer = - isCustomRpcEnabled || - sponsorDisabledForBatch || - (megafuelDisabledForPrivateSend && serverPayer === 'megafuel') || - (gasAccountDisabledByScenario && serverPayer === 'gasAccount') || - (gasAccountTemporarilyDisabled && serverPayer === 'gasAccount') - ? 'user' - : serverPayer; + const { + effectiveFeePayer: nextEffectiveFeePayer, + selectedPayer: nextSelectedPayer, + } = resolveSponsorPayerState({ + serverPayer, + megafuelSponsorable: !!r.megafuelEligible?.sponsorable, + gasAccountQuoteEligible: isGasAccountQuoteEligible({ + gasAccountEligible: r.gasAccountEligible, + gasAccountQuote: r.gasAccountQuote, + }), + isCustomRpcEnabled, + sponsorDisabledForBatch, + megafuelDisabledForPrivateSend, + gasAccountDisabledByScenario, + gasAccountTemporarilyDisabled, + }); updateEffectiveFeePayer(nextEffectiveFeePayer); if ( @@ -626,11 +644,6 @@ function TxFeeInfo(props: IProps) { } } else if (r.gasAccountEligible && r.gasAccountQuote) { resetGasAccountTemporarilyDisabled(); - const nextSelectedPayer = - r.megafuelEligible?.sponsorable || r.payer !== 'gasAccount' - ? 'user' - : 'gasAccount'; - updateGasAccountUiState({ payer: r.payer, gasAccountEligible: true, diff --git a/packages/kit/src/views/SignatureConfirm/utils/gasAccountPayerSelection.test.ts b/packages/kit/src/views/SignatureConfirm/utils/gasAccountPayerSelection.test.ts new file mode 100644 index 000000000000..b9e908bbf54f --- /dev/null +++ b/packages/kit/src/views/SignatureConfirm/utils/gasAccountPayerSelection.test.ts @@ -0,0 +1,336 @@ +import { + isGasAccountQuoteEligible, + resolveSponsorPayerState, +} from './gasAccountPayerSelection'; + +const baseParams = { + serverPayer: 'user' as const, + megafuelSponsorable: false, + gasAccountQuoteEligible: false, + isCustomRpcEnabled: false, + sponsorDisabledForBatch: false, + megafuelDisabledForPrivateSend: false, + gasAccountDisabledByScenario: false, + gasAccountTemporarilyDisabled: false, +}; + +describe('isGasAccountQuoteEligible', () => { + const quote = { + quoteId: 'quote-1', + maxFee: '1000', + expiresAt: '2026-01-01T00:00:00Z', + }; + + it('is eligible only with both the eligible flag and a non-empty quoteId', () => { + expect( + isGasAccountQuoteEligible({ + gasAccountEligible: true, + gasAccountQuote: quote, + }), + ).toBe(true); + }); + + it('rejects a quote object whose quoteId is empty', () => { + expect( + isGasAccountQuoteEligible({ + gasAccountEligible: true, + gasAccountQuote: { ...quote, quoteId: '' }, + }), + ).toBe(false); + }); + + it('rejects a missing quote or a missing eligible flag', () => { + expect( + isGasAccountQuoteEligible({ + gasAccountEligible: true, + gasAccountQuote: undefined, + }), + ).toBe(false); + expect( + isGasAccountQuoteEligible({ + gasAccountEligible: undefined, + gasAccountQuote: quote, + }), + ).toBe(false); + expect( + isGasAccountQuoteEligible({ + gasAccountEligible: false, + gasAccountQuote: quote, + }), + ).toBe(false); + }); + + it('resolves to user/user when the quote object exists without a quoteId', () => { + // End-to-end invariant: an id-less quote must never surface the sponsored + // UI (effectiveFeePayer) nor wire the submit path (selectedPayer), even + // on the Private Send megafuel fallback where the window is widest. + expect( + resolveSponsorPayerState({ + ...baseParams, + serverPayer: 'megafuel', + megafuelSponsorable: true, + megafuelDisabledForPrivateSend: true, + gasAccountQuoteEligible: isGasAccountQuoteEligible({ + gasAccountEligible: true, + gasAccountQuote: { ...quote, quoteId: '' }, + }), + }), + ).toEqual({ + effectiveFeePayer: 'user', + selectedPayer: 'user', + }); + }); + + it('resolves to user/user when the server prefers gasAccount but the quoteId is empty', () => { + // Same invariant on the direct serverPayer === 'gasAccount' path: with no + // suppression flags set, only quote eligibility stands between an id-less + // quote and the sponsored display state. + expect( + resolveSponsorPayerState({ + ...baseParams, + serverPayer: 'gasAccount', + gasAccountQuoteEligible: isGasAccountQuoteEligible({ + gasAccountEligible: true, + gasAccountQuote: { ...quote, quoteId: '' }, + }), + }), + ).toEqual({ + effectiveFeePayer: 'user', + selectedPayer: 'user', + }); + }); +}); + +describe('resolveSponsorPayerState', () => { + describe('default flow (no suppression)', () => { + it('keeps user payer when the server does not sponsor', () => { + expect(resolveSponsorPayerState(baseParams)).toEqual({ + effectiveFeePayer: 'user', + selectedPayer: 'user', + }); + }); + + it('selects gas account when the server prefers it and a quote is eligible', () => { + expect( + resolveSponsorPayerState({ + ...baseParams, + serverPayer: 'gasAccount', + gasAccountQuoteEligible: true, + }), + ).toEqual({ + effectiveFeePayer: 'gasAccount', + selectedPayer: 'gasAccount', + }); + }); + + it('keeps user submit when megafuel sponsors even if a gas account quote exists', () => { + expect( + resolveSponsorPayerState({ + ...baseParams, + serverPayer: 'megafuel', + megafuelSponsorable: true, + gasAccountQuoteEligible: true, + }), + ).toEqual({ + effectiveFeePayer: 'megafuel', + selectedPayer: 'user', + }); + }); + + it('lets megafuel win submit wiring when sponsorable alongside a gasAccount payer', () => { + expect( + resolveSponsorPayerState({ + ...baseParams, + serverPayer: 'gasAccount', + megafuelSponsorable: true, + gasAccountQuoteEligible: true, + }), + ).toEqual({ + effectiveFeePayer: 'gasAccount', + selectedPayer: 'user', + }); + }); + + it('resets both payers to user without an eligible quote', () => { + // The display payer must be gated by quote eligibility as well: + // keeping effectiveFeePayer at 'gasAccount' here would show the + // sponsored UI while the submit path broadcasts user-paid. + expect( + resolveSponsorPayerState({ + ...baseParams, + serverPayer: 'gasAccount', + gasAccountQuoteEligible: false, + }), + ).toEqual({ + effectiveFeePayer: 'user', + selectedPayer: 'user', + }); + }); + }); + + describe('global suppressions', () => { + it('forces user for both payers when a custom RPC is enabled', () => { + expect( + resolveSponsorPayerState({ + ...baseParams, + serverPayer: 'gasAccount', + gasAccountQuoteEligible: true, + isCustomRpcEnabled: true, + }), + ).toEqual({ + effectiveFeePayer: 'user', + selectedPayer: 'user', + }); + }); + + it('forces user for both payers on batch transactions', () => { + expect( + resolveSponsorPayerState({ + ...baseParams, + serverPayer: 'megafuel', + megafuelSponsorable: true, + gasAccountQuoteEligible: true, + sponsorDisabledForBatch: true, + }), + ).toEqual({ + effectiveFeePayer: 'user', + selectedPayer: 'user', + }); + }); + + it('forces user when gas account is temporarily disabled after a fallback', () => { + expect( + resolveSponsorPayerState({ + ...baseParams, + serverPayer: 'gasAccount', + gasAccountQuoteEligible: true, + gasAccountTemporarilyDisabled: true, + }), + ).toEqual({ + effectiveFeePayer: 'user', + selectedPayer: 'user', + }); + }); + + it('forces user when the frontend scenario disables gas account', () => { + expect( + resolveSponsorPayerState({ + ...baseParams, + serverPayer: 'gasAccount', + gasAccountQuoteEligible: true, + gasAccountDisabledByScenario: true, + }), + ).toEqual({ + effectiveFeePayer: 'user', + selectedPayer: 'user', + }); + }); + + it('still surfaces megafuel display when only the gas account path is scenario-disabled', () => { + expect( + resolveSponsorPayerState({ + ...baseParams, + serverPayer: 'megafuel', + megafuelSponsorable: true, + gasAccountQuoteEligible: true, + gasAccountDisabledByScenario: true, + }), + ).toEqual({ + effectiveFeePayer: 'megafuel', + selectedPayer: 'user', + }); + }); + }); + + describe('private send (megafuel suppressed)', () => { + it('falls back to the eligible gas account quote when the server prefers megafuel', () => { + expect( + resolveSponsorPayerState({ + ...baseParams, + serverPayer: 'megafuel', + megafuelSponsorable: true, + gasAccountQuoteEligible: true, + megafuelDisabledForPrivateSend: true, + }), + ).toEqual({ + effectiveFeePayer: 'gasAccount', + selectedPayer: 'gasAccount', + }); + }); + + it('falls back to user when the server prefers megafuel and no quote is eligible', () => { + expect( + resolveSponsorPayerState({ + ...baseParams, + serverPayer: 'megafuel', + megafuelSponsorable: true, + gasAccountQuoteEligible: false, + megafuelDisabledForPrivateSend: true, + }), + ).toEqual({ + effectiveFeePayer: 'user', + selectedPayer: 'user', + }); + }); + + it('selects gas account when the server prefers it even with megafuel sponsorable', () => { + expect( + resolveSponsorPayerState({ + ...baseParams, + serverPayer: 'gasAccount', + megafuelSponsorable: true, + gasAccountQuoteEligible: true, + megafuelDisabledForPrivateSend: true, + }), + ).toEqual({ + effectiveFeePayer: 'gasAccount', + selectedPayer: 'gasAccount', + }); + }); + + it('keeps user payer when the server does not sponsor', () => { + expect( + resolveSponsorPayerState({ + ...baseParams, + serverPayer: 'user', + megafuelDisabledForPrivateSend: true, + }), + ).toEqual({ + effectiveFeePayer: 'user', + selectedPayer: 'user', + }); + }); + + it('does not fall back to gas account when a custom RPC is enabled', () => { + expect( + resolveSponsorPayerState({ + ...baseParams, + serverPayer: 'megafuel', + megafuelSponsorable: true, + gasAccountQuoteEligible: true, + megafuelDisabledForPrivateSend: true, + isCustomRpcEnabled: true, + }), + ).toEqual({ + effectiveFeePayer: 'user', + selectedPayer: 'user', + }); + }); + + it('does not fall back to gas account while it is temporarily disabled', () => { + expect( + resolveSponsorPayerState({ + ...baseParams, + serverPayer: 'megafuel', + megafuelSponsorable: true, + gasAccountQuoteEligible: true, + megafuelDisabledForPrivateSend: true, + gasAccountTemporarilyDisabled: true, + }), + ).toEqual({ + effectiveFeePayer: 'user', + selectedPayer: 'user', + }); + }); + }); +}); diff --git a/packages/kit/src/views/SignatureConfirm/utils/gasAccountPayerSelection.ts b/packages/kit/src/views/SignatureConfirm/utils/gasAccountPayerSelection.ts new file mode 100644 index 000000000000..31fd2d43f765 --- /dev/null +++ b/packages/kit/src/views/SignatureConfirm/utils/gasAccountPayerSelection.ts @@ -0,0 +1,100 @@ +import type { IGasAccountQuote, IGasPayer } from '@onekeyhq/shared/types/fee'; + +/** + * `IGasAccountQuote.quoteId` is typed as a required string but comes from a + * network response with no runtime guarantee. Every downstream consumer (fee + * display, balance precheck, broadcast gate) additionally requires a + * non-empty `quoteId`, so an id-less quote must not count as eligible here + * either — otherwise the UI would show the sponsored state while the + * broadcast silently falls back to user-paid. + */ +export function isGasAccountQuoteEligible({ + gasAccountEligible, + gasAccountQuote, +}: { + gasAccountEligible: boolean | undefined; + gasAccountQuote: IGasAccountQuote | undefined; +}): boolean { + return !!(gasAccountEligible && gasAccountQuote?.quoteId); +} + +export interface IResolveSponsorPayerStateParams { + /** Raw `payer` returned by the fee service (defaulted to 'user'). */ + serverPayer: IGasPayer; + /** Raw `megafuelEligible.sponsorable` from the fee service, pre-filtering. */ + megafuelSponsorable: boolean; + /** Whether the estimate carries an eligible gas account quote. */ + gasAccountQuoteEligible: boolean; + isCustomRpcEnabled: boolean; + sponsorDisabledForBatch: boolean; + megafuelDisabledForPrivateSend: boolean; + gasAccountDisabledByScenario: boolean; + gasAccountTemporarilyDisabled: boolean; +} + +export interface ISponsorPayerState { + effectiveFeePayer: IGasPayer; + selectedPayer: 'user' | 'gasAccount'; +} + +/** + * Derives the display payer (`effectiveFeePayer`) and the submit wiring + * (`selectedPayer`) from the post-filtered sponsor state, in one place so the + * two can never drift apart (see the atom docs in + * `states/jotai/contexts/signatureConfirm/atoms.ts`). + * + * Megafuel wins over a coexisting gas account quote when it is actually + * available: it sponsors at the chain level (zeroed gas price), so the quote + * must not be attached on top of it. When megafuel is suppressed for the + * scenario (Private Send), the server's megafuel preference falls through to + * an eligible gas account quote instead of silently degrading to user-paid. + */ +export function resolveSponsorPayerState({ + serverPayer, + megafuelSponsorable, + gasAccountQuoteEligible, + isCustomRpcEnabled, + sponsorDisabledForBatch, + megafuelDisabledForPrivateSend, + gasAccountDisabledByScenario, + gasAccountTemporarilyDisabled, +}: IResolveSponsorPayerStateParams): ISponsorPayerState { + const gasAccountSuppressed = + isCustomRpcEnabled || + sponsorDisabledForBatch || + gasAccountDisabledByScenario || + gasAccountTemporarilyDisabled; + const megafuelSuppressed = + isCustomRpcEnabled || + sponsorDisabledForBatch || + megafuelDisabledForPrivateSend; + + const megafuelAvailable = !megafuelSuppressed && megafuelSponsorable; + const payerPreference = + megafuelDisabledForPrivateSend && serverPayer === 'megafuel' + ? 'gasAccount' + : serverPayer; + + const selectedPayer: 'user' | 'gasAccount' = + gasAccountQuoteEligible && + !gasAccountSuppressed && + !megafuelAvailable && + payerPreference === 'gasAccount' + ? 'gasAccount' + : 'user'; + + let effectiveFeePayer: IGasPayer = serverPayer; + if ( + isCustomRpcEnabled || + sponsorDisabledForBatch || + (!gasAccountQuoteEligible && serverPayer === 'gasAccount') || + (gasAccountDisabledByScenario && serverPayer === 'gasAccount') || + (gasAccountTemporarilyDisabled && serverPayer === 'gasAccount') + ) { + effectiveFeePayer = 'user'; + } else if (megafuelDisabledForPrivateSend && serverPayer === 'megafuel') { + effectiveFeePayer = selectedPayer === 'gasAccount' ? 'gasAccount' : 'user'; + } + + return { effectiveFeePayer, selectedPayer }; +}