mirror of
https://github.com/kremalicious/metamask-extension.git
synced 2024-11-22 18:00:18 +01:00
Replace uses of updateTransaction (from the tx controller) with less powerful methods for each specific state transition (#13496)
* Draft methods to brak updateTransaction into smaller more targeted
methods.
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* normalize and validate tx params.
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Method to normalize tx and check if it's unapproved.
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Move the methods to controllers/transactions/index.js
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Flesh out the methods to update transaction with custom notes.
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* enforce that only the properties for the specific methid can be updated via the method.
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Rename calls to check transaction status.
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Test update gas fees
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Test update estimated base fee
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Update swap approval transaction
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* use lodash to remove undefined properties
update swap transaction tests
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Updates transaction user settings.
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Lint fixes.
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Remove commented log lines
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Add more parameters to updateSwapTransaction
approvalTxId
estimatedBaseFee
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Fix documentation for updateSwapTransaction
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Add Update Transaction Metrics
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Update transaction gas fees actions.js
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Lint fixes.
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Update EIP 1559 Params.
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Lint Fixes.
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Documentations.
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Remove metrics from this PR
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Lint fixes: Removed unused variables
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Add more params to updateTransactionGasFees.
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Update eip1559 method to editableParams.
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* lint fixes.
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Fix Mocha tests
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* add gasPrice to updateEditableParams
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* Remove duplicated Params in notes.
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
* A few more tests to cover if
transaction status is not unapproved
transaction is passed more parameters than it requires.
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
This commit is contained in:
Olusegun Akintayo
GitHub
parent
19098f7d69
commit
028850899e
@ -10,6 +10,7 @@ import { ethers } from 'ethers';
|
||||
import NonceTracker from 'nonce-tracker';
|
||||
import log from 'loglevel';
|
||||
import BigNumber from 'bignumber.js';
|
||||
import { merge, pickBy } from 'lodash';
|
||||
import cleanErrorStack from '../../lib/cleanErrorStack';
|
||||
import {
|
||||
hexToBn,
|
||||
@ -349,6 +350,268 @@ export default class TransactionController extends EventEmitter {
|
||||
});
|
||||
}
|
||||
|
||||
// ====================================================================================================================================================
|
||||
|
||||
/**
|
||||
* @param {number} txId
|
||||
* @returns {TransactionMeta} the txMeta who matches the given id if none found
|
||||
* for the network returns undefined
|
||||
*/
|
||||
_getTransaction(txId) {
|
||||
const { transactions } = this.store.getState();
|
||||
return transactions[txId];
|
||||
}
|
||||
|
||||
_checkIfTxStatusIsUnapproved(txId) {
|
||||
return (
|
||||
this.txStateManager.getTransaction(txId).status ===
|
||||
TRANSACTION_STATUSES.UNAPPROVED
|
||||
);
|
||||
}
|
||||
|
||||
_updateTransaction(txId, proposedUpdate, note) {
|
||||
const txMeta = this.txStateManager.getTransaction(txId);
|
||||
const updated = merge(txMeta, proposedUpdate);
|
||||
this.txStateManager.updateTransaction(updated, note);
|
||||
}
|
||||
|
||||
/**
|
||||
*
|
||||
* @param {string} txId - transaction id
|
||||
* @param {object} editableParams - holds the eip1559 fees parameters
|
||||
* @param editableParams.data
|
||||
* @param editableParams.from
|
||||
* @param editableParams.to
|
||||
* @param editableParams.value
|
||||
* @param editableParams.gas
|
||||
* @param editableParams.gasPrice
|
||||
*/
|
||||
updateEditableParams(txId, { data, from, to, value, gas, gasPrice }) {
|
||||
if (!this._checkIfTxStatusIsUnapproved(txId)) {
|
||||
return;
|
||||
}
|
||||
|
||||
const editableParams = {
|
||||
txParams: {
|
||||
data,
|
||||
from,
|
||||
to,
|
||||
value,
|
||||
gas,
|
||||
gasPrice,
|
||||
},
|
||||
};
|
||||
|
||||
// only update what is defined
|
||||
editableParams.txParams = pickBy(editableParams.txParams);
|
||||
const note = `Update Editable Params for ${txId}`;
|
||||
this._updateTransaction(txId, editableParams, note);
|
||||
}
|
||||
|
||||
/**
|
||||
* updates the gas fees of the transaction with id if the transaction state is unapproved
|
||||
*
|
||||
* @param {string} txId - transaction id
|
||||
* @param {object} txGasFees - holds the gas fees parameters
|
||||
* {
|
||||
* gasLimit,
|
||||
* gasPrice,
|
||||
* maxPriorityFeePerGas,
|
||||
* maxFeePerGas,
|
||||
* estimateUsed,
|
||||
* estimateSuggested
|
||||
* }
|
||||
* @param txGasFees.gasLimit
|
||||
* @param txGasFees.gasPrice
|
||||
* @param txGasFees.maxPriorityFeePerGas
|
||||
* @param txGasFees.maxFeePerGas
|
||||
* @param txGasFees.estimateUsed
|
||||
* @param txGasFees.estimateSuggested
|
||||
* @param txGasFees.defaultGasEstimates
|
||||
* @param txGasFees.gas
|
||||
* @param txGasFees.originalGasEstimate
|
||||
*/
|
||||
updateTransactionGasFees(
|
||||
txId,
|
||||
{
|
||||
gas,
|
||||
gasLimit,
|
||||
gasPrice,
|
||||
maxPriorityFeePerGas,
|
||||
maxFeePerGas,
|
||||
estimateUsed,
|
||||
estimateSuggested,
|
||||
defaultGasEstimates,
|
||||
originalGasEstimate,
|
||||
},
|
||||
) {
|
||||
if (!this._checkIfTxStatusIsUnapproved(txId)) {
|
||||
return;
|
||||
}
|
||||
|
||||
let txGasFees = {
|
||||
txParams: {
|
||||
gas,
|
||||
gasLimit,
|
||||
gasPrice,
|
||||
maxPriorityFeePerGas,
|
||||
maxFeePerGas,
|
||||
},
|
||||
estimateUsed,
|
||||
estimateSuggested,
|
||||
defaultGasEstimates,
|
||||
originalGasEstimate,
|
||||
};
|
||||
|
||||
// only update what is defined
|
||||
txGasFees.txParams = pickBy(txGasFees.txParams);
|
||||
txGasFees = pickBy(txGasFees);
|
||||
const note = `Update Transaction Gas Fees for ${txId}`;
|
||||
this._updateTransaction(txId, txGasFees, note);
|
||||
}
|
||||
|
||||
/**
|
||||
* updates the estimate base fees of the transaction with id if the transaction state is unapproved
|
||||
*
|
||||
* @param {string} txId - transaction id
|
||||
* @param {object} txEstimateBaseFees - holds the estimate base fees parameters
|
||||
* {
|
||||
* estimatedBaseFee,
|
||||
* decEstimatedBaseFee
|
||||
* }
|
||||
* @param txEstimateBaseFees.estimatedBaseFee
|
||||
* @param txEstimateBaseFees.decEstimatedBaseFee
|
||||
*/
|
||||
updateTransactionEstimatedBaseFee(
|
||||
txId,
|
||||
{ estimatedBaseFee, decEstimatedBaseFee },
|
||||
) {
|
||||
if (!this._checkIfTxStatusIsUnapproved(txId)) {
|
||||
return;
|
||||
}
|
||||
|
||||
let txEstimateBaseFees = { estimatedBaseFee, decEstimatedBaseFee };
|
||||
// only update what is defined
|
||||
txEstimateBaseFees = pickBy(txEstimateBaseFees);
|
||||
|
||||
const note = `Update Transaction Estimated Base Fees for ${txId}`;
|
||||
this._updateTransaction(txId, txEstimateBaseFees, note);
|
||||
}
|
||||
|
||||
/**
|
||||
* updates a swap approval transaction with provided metadata and source token symbol
|
||||
* if the transaction state is unapproved.
|
||||
*
|
||||
* @param {string} txId
|
||||
* @param {object} swapApprovalTransaction - holds the metadata and token symbol
|
||||
* {
|
||||
* type,
|
||||
* sourceTokenSymbol
|
||||
* }
|
||||
* @param swapApprovalTransaction.type
|
||||
* @param swapApprovalTransaction.sourceTokenSymbol
|
||||
*/
|
||||
updateSwapApprovalTransaction(txId, { type, sourceTokenSymbol }) {
|
||||
if (!this._checkIfTxStatusIsUnapproved(txId)) {
|
||||
return;
|
||||
}
|
||||
|
||||
let swapApprovalTransaction = { type, sourceTokenSymbol };
|
||||
// only update what is defined
|
||||
swapApprovalTransaction = pickBy(swapApprovalTransaction);
|
||||
|
||||
const note = `Update Swap Approval Transaction for ${txId}`;
|
||||
this._updateTransaction(txId, swapApprovalTransaction, note);
|
||||
}
|
||||
|
||||
/**
|
||||
* updates a swap transaction with provided metadata and source token symbol
|
||||
* if the transaction state is unapproved.
|
||||
*
|
||||
* @param {string} txId
|
||||
* @param {object} swapTransaction - holds the metadata
|
||||
* {
|
||||
* sourceTokenSymbol,
|
||||
* destinationTokenSymbol,
|
||||
* type,
|
||||
* destinationTokenDecimals,
|
||||
* destinationTokenAddress,
|
||||
* swapMetaData,
|
||||
* swapTokenValue,
|
||||
* estimatedBaseFee,
|
||||
* approvalTxId
|
||||
*}
|
||||
* @param swapTransaction.sourceTokenSymbol
|
||||
* @param swapTransaction.destinationTokenSymbol
|
||||
* @param swapTransaction.type
|
||||
* @param swapTransaction.destinationTokenDecimals
|
||||
* @param swapTransaction.destinationTokenAddress
|
||||
* @param swapTransaction.swapMetaData
|
||||
* @param swapTransaction.swapTokenValue
|
||||
* @param swapTransaction.estimatedBaseFee
|
||||
* @param swapTransaction.approvalTxId
|
||||
*/
|
||||
updateSwapTransaction(
|
||||
txId,
|
||||
{
|
||||
sourceTokenSymbol,
|
||||
destinationTokenSymbol,
|
||||
type,
|
||||
destinationTokenDecimals,
|
||||
destinationTokenAddress,
|
||||
swapMetaData,
|
||||
swapTokenValue,
|
||||
estimatedBaseFee,
|
||||
approvalTxId,
|
||||
},
|
||||
) {
|
||||
if (!this._checkIfTxStatusIsUnapproved(txId)) {
|
||||
return;
|
||||
}
|
||||
|
||||
let swapTransaction = {
|
||||
sourceTokenSymbol,
|
||||
destinationTokenSymbol,
|
||||
type,
|
||||
destinationTokenDecimals,
|
||||
destinationTokenAddress,
|
||||
swapMetaData,
|
||||
swapTokenValue,
|
||||
estimatedBaseFee,
|
||||
approvalTxId,
|
||||
};
|
||||
|
||||
// only update what is defined
|
||||
swapTransaction = pickBy(swapTransaction);
|
||||
|
||||
const note = `Update Swap Transaction for ${txId}`;
|
||||
this._updateTransaction(txId, swapTransaction, note);
|
||||
}
|
||||
|
||||
/**
|
||||
* updates a transaction's user settings only if the transaction state is unapproved
|
||||
*
|
||||
* @param {string} txId
|
||||
* @param {object} userSettings - holds the metadata
|
||||
* { userEditedGasLimit, userFeeLevel }
|
||||
* @param userSettings.userEditedGasLimit
|
||||
* @param userSettings.userFeeLevel
|
||||
*/
|
||||
updateTransactionUserSettings(txId, { userEditedGasLimit, userFeeLevel }) {
|
||||
if (!this._checkIfTxStatusIsUnapproved(txId)) {
|
||||
return;
|
||||
}
|
||||
|
||||
let userSettings = { userEditedGasLimit, userFeeLevel };
|
||||
// only update what is defined
|
||||
userSettings = pickBy(userSettings);
|
||||
|
||||
const note = `Update User Settings for ${txId}`;
|
||||
this._updateTransaction(txId, userSettings, note);
|
||||
}
|
||||
|
||||
// ====================================================================================================================================================
|
||||
|
||||
/**
|
||||
* Validates and generates a txMeta with defaults and puts it in txStateManager
|
||||
* store.
|
||||
|
||||
@ -2175,4 +2175,203 @@ describe('Transaction Controller', function () {
|
||||
assert.deepEqual(result, expectedParams);
|
||||
});
|
||||
});
|
||||
|
||||
describe('update transaction methods', function () {
|
||||
let txStateManager;
|
||||
|
||||
beforeEach(function () {
|
||||
txStateManager = txController.txStateManager;
|
||||
txStateManager.addTransaction({
|
||||
id: '1',
|
||||
status: TRANSACTION_STATUSES.UNAPPROVED,
|
||||
metamaskNetworkId: currentNetworkId,
|
||||
txParams: {
|
||||
gasLimit: '0x001',
|
||||
gasPrice: '0x002',
|
||||
// max fees can not be mixed with gasPrice
|
||||
// maxPriorityFeePerGas: '0x003',
|
||||
// maxFeePerGas: '0x004',
|
||||
to: VALID_ADDRESS,
|
||||
from: VALID_ADDRESS,
|
||||
},
|
||||
estimateUsed: '0x005',
|
||||
estimatedBaseFee: '0x006',
|
||||
decEstimatedBaseFee: '6',
|
||||
type: 'swap',
|
||||
sourceTokenSymbol: 'ETH',
|
||||
destinationTokenSymbol: 'UNI',
|
||||
destinationTokenDecimals: 16,
|
||||
destinationTokenAddress: VALID_ADDRESS,
|
||||
swapMetaData: {},
|
||||
swapTokenValue: '0x007',
|
||||
userEditedGasLimit: '0x008',
|
||||
userFeeLevel: 'medium',
|
||||
});
|
||||
});
|
||||
|
||||
it('updates transaction gas fees', function () {
|
||||
// test update gasFees
|
||||
txController.updateTransactionGasFees('1', {
|
||||
gasPrice: '0x0022',
|
||||
gasLimit: '0x0011',
|
||||
});
|
||||
let result = txStateManager.getTransaction('1');
|
||||
assert.equal(result.txParams.gasPrice, '0x0022');
|
||||
// TODO: weird behavior here...only gasPrice gets returned.
|
||||
// assert.equal(result.txParams.gasLimit, '0x0011');
|
||||
|
||||
// test update maxPriorityFeePerGas
|
||||
txStateManager.addTransaction({
|
||||
id: '2',
|
||||
status: TRANSACTION_STATUSES.UNAPPROVED,
|
||||
metamaskNetworkId: currentNetworkId,
|
||||
txParams: {
|
||||
maxPriorityFeePerGas: '0x003',
|
||||
to: VALID_ADDRESS,
|
||||
from: VALID_ADDRESS,
|
||||
},
|
||||
estimateUsed: '0x005',
|
||||
});
|
||||
txController.updateTransactionGasFees('2', {
|
||||
maxPriorityFeePerGas: '0x0033',
|
||||
});
|
||||
result = txStateManager.getTransaction('2');
|
||||
assert.equal(result.txParams.maxPriorityFeePerGas, '0x0033');
|
||||
|
||||
// test update maxFeePerGas
|
||||
txStateManager.addTransaction({
|
||||
id: '3',
|
||||
status: TRANSACTION_STATUSES.UNAPPROVED,
|
||||
metamaskNetworkId: currentNetworkId,
|
||||
txParams: {
|
||||
maxPriorityFeePerGas: '0x003',
|
||||
maxFeePerGas: '0x004',
|
||||
to: VALID_ADDRESS,
|
||||
from: VALID_ADDRESS,
|
||||
},
|
||||
estimateUsed: '0x005',
|
||||
});
|
||||
txController.updateTransactionGasFees('3', { maxFeePerGas: '0x0044' });
|
||||
result = txStateManager.getTransaction('3');
|
||||
assert.equal(result.txParams.maxFeePerGas, '0x0044');
|
||||
|
||||
// test update estimate used
|
||||
txController.updateTransactionGasFees('3', { estimateUsed: '0x0055' });
|
||||
result = txStateManager.getTransaction('3');
|
||||
assert.equal(result.estimateUsed, '0x0055');
|
||||
});
|
||||
|
||||
it('updates estimated base fee', function () {
|
||||
txController.updateTransactionEstimatedBaseFee('1', {
|
||||
estimatedBaseFee: '0x0066',
|
||||
decEstimatedBaseFee: '66',
|
||||
});
|
||||
const result = txStateManager.getTransaction('1');
|
||||
assert.equal(result.estimatedBaseFee, '0x0066');
|
||||
assert.equal(result.decEstimatedBaseFee, '66');
|
||||
});
|
||||
|
||||
it('updates swap approval transaction', function () {
|
||||
txController.updateSwapApprovalTransaction('1', {
|
||||
type: 'swapApproval',
|
||||
sourceTokenSymbol: 'XBN',
|
||||
});
|
||||
|
||||
const result = txStateManager.getTransaction('1');
|
||||
assert.equal(result.type, 'swapApproval');
|
||||
assert.equal(result.sourceTokenSymbol, 'XBN');
|
||||
});
|
||||
|
||||
it('updates swap transaction', function () {
|
||||
txController.updateSwapTransaction('1', {
|
||||
sourceTokenSymbol: 'BTCX',
|
||||
destinationTokenSymbol: 'ETH',
|
||||
});
|
||||
|
||||
const result = txStateManager.getTransaction('1');
|
||||
assert.equal(result.sourceTokenSymbol, 'BTCX');
|
||||
assert.equal(result.destinationTokenSymbol, 'ETH');
|
||||
assert.equal(result.destinationTokenDecimals, 16);
|
||||
assert.equal(result.destinationTokenAddress, VALID_ADDRESS);
|
||||
assert.equal(result.swapTokenValue, '0x007');
|
||||
|
||||
txController.updateSwapTransaction('1', {
|
||||
type: 'swapped',
|
||||
destinationTokenDecimals: 8,
|
||||
destinationTokenAddress: VALID_ADDRESS_TWO,
|
||||
swapTokenValue: '0x0077',
|
||||
});
|
||||
assert.equal(result.sourceTokenSymbol, 'BTCX');
|
||||
assert.equal(result.destinationTokenSymbol, 'ETH');
|
||||
assert.equal(result.type, 'swapped');
|
||||
assert.equal(result.destinationTokenDecimals, 8);
|
||||
assert.equal(result.destinationTokenAddress, VALID_ADDRESS_TWO);
|
||||
assert.equal(result.swapTokenValue, '0x0077');
|
||||
});
|
||||
|
||||
it('updates transaction user settings', function () {
|
||||
txController.updateTransactionUserSettings('1', {
|
||||
userEditedGasLimit: '0x0088',
|
||||
userFeeLevel: 'high',
|
||||
});
|
||||
|
||||
const result = txStateManager.getTransaction('1');
|
||||
assert.equal(result.userEditedGasLimit, '0x0088');
|
||||
assert.equal(result.userFeeLevel, 'high');
|
||||
});
|
||||
|
||||
it('does not update if status is not unapproved', function () {
|
||||
txStateManager.addTransaction({
|
||||
id: '4',
|
||||
status: TRANSACTION_STATUSES.APPROVED,
|
||||
metamaskNetworkId: currentNetworkId,
|
||||
txParams: {
|
||||
maxPriorityFeePerGas: '0x007',
|
||||
maxFeePerGas: '0x008',
|
||||
to: VALID_ADDRESS,
|
||||
from: VALID_ADDRESS,
|
||||
},
|
||||
estimateUsed: '0x009',
|
||||
});
|
||||
|
||||
txController.updateTransactionGasFees('4', { maxFeePerGas: '0x0088' });
|
||||
let result = txStateManager.getTransaction('4');
|
||||
assert.equal(result.txParams.maxFeePerGas, '0x008');
|
||||
|
||||
// test update estimate used
|
||||
txController.updateTransactionGasFees('4', { estimateUsed: '0x0099' });
|
||||
result = txStateManager.getTransaction('4');
|
||||
assert.equal(result.estimateUsed, '0x009');
|
||||
});
|
||||
|
||||
it('does not update unknown parameters in update method', function () {
|
||||
txController.updateSwapTransaction('1', {
|
||||
type: 'swapped',
|
||||
destinationTokenDecimals: 8,
|
||||
destinationTokenAddress: VALID_ADDRESS_TWO,
|
||||
swapTokenValue: '0x011',
|
||||
gasPrice: '0x12',
|
||||
});
|
||||
|
||||
let result = txStateManager.getTransaction('1');
|
||||
|
||||
assert.equal(result.type, 'swapped');
|
||||
assert.equal(result.destinationTokenDecimals, 8);
|
||||
assert.equal(result.destinationTokenAddress, VALID_ADDRESS_TWO);
|
||||
assert.equal(result.swapTokenValue, '0x011');
|
||||
assert.equal(result.txParams.gasPrice, '0x002'); // not updated even though it's passed in to update
|
||||
|
||||
txController.updateTransactionGasFees('1', {
|
||||
estimateUsed: '0x13',
|
||||
gasPrice: '0x14',
|
||||
destinationTokenAddress: VALID_ADDRESS,
|
||||
});
|
||||
|
||||
result = txStateManager.getTransaction('1');
|
||||
console.log(result);
|
||||
assert.equal(result.estimateUsed, '0x13');
|
||||
assert.equal(result.txParams.gasPrice, '0x14');
|
||||
assert.equal(result.destinationTokenAddress, VALID_ADDRESS_TWO); // not updated even though it's passed in to update
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Loading…
Reference in New Issue
Block a user