Skip to content

Commit

Permalink
fix(bridge-ui): issue with decimals (#13892)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jscriptcoder
jscriptcoder authored Jun 6, 2023
1 parent d97c4fa commit fbed474
Show file tree
Hide file tree
Showing 12 changed files with 96 additions and 85 deletions.
42 changes: 14 additions & 28 deletions packages/bridge-ui/src/bridge/ERC20Bridge.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ jest.mock('ethers', () => ({
const wallet = new Wallet('0x');

const opts: BridgeOpts = {
amountInWei: BigNumber.from(1),
amount: BigNumber.from(1),
signer: wallet,
tokenAddress: '0xtoken',
srcChainId: L1_CHAIN_ID,
Expand All @@ -54,7 +54,7 @@ const opts: BridgeOpts = {
};

const approveOpts: ApproveOpts = {
amountInWei: BigNumber.from(1),
amount: BigNumber.from(1),
signer: wallet,
contractAddress: '0x456',
spenderAddress: '0x789',
Expand All @@ -66,9 +66,7 @@ describe('bridge tests', () => {
});

it('requires allowance returns true when allowance has not been set', async () => {
mockContract.allowance.mockImplementationOnce(() =>
opts.amountInWei.sub(1),
);
mockContract.allowance.mockImplementationOnce(() => opts.amount.sub(1));

mockSigner.getAddress.mockImplementationOnce(() => '0xfake');

Expand All @@ -86,9 +84,7 @@ describe('bridge tests', () => {
});

it('requires allowance returns true when allowance is > than amount', async () => {
mockContract.allowance.mockImplementationOnce(() =>
opts.amountInWei.add(1),
);
mockContract.allowance.mockImplementationOnce(() => opts.amount.add(1));
mockSigner.getAddress.mockImplementationOnce(() => '0xfake');

const bridge: Bridge = new ERC20Bridge(null);
Expand All @@ -105,7 +101,7 @@ describe('bridge tests', () => {
});

it('requires allowance returns true when allowance is === amount', async () => {
mockContract.allowance.mockImplementationOnce(() => opts.amountInWei);
mockContract.allowance.mockImplementationOnce(() => opts.amount);
mockSigner.getAddress.mockImplementationOnce(() => '0xfake');

const bridge: Bridge = new ERC20Bridge(null);
Expand All @@ -122,9 +118,7 @@ describe('bridge tests', () => {
});

it('approve throws when amount is already greater than whats set', async () => {
mockContract.allowance.mockImplementationOnce(() =>
opts.amountInWei.add(1),
);
mockContract.allowance.mockImplementationOnce(() => opts.amount.add(1));

mockSigner.getAddress.mockImplementationOnce(() => '0xfake');

Expand All @@ -143,9 +137,7 @@ describe('bridge tests', () => {
});

it('approve succeeds when allowance is less than what is being requested', async () => {
mockContract.allowance.mockImplementationOnce(() =>
opts.amountInWei.sub(1),
);
mockContract.allowance.mockImplementationOnce(() => opts.amount.sub(1));

mockSigner.getAddress.mockImplementationOnce(() => '0xfake');

Expand All @@ -161,14 +153,12 @@ describe('bridge tests', () => {
);
expect(mockContract.approve).toHaveBeenCalledWith(
approveOpts.spenderAddress,
approveOpts.amountInWei,
approveOpts.amount,
);
});

it('bridge throws when requires approval', async () => {
mockContract.allowance.mockImplementationOnce(() =>
opts.amountInWei.sub(1),
);
mockContract.allowance.mockImplementationOnce(() => opts.amount.sub(1));

const bridge: Bridge = new ERC20Bridge(null);

Expand All @@ -182,9 +172,7 @@ describe('bridge tests', () => {
});

it('bridge calls senderc20 when doesnt require approval', async () => {
mockContract.allowance.mockImplementationOnce(() =>
opts.amountInWei.add(1),
);
mockContract.allowance.mockImplementationOnce(() => opts.amount.add(1));
mockSigner.getAddress.mockImplementation(() => '0xfake');

const bridge: Bridge = new ERC20Bridge(null);
Expand All @@ -198,7 +186,7 @@ describe('bridge tests', () => {
opts.destChainId,
'0x',
opts.tokenAddress,
opts.amountInWei,
opts.amount,
BigNumber.from(3140000),
opts.processingFeeInWei,
'0xfake',
Expand All @@ -210,17 +198,15 @@ describe('bridge tests', () => {
});

it('bridge calls senderc20 when doesnt requires approval, with no processing fee and memo', async () => {
mockContract.allowance.mockImplementationOnce(() =>
opts.amountInWei.add(1),
);
mockContract.allowance.mockImplementationOnce(() => opts.amount.add(1));
mockSigner.getAddress.mockImplementation(() => '0xfake');

const bridge: Bridge = new ERC20Bridge(null);

expect(mockContract.sendERC20).not.toHaveBeenCalled();

const opts: BridgeOpts = {
amountInWei: BigNumber.from(1),
amount: BigNumber.from(1),
signer: wallet,
tokenAddress: '0xtoken',
srcChainId: L1_CHAIN_ID,
Expand All @@ -235,7 +221,7 @@ describe('bridge tests', () => {
opts.destChainId,
'0xfake',
opts.tokenAddress,
opts.amountInWei,
opts.amount,
BigNumber.from(3000000),
BigNumber.from(0),
'0xfake',
Expand Down
16 changes: 8 additions & 8 deletions packages/bridge-ui/src/bridge/ERC20Bridge.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export class ERC20Bridge implements Bridge {
srcChainId: opts.srcChainId,
destChainId: opts.destChainId,

depositValue: opts.amountInWei,
depositValue: opts.amount,
callValue: 0,
processingFee,
gasLimit,
Expand Down Expand Up @@ -104,7 +104,7 @@ export class ERC20Bridge implements Bridge {
return this.spenderRequiresAllowance(
opts.contractAddress,
opts.signer,
opts.amountInWei,
opts.amount,
opts.spenderAddress,
);
}
Expand All @@ -113,7 +113,7 @@ export class ERC20Bridge implements Bridge {
const requiresAllowance = await this.spenderRequiresAllowance(
opts.contractAddress,
opts.signer,
opts.amountInWei,
opts.amount,
opts.spenderAddress,
);

Expand All @@ -129,10 +129,10 @@ export class ERC20Bridge implements Bridge {

try {
log(
`Approving ${opts.amountInWei} tokens for spender "${opts.spenderAddress}"`,
`Approving ${opts.amount} tokens for spender "${opts.spenderAddress}"`,
);

const tx = await contract.approve(opts.spenderAddress, opts.amountInWei);
const tx = await contract.approve(opts.spenderAddress, opts.amount);

log('Approval sent with transaction', tx);

Expand All @@ -149,7 +149,7 @@ export class ERC20Bridge implements Bridge {
const requiresAllowance = await this.spenderRequiresAllowance(
opts.tokenAddress,
opts.signer,
opts.amountInWei,
opts.amount,
opts.tokenVaultAddress,
);

Expand All @@ -168,7 +168,7 @@ export class ERC20Bridge implements Bridge {
message.destChainId,
message.to,
opts.tokenAddress,
opts.amountInWei,
opts.amount,
message.gasLimit,
message.processingFee,
message.refundAddress,
Expand Down Expand Up @@ -199,7 +199,7 @@ export class ERC20Bridge implements Bridge {
message.destChainId,
message.to,
opts.tokenAddress,
opts.amountInWei,
opts.amount,
message.gasLimit,
message.processingFee,
message.refundAddress,
Expand Down
8 changes: 4 additions & 4 deletions packages/bridge-ui/src/bridge/ETHBridge.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ describe('bridge tests', () => {
const bridge: Bridge = new ETHBridge(null);

const requires = await bridge.requiresAllowance({
amountInWei: BigNumber.from(1),
amount: BigNumber.from(1),
signer: new Wallet('0x'),
contractAddress: '0x1234',
spenderAddress: '0x',
Expand All @@ -63,7 +63,7 @@ describe('bridge tests', () => {
const bridge: Bridge = new ETHBridge(null);

const tx = await bridge.approve({
amountInWei: BigNumber.from(1),
amount: BigNumber.from(1),
signer: new Wallet('0x'),
contractAddress: '0x1234',
spenderAddress: '0x',
Expand All @@ -81,7 +81,7 @@ describe('bridge tests', () => {
});

const opts: BridgeOpts = {
amountInWei: BigNumber.from(1),
amount: BigNumber.from(1),
signer: wallet,
tokenAddress: '',
srcChainId: L1_CHAIN_ID,
Expand Down Expand Up @@ -126,7 +126,7 @@ describe('bridge tests', () => {
});

const opts: BridgeOpts = {
amountInWei: BigNumber.from(1),
amount: BigNumber.from(1),
signer: wallet,
tokenAddress: '',
srcChainId: L1_CHAIN_ID,
Expand Down
4 changes: 2 additions & 2 deletions packages/bridge-ui/src/bridge/ETHBridge.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ export class ETHBridge implements Bridge {

const depositValue =
opts.to.toLowerCase() === owner.toLowerCase()
? opts.amountInWei
? opts.amount
: BigNumber.from(0);

const callValue =
opts.to.toLowerCase() === owner.toLowerCase()
? BigNumber.from(0)
: opts.amountInWei;
: opts.amount;

const processingFee = opts.processingFeeInWei ?? BigNumber.from(0);

Expand Down
21 changes: 13 additions & 8 deletions packages/bridge-ui/src/components/BridgeForm/BridgeForm.svelte
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,14 @@
signer,
);
log(`Checking allowance for token ${token.symbol}`);
const parsedAmount = ethers.utils.parseUnits(amount, token.decimals);
log(
`Checking allowance for token ${token.symbol} and amount ${parsedAmount}`,
);
const isRequired = await $activeBridge.requiresAllowance({
amountInWei: ethers.utils.parseUnits(amount, token.decimals),
amount: parsedAmount,
signer: signer,
contractAddress: address,
spenderAddress: tokenVaults[srcChain.id],
Expand Down Expand Up @@ -201,11 +205,12 @@
);
const spenderAddress = tokenVaults[$srcChain.id];
const parsedAmount = ethers.utils.parseUnits(amount, _token.decimals);
log(`Approving token ${_token.symbol}`);
const tx = await $activeBridge.approve({
amountInWei: ethers.utils.parseUnits(amount, _token.decimals),
amount: parsedAmount,
signer: $signer,
contractAddress,
spenderAddress,
Expand Down Expand Up @@ -253,7 +258,7 @@
const gasEstimate = await $activeBridge.estimateGas({
...bridgeOpts,
// We need an amount, and user might not have entered one at this point
amountInWei: BigNumber.from(1),
amount: BigNumber.from(1),
});
const feeData = await fetchFeeData();
Expand Down Expand Up @@ -296,7 +301,7 @@
return;
}
const amountInWei = ethers.utils.parseUnits(amount, _token.decimals);
const parsedAmount = ethers.utils.parseUnits(amount, _token.decimals);
const provider = providers[$destChain.id];
const destTokenVaultAddress = tokenVaults[$destChain.id];
Expand All @@ -322,7 +327,7 @@
);
const bridgeOpts: BridgeOpts = {
amountInWei,
amount: parsedAmount,
signer: $signer,
tokenAddress,
srcChainId: $srcChain.id,
Expand Down Expand Up @@ -368,7 +373,7 @@
srcChainId: $srcChain.id,
destChainId: $destChain.id,
symbol: _token.symbol,
amountInWei,
amount: parsedAmount,
from: tx.from,
hash: tx.hash,
status: MessageStatus.New,
Expand Down Expand Up @@ -444,7 +449,7 @@
try {
const feeData = await fetchFeeData();
const gasEstimate = await $activeBridge.estimateGas({
amountInWei: BigNumber.from(1),
amount: BigNumber.from(1),
signer: $signer,
tokenAddress: await getAddressForToken(
$token,
Expand Down
2 changes: 1 addition & 1 deletion packages/bridge-ui/src/components/Transactions/Transaction.svelte
View file
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@
{@const { depositValue, callValue } = transaction.message}
{utils.formatEther(depositValue.eq(0) ? callValue : depositValue)}
{:else}
{utils.formatUnits(transaction.amountInWei)}
{utils.formatUnits(transaction.amount, transaction.decimals)}
{/if}
{transaction.symbol ?? 'ETH'}
</td>
Expand Down
4 changes: 2 additions & 2 deletions packages/bridge-ui/src/domain/bridge.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,14 @@ export enum BridgeType {
}

export type ApproveOpts = {
amountInWei: BigNumber;
amount: BigNumber;
contractAddress: string;
signer: ethers.Signer;
spenderAddress: string;
};

export type BridgeOpts = {
amountInWei: BigNumber;
amount: BigNumber;
signer: ethers.Signer;
tokenAddress: string;
srcChainId: ChainID;
Expand Down
3 changes: 2 additions & 1 deletion packages/bridge-ui/src/domain/transaction.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,9 @@ export type BridgeTransaction = {
msgHash?: string;
message?: Message;
interval?: NodeJS.Timer;
amountInWei?: BigNumber;
amount?: BigNumber;
symbol?: string;
decimals?: number;
srcChainId: ChainID;
destChainId: ChainID;
};
Expand Down
3 changes: 3 additions & 0 deletions packages/bridge-ui/src/relayer-api/RelayerAPIService.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ const mockContract = {
queryFilter: jest.fn(),
getMessageStatus: jest.fn(),
symbol: jest.fn(),
decimals: jest.fn(),
filters: {
ERC20Sent: () => 'ERC20Sent',
},
Expand Down Expand Up @@ -82,6 +83,7 @@ describe('RelayerAPIService', () => {
mockContract.getMessageStatus.mockResolvedValue(MessageStatus.New);
mockContract.queryFilter.mockResolvedValue(mockErc20Query);
mockContract.symbol.mockResolvedValue('BLL');
mockContract.decimals.mockResolvedValue(18);
});

it('should get transactions from API', async () => {
Expand Down Expand Up @@ -254,6 +256,7 @@ describe('RelayerAPIService', () => {
expect(mockContract.getMessageStatus).toHaveBeenCalledTimes(10);
expect(mockContract.queryFilter).toHaveBeenCalledTimes(9);
expect(mockContract.symbol).toHaveBeenCalledTimes(9);
expect(mockContract.decimals).toHaveBeenCalledTimes(9);
});

it('should not get transactions with wrong address', async () => {
Expand Down
Loading

0 comments on commit fbed474

Please sign in to comment.