1
0
mirror of https://github.com/kremalicious/metamask-extension.git synced 2024-11-23 02:10:12 +01:00

Synchronously update transaction status (#8563)

All transaction status updates were moved into a `setTimeout` callback
and wrapped in a `try...catch` block in #4131, apparently in an attempt
to prevent failures in event subscribers from interrupting the
transaction logic. The `try...catch` block did accomplish that, but by
putting the status update in a `setTimeout` callback the operation was
made asynchronous.

Transaction status updates now happen unpredictably, in some future
event loop from when they're triggered. This creates a race condition,
where the transaction status update may occur before or after
subsequent state changes. This also introduces a risk of accidentally
undoing a change to the transaction state, as the update made to the
transaction inside the `setTimeout` callback uses a reference to
`txMeta` obtained synchronously before the `setTimeout` call. Any
replacement of the `txMeta` between the `setTxStatus` call and the
execution of the timeout would be erased. Luckily the `txMeta` object
is more often than not mutated rather than replaced, which may explain
why we haven't seen this happen yet.

Everything seems to work correctly with the `setTimeout` call removed,
and now the transaction logic is easier to understand.
This commit is contained in:
Mark Stacey 2020-05-09 10:46:58 -03:00 committed by GitHub
parent d8a4b7fc9b
commit 9c06b07c3c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -455,19 +455,17 @@ export default class TransactionStateManager extends EventEmitter {
} }
txMeta.status = status txMeta.status = status
setTimeout(() => { try {
try { this.updateTx(txMeta, `txStateManager: setting status to ${status}`)
this.updateTx(txMeta, `txStateManager: setting status to ${status}`) this.emit(`${txMeta.id}:${status}`, txId)
this.emit(`${txMeta.id}:${status}`, txId) this.emit(`tx:status-update`, txId, status)
this.emit(`tx:status-update`, txId, status) if (['submitted', 'rejected', 'failed'].includes(status)) {
if (['submitted', 'rejected', 'failed'].includes(status)) { this.emit(`${txMeta.id}:finished`, txMeta)
this.emit(`${txMeta.id}:finished`, txMeta)
}
this.emit('update:badge')
} catch (error) {
log.error(error)
} }
}) this.emit('update:badge')
} catch (error) {
log.error(error)
}
} }
/** /**