Skip to content

Commit

Permalink
Decouple swaps metric logic from TransactionController (#21559)
Browse files Browse the repository at this point in the history
## **Description**
With ongoing effort of `TransactionController` alignment between mobile
and extension, this PR aims to decouple swaps metric logic from
`TransactionController` and make them client specific.

Also removed `transaction-finalized` and put `transaction-confirmed` and
`transaction-failed`. This should've been done before but I couldn't
notice when I was working mainly focused on metric events.

## **Manual testing steps**

No functional changes. 

## **Related issues**

Fixes MetaMask/MetaMask-planning#1053

## **Pre-merge author checklist**

- [X] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [X] I've clearly explained:
  - [X] What problem this PR is solving.
  - [X] How this problem was solved.
  - [X] How reviewers can test my changes.
- [X] I’ve indicated what issue this PR is linked to: Fixes #???
- [X] I’ve included tests if applicable.
- [X] I’ve documented any added code.
- [X] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
- [x] I’ve properly set the pull request status:
  - [x] In case it's not yet "ready for review", I've set it to "draft".
- [x] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

## **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 Nov 1, 2023
1 parent e6eb0af commit 3fa27cd
Show file tree
Hide file tree
Showing 5 changed files with 565 additions and 310 deletions.
102 changes: 18 additions & 84 deletions app/scripts/controllers/transactions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
getChainType,
} from '../../lib/util';
import {
TransactionEvent,
TransactionStatus,
TransactionType,
TransactionEnvelopeType,
Expand All @@ -38,10 +39,6 @@ import {
hexWEIToDecGWEI,
} from '../../../../shared/modules/conversion.utils';
import { isSwapsDefaultTokenAddress } from '../../../../shared/modules/swaps.utils';
import {
MetaMetricsEventCategory,
MetaMetricsEventName,
} from '../../../../shared/constants/metametrics';
import {
CHAIN_ID_TO_GAS_LIMIT_BUFFER_MAP,
NETWORK_TYPES,
Expand All @@ -53,10 +50,7 @@ import {
isEIP1559Transaction,
} from '../../../../shared/modules/transaction.utils';
import { ORIGIN_METAMASK } from '../../../../shared/constants/app';
import {
calcGasTotal,
getSwapsTokensReceivedFromTxMeta,
} from '../../../../shared/lib/transactions-controller-utils';
import { calcGasTotal } from '../../../../shared/lib/transactions-controller-utils';
import { Numeric } from '../../../../shared/modules/Numeric';
import TransactionStateManager from './tx-state-manager';
import TxGasUtil from './tx-gas-utils';
Expand Down Expand Up @@ -705,7 +699,7 @@ export default class TransactionController extends EventEmitter {
this.txStateManager.setTxStatusConfirmed(txId);
this._markNonceDuplicatesDropped(txId);

this.emit('transaction-finalized', {
this.emit(TransactionEvent.confirmed, {
transactionMeta: txMeta,
});

Expand Down Expand Up @@ -898,7 +892,7 @@ export default class TransactionController extends EventEmitter {
this.txStateManager.setTxStatusConfirmed(txId);
this._markNonceDuplicatesDropped(txId);

this.emit('transaction-finalized', {
this.emit(TransactionEvent.confirmed, {
transactionMeta: txMeta,
});

Expand Down Expand Up @@ -1218,7 +1212,10 @@ export default class TransactionController extends EventEmitter {
latestTxMeta,
'transactions#confirmTransaction - add postTxBalance',
);
this._trackSwapsMetrics(latestTxMeta, approvalTxMeta);
this.emit(TransactionEvent.postTransactionBalanceUpdated, {
transactionMeta: latestTxMeta,
approvalTransactionMeta: approvalTxMeta,
});
}
}

Expand Down Expand Up @@ -1256,7 +1253,7 @@ export default class TransactionController extends EventEmitter {

this.txStateManager.setTxStatusSubmitted(txId);

this.emit('transaction-submitted', {
this.emit(TransactionEvent.submitted, {
transactionMeta: txMeta,
actionId,
});
Expand Down Expand Up @@ -1542,13 +1539,13 @@ export default class TransactionController extends EventEmitter {
txMeta = await this._addTransactionGasDefaults(txMeta);

if ([TransactionType.swap, TransactionType.swapApproval].includes(type)) {
txMeta = await this._createSwapsTransaction(swaps, type, txMeta);
txMeta = await this._updateSwapsTransaction(swaps, type, txMeta);
}

return { txMeta, isExisting: false };
}

async _createSwapsTransaction(swapOptions, transactionType, txMeta) {
async _updateSwapsTransaction(swapOptions, transactionType, txMeta) {
// The simulationFails property is added if the estimateGas call fails. In cases
// when no swaps approval tx is required, this indicates that the swap will likely
// fail. There was an earlier estimateGas call made by the swaps controller,
Expand All @@ -1573,12 +1570,12 @@ export default class TransactionController extends EventEmitter {
}

if (transactionType === TransactionType.swapApproval) {
this.emit('newSwapApproval', txMeta);
this.emit(TransactionEvent.newSwapApproval, { transactionMeta: txMeta });
return this._updateSwapApprovalTransaction(txMeta.id, swapsMeta);
}

if (transactionType === TransactionType.swap) {
this.emit('newSwap', txMeta);
this.emit(TransactionEvent.newSwap, { transactionMeta: txMeta });
return this._updateSwapTransaction(txMeta.id, swapsMeta);
}

Expand Down Expand Up @@ -1783,7 +1780,7 @@ export default class TransactionController extends EventEmitter {
// sign transaction
const rawTx = await this._signTransaction(txId);
await this._publishTransaction(txId, rawTx, actionId);
this.emit('transaction-approved', {
this.emit(TransactionEvent.approved, {
transactionMeta: txMeta,
actionId,
});
Expand Down Expand Up @@ -1817,7 +1814,7 @@ export default class TransactionController extends EventEmitter {
async _cancelTransaction(txId, actionId) {
const txMeta = this.txStateManager.getTransaction(txId);
this.txStateManager.setTxStatusRejected(txId);
this.emit('transaction-rejected', {
this.emit(TransactionEvent.rejected, {
transactionMeta: txMeta,
actionId,
});
Expand Down Expand Up @@ -2081,69 +2078,6 @@ export default class TransactionController extends EventEmitter {
};
}

_trackSwapsMetrics(txMeta, approvalTxMeta) {
if (this._getParticipateInMetrics() && txMeta.swapMetaData) {
if (txMeta.txReceipt.status === '0x0') {
this.emit('transaction-swap-failed', {
event: 'Swap Failed',
sensitiveProperties: { ...txMeta.swapMetaData },
category: MetaMetricsEventCategory.Swaps,
});
} else {
const tokensReceived = getSwapsTokensReceivedFromTxMeta(
txMeta.destinationTokenSymbol,
txMeta,
txMeta.destinationTokenAddress,
txMeta.txParams.from,
txMeta.destinationTokenDecimals,
approvalTxMeta,
txMeta.chainId,
);

const quoteVsExecutionRatio = tokensReceived
? `${new BigNumber(tokensReceived, 10)
.div(txMeta.swapMetaData.token_to_amount, 10)
.times(100)
.round(2)}%`
: null;

const estimatedVsUsedGasRatio =
txMeta.txReceipt.gasUsed && txMeta.swapMetaData.estimated_gas
? `${new BigNumber(txMeta.txReceipt.gasUsed, 16)
.div(txMeta.swapMetaData.estimated_gas, 10)
.times(100)
.round(2)}%`
: null;

const transactionsCost = this._calculateTransactionsCost(
txMeta,
approvalTxMeta,
);

this.emit('transaction-swap-finalized', {
event: MetaMetricsEventName.SwapCompleted,
category: MetaMetricsEventCategory.Swaps,
sensitiveProperties: {
...txMeta.swapMetaData,
token_to_amount_received: tokensReceived,
quote_vs_executionRatio: quoteVsExecutionRatio,
estimated_vs_used_gasRatio: estimatedVsUsedGasRatio,
approval_gas_cost_in_eth: transactionsCost.approvalGasCostInEth,
trade_gas_cost_in_eth: transactionsCost.tradeGasCostInEth,
trade_and_approval_gas_cost_in_eth:
transactionsCost.tradeAndApprovalGasCostInEth,
// Firefox and Chrome have different implementations of the APIs
// that we rely on for communication accross the app. On Chrome big
// numbers are converted into number strings, on firefox they remain
// Big Number objects. As such, we convert them here for both
// browsers.
token_to_amount: txMeta.swapMetaData.token_to_amount.toString(10),
},
});
}
}
}

/**
* The allowance amount in relation to the dapp proposed amount for specific token
*
Expand Down Expand Up @@ -2185,7 +2119,7 @@ export default class TransactionController extends EventEmitter {
_failTransaction(txId, error, actionId) {
this.txStateManager.setTxStatusFailed(txId, error);
const txMeta = this.txStateManager.getTransaction(txId);
this.emit('transaction-finalized', {
this.emit(TransactionEvent.failed, {
actionId,
error: error.message,
transactionMeta: txMeta,
Expand All @@ -2195,7 +2129,7 @@ export default class TransactionController extends EventEmitter {
_dropTransaction(txId) {
this.txStateManager.setTxStatusDropped(txId);
const txMeta = this.txStateManager.getTransaction(txId);
this.emit('transaction-dropped', {
this.emit(TransactionEvent.dropped, {
transactionMeta: txMeta,
});
}
Expand All @@ -2209,7 +2143,7 @@ export default class TransactionController extends EventEmitter {
_addTransaction(txMeta) {
this.txStateManager.addTransaction(txMeta);
this.emit(`${txMeta.id}:unapproved`, txMeta);
this.emit('transaction-added', {
this.emit(TransactionEvent.added, {
transactionMeta: txMeta,
actionId: txMeta.actionId,
});
Expand Down
Loading

0 comments on commit 3fa27cd

Please sign in to comment.