-
Notifications
You must be signed in to change notification settings - Fork 7
fix(generic-delegator): correct gas estimation for smart-account txs #623
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.
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
Changes from 4 commits
77e2aeb
773b458
2a9cd70
e3d1ba8
4ded349
aa0f527
1949da9
1edaf19
3125daa
51f192e
21946bb
3401b5b
5a7b685
a305562
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,39 @@ import { getConfig } from '@/config'; | |
| import { useVeChainKitConfig } from '@/providers'; | ||
| import { useCallback } from 'react'; | ||
|
|
||
| /** | ||
| * Safety multiplier applied on top of the locally-estimated gas to absorb | ||
| * variance between simulation and on-chain execution. Mirrors VeWorld | ||
| * mobile's heuristic. | ||
| */ | ||
| export const GENERIC_DELEGATOR_GAS_SAFETY_MULTIPLIER = 1.1; | ||
|
|
||
| /** | ||
| * Fixed gas cost for the extra transfer clause that pays the generic | ||
| * delegator's deposit account. VET transfers are bare value transfers | ||
| * (~21k), ERC-20 transfers (VTHO, B3TR) are ~50-55k depending on the | ||
| * recipient cold/warm state. | ||
| */ | ||
| export const GENERIC_DELEGATOR_FEE_PAYER_OVERHEAD_GAS: Record<GasTokenType, number> = { | ||
| VET: 21_000, | ||
| VTHO: 55_000, | ||
| B3TR: 55_000, | ||
| }; | ||
|
|
||
| /** | ||
| * Gas overhead added on top of the raw user-clause estimate to account for | ||
| * the smart-account `executeWithAuthorization` / `executeBatchWithAuthorization` | ||
| * wrapper (signature verification, calldata decoding, per-clause dispatch). | ||
| */ | ||
| export const GENERIC_DELEGATOR_WRAPPER_OVERHEAD_GAS: Record<number, number> = { | ||
| 1: 80_000, | ||
| 3: 120_000, | ||
| }; | ||
|
|
||
| const getWrapperOverheadGas = (version: number): number => | ||
| GENERIC_DELEGATOR_WRAPPER_OVERHEAD_GAS[version] ?? | ||
| GENERIC_DELEGATOR_WRAPPER_OVERHEAD_GAS[3]; | ||
|
|
||
| export const estimateGas = async ( | ||
| signerAddress: string, | ||
| genericDelegatorUrl: string, | ||
|
|
@@ -100,6 +133,162 @@ export const estimateAndBuildTxBody = async ( | |
| ); | ||
| }; | ||
|
|
||
| /** | ||
| * Hard timeout (ms) applied to the local Thor gas estimation. Stops the | ||
| * fee-estimation UI from hanging if the node is slow or unreachable. | ||
| */ | ||
| export const GENERIC_DELEGATOR_LOCAL_ESTIMATE_TIMEOUT_MS = 6_000; | ||
|
|
||
| const withTimeout = <T>( | ||
| promise: Promise<T>, | ||
| ms: number, | ||
| ): Promise<T | null> => | ||
| new Promise<T | null>((resolve) => { | ||
| const timer = setTimeout(() => resolve(null), ms); | ||
| promise | ||
| .then((value) => { | ||
| clearTimeout(timer); | ||
| resolve(value); | ||
| }) | ||
| .catch(() => { | ||
| clearTimeout(timer); | ||
| resolve(null); | ||
| }); | ||
| }); | ||
|
|
||
| /** | ||
| * Run the local Thor gas estimation for the user's raw clauses (caller = | ||
| * smart account) and return the gas-token-agnostic total: raw gas + wrapper | ||
| * overhead, padded by the safety multiplier. Returns `null` if the | ||
| * simulation reverts, times out, or returns a non-positive number — the | ||
| * caller should then fall back to a delegator-derived estimate. | ||
| * | ||
| * The output is independent of the gas token, so callers iterating over | ||
| * a token-priority list should call this once and reuse the result. | ||
| */ | ||
| export const computeCorrectedTotalGasNoFeePayer = async ({ | ||
| thor, | ||
| clauses, | ||
| smartAccountAddress, | ||
| version, | ||
| timeoutMs = GENERIC_DELEGATOR_LOCAL_ESTIMATE_TIMEOUT_MS, | ||
| }: { | ||
| thor: ThorClient; | ||
| clauses: TransactionClause[]; | ||
| smartAccountAddress: string; | ||
| version: number; | ||
| timeoutMs?: number; | ||
| }): Promise<number | null> => { | ||
| const rawGasResult = await withTimeout( | ||
| thor.gas.estimateGas(clauses, smartAccountAddress), | ||
| timeoutMs, | ||
| ); | ||
|
|
||
| const rawGas = rawGasResult?.totalGas; | ||
| if (!rawGas || rawGas <= 0) { | ||
| return null; | ||
| } | ||
|
|
||
| const wrapperOverhead = getWrapperOverheadGas(version); | ||
| return Math.ceil( | ||
| (rawGas + wrapperOverhead) * GENERIC_DELEGATOR_GAS_SAFETY_MULTIPLIER, | ||
| ); | ||
| }; | ||
|
|
||
| /** | ||
| * Convert a gas number (without the fee-payer transfer overhead) into the | ||
| * gas-token amount required to cover the transaction, using the per-gas | ||
| * rate returned by the delegator's `/estimate/clauses` response (which is | ||
| * accurate even when the absolute gas number from the same response is | ||
| * not). Adds the gas-token-specific fee-payer transfer overhead. | ||
| */ | ||
| export const convertGasToGasTokenAmount = ({ | ||
| totalGasNoFeePayer, | ||
| gasToken, | ||
| estimationResponse, | ||
| }: { | ||
| totalGasNoFeePayer: number; | ||
| gasToken: GasTokenType; | ||
| estimationResponse: EstimationResponse; | ||
| }): number => { | ||
| const totalGas = | ||
| totalGasNoFeePayer + | ||
| GENERIC_DELEGATOR_FEE_PAYER_OVERHEAD_GAS[gasToken]; | ||
|
|
||
| let gasTokenPerGas = 0; | ||
| if ( | ||
| estimationResponse.transactionCost && | ||
| estimationResponse.estimatedGas && | ||
| estimationResponse.estimatedGas > 0 | ||
| ) { | ||
| gasTokenPerGas = | ||
| estimationResponse.transactionCost / | ||
| estimationResponse.estimatedGas; | ||
| } else if (estimationResponse.vthoPerGasAtSpeed) { | ||
| const rate = estimationResponse.rate ?? 1; | ||
| const serviceFee = estimationResponse.serviceFee ?? 0; | ||
| gasTokenPerGas = | ||
| estimationResponse.vthoPerGasAtSpeed * rate * (1 + serviceFee); | ||
| } | ||
|
|
||
| if (!gasTokenPerGas || gasTokenPerGas <= 0) { | ||
| return 0; | ||
| } | ||
|
|
||
| return totalGas * gasTokenPerGas; | ||
| }; | ||
|
|
||
| /** | ||
| * Compute the gas-token amount the smart account must transfer to the | ||
| * generic delegator's deposit account to cover the transaction. | ||
| * | ||
| * The delegator's `/estimate/clauses` endpoint simulates the user's raw | ||
| * clauses as if executed directly by the smart account, with no | ||
| * `executeWithAuthorization` wrapper and no embedded-wallet signature, so | ||
| * it under-estimates (and for NFT-heavy clauses can revert outright). We | ||
| * trust its **rate** information (the gas-token-per-gas ratio is just a | ||
| * market price and doesn't depend on the gas amount) but recompute the | ||
| * gas number locally — including the wrapper overhead, fee-payer overhead, | ||
| * and a 10% safety multiplier — and reapply the rate. | ||
| */ | ||
| export const computeCorrectedGasTokenCost = async ({ | ||
| thor, | ||
| clauses, | ||
| smartAccountAddress, | ||
| version, | ||
| estimationResponse, | ||
| gasToken, | ||
| timeoutMs, | ||
| }: { | ||
| thor: ThorClient; | ||
| clauses: TransactionClause[]; | ||
| smartAccountAddress: string; | ||
| version: number; | ||
| estimationResponse: EstimationResponse; | ||
| gasToken: GasTokenType; | ||
| timeoutMs?: number; | ||
| }): Promise<number> => { | ||
| const fallbackCost = (estimationResponse.transactionCost ?? 0) * 2; | ||
|
|
||
| const totalGasNoFeePayer = await computeCorrectedTotalGasNoFeePayer({ | ||
| thor, | ||
| clauses, | ||
| smartAccountAddress, | ||
| version, | ||
| timeoutMs, | ||
| }); | ||
| if (totalGasNoFeePayer === null) { | ||
| return fallbackCost; | ||
| } | ||
|
|
||
| const cost = convertGasToGasTokenAmount({ | ||
| totalGasNoFeePayer, | ||
| gasToken, | ||
| estimationResponse, | ||
| }); | ||
| return cost > 0 ? cost : fallbackCost; | ||
| }; | ||
|
Comment on lines
+271
to
+290
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fail fast when fallback cost is non-positive.
Proposed fix export const computeCorrectedGasTokenCost = async ({
@@
}): Promise<number> => {
const fallbackCost = (estimationResponse.transactionCost ?? 0) * 2;
@@
if (totalGasNoFeePayer === null) {
- return fallbackCost;
+ if (fallbackCost > 0) return fallbackCost;
+ throw new Error('Unable to compute a positive fallback gas-token cost');
}
@@
const cost = convertGasToGasTokenAmount({
totalGasNoFeePayer,
gasToken,
estimationResponse,
});
- return cost > 0 ? cost : fallbackCost;
+ if (cost > 0) return cost;
+ if (fallbackCost > 0) return fallbackCost;
+ throw new Error('Unable to compute a positive gas-token cost');
};As per coding guidelines, "Always validate inputs in transaction and contract interaction functions - check address validity with isAddress() and ensure amounts are positive using BigInt comparisons". 🤖 Prompt for AI Agents |
||
|
|
||
| /** | ||
| * Sign the final transaction with the given private key and signature | ||
| * returned by the generic delegator. | ||
|
|
@@ -155,18 +344,41 @@ export const useGenericDelegator = () => { | |
| }): Promise<string> => { | ||
| try { | ||
| const gasToken = preferences.gasTokenToUse; | ||
| const gasEstimationResponse: EstimationResponse = await estimateGas(smartAccount?.address ?? '', genericDelegatorUrl, clauses as TransactionClause[], gasToken, 'medium'); | ||
| const gasEstimationResponse: EstimationResponse = await estimateGas( | ||
| smartAccount?.address ?? '', | ||
| genericDelegatorUrl, | ||
| clauses as TransactionClause[], | ||
| gasToken, | ||
| 'medium', | ||
| ); | ||
|
|
||
| const depositAccount: DepositAccount = await getDepositAccount(genericDelegatorUrl); | ||
|
|
||
| const correctedTransactionCost = await computeCorrectedGasTokenCost({ | ||
| thor, | ||
| clauses, | ||
| smartAccountAddress: smartAccount?.address ?? '', | ||
| version: smartAccountVersion?.version ?? 0, | ||
| estimationResponse: gasEstimationResponse, | ||
| gasToken, | ||
| }); | ||
|
|
||
| const transferAmountWei = parseEther( | ||
| correctedTransactionCost.toString(), | ||
| ).toString(); | ||
|
|
||
| const transferToGenericDelegatorClause = { | ||
| to: gasToken === 'VET' ? depositAccount.depositAccount : SUPPORTED_GAS_TOKENS[gasToken as GasTokenType].address, | ||
| value: gasToken === 'VET' ? parseEther(gasEstimationResponse.transactionCost?.toString() ?? '0').toString() : '0x0', | ||
| data: gasToken === 'VET' ? '0x' : ERC20Interface.encodeFunctionData('transfer', [ | ||
| depositAccount.depositAccount, | ||
| parseEther(gasEstimationResponse.transactionCost?.toString() ?? '0'), | ||
| ]), | ||
| comment: `Transfer ${gasEstimationResponse.transactionCost} ${gasToken} to ${depositAccount.depositAccount}`, | ||
| to: gasToken === 'VET' | ||
| ? depositAccount.depositAccount | ||
| : SUPPORTED_GAS_TOKENS[gasToken as GasTokenType].address, | ||
| value: gasToken === 'VET' ? transferAmountWei : '0x0', | ||
| data: gasToken === 'VET' | ||
| ? '0x' | ||
| : ERC20Interface.encodeFunctionData('transfer', [ | ||
| depositAccount.depositAccount, | ||
| transferAmountWei, | ||
| ]), | ||
| comment: `Transfer ${correctedTransactionCost} ${gasToken} to ${depositAccount.depositAccount}`, | ||
| abi: gasToken === 'VET' ? undefined : ERC20Interface.getFunction('transfer'), | ||
| }; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -29,7 +29,19 @@ export const useGenericDelegatorFeeEstimation = ({ | |||||||||
| const { updatePreferences } = useGasTokenSelection(); | ||||||||||
| // Only include essential data in query key to prevent unnecessary refetches | ||||||||||
| const queryKey = ['gas-estimation', JSON.stringify(clauses), JSON.stringify(tokens), sendingAmount, sendingTokenSymbol]; | ||||||||||
|
Comment on lines
46
to
47
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Query key missing Similar to Proposed fix- const queryKey = ['gas-estimation', JSON.stringify(clauses), JSON.stringify(tokens), sendingAmount, sendingTokenSymbol];
+ const queryKey = ['gas-estimation', JSON.stringify(clauses), JSON.stringify(tokens), sendingAmount, sendingTokenSymbol, smartAccountVersion?.version];📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||
|
|
||||||||||
|
|
||||||||||
| // eslint-disable-next-line no-console | ||||||||||
| console.log('[vk-debug] useGenericDelegatorFeeEstimation inputs:', { | ||||||||||
| callerEnabled: enabled, | ||||||||||
| clausesLen: clauses.length, | ||||||||||
| accountAddress: account?.address, | ||||||||||
| smartAccountAddress: smartAccount?.address, | ||||||||||
| genericDelegatorUrl: feeDelegation?.genericDelegatorUrl, | ||||||||||
| delegatorUrl: feeDelegation?.delegatorUrl, | ||||||||||
| tokensLen: tokens.length, | ||||||||||
| balancesLen: balances.length, | ||||||||||
| }); | ||||||||||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||||||||||
|
|
||||||||||
| return useQuery<EstimationResponse & { usedToken: string }, Error>({ | ||||||||||
| queryKey, | ||||||||||
| queryFn: async () => { | ||||||||||
|
|
||||||||||
Uh oh!
There was an error while loading. Please reload this page.