Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(bridge-ui): pending transactions custom store with better error handling #13581

Merged
merged 11 commits into from
Apr 19, 2023
24 changes: 1 addition & 23 deletions packages/bridge-ui/src/App.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,7 @@
import { MetaMaskConnector } from '@wagmi/core/connectors/metaMask';

import { setupI18n } from './i18n';
import {
pendingTransactions,
transactioner,
transactions,
} from './store/transactions';
import { transactioner, transactions } from './store/transactions';
import Navbar from './components/Navbar.svelte';
import Toast, { successToast } from './components/Toast.svelte';
import { signer } from './store/signer';
Expand Down Expand Up @@ -130,24 +126,6 @@
}
});

pendingTransactions.subscribe((store) => {
(async () => {
const confirmedPendingTxIndex = await Promise.race(
store.map((tx, index) => {
return new Promise<number>((resolve) => {
$signer.provider
.waitForTransaction(tx.hash, 1)
.then(() => resolve(index));
});
}),
);
successToast('Transaction completed!');
let s = store;
s.splice(confirmedPendingTxIndex, 1);
pendingTransactions.set(s);
})();
});

const transactionToIntervalMap = new Map();

transactions.subscribe((store) => {
Expand Down
21 changes: 11 additions & 10 deletions packages/bridge-ui/src/components/Transactions/Transaction.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -108,15 +108,16 @@
srcBridgeAddress: chains[bridgeTx.fromChainId].bridgeAddress,
});

pendingTransactions.update((store) => {
store.push(tx);
return store;
});

successToast($_('toast.transactionSent'));

await pendingTransactions.add(tx, $signer);

// TODO: keep the MessageStatus as contract and use another way.
transaction.status = MessageStatus.ClaimInProgress;

successToast('Transaction completed!');
} catch (e) {
// TODO: handle potential transaction failure
console.error(e);
errorToast($_('toast.errorSendingTransaction'));
} finally {
Expand Down Expand Up @@ -153,13 +154,13 @@
srcTokenVaultAddress: tokenVaults[bridgeTx.fromChainId],
});

pendingTransactions.update((store) => {
store.push(tx);
return store;
});

successToast($_('toast.transactionSent'));

pendingTransactions.add(tx, $signer);

successToast('Transaction completed!');
} catch (e) {
// TODO: handle potential transaction failure
console.error(e);
errorToast($_('toast.errorSendingTransaction'));
} finally {
Expand Down
29 changes: 16 additions & 13 deletions packages/bridge-ui/src/components/form/BridgeForm.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@
async function getUserBalance(
signer: ethers.Signer,
token: Token,
fromChain: Chain,

Check warning on line 88 in packages/bridge-ui/src/components/form/BridgeForm.svelte

View workflow job for this annotation

GitHub Actions / build

'fromChain' is defined but never used
) {
if (signer && token) {
if (token.symbol == ETHToken.symbol) {
Expand Down Expand Up @@ -171,17 +171,18 @@
spenderAddress: tokenVaults[$fromChain.id],
});

pendingTransactions.update((store) => {
store.push(tx);
return store;
});

successToast($_('toast.transactionSent'));
await $signer.provider.waitForTransaction(tx.hash, 1);

await pendingTransactions.add(tx, $signer);

requiresAllowance = false;

successToast('Transaction completed!');
} catch (e) {
console.error(e);
// TODO: if we have TransactionReceipt here means the tx failed
// We might want to give the user a link to etherscan
// to see the tx details
errorToast($_('toast.errorSendingTransaction'));
} finally {
loading = false;
Expand Down Expand Up @@ -262,12 +263,17 @@
);

if (!doesUserHaveEnoughBalance) {
// TODO: about custom errors and catch it in the catch block?
errorToast('Insufficient ETH balance');
return;
}

const tx = await $activeBridge.Bridge(bridgeOpts);

successToast($_('toast.transactionSent'));

await pendingTransactions.add(tx, $signer);

// tx.chainId is not set immediately but we need it later. set it
// manually.
tx.chainId = $fromChain.id;
Expand All @@ -284,6 +290,7 @@
hash: tx.hash,
status: MessageStatus.New,
};

if (!transactions) {
transactions = [bridgeTransaction];
} else {
Expand All @@ -292,11 +299,6 @@

$transactioner.updateStorageByAddress(userAddress, transactions);

pendingTransactions.update((store) => {
store.push(tx);
return store;
});

const allTransactions = $transactionsStore;

// get full BridgeTransaction object
Expand All @@ -307,11 +309,12 @@

transactionsStore.set([bridgeTransaction, ...allTransactions]);

successToast($_('toast.transactionSent'));
await $signer.provider.waitForTransaction(tx.hash, 1);
memo = '';

successToast('Transaction completed!');
} catch (e) {
console.error(e);
// TODO: Same as in approve()
errorToast($_('toast.errorSendingTransaction'));
} finally {
loading = false;
Expand Down
12 changes: 6 additions & 6 deletions packages/bridge-ui/src/components/modals/FaucetModal.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -73,17 +73,17 @@
const address = await $signer.getAddress();
const tx = await contract.mint(address);

pendingTransactions.update((store) => {
store.push(tx);
return store;
});

successToast($_('toast.transactionSent'));

await pendingTransactions.add(tx, $signer);

isOpen = false;
await $signer.provider.waitForTransaction(tx.hash, 1);

successToast('Transaction completed!');

await onMint();
} catch (e) {
// TODO: handle potential transaction failure
console.error(e);
errorToast($_('toast.errorSendingTransaction'));
}
Expand Down
67 changes: 67 additions & 0 deletions packages/bridge-ui/src/store/transactions.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import { get } from 'svelte/store';
import type { Signer, Transaction, ethers } from 'ethers';
import { pendingTransactions } from './transactions';

jest.mock('../constants/envVars');

// Transaction we're going to add to the store
const tx = { hash: '0x789' } as Transaction;

// These are the pending transactions we'll have initially in the store
const initialTxs = [{ hash: '0x123' }, { hash: '0x456' }] as Transaction[];

const mockSigner = (receipt: ethers.providers.TransactionReceipt) => {
const waitForTransaction = jest
.fn()
.mockImplementation(() => Promise.resolve(receipt));

return {
provider: { waitForTransaction },
} as unknown as Signer;
};

describe('transaction stores', () => {
beforeEach(() => {
pendingTransactions.set(initialTxs);
});

it('tests a successful pendingTransactions', () => {
const txTeceipt = { status: 1 } as ethers.providers.TransactionReceipt;
const signer = mockSigner(txTeceipt);

pendingTransactions
.add(tx, signer)
.then((receipt) => {
// The transaction should have been removed from the store
expect(get(pendingTransactions)).toStrictEqual(initialTxs);

expect(receipt).toEqual(txTeceipt);
})
.catch(() => {
throw new Error('Should not have thrown');
});

// The transaction should have added to the store
expect(get(pendingTransactions)).toStrictEqual([...initialTxs, tx]);
});

it('tests a failed pendingTransactions custom store', () => {
const txTeceipt = { status: 0 } as ethers.providers.TransactionReceipt;
const signer = mockSigner(txTeceipt);

pendingTransactions
.add(tx, signer)
.then(() => {
throw new Error('Should have thrown');
})
.catch((receipt) => {
// The transaction should have been removed from the store
expect(get(pendingTransactions)).toStrictEqual(initialTxs);

expect(receipt).toEqual(txTeceipt);
});

// The transaction should have added to the store
expect(get(pendingTransactions)).toStrictEqual([...initialTxs, tx]);
});
});
71 changes: 65 additions & 6 deletions packages/bridge-ui/src/store/transactions.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,69 @@
import { writable } from 'svelte/store';

import type { Transaction } from 'ethers';
import type { Signer, Transaction, ethers } from 'ethers';
import type { BridgeTransaction, Transactioner } from '../domain/transactions';
import { Deferred } from '../utils/Deferred';

export const transactions = writable<BridgeTransaction[]>([]);
export const transactioner = writable<Transactioner>();

// Custom store: pendingTransactions
const { subscribe, set, update } = writable<Transaction[]>([]);
export const pendingTransactions = {
/**
* We're creating here a custom store, which is a writable store.
* We must stick to the store contract, which is:
*/
set,
subscribe,
// update, // this method is optional.

/**
* Custom method, which will help us add a new transaction to the store
* and get it removed onces the transaction is mined.
*/
add: (tx: Transaction, signer: Signer) => {
const deferred = new Deferred<ethers.providers.TransactionReceipt>();

update((txs: Transaction[]) => {
// New array with the new transaction appended
const newPendingTransactions = [...txs, tx];

// Save the index of the new transaction to later on remove it
// from the list of pending transactions.
const idxAppendedTransaction = newPendingTransactions.length - 1;

// Next step is to wait for the transaction to be mined
// before removing it from the store.

/**
* Returns a Promise which will not resolve until transactionHash is mined.
* If confirms is 0, this method is non-blocking and if the transaction
* has not been mined returns null. Otherwise, this method will block until
* the transaction has confirms blocks mined on top of the block in which
* is was mined.
* See https://docs.ethers.org/v5/api/providers/provider/#Provider-waitForTransaction
*/
signer.provider.waitForTransaction(tx.hash, 1).then((receipt) => {
// The transaction has been mined.

// Removes the transaction from the store
update((txs: Transaction[]) => {
const copyPendingTransactions = [...txs];
copyPendingTransactions.splice(idxAppendedTransaction, 1);
return copyPendingTransactions;
});

// Resolves or rejects the promise depending on the transaction status.
if (receipt.status === 1) {
deferred.resolve(receipt);
} else {
deferred.reject(receipt);
}
});

const pendingTransactions = writable<Transaction[]>([]);
const transactions = writable<BridgeTransaction[]>([]);
const transactioner = writable<Transactioner>();
return newPendingTransactions;
});

export { pendingTransactions, transactions, transactioner };
return deferred.promise;
},
};
25 changes: 25 additions & 0 deletions packages/bridge-ui/src/utils/Deferred.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { Deferred } from './Deferred';

describe('Deferred', () => {
it('should resolve', async () => {
const deferred = new Deferred();
deferred.resolve('test');

try {
await expect(deferred.promise).resolves.toBe('test');
} catch (e) {
throw Error('This should never happen');
}
});

it('should reject', async () => {
const deferred = new Deferred();
deferred.reject('test');

try {
await deferred.promise;
} catch (err) {
expect(err).toBe('test');
}
});
});
12 changes: 12 additions & 0 deletions packages/bridge-ui/src/utils/Deferred.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
export class Deferred<T> {
public promise: Promise<T>;
public resolve: (value?: T) => void;
public reject: (reason?: T) => void;

constructor() {
this.promise = new Promise((resolve, reject) => {
this.resolve = resolve;
this.reject = reject;
});
}
}