From ea88b32821926ded9035014a42ca8b232b3da068 Mon Sep 17 00:00:00 2001 From: dekanbro Date: Tue, 7 Nov 2023 15:40:15 -0700 Subject: [PATCH 1/3] better gas estimation by incorporating estimated overhead for each action --- libs/tx-builder/src/utils/constants.ts | 2 +- libs/tx-builder/src/utils/multicall.ts | 27 +++++++++++++++++++++----- libs/utils/src/constants/proposals.ts | 8 ++++++-- libs/utils/src/utils/gas.ts | 2 +- 4 files changed, 30 insertions(+), 9 deletions(-) diff --git a/libs/tx-builder/src/utils/constants.ts b/libs/tx-builder/src/utils/constants.ts index c73cf36a..2d07f88e 100644 --- a/libs/tx-builder/src/utils/constants.ts +++ b/libs/tx-builder/src/utils/constants.ts @@ -4,7 +4,7 @@ import { LOCAL_ABI } from '@daohaus/abis'; export const EXPIRY = '.proposalExpiry'; export const FORM = '.formValues'; export const CURRENT_DAO = '.daoId'; -export const gasBufferMultiplier = 5; +export const gasBufferMultiplier = 1.2; // buffers baalgas estimate export const BaalContractBase = { type: 'local', contractName: 'Baal', diff --git a/libs/tx-builder/src/utils/multicall.ts b/libs/tx-builder/src/utils/multicall.ts index e0cfc4b7..76508468 100644 --- a/libs/tx-builder/src/utils/multicall.ts +++ b/libs/tx-builder/src/utils/multicall.ts @@ -14,6 +14,7 @@ import { MulticallArg, StringSearch, TXLego, + ACTION_GAS_LIMIT_ADDITION, } from '@daohaus/utils'; import { CONTRACT_KEYCHAINS, @@ -39,17 +40,19 @@ import { createViemClient } from '@daohaus/utils'; export const estimateFunctionalGas = async ({ chainId, - constractAddress, + contractAddress, from, value, data, + actionsCount, rpcs = HAUS_RPC, }: { chainId: ValidNetwork; - constractAddress: string; + contractAddress: string; from: string; value: bigint; data: string; + actionsCount: number; rpcs?: Keychain; }): Promise => { const client = createViemClient({ @@ -59,12 +62,20 @@ export const estimateFunctionalGas = async ({ const functionGasFees = await client.estimateGas({ account: from as EthAddress, - to: constractAddress as EthAddress, + to: contractAddress as EthAddress, value, data: data as `0x${string}`, }); - return Number(functionGasFees); + console.log('functionGasFees', functionGasFees) + + // extra gas overhead when calling the dao from the baal safe + console.log('contractAddress', contractAddress); + console.log('from', from); + const baalOnlyGas = actionsCount * ACTION_GAS_LIMIT_ADDITION; + console.log('baalOnlyGas', baalOnlyGas); + console.log('functionGasFees', functionGasFees) + return Number(functionGasFees + BigInt(baalOnlyGas)); }; export const txActionToMetaTx = ({ @@ -284,23 +295,27 @@ export const handleMulticallArg = async ({ export const gasEstimateFromActions = async ({ actions, + actionsCount, chainId, daoId, }: { actions: MetaTransaction[]; + actionsCount: number; chainId: ValidNetwork; daoId: string; safeId: string; // not used at the moment }) => { + const esitmatedGases = await Promise.all( actions.map( async (action) => await estimateFunctionalGas({ chainId: chainId, - constractAddress: action.to, + contractAddress: action.to, from: daoId, // from value needs to be the safe module (baal) to estimate without revert value: BigInt(Number(action.value)), data: action.data, + actionsCount }) ) ); @@ -386,12 +401,14 @@ export const handleGasEstimate = async ({ } as MetaTransaction; const gasEstimate = await gasEstimateFromActions({ actions: encodeExecFromModule({ safeId, metaTx }), + actionsCount: actions.length, chainId, daoId, safeId, }); if (gasEstimate) { + // adds buffer to baalgas estimate const buffer = arg.bufferPercentage || gasBufferMultiplier; return Math.round(Number(gasEstimate) * Number(buffer)); } else { diff --git a/libs/utils/src/constants/proposals.ts b/libs/utils/src/constants/proposals.ts index 21c578b1..8497ea64 100644 --- a/libs/utils/src/constants/proposals.ts +++ b/libs/utils/src/constants/proposals.ts @@ -105,8 +105,12 @@ export const PROPOSAL_FILTERS: Record = { failed: 'Defeated', expired: 'Expired', }; - -export const GAS_BUFFER_MULTIPLIER = 5; +// Processing gas estimate buffer +export const GAS_BUFFER_MULTIPLIER = 2; // Adding to the gas limit to account for cost of processProposal export const PROCESS_PROPOSAL_GAS_LIMIT_ADDITION = 150000; +// Adding to the gas limit to account for cost of each action +export const ACTION_GAS_LIMIT_ADDITION = 150000; + export const L2_ADDITIONAL_GAS = 5000000; + diff --git a/libs/utils/src/utils/gas.ts b/libs/utils/src/utils/gas.ts index 8cfd470e..4d6ef387 100644 --- a/libs/utils/src/utils/gas.ts +++ b/libs/utils/src/utils/gas.ts @@ -16,7 +16,7 @@ export const getGasCostEstimate = async ( ): Promise => { const feeDataNew = await fetchFeeData({ chainId: chainId as ValidNetwork }); - console.log('feeDataNew', feeDataNew); + // console.log('feeDataNew', feeDataNew); return ( Number(getProcessingGasLimit(gasLimit, chainId)) * Number(feeDataNew.maxFeePerGas || 0) From e53f828a3ebe4dd0533d6cdcf01c096016af9ced Mon Sep 17 00:00:00 2001 From: dekanbro Date: Wed, 8 Nov 2023 10:53:56 -0700 Subject: [PATCH 2/3] baalOnlyGas addition to the end of the gasEstimateFromActions function instead of estimateFunctionalGas --- libs/tx-builder/src/utils/multicall.ts | 23 +++++++++-------------- libs/utils/src/constants/proposals.ts | 3 +-- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/libs/tx-builder/src/utils/multicall.ts b/libs/tx-builder/src/utils/multicall.ts index 76508468..c470722b 100644 --- a/libs/tx-builder/src/utils/multicall.ts +++ b/libs/tx-builder/src/utils/multicall.ts @@ -44,7 +44,6 @@ export const estimateFunctionalGas = async ({ from, value, data, - actionsCount, rpcs = HAUS_RPC, }: { chainId: ValidNetwork; @@ -52,7 +51,6 @@ export const estimateFunctionalGas = async ({ from: string; value: bigint; data: string; - actionsCount: number; rpcs?: Keychain; }): Promise => { const client = createViemClient({ @@ -67,15 +65,9 @@ export const estimateFunctionalGas = async ({ data: data as `0x${string}`, }); - console.log('functionGasFees', functionGasFees) + console.log('functionGasFees', functionGasFees); - // extra gas overhead when calling the dao from the baal safe - console.log('contractAddress', contractAddress); - console.log('from', from); - const baalOnlyGas = actionsCount * ACTION_GAS_LIMIT_ADDITION; - console.log('baalOnlyGas', baalOnlyGas); - console.log('functionGasFees', functionGasFees) - return Number(functionGasFees + BigInt(baalOnlyGas)); + return Number(functionGasFees); }; export const txActionToMetaTx = ({ @@ -305,7 +297,6 @@ export const gasEstimateFromActions = async ({ daoId: string; safeId: string; // not used at the moment }) => { - const esitmatedGases = await Promise.all( actions.map( async (action) => @@ -314,8 +305,7 @@ export const gasEstimateFromActions = async ({ contractAddress: action.to, from: daoId, // from value needs to be the safe module (baal) to estimate without revert value: BigInt(Number(action.value)), - data: action.data, - actionsCount + data: action.data }) ) ); @@ -325,9 +315,14 @@ export const gasEstimateFromActions = async ({ (a, b) => (a || 0) + (b || 0), 0 ); + + + // extra gas overhead when calling the dao from the baal safe + const baalOnlyGas = actionsCount * ACTION_GAS_LIMIT_ADDITION; + console.log('totalGasEstimate', totalGasEstimate); - return totalGasEstimate; + return (totalGasEstimate || 0) + baalOnlyGas; }; export const handleEncodeMulticallArg = async ({ diff --git a/libs/utils/src/constants/proposals.ts b/libs/utils/src/constants/proposals.ts index 8497ea64..6f025533 100644 --- a/libs/utils/src/constants/proposals.ts +++ b/libs/utils/src/constants/proposals.ts @@ -110,7 +110,6 @@ export const GAS_BUFFER_MULTIPLIER = 2; // Adding to the gas limit to account for cost of processProposal export const PROCESS_PROPOSAL_GAS_LIMIT_ADDITION = 150000; // Adding to the gas limit to account for cost of each action -export const ACTION_GAS_LIMIT_ADDITION = 150000; +export const ACTION_GAS_LIMIT_ADDITION = 150000; export const L2_ADDITIONAL_GAS = 5000000; - From c810678da47bce52425ec36db3b7b20cebe60c32 Mon Sep 17 00:00:00 2001 From: dekanbro Date: Thu, 9 Nov 2023 15:40:07 -0700 Subject: [PATCH 3/3] address review issues --- libs/tx-builder/src/utils/multicall.ts | 5 ++--- libs/utils/src/utils/gas.ts | 1 - 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/libs/tx-builder/src/utils/multicall.ts b/libs/tx-builder/src/utils/multicall.ts index c470722b..850894ae 100644 --- a/libs/tx-builder/src/utils/multicall.ts +++ b/libs/tx-builder/src/utils/multicall.ts @@ -305,7 +305,7 @@ export const gasEstimateFromActions = async ({ contractAddress: action.to, from: daoId, // from value needs to be the safe module (baal) to estimate without revert value: BigInt(Number(action.value)), - data: action.data + data: action.data, }) ) ); @@ -316,10 +316,9 @@ export const gasEstimateFromActions = async ({ 0 ); - // extra gas overhead when calling the dao from the baal safe const baalOnlyGas = actionsCount * ACTION_GAS_LIMIT_ADDITION; - + console.log('baalOnlyGas addtition', baalOnlyGas); console.log('totalGasEstimate', totalGasEstimate); return (totalGasEstimate || 0) + baalOnlyGas; diff --git a/libs/utils/src/utils/gas.ts b/libs/utils/src/utils/gas.ts index 4d6ef387..75094790 100644 --- a/libs/utils/src/utils/gas.ts +++ b/libs/utils/src/utils/gas.ts @@ -16,7 +16,6 @@ export const getGasCostEstimate = async ( ): Promise => { const feeDataNew = await fetchFeeData({ chainId: chainId as ValidNetwork }); - // console.log('feeDataNew', feeDataNew); return ( Number(getProcessingGasLimit(gasLimit, chainId)) * Number(feeDataNew.maxFeePerGas || 0)