Skip to content

Commit

Permalink
fix: Add networkClientId to estimateGas function (#12958)
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 aims to add `networkClientId` argument to `estimateGas`
function.

## **Related issues**

Fixes: #12612

## **Manual testing steps**

1. Try sending non-native asset initiated from app
2. See no "Instricted gas too low" error 
3. Expect transaction to be passed

## **Screenshots/Recordings**

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

### **Before**

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

### **After**

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

## **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
- [X] 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
OGPoyraz authored Jan 13, 2025
1 parent 68efd5e commit 99a2404
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 10 deletions.
10 changes: 8 additions & 2 deletions app/components/Views/confirmations/ApproveView/Approve/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,8 @@ class Approve extends PureComponent {
};

setNetworkNonce = async () => {
const { networkClientId, setNonce, setProposedNonce, transaction } = this.props;
const { networkClientId, setNonce, setProposedNonce, transaction } =
this.props;
const proposedNonce = await getNetworkNonce(transaction, networkClientId);
setNonce(proposedNonce);
setProposedNonce(proposedNonce);
Expand All @@ -308,8 +309,13 @@ class Approve extends PureComponent {
};

handleGetGasLimit = async () => {
const { networkClientId } = this.props;
const { setTransactionObject, transaction } = this.props;
const estimation = await getGasLimit({ ...transaction, gas: undefined });
const estimation = await getGasLimit(
{ ...transaction, gas: undefined },
false,
networkClientId,
);
setTransactionObject({ gas: estimation.gas });
};

Expand Down
21 changes: 16 additions & 5 deletions app/components/Views/confirmations/SendFlow/Amount/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ import Alert, { AlertType } from '../../../../Base/Alert';

import {
selectChainId,
selectNetworkClientId,
selectProviderType,
selectTicker,
} from '../../../../../selectors/networkController';
Expand Down Expand Up @@ -490,6 +491,10 @@ class Amount extends PureComponent {
* Function that sets the max value mode
*/
setMaxValueMode: PropTypes.func,
/**
* Network client id
*/
networkClientId: PropTypes.string,
};

state = {
Expand Down Expand Up @@ -877,10 +882,15 @@ class Amount extends PureComponent {
transaction: { from },
transactionTo,
} = this.props.transactionState;
const { gas } = await getGasLimit({
from,
to: transactionTo,
});
const { networkClientId } = this.props;
const { gas } = await getGasLimit(
{
from,
to: transactionTo,
},
false,
networkClientId,
);

return gas;
};
Expand Down Expand Up @@ -940,7 +950,7 @@ class Amount extends PureComponent {
} = this.props;
const { internalPrimaryCurrencyIsCrypto } = this.state;

setMaxValueMode(useMax ?? false)
setMaxValueMode(useMax ?? false);

let inputValueConversion,
renderableInputValueConversion,
Expand Down Expand Up @@ -1570,6 +1580,7 @@ const mapStateToProps = (state, ownProps) => ({
),
swapsIsLive: swapsLivenessSelector(state),
chainId: selectChainId(state),
networkClientId: selectNetworkClientId(state),
});

const mapDispatchToProps = (dispatch) => ({
Expand Down
3 changes: 2 additions & 1 deletion app/components/Views/confirmations/SendFlow/Confirm/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -718,7 +718,8 @@ class Confirm extends PureComponent {
prepareTransaction,
transactionState: { transaction },
} = this.props;
const estimation = await getGasLimit(transaction, true);
const { networkClientId } = this.props;
const estimation = await getGasLimit(transaction, true, networkClientId);
prepareTransaction({ ...transaction, ...estimation });
};

Expand Down
8 changes: 6 additions & 2 deletions app/util/custom-gas/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,14 +107,18 @@ export function parseWaitTime(min) {
return parsed.trim();
}

export async function getGasLimit(transaction, resetGas = false) {
export async function getGasLimit(
transaction,
resetGas = false,
networkClientId,
) {
let estimation;
try {
const newTransactionObj = resetGas
? { ...transaction, gas: undefined, gasPrice: undefined }
: transaction;

estimation = await estimateGas(newTransactionObj);
estimation = await estimateGas(newTransactionObj, networkClientId);
} catch (error) {
estimation = {
gas: TransactionTypes.CUSTOM_GAS.DEFAULT_GAS_LIMIT,
Expand Down

0 comments on commit 99a2404

Please sign in to comment.