-
Notifications
You must be signed in to change notification settings - Fork 5.5k
fix: swap overestimates gas on EVM networks cp-13.13.0 #38642
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 all commits
afeda72
2a89e9c
a6a078c
0f49812
8640b45
2281f02
198a5e9
0f7d3bb
4c29ae4
40a2781
c7df723
036db5b
e72c0b8
f7a5fa7
f80b03b
37d5370
01eb8be
7d50dbf
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 |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| diff --git a/dist/selectors.cjs b/dist/selectors.cjs | ||
| index 3afe13e4fc960c87f98c85d323dd992c5d5ac7f2..0d585efbbd5043ae092a1a8cb497f772ebd4b548 100644 | ||
| --- a/dist/selectors.cjs | ||
| +++ b/dist/selectors.cjs | ||
| @@ -135,8 +135,8 @@ exports.selectIsAssetExchangeRateInState = selectIsAssetExchangeRateInState; | ||
| */ | ||
| const selectBridgeFeesPerGas = createStructuredBridgeSelector({ | ||
| estimatedBaseFeeInDecGwei: ({ gasFeeEstimates }) => gasFeeEstimates?.estimatedBaseFee, | ||
| - maxPriorityFeePerGasInDecGwei: ({ gasFeeEstimates }) => gasFeeEstimates?.[bridge_1.BRIDGE_PREFERRED_GAS_ESTIMATE] | ||
| - ?.suggestedMaxPriorityFeePerGas, | ||
| + feePerGasInDecGwei: ({ gasFeeEstimates }) => gasFeeEstimates?.[bridge_1.BRIDGE_PREFERRED_GAS_ESTIMATE] | ||
| + ?.suggestedMaxFeePerGas, | ||
| maxFeePerGasInDecGwei: ({ gasFeeEstimates }) => gasFeeEstimates?.high?.suggestedMaxFeePerGas, | ||
| }); | ||
| // Selects cross-chain swap quotes including their metadata | ||
| diff --git a/dist/utils/quote.cjs b/dist/utils/quote.cjs | ||
| index f09b7c3e34ca626c93a80cf7c1a2b528d5e07e3f..d9929ff35b0e9534e01ee580c94bf2bae1345f0d 100644 | ||
| --- a/dist/utils/quote.cjs | ||
| +++ b/dist/utils/quote.cjs | ||
| @@ -130,14 +130,13 @@ const calcRelayerFee = (quoteResponse, { exchangeRate, usdExchangeRate }) => { | ||
| }; | ||
| }; | ||
| exports.calcRelayerFee = calcRelayerFee; | ||
| -const calcTotalGasFee = ({ approvalGasLimit, resetApprovalGasLimit, tradeGasLimit, l1GasFeesInHexWei, feePerGasInDecGwei, priorityFeePerGasInDecGwei, nativeToDisplayCurrencyExchangeRate, nativeToUsdExchangeRate, }) => { | ||
| +const calcTotalGasFee = ({ approvalGasLimit, resetApprovalGasLimit, tradeGasLimit, l1GasFeesInHexWei, feePerGasInDecGwei, nativeToDisplayCurrencyExchangeRate, nativeToUsdExchangeRate, }) => { | ||
| const totalGasLimitInDec = new bignumber_js_1.BigNumber(tradeGasLimit?.toString() ?? '0') | ||
| .plus(approvalGasLimit?.toString() ?? '0') | ||
| .plus(resetApprovalGasLimit?.toString() ?? '0'); | ||
| - const totalFeePerGasInDecGwei = new bignumber_js_1.BigNumber(feePerGasInDecGwei).plus(priorityFeePerGasInDecGwei); | ||
| const l1GasFeesInDecGWei = (0, controller_utils_1.weiHexToGweiDec)((0, controller_utils_1.toHex)(l1GasFeesInHexWei ?? '0')); | ||
| const gasFeesInDecGwei = totalGasLimitInDec | ||
| - .times(totalFeePerGasInDecGwei) | ||
| + .times(feePerGasInDecGwei) | ||
| .plus(l1GasFeesInDecGWei); | ||
| const gasFeesInDecEth = gasFeesInDecGwei.times(new bignumber_js_1.BigNumber(10).pow(-9)); | ||
| const gasFeesInDisplayCurrency = nativeToDisplayCurrencyExchangeRate | ||
| @@ -152,7 +151,7 @@ const calcTotalGasFee = ({ approvalGasLimit, resetApprovalGasLimit, tradeGasLimi | ||
| usd: gasFeesInUSD?.toString() ?? null, | ||
| }; | ||
| }; | ||
| -const calcEstimatedAndMaxTotalGasFee = ({ bridgeQuote: { approval, trade, l1GasFeesInHexWei, resetApproval }, estimatedBaseFeeInDecGwei, maxFeePerGasInDecGwei, maxPriorityFeePerGasInDecGwei, exchangeRate: nativeToDisplayCurrencyExchangeRate, usdExchangeRate: nativeToUsdExchangeRate, }) => { | ||
| +const calcEstimatedAndMaxTotalGasFee = ({ bridgeQuote: { approval, trade, l1GasFeesInHexWei, resetApproval }, estimatedBaseFeeInDecGwei, maxFeePerGasInDecGwei, feePerGasInDecGwei, exchangeRate: nativeToDisplayCurrencyExchangeRate, usdExchangeRate: nativeToUsdExchangeRate, }) => { | ||
| // Estimated gas fees spent after receiving refunds, this is shown to the user | ||
| const { amount: amountEffective, valueInCurrency: valueInCurrencyEffective, usd: usdEffective, } = calcTotalGasFee({ | ||
| // Fallback to gasLimit if effectiveGas is not available | ||
| @@ -160,8 +159,7 @@ const calcEstimatedAndMaxTotalGasFee = ({ bridgeQuote: { approval, trade, l1GasF | ||
| resetApprovalGasLimit: resetApproval?.effectiveGas ?? resetApproval?.gasLimit, | ||
| tradeGasLimit: trade?.effectiveGas ?? trade?.gasLimit, | ||
| l1GasFeesInHexWei, | ||
| - feePerGasInDecGwei: estimatedBaseFeeInDecGwei, | ||
| - priorityFeePerGasInDecGwei: maxPriorityFeePerGasInDecGwei, | ||
| + feePerGasInDecGwei: feePerGasInDecGwei, | ||
| nativeToDisplayCurrencyExchangeRate, | ||
| nativeToUsdExchangeRate, | ||
| }); | ||
| @@ -171,8 +169,7 @@ const calcEstimatedAndMaxTotalGasFee = ({ bridgeQuote: { approval, trade, l1GasF | ||
| resetApprovalGasLimit: resetApproval?.gasLimit, | ||
| tradeGasLimit: trade?.gasLimit, | ||
| l1GasFeesInHexWei, | ||
| - feePerGasInDecGwei: estimatedBaseFeeInDecGwei, | ||
| - priorityFeePerGasInDecGwei: maxPriorityFeePerGasInDecGwei, | ||
| + feePerGasInDecGwei: feePerGasInDecGwei, | ||
| nativeToDisplayCurrencyExchangeRate, | ||
| nativeToUsdExchangeRate, | ||
| }); | ||
| @@ -183,7 +180,6 @@ const calcEstimatedAndMaxTotalGasFee = ({ bridgeQuote: { approval, trade, l1GasF | ||
| tradeGasLimit: trade?.gasLimit, | ||
| l1GasFeesInHexWei, | ||
| feePerGasInDecGwei: maxFeePerGasInDecGwei, | ||
| - priorityFeePerGasInDecGwei: maxPriorityFeePerGasInDecGwei, | ||
| nativeToDisplayCurrencyExchangeRate, | ||
| nativeToUsdExchangeRate, | ||
| }); | ||
| @@ -214,7 +210,7 @@ exports.calcEstimatedAndMaxTotalGasFee = calcEstimatedAndMaxTotalGasFee; | ||
| * @param relayerFee - The relayer fee paid to bridge providers | ||
| * @returns The total estimated network fee for the bridge transaction, including the relayer fee paid to bridge providers | ||
| */ | ||
| -const calcTotalEstimatedNetworkFee = ({ effective: gasFeeToDisplay, }, relayerFee) => { | ||
| +const calcTotalEstimatedNetworkFee = ({ total: gasFeeToDisplay, }, relayerFee) => { | ||
|
Member
Author
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. Changes |
||
| return { | ||
| amount: new bignumber_js_1.BigNumber(gasFeeToDisplay?.amount ?? '0') | ||
| .plus(relayerFee.amount) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -945,35 +945,6 @@ | |
| "uuid": true | ||
| } | ||
| }, | ||
| "@metamask/transaction-pay-controller>@metamask/bridge-controller": { | ||
| "globals": { | ||
| "AbortController": true, | ||
| "TextDecoder": true, | ||
| "URLSearchParams": true, | ||
| "console.error": true, | ||
| "console.log": true, | ||
| "console.warn": true | ||
|
Comment on lines
-948
to
-955
Member
Author
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. @MetaMask/policy-reviewers these policies were removed after I patched the bridge-controller. There are no breaking changes from bridge-controller v63 to v64 that affect the transaction-pay controller |
||
| }, | ||
| "packages": { | ||
| "ethers>@ethersproject/address": true, | ||
| "ethers>@ethersproject/constants": true, | ||
| "@ethersproject/contracts": true, | ||
| "@ethersproject/providers": true, | ||
| "@metamask/controller-utils": true, | ||
| "@metamask/keyring-api": true, | ||
| "@metamask/metamask-eth-abis": true, | ||
| "@metamask/transaction-pay-controller>@metamask/bridge-controller>@metamask/multichain-network-controller": true, | ||
| "@metamask/transaction-pay-controller>@metamask/bridge-controller>@metamask/polling-controller": true, | ||
| "@metamask/superstruct": true, | ||
| "@metamask/transaction-controller": true, | ||
| "@metamask/utils": true, | ||
| "@metamask/transaction-pay-controller>bignumber.js": true, | ||
| "browserify>buffer": true, | ||
| "lodash": true, | ||
| "reselect": true, | ||
| "uuid": true | ||
| } | ||
| }, | ||
| "@metamask/bridge-status-controller": { | ||
| "globals": { | ||
| "URLSearchParams": true, | ||
|
|
@@ -1622,19 +1593,6 @@ | |
| "lodash": true | ||
| } | ||
| }, | ||
| "@metamask/transaction-pay-controller>@metamask/bridge-controller>@metamask/multichain-network-controller": { | ||
| "globals": { | ||
| "URL": true | ||
| }, | ||
| "packages": { | ||
| "@metamask/base-controller": true, | ||
| "@metamask/keyring-api": true, | ||
| "@metamask/transaction-pay-controller>@metamask/network-controller": true, | ||
| "@metamask/superstruct": true, | ||
| "@metamask/utils": true, | ||
| "lodash": true | ||
| } | ||
| }, | ||
| "@metamask/multichain-transactions-controller": { | ||
| "globals": { | ||
| "console.error": true | ||
|
|
@@ -1684,33 +1642,6 @@ | |
| "uuid": true | ||
| } | ||
| }, | ||
| "@metamask/transaction-pay-controller>@metamask/network-controller": { | ||
| "globals": { | ||
| "Intl.NumberFormat": true, | ||
| "URL": true, | ||
| "setTimeout": true | ||
| }, | ||
| "packages": { | ||
| "@metamask/base-controller": true, | ||
| "@metamask/controller-utils": true, | ||
| "@metamask/network-controller>@metamask/eth-block-tracker": true, | ||
| "@metamask/network-controller>@metamask/eth-json-rpc-infura": true, | ||
| "@metamask/eth-json-rpc-middleware": true, | ||
| "@metamask/eth-json-rpc-provider": true, | ||
| "@metamask/controller-utils>@metamask/eth-query": true, | ||
| "@metamask/json-rpc-engine": true, | ||
| "@metamask/rpc-errors": true, | ||
| "@metamask/network-controller>@metamask/swappable-obj-proxy": true, | ||
| "@metamask/utils": true, | ||
| "addons-linter>deepmerge": true, | ||
| "eslint>fast-deep-equal": true, | ||
| "immer": true, | ||
| "lodash": true, | ||
| "reselect": true, | ||
| "uri-js": true, | ||
| "uuid": true | ||
| } | ||
| }, | ||
| "@metamask/network-enablement-controller": { | ||
| "packages": { | ||
| "@metamask/base-controller": true, | ||
|
|
@@ -1838,18 +1769,6 @@ | |
| "uuid": true | ||
| } | ||
| }, | ||
| "@metamask/transaction-pay-controller>@metamask/bridge-controller>@metamask/polling-controller": { | ||
| "globals": { | ||
| "clearTimeout": true, | ||
| "console.error": true, | ||
| "setTimeout": true | ||
| }, | ||
| "packages": { | ||
| "@metamask/base-controller": true, | ||
| "@metamask/snaps-utils>fast-json-stable-stringify": true, | ||
| "uuid": true | ||
| } | ||
| }, | ||
| "@metamask/gas-fee-controller>@metamask/polling-controller": { | ||
| "globals": { | ||
| "clearTimeout": true, | ||
|
|
@@ -2365,7 +2284,7 @@ | |
| "packages": { | ||
| "@ethersproject/abi": true, | ||
| "@metamask/base-controller": true, | ||
| "@metamask/transaction-pay-controller>@metamask/bridge-controller": true, | ||
| "@metamask/bridge-controller": true, | ||
| "@metamask/controller-utils": true, | ||
| "@metamask/metamask-eth-abis": true, | ||
| "@metamask/transaction-controller": true, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixes medium fee per gas used for calculations