instead of consuming estimateFees() in useGetEstimatedFeesQuery, use getFees() or getFeesWithWallet which is not hacked around sends, the current estimateFees abusing the memo field for calldata (and also missing EIP1559 fees estimation)
In other words, all 3 branches should have their fees estimated with the same CreateBuildCustomTxInputArgs (or similar for other chains) payload.
To get there, we will most likely want to decouple things into: createTxInput, which can be used both for estimating fees and for building/broadcasting. This already exists for EVM as createBuildCustomTxInput, so we'll need to build something similar for other chains```
Pretty much what this comment says. The current estimateFees is a relic of the past with a suboptimal API which often ends up in us getting burned, and fees estimation logic is disparate across the board:
While EVM chains have the newest, shiny getFees / getFeesWithWallet that estimates fees in a very typical JSON-RPC spec fashion (except for the adapter and supportsEIP1559 specificities, other chains still deal with estimateFees.
Furthermore, many (most?) places currently use estimateFees, meaning that we don't leverage the EIP-1559 support bits and are also quite prone to bugs related to its API.
Let's make estimateFees great again by removing it.
All chain leverage GetFeesArgs, with chain-specific discrimination handled gracefully (discriminated union, multiple types or whatever is the most elegant and type-safe option). Note we already have ChainSpecificGetFeeDataInput, used in the generic GetFeeDataInput so there may not be a need for a type change here
getFees is either duplicated (preferred while starting to dev this, though probably not the best option as a final version when opening the PR), or made to work with all chains
estimateFees is gone, and so are its usages in profit of getFees
Overview
(without the faster part, but better and safer still)
See https://github.com/shapeshift/web/pull/6700#discussion_r1569323927
Pretty much what this comment says. The current
estimateFees
is a relic of the past with a suboptimal API which often ends up in us getting burned, and fees estimation logic is disparate across the board: While EVM chains have the newest, shinygetFees
/getFeesWithWallet
that estimates fees in a very typical JSON-RPC spec fashion (except for theadapter
andsupportsEIP1559
specificities, other chains still deal withestimateFees
. Furthermore, many (most?) places currently useestimateFees
, meaning that we don't leverage the EIP-1559 support bits and are also quite prone to bugs related to its API.Let's make
estimateFees
great again by removing it.References and additional details
https://github.com/shapeshift/web/blob/d0e81fbbc32aa56f484bbd807798f43f95bec356/src/lib/utils/evm.ts#L94-L113 https://github.com/shapeshift/web/blob/d0e81fbbc32aa56f484bbd807798f43f95bec356/src/lib/utils/evm.ts#L151-L155
Acceptance Criteria
GetFeesArgs
, with chain-specific discrimination handled gracefully (discriminated union, multiple types or whatever is the most elegant and type-safe option). Note we already haveChainSpecificGetFeeDataInput
, used in the genericGetFeeDataInput
so there may not be a need for a type change heregetFees
is either duplicated (preferred while starting to dev this, though probably not the best option as a final version when opening the PR), or made to work with all chainsestimateFees
is gone, and so are its usages in profit ofgetFees
Need By Date
No response
Screenshots/Mockups
No response
Estimated effort
No response