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): issue with decimals #13892

Merged
merged 2 commits into from
Jun 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 14 additions & 28 deletions packages/bridge-ui/src/bridge/ERC20Bridge.spec.ts
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
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
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
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
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
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
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
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
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