Skip to content

Commit

Permalink
fix: Optimism quotes failing (#10687)
Browse files Browse the repository at this point in the history 
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

This PR fixes an issue in Swaps on Optimism where users were unable to
fetch quotes.

`fetchEstimatedMultiLayerL1Fee` is accessed in multiple places: Swaps,
Send, Dapp transactions.

I've made this PR backwards compatible with the behavior in previous
functioning versions of the app which is to return a non `0x` prefixed
hex string for the gas fee.

## **Related issues**

Fixes:

## **Manual testing steps**

There's a number of places `fetchEstimatedMultiLayerL1Fee` is used.

Swap from ETH to ERC20:
1. Go to Swaps
2. Request a quote for ETH > ERC20
3. Receive quotes

Swap from ERC20 that requires an allowance approval:
1. Go to Swaps
2. Request a quote for ERC20 (that requires an allowance approval) >
ERC20
3. Receive quotes

Send
1. Go to Send
1. Start a send flow (don't need to actually execute the send though)
2. No errors should show

Dapp tx
1. Go to a dapp
3. Start a tx (don't need to execute just need to go to the tx
confirmation screen)
4. No errors should show

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->


https://github.com/user-attachments/assets/b50d1c31-7756-495b-a351-e69ee71cd50d



## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
  • Loading branch information
@infiniteflower
infiniteflower authored Aug 30, 2024
1 parent 42b15d7 commit c05e7dd
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 423 deletions.
37 changes: 14 additions & 23 deletions app/util/networks/index.js
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,4 @@
import URL from 'url-parse';
import { utils } from 'ethers';
import EthContract from 'ethjs-contract';
import { getContractFactory } from '@eth-optimism/contracts/dist/contract-defs';
import { predeploys } from '@eth-optimism/contracts/dist/predeploys';
import networksWithImages from 'images/image-icons';
import {
MAINNET,
Expand All @@ -24,7 +20,6 @@ import { isStrictHexString } from '@metamask/utils';
import Engine from '../../core/Engine';
import { toLowerCaseEquals } from '../general';
import { fastSplit } from '../number';
import buildUnserializedTransaction from '../transactions/optimismTransaction';
import handleNetworkSwitch from './handleNetworkSwitch';
import { regex } from '../../../app/util/regex';

Expand Down Expand Up @@ -438,32 +433,28 @@ export const getNetworkImageSource = ({ networkType, chainId }) => {
return getTestNetImage(networkType);
};

// The code in this file is largely drawn from https://community.optimism.io/docs/developers/l2/new-fees.html#for-frontend-and-wallet-developers
const buildOVMGasPriceOracleContract = (eth) => {
const OVMGasPriceOracle = getContractFactory('OVM_GasPriceOracle').attach(
predeploys.OVM_GasPriceOracle,
);
const abi = JSON.parse(
OVMGasPriceOracle.interface.format(utils.FormatTypes.json),
);
const contract = new EthContract(eth);
return contract(abi).at(OVMGasPriceOracle.address);
};

/**
* It returns an estimated L1 fee for a multi layer network.
* Currently only for the Optimism network, but can be extended to other networks.
*
* @param {Object} eth
* @param {Object} txMeta
* @returns {String}
* @returns {String} Hex string gas fee, with no 0x prefix
*/
export const fetchEstimatedMultiLayerL1Fee = async (eth, txMeta) => {
const contract = buildOVMGasPriceOracleContract(eth);
const serializedTransaction =
buildUnserializedTransaction(txMeta).serialize();
const result = await contract.getL1Fee(serializedTransaction);
return result?.[0]?.toString(16);
const chainId = txMeta.chainId;

const layer1GasFee =
await Engine.context.TransactionController.getLayer1GasFee({
transactionParams: txMeta.txParams,
chainId,
});

const layer1GasFeeNoPrefix = layer1GasFee.startsWith('0x')
? layer1GasFee.slice(2)
: layer1GasFee;

return layer1GasFeeNoPrefix;
};

/**
Expand Down
24 changes: 24 additions & 0 deletions app/util/networks/index.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
getNetworkNonce,
convertNetworkId,
deprecatedGetNetworkId,
fetchEstimatedMultiLayerL1Fee,
} from '.';
import {
MAINNET,
Expand Down Expand Up @@ -45,6 +46,9 @@ jest.mock('./../../core/Engine', () => ({
PreferencesController: {
state: {},
},
TransactionController: {
getLayer1GasFee: jest.fn(async () => '0x0a25339d61'),
},
},
}));

Expand Down Expand Up @@ -414,4 +418,24 @@ describe('network-utils', () => {
);
});
});

describe('fetchEstimatedMultiLayerL1Fee', () => {
it('returns a non-0x prefixed hex string', async () => {
const txMeta = {
chainId: '0xa',
txParams: {
data: '0x5f57552900000000000000000000000000000000000000000000000000000000000000800000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000002386f26fc1000000000000000000000000000000000000000000000000000000000000000000c0000000000000000000000000000000000000000000000000000000000000000c307846656544796e616d69630000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000084000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000b2c639c533813f4aa9d7837caf62653d097ff850000000000000000000000000000000000000000000000000023375dc1560800000000000000000000000000000000000000000000000000000000000184ebd9000000000000000000000000000000000000000000000000000000000000012000000000000000000000000000000000000000000000000000004f94ae6af80000000000000000000000000045d047bfcb1055b9dd531ef9605e8f0b0dc285f300000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000708415565b0000000000000000000000000eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee0000000000000000000000000b2c639c533813f4aa9d7837caf62653d097ff850000000000000000000000000000000000000000000000000023375dc1560800000000000000000000000000000000000000000000000000000000000184ebd800000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000030000000000000000000000000000000000000000000000000000000000000060000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000004c0000000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000000000000000000040000000000000000000000000eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee0000000000000000000000000000000000000000000000000023375dc15608000000000000000000000000000000000000000000000000000000000000000011000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000003600000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000000000000000000000000000042000000000000000000000000000000000000060000000000000000000000000b2c639c533813f4aa9d7837caf62653d097ff8500000000000000000000000000000000000000000000000000000000000001400000000000000000000000000000000000000000000000000000000000000320000000000000000000000000000000000000000000000000000000000000032000000000000000000000000000000000000000000000000000000000000002e00000000000000000000000000000000000000000000000000023375dc1560800000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000003200000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000012556e69737761705633000000000000000000000000000000000000000000000000000000000000000023375dc1560800000000000000000000000000000000000000000000000000000000000184ebd8000000000000000000000000000000000000000000000000000000000000008000000000000000000000000000000000000000000000000000000000000000c0000000000000000000000000e592427a0aece92de3edee1f18e0157c0586156400000000000000000000000000000000000000000000000000000000000000600000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000002b42000000000000000000000000000000000000060001f40b2c639c533813f4aa9d7837caf62653d097ff85000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000b000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000e00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000020000000000000000000000004200000000000000000000000000000000000006000000000000000000000000eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee0000000000000000000000000000000000000000000000000000000000000000869584cd00000000000000000000000011ededebf63bef0ea2d2d071bdf88f71543ec6fb00000000000000000000000000000000000000000df057b339e721ee3d141aef0000000000000000000000000000000000000000000000000090',
from: '0xc5fe6ef47965741f6f7a4734bf784bf3ae3f2452',
gas: '0x1e583e',
to: '0x9dda6ef3d919c9bc8885d5560999a3640431e8e6',
value: '0x2386f26fc10000',
},
};

const layer1GasFee = await fetchEstimatedMultiLayerL1Fee({}, txMeta);

expect(layer1GasFee.startsWith('0x')).toEqual(false);
expect(layer1GasFee).toEqual('0a25339d61');
});
});
});
26 changes: 0 additions & 26 deletions app/util/transactions/index.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ import {
TOKEN_METHOD_APPROVE,
getTransactionReviewActionKey,
} from '.';
import buildUnserializedTransaction from './optimismTransaction';
import Engine from '../../core/Engine';
import { strings } from '../../../locales/i18n';

Expand Down Expand Up @@ -736,31 +735,6 @@ describe('Transaction utils :: calculateEIP1559Times', () => {
});
});

describe('Transactions utils :: buildUnserializedTransaction', () => {
it('returns a transaction that can be serialized and fed to an Optimism smart contract', () => {
const unserializedTransaction = buildUnserializedTransaction({
txParams: {
nonce: '0x0',
gasPrice: `0x${new BN('100').toString(16)}`,
gas: `0x${new BN('21000').toString(16)}`,
to: '0x0000000000000000000000000000000000000000',
value: `0x${new BN('10000000000000').toString(16)}`,
data: '0x0',
},
chainId: '10',
metamaskNetworkId: '10',
});
expect(unserializedTransaction.toJSON()).toMatchObject({
nonce: '0x0',
gasPrice: '0x64',
gasLimit: '0x5208',
to: '0x0000000000000000000000000000000000000000',
value: '0x9184e72a000',
data: '0x00',
});
});
});

const dappTxMeta = {
chainId: '0x1',
origin: 'pancakeswap.finance',
Expand Down
29 changes: 0 additions & 29 deletions app/util/transactions/optimismTransaction.ts
View file

This file was deleted.

7 changes: 1 addition & 6 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,6 @@
},
"dependencies": {
"@consensys/on-ramp-sdk": "1.28.3",
"@eth-optimism/contracts": "0.0.0-2021919175625",
"@ethereumjs/tx": "^3.2.1",
"@ethersproject/abi": "^5.7.0",
"@keystonehq/bc-ur-registry-eth": "^0.19.1",
"@keystonehq/metamask-airgapped-keyring": "^0.13.1",
Expand Down Expand Up @@ -548,10 +546,7 @@
},
"lavamoat": {
"allowScripts": {
"@eth-optimism/contracts>@ethersproject/hardware-wallets>@ledgerhq/hw-transport-node-hid>@ledgerhq/hw-transport-node-hid-noevents>node-hid": false,
"@eth-optimism/contracts>@ethersproject/hardware-wallets>@ledgerhq/hw-transport-node-hid>node-hid": false,
"@eth-optimism/contracts>@ethersproject/hardware-wallets>@ledgerhq/hw-transport-node-hid>usb": false,
"@ethereumjs/tx>ethereumjs-util>ethereum-cryptography>keccak": true,
"ethereumjs-abi>ethereumjs-util>ethereum-cryptography>keccak": true,
"@sentry/react-native>@sentry/cli": true,
"@storybook/manager-webpack5>@storybook/core-common>webpack>watchpack>watchpack-chokidar2>chokidar>fsevents": false,
"@storybook/addon-controls>@storybook/core-common>esbuild": false,
Expand Down
Loading

0 comments on commit c05e7dd

Please sign in to comment.