This repository was archived by the owner on Nov 10, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 360
feat: watch pending txs for success #3537
Merged
Merged
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
c388c76
fix: throw when tx isn't mined
2f79e3e
fix: remove unnecessary throw
22c160a
fix: propose once `txHash` + store with pending
f4fcfda
wip: watch loaded pending txs
e3ff1c6
Merge branch 'dev' into pending-watcher
75d6c36
fix: move watcher to middleware + add notification
c3526d3
fix: abstract monitor + add tests
0667c52
fix: migrate to wallet connection + tweak monitor
0145293
fix: remove chains with no pending txs
23a2b40
fix: extract back off consts + attempt loops
148bb3b
fix: split monitor + pass backOff options as param
e5a70ec
fix: tests
74fe1cb
fix: tests
5553c66
fix: check `blockHash` of receipt
a144665
fix: test
e239a22
fix: only remove pending in catch
64dcdd6
fix: test
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -126,7 +126,7 @@ export class TxSender { | |
| } | ||
|
|
||
| async onError(err: Error & { code: number }, errorCallback?: ErrorEventHandler): Promise<void> { | ||
| const { txArgs, isFinalization, from, txProps, dispatch, notifications, safeInstance, txId, txHash } = this | ||
| const { txArgs, isFinalization, from, txProps, dispatch, notifications, safeInstance, txId } = this | ||
|
|
||
| errorCallback?.() | ||
|
|
||
|
|
@@ -159,7 +159,7 @@ export class TxSender { | |
| txArgs.sigs, | ||
| ) | ||
| .encodeABI() | ||
| : txHash && safeInstance.methods.approveHash(txHash).encodeABI() | ||
| : this.txHash && safeInstance.methods.approveHash(this.txHash).encodeABI() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why this change? I see that before we were destructuring
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| if (!executeData) { | ||
| return | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
161 changes: 161 additions & 0 deletions
161
src/logic/safe/transactions/__tests__/pendingTxMonitor.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,161 @@ | ||
| import { Transaction } from 'web3-core' | ||
|
|
||
| import * as store from 'src/store' | ||
| import * as web3 from 'src/logic/wallets/getWeb3' | ||
| import { PendingTxMonitor } from 'src/logic/safe/transactions/pendingTxMonitor' | ||
|
|
||
| const originalIsTxMined = PendingTxMonitor._isTxMined | ||
|
|
||
| describe('PendingTxMonitor', () => { | ||
| beforeEach(() => { | ||
| jest.restoreAllMocks() | ||
| }) | ||
|
|
||
| afterEach(() => { | ||
| PendingTxMonitor._isTxMined = originalIsTxMined | ||
| }) | ||
| describe('_isTxMined', () => { | ||
| it("doesn't throw if a transaction receipt exists", async () => { | ||
| jest.spyOn(web3.getWeb3().eth, 'getTransactionReceipt').mockImplementationOnce(() => | ||
| Promise.resolve({ | ||
| blockHash: '0x123', | ||
| blockNumber: 1, | ||
| transactionHash: 'fakeTxHash', | ||
| transactionIndex: 0, | ||
| from: '0x123', | ||
| to: '0x123', | ||
| cumulativeGasUsed: 1, | ||
| gasUsed: 1, | ||
| contractAddress: '0x123', | ||
| logs: [], | ||
| status: true, | ||
| logsBloom: '0x123', | ||
| }), | ||
| ) | ||
|
|
||
| expect(async () => await PendingTxMonitor._isTxMined(0, 'fakeTxHash')).not.toThrow() | ||
| }) | ||
| it('throws if no transaction receipt exists within 50 blocks', async () => { | ||
| jest | ||
| .spyOn(web3.getWeb3().eth, 'getTransactionReceipt') | ||
| // Returns `null` if transaction is pending: https://web3js.readthedocs.io/en/v1.2.11/web3-eth.html#gettransactionreceipt | ||
| .mockImplementation(() => Promise.resolve(null as any)) | ||
| jest.spyOn(web3.getWeb3().eth, 'getBlockNumber').mockImplementation(() => Promise.resolve(50)) | ||
|
|
||
| try { | ||
| await PendingTxMonitor._isTxMined(0, 'fakeTxHash') | ||
|
|
||
| // Fail test if above expression doesn't throw anything | ||
| expect(true).toBe(false) | ||
| } catch (e) { | ||
| expect(e.message).toEqual('Pending transaction not found') | ||
| } | ||
| }) | ||
| it("doesn't throw if no transaction receipt exists and after 50 blocks", async () => { | ||
| jest | ||
| .spyOn(web3.getWeb3().eth, 'getTransactionReceipt') | ||
| // Returns `null` if transaction is pending: https://web3js.readthedocs.io/en/v1.2.11/web3-eth.html#gettransactionreceipt | ||
| .mockImplementation(() => Promise.resolve(null as any)) | ||
| jest.spyOn(web3.getWeb3().eth, 'getBlockNumber').mockImplementation(() => Promise.resolve(51)) | ||
|
|
||
| expect(async () => await PendingTxMonitor._isTxMined(0, 'fakeTxHash')).not.toThrow() | ||
| }) | ||
| }) | ||
|
|
||
| describe('monitorTx', () => { | ||
| it("doesn't clear the pending transaction if it was mined", async () => { | ||
| PendingTxMonitor._isTxMined = jest.fn(() => Promise.resolve()) | ||
|
|
||
| const dispatchSpy = jest.spyOn(store.store, 'dispatch').mockImplementation(() => jest.fn()) | ||
|
|
||
| await PendingTxMonitor.monitorTx(0, 'fakeTxId', 'fakeTxHash', { | ||
| numOfAttempts: 1, | ||
| startingDelay: 0, | ||
| timeMultiple: 0, | ||
| }) | ||
|
|
||
| expect(dispatchSpy).not.toBeCalled() | ||
| }) | ||
|
|
||
| it('clears the pending transaction it the tx was not mined within 50 blocks', async () => { | ||
| PendingTxMonitor._isTxMined = jest.fn(() => Promise.reject()) | ||
|
|
||
| const dispatchSpy = jest.spyOn(store.store, 'dispatch').mockImplementation(() => jest.fn()) | ||
|
|
||
| await PendingTxMonitor.monitorTx(0, 'fakeTxId', 'fakeTxHash', { | ||
| numOfAttempts: 1, | ||
| startingDelay: 0, | ||
| timeMultiple: 0, | ||
| }) | ||
|
|
||
| expect(dispatchSpy).toHaveBeenCalledTimes(2) | ||
| }) | ||
| }) | ||
|
|
||
| describe('monitorAllPendingTxs', () => { | ||
| it('breaks if there are no pending txs', async () => { | ||
| jest.spyOn(store.store, 'getState').mockImplementation(() => ({ | ||
| pendingTransactions: {}, | ||
| config: { | ||
| chainId: '4', | ||
| }, | ||
| })) | ||
|
|
||
| const getWeb3Spy = jest.spyOn(web3, 'getWeb3') | ||
|
|
||
| await PendingTxMonitor.monitorAllTxs() | ||
|
|
||
| expect(getWeb3Spy).not.toHaveBeenCalled() | ||
| }) | ||
| it('breaks if no block number returns', async () => { | ||
| jest.spyOn(store.store, 'getState').mockImplementation(() => ({ | ||
| pendingTransactions: { | ||
| '4': { fakeTxId: 'fakeTxHash' }, | ||
| }, | ||
| config: { | ||
| chainId: '4', | ||
| }, | ||
| })) | ||
|
|
||
| jest.spyOn(web3.getWeb3().eth, 'getBlockNumber').mockImplementation(() => Promise.reject()) | ||
|
|
||
| const isPendingSpy = jest.spyOn(PendingTxMonitor, '_isTxMined').mockImplementation(jest.fn()) | ||
|
|
||
| try { | ||
| await PendingTxMonitor.monitorAllTxs() | ||
|
|
||
| // Fail test if above expression doesn't throw anything | ||
| expect(true).toBe(false) | ||
| } catch (e) { | ||
| expect(isPendingSpy).not.toHaveBeenCalled() | ||
| } | ||
| }) | ||
|
|
||
| it('checks each pending tx', async () => { | ||
| jest.spyOn(store.store, 'getState').mockImplementation(() => ({ | ||
| pendingTransactions: { | ||
| '4': { | ||
| fakeTxId: 'fakeTxHash', | ||
| fakeTxId2: 'fakeTxHash2', | ||
| fakeTxId3: 'fakeTxHash3', | ||
| }, | ||
| }, | ||
| config: { | ||
| chainId: '4', | ||
| }, | ||
| })) | ||
|
|
||
| jest.spyOn(web3.getWeb3().eth, 'getBlockNumber').mockImplementation(() => Promise.resolve(0)) | ||
|
|
||
| PendingTxMonitor._isTxMined = jest.fn(() => Promise.resolve()) | ||
|
|
||
| await PendingTxMonitor.monitorAllTxs() | ||
|
|
||
| expect((PendingTxMonitor._isTxMined as jest.Mock).mock.calls).toEqual([ | ||
| [0, 'fakeTxHash'], | ||
| [0, 'fakeTxHash2'], | ||
| [0, 'fakeTxHash3'], | ||
| ]) | ||
| }) | ||
| }) | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| import { backOff, IBackOffOptions } from 'exponential-backoff' | ||
|
|
||
| import { NOTIFICATIONS } from 'src/logic/notifications' | ||
| import enqueueSnackbar from 'src/logic/notifications/store/actions/enqueueSnackbar' | ||
| import { getWeb3 } from 'src/logic/wallets/getWeb3' | ||
| import { store } from 'src/store' | ||
| import { removePendingTransaction } from 'src/logic/safe/store/actions/pendingTransactions' | ||
| import { pendingTxIdsByChain } from 'src/logic/safe/store/selectors/pendingTransactions' | ||
|
|
||
| const _isTxMined = async (sessionBlockNumber: number, txHash: string): Promise<void> => { | ||
| const MAX_WAITING_BLOCK = sessionBlockNumber + 50 | ||
|
|
||
| const web3 = getWeb3() | ||
|
|
||
| if ( | ||
| // Transaction hasn't yet been mined | ||
| !(await web3.eth.getTransactionReceipt(txHash)) && | ||
| // The current block is within waiting window | ||
| (await web3.eth.getBlockNumber()) <= MAX_WAITING_BLOCK | ||
| ) { | ||
| throw new Error('Pending transaction not found') | ||
| } | ||
| } | ||
|
|
||
| // Progressively after 10s, 20s, 40s, 80s, 160s, 320s - total of 6.5 minutes | ||
| const INITIAL_TIMEOUT = 10_000 | ||
| const TIMEOUT_MULTIPLIER = 2 | ||
| const MAX_ATTEMPTS = 6 | ||
|
|
||
| const monitorTx = async ( | ||
| sessionBlockNumber: number, | ||
| txId: string, | ||
| txHash: string, | ||
| options: Partial<IBackOffOptions> = { | ||
| startingDelay: INITIAL_TIMEOUT, | ||
| timeMultiple: TIMEOUT_MULTIPLIER, | ||
| numOfAttempts: MAX_ATTEMPTS, | ||
| }, | ||
| ): Promise<void> => { | ||
| return backOff(() => PendingTxMonitor._isTxMined(sessionBlockNumber, txHash), options).catch(() => { | ||
| // Unsuccessfully mined (threw in last backOff attempt) | ||
| store.dispatch(removePendingTransaction({ id: txId })) | ||
| store.dispatch(enqueueSnackbar(NOTIFICATIONS.TX_PENDING_FAILED_MSG)) | ||
| }) | ||
| // If mined, pending status is removed in the transaction middleware | ||
| // when a transaction is added to historical transactions list | ||
| } | ||
|
|
||
| const monitorAllTxs = async (): Promise<void> => { | ||
| const pendingTxsOnChain = pendingTxIdsByChain(store.getState()) | ||
| const pendingTxs = Object.entries(pendingTxsOnChain || {}) | ||
|
|
||
| // Don't check pending transactions if there are none | ||
| if (pendingTxs.length === 0) { | ||
| return | ||
| } | ||
|
|
||
| const web3 = getWeb3() | ||
|
|
||
| try { | ||
| const sessionBlockNumber = await web3.eth.getBlockNumber() | ||
| await Promise.all( | ||
katspaugh marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| pendingTxs.map(([txId, txHash]) => { | ||
| return PendingTxMonitor.monitorTx(sessionBlockNumber, txId, txHash) | ||
| }), | ||
| ) | ||
| } catch { | ||
| // Ignore | ||
| } | ||
| } | ||
|
|
||
| export const PendingTxMonitor = { _isTxMined, monitorTx, monitorAllTxs } | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.