Skip to content

refactor: accept transactionRequests[] in TransactionFee and useGas #2237

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
janek26 wants to merge 1 commit into from
Closed

refactor: accept transactionRequests[] in TransactionFee and useGas #2237

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions src/entries/popup/components/TransactionFee/TransactionFee.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,25 +286,29 @@ type TransactionFeeProps = {
disableShortcuts?: boolean;
address?: Address;
defaultSpeed?: GasSpeed;
transactionRequest: TransactionRequest;
transactionRequests: TransactionRequest[];
accentColor?: string;
plainTriggerBorder?: boolean;
analyticsEvents?: {
customGasClicked: keyof EventProperties;
transactionSpeedSwitched: keyof EventProperties;
transactionSpeedClicked: keyof EventProperties;
};
feeLabel?: string;
feeInfoButton?: { onClick: () => void };
};

export function TransactionFee({
chainId,
disableShortcuts,
address,
defaultSpeed,
transactionRequest,
transactionRequests,
accentColor,
plainTriggerBorder,
analyticsEvents,
feeLabel,
feeInfoButton,
}: TransactionFeeProps) {
const { defaultTxSpeed } = useDefaultTxSpeed({ chainId });
const {
Expand All @@ -322,7 +326,7 @@ export function TransactionFee({
chainId,
address,
defaultSpeed: defaultSpeed || defaultTxSpeed,
transactionRequest,
transactionRequests,
});

return (
Expand All @@ -342,6 +346,8 @@ export function TransactionFee({
currentBaseFee={currentBaseFee}
baseFeeTrend={baseFeeTrend}
feeType={feeType}
feeLabel={feeLabel}
feeInfoButton={feeInfoButton}
/>
);
}
Expand Down
60 changes: 53 additions & 7 deletions src/entries/popup/hooks/useGas.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { isLegacyMeteorologyFeeData } from '~/core/resources/gas/classification'
import { useEstimateApprovalGasLimit } from '~/core/resources/gas/estimateApprovalGasLimit';
import { useEstimateSwapGasLimit } from '~/core/resources/gas/estimateSwapGasLimit';
import { useOptimismL1SecurityFee } from '~/core/resources/gas/optimismL1SecurityFee';
import { estimateTransactionsGasLimit } from '~/core/resources/transactions/simulation';
import { useCurrentCurrencyStore, useGasStore } from '~/core/state';
import { useNetworkStore } from '~/core/state/networks/networks';
import { ParsedAsset, ParsedSearchAsset } from '~/core/types/assets';
Expand Down Expand Up @@ -365,27 +366,72 @@ const useGas = ({
};
};

const toSimulationTransaction = (
tx: TransactionRequest,
): { from: string; to: string; value: string; data: string } => ({
from: tx.from?.toString() ?? '',
to: tx.to?.toString() ?? '',
Copy link
Collaborator

@DanielSinclair DanielSinclair Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fallback values are concerning here because they differ from the transaction submission flow. If missing here, the simulation would fail and the UI handles gracefully, but could still be destructive

Copy link
Collaborator

@DanielSinclair DanielSinclair Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'd want to use the selected address instead here, I think. I assume the stack won't let you send to a null address with '', but maybe from as the default for to as well

value: tx.value?.toString() ?? '0',
data: tx.data?.toString() ?? '0x',
});

/**
* Gas estimation for one or more transactions.
* Single tx: uses provider estimateGas. Batch: sums gas from simulation API.
*/
export const useTransactionGas = ({
chainId,
address,
defaultSpeed,
transactionRequest,
transactionRequests,
}: {
chainId: ChainId;
address?: Address;
defaultSpeed?: GasSpeed;
transactionRequest: TransactionRequest;
transactionRequests: TransactionRequest[];
}) => {
const { data: estimatedGasLimit } = useEstimateGasLimit({
chainId,
transactionRequest: useDebounce(transactionRequest, 500),
const debouncedRequests = useDebounce(transactionRequests, 500);
const isSingle = (debouncedRequests?.length ?? 0) <= 1;

const { data: singleGasLimit } = useEstimateGasLimit(
{
chainId,
transactionRequest: debouncedRequests?.[0] ?? {},
},
{ enabled: !!chainId && isSingle && !!debouncedRequests?.length },
);

const { data: batchGasLimit } = useQuery({
queryKey: [
'estimateBatchGasLimit',
chainId,
address,
debouncedRequests
?.map(
(r) =>
`${r.from ?? ''}-${r.to}-${r.value?.toString() ?? '0'}-${r.data}`,
)
.join(','),
],
queryFn: async () => {
if (!debouncedRequests?.length || isSingle) return undefined;
const steps = debouncedRequests.map((tx, i) => ({
transaction: toSimulationTransaction(tx),
label: `Call ${i + 1}`,
}));
return estimateTransactionsGasLimit({ chainId, steps });
},
enabled: !!chainId && !isSingle && !!debouncedRequests?.length,
});

const estimatedGasLimit = isSingle ? singleGasLimit : batchGasLimit;

return useGas({
chainId,
address,
defaultSpeed,
estimatedGasLimit,
transactionRequest,
estimatedGasLimit: estimatedGasLimit ?? undefined,
transactionRequest: debouncedRequests?.[0] ?? null,
Copy link
Collaborator

@DanielSinclair DanielSinclair Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unintended side effects on gas. I think we want to avoid conflating using sequential simulation for display values, vs gas estimates; we really need to estimate the batch as a whole.

useOptimismL1SecurityFee only accounts for the first transaction's calldata on OP-stack
chains, silently underestimating fees for multi-tx batches.

That matters on chains like Optimism and Base because the extra L1 security fee depends on the
calldata size of the full transaction. A batch with 3 calls has more calldata than just the first
call, so using only the first request makes the L1 fee too small. The result is the fee preview and
speed options are underestimated for multi-call batches.

enabled: true,
});
};
Expand Down
16 changes: 9 additions & 7 deletions src/entries/popup/pages/home/Delegations/RevokeDelegationPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -634,13 +634,15 @@ export const RevokeDelegationPage = () => {
<TransactionFee
chainId={currentDelegation.chainId}
address={revokeAddress}
transactionRequest={{
to: revokeAddress,
from: revokeAddress,
chainId: currentDelegation.chainId,
data: '0x',
value: '0x0',
}}
transactionRequests={[
{
to: revokeAddress,
from: revokeAddress,
chainId: currentDelegation.chainId,
data: '0x',
value: '0x0',
},
]}
/>
</Row>
<Row>
Expand Down
6 changes: 5 additions & 1 deletion src/entries/popup/pages/messages/SendTransaction/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,11 @@ export function SendTransaction({
}}
chainId={activeSession?.chainId || ChainId.mainnet}
address={activeSession?.address}
transactionRequest={request?.params?.[0] as TransactionRequest}
transactionRequests={
request?.params?.[0]
? [request.params[0] as TransactionRequest]
: []
}
plainTriggerBorder
/>
<SendTransactionActions
Expand Down
6 changes: 5 additions & 1 deletion src/entries/popup/pages/send/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -767,7 +767,11 @@ export function Send() {
<TransactionFee
disableShortcuts={contactSaveAction.show}
chainId={chainId}
transactionRequest={transactionRequestForGas}
transactionRequests={
transactionRequestForGas
? [transactionRequestForGas]
: []
}
accentColor={assetAccentColor}
/>
</Row>
Expand Down
4 changes: 3 additions & 1 deletion src/entries/popup/pages/speedUpAndCancelSheet/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,9 @@ export function SpeedUpAndCancelSheet({
<TransactionFee
chainId={transaction.chainId}
defaultSpeed={GasSpeed.URGENT}
transactionRequest={transactionRequest}
transactionRequests={
transactionRequest ? [transactionRequest] : []
}
/>
)}
</Box>
Expand Down
Loading