Split `BaseWallet.completeFeeOptions` into `decideAccountFeePaymentMethodOptions` and `calculateGasSettings`

[shiqicao]

Summary

BaseWallet.completeFeeOptions computes two entirely unrelated things and returns them fused into a single FeeOptions object. I'd like to propose splitting it into two functions:

decideAccountFeePaymentMethodOptions(
  from: AztecAddress | NoFrom,
  feePayer?: AztecAddress,
): AccountFeePaymentMethodOptions | undefined;

calculateGasSettings(
  initGasSettings: Partial<FieldsOf<GasSettings>> | undefined,
  forEstimation?: boolean,
  congestionEstimate?: ManaUsageEstimate,
): Promise<GasSettings>;

Observed in @aztec/wallet-sdk@5.0.1, src/base-wallet/base_wallet.ts.

Today

protected completeFeeOptions(config: CompleteFeeOptionsConfig): Promise<FeeOptions>;

The body is two independent computations that never exchange a value:

• accountFeePaymentMethodOptions is derived from from and feePayer alone. It is pure and synchronous — a couple of comparisons picking between PREEXISTING_FEE_JUICE, FEE_JUICE_WITH_CLAIM and EXTERNAL.
• fullGasSettings is derived from gasSettings, forEstimation and congestionEstimate alone. It is async and does network I/O (getMinFees, and getMaxTxGasLimits on the non-estimation path).

maxFeesPerGas feeds only the second. Nothing feeds the first. The two halves could be moved into separate files without a single edit to either.

Why split

A. It removes two awkward types. CompleteFeeOptionsConfig exists only to bundle five arguments for one call, and FeeOptions exists only to carry the two unrelated results back. Neither models a domain concept — they're parameter-object and return-tuple wrappers around a function that shouldn't have been one function.

FeeOptions is actively confusing to read. Its name suggests "the fee options for this transaction", but it is really three unrelated fields, one of which (walletFeePaymentMethod) completeFeeOptions always returns as undefined. So every caller of completeFeeOptions gets a field that is dead by construction, while computeAppCallOffset reads that same field expecting it to be populated by somebody else. A reader has to trace all producers to learn which fields are live on any given path.

With the split, both types disappear: one function takes two plain arguments and returns an enum, the other takes three and returns GasSettings.

B. The two halves are fully disjoint, as above — no shared inputs, no shared intermediates, and they don't even agree on being async. Fusing them means the pure, synchronous, offline half is only reachable by awaiting a function that may make two network round trips.

C. The new names say what they do. "Complete fee options" says a bag of options is being filled in, but not with what or from where. decideAccountFeePaymentMethodOptions and calculateGasSettings name their outputs, and the verbs are honest about the difference: one decides between three enum cases, the other calculates against network state.

D. Callers could then call them separately — which is the practical motivation. A wallet that defers its fee payment method until gas settings are known needs these two facts at different times:

• The account fee payment mode has to be decided before any payload exists, because it determines where the account entrypoint ends the setup phase. It's derivable from the payer address alone, which is known as soon as the payer is chosen.
• The gas settings have to be pinned to a specific value, because a sponsor contract that fronts gas recomputes what it requires from the settings the transaction declares, and rejects a charge authorized against a smaller ceiling.

With one fused function, getting both means either calling it twice (paying for the network round trips twice, and having to thread feePayer in on the second call once a payload finally exists) or calling it once and overwriting fields of the returned FeeOptions — I've ended up writing both, and neither reads like intended usage. Split, it's two direct calls in the order the caller actually needs them.

Notes

• Both are protected, so this is a subclass-facing change rather than a public API break; completeFeeOptions could be kept for a release as a thin wrapper over the two, if that's preferred.
• Worth considering separately: whether walletFeePaymentMethod belongs in the same type as gasSettings at all, given completeFeeOptions never sets it.

Happy to open a PR if the direction sounds right.