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.

Sign up for GitHub

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: bug where user could proceed with invalid destination address #176

Merged
merged 8 commits into from
Apr 19, 2024
Merged

fix: bug where user could proceed with invalid destination address #176

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1d7ebe6
wip
Lykhoyda Apr 16, 2024
2466978
fix transfer button
Lykhoyda Apr 17, 2024
faf4ea6
fix tests
Lykhoyda Apr 17, 2024
6f55392
lykhoyda/network_whitelisting
Lykhoyda Apr 2, 2024
f45af40
fix
Lykhoyda Apr 18, 2024
35d8576
nit
Lykhoyda Apr 18, 2024
2bd0c5b
update]
Lykhoyda Apr 19, 2024
d49dc4e
fix tests
Lykhoyda Apr 19, 2024
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
18 changes: 8 additions & 10 deletions packages/widget/src/components/address-input/address-input.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { when } from 'lit/directives/when.js';

import type { PropertyValues } from '@lit/reactive-element';
import { validateAddress } from '../../utils';
import { BaseComponent } from '../common/base-component/base-component';
import { BaseComponent } from '../common';

import { styles } from './styles';

Expand All @@ -20,11 +20,10 @@ export class AddressInput extends BaseComponent {
address: string = '';

@property({ attribute: false })
onAddressChange: (address: string) => void = () => {};
onAddressChange: (address: string, errorMessage?: string | null) => void =
() => {};

@property({
type: String
})
@property({ attribute: false })
networkType: Network = Network.EVM;

@state()
Expand All @@ -43,13 +42,12 @@ export class AddressInput extends BaseComponent {
}

if (!trimedValue) {
void this.onAddressChange('');
void this.onAddressChange('', null);
return;
}

this.errorMessage = validateAddress(trimedValue, this.networkType);

this.onAddressChange(trimedValue);
this.onAddressChange(trimedValue, this.errorMessage);
};

protected updated(changedProperties: PropertyValues): void {
Expand All @@ -60,7 +58,7 @@ export class AddressInput extends BaseComponent {
}

render(): HTMLTemplateResult {
return html` <section class="inputAddressSection">
return html`<section class="inputAddressSection">
<div class="inputAddressContainer">
<label class="labelContainer">
<span>Send to </span>
Expand All @@ -79,7 +77,7 @@ export class AddressInput extends BaseComponent {
}
}}
@input=${(evt: Event) =>
this.handleAddressChange((evt.target as HTMLInputElement).value)}
this.handleAddressChange((evt.target as HTMLTextAreaElement).value)}
></textarea>
</div>
</section>`;
Expand Down
12 changes: 8 additions & 4 deletions packages/widget/src/components/transfer/fungible/transfer-button/transfer-button.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ import type { Button } from '../../../common';
import { BaseComponent } from '../../../common';

const enabledStates = [
FungibleTransferState.WRONG_CHAIN,
FungibleTransferState.WALLET_NOT_CONNECTED,
FungibleTransferState.PENDING_APPROVALS,
FungibleTransferState.PENDING_TRANSFER,
FungibleTransferState.COMPLETED
FungibleTransferState.COMPLETED,
FungibleTransferState.WRONG_CHAIN,
FungibleTransferState.WALLET_NOT_CONNECTED
];

const loadingStates = [
Expand Down Expand Up @@ -54,6 +54,10 @@ export class FungibleTransferButton extends BaseComponent {
],
[FungibleTransferState.WALLET_NOT_CONNECTED, () => 'Connect Wallet'],
[FungibleTransferState.WRONG_CHAIN, () => 'Switch chain'],
[
FungibleTransferState.WRONG_DESTINATION_ADDRESS,
() => 'Wrong Address'
Lykhoyda marked this conversation as resolved.
Show resolved Hide resolved
],
[FungibleTransferState.PENDING_APPROVALS, () => 'Approve token'],
[FungibleTransferState.PENDING_TRANSFER, () => 'Transfer'],
[
Expand All @@ -67,7 +71,7 @@ export class FungibleTransferButton extends BaseComponent {
[FungibleTransferState.COMPLETED, () => 'Start new transfer']
],
() => 'Loading'
)!}
)}
@click=${this.onClick}
></sygma-action-button>`;
}
Expand Down
37 changes: 30 additions & 7 deletions packages/widget/src/controllers/transfers/fungible-token-transfer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export enum FungibleTransferState {
MISSING_RESOURCE,
MISSING_RESOURCE_AMOUNT,
MISSING_DESTINATION_ADDRESS,
WRONG_DESTINATION_ADDRESS,
WALLET_NOT_CONNECTED,
WRONG_CHAIN,
PENDING_APPROVALS,
Expand All @@ -35,6 +36,7 @@ export class FungibleTokenTransferController implements ReactiveController {
public waitingTxExecution: boolean = false;
public transferTransactionId?: string;
public errorMessage: string | null = null;
public invalidDestinationAddressErrorMessage: string | null = null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since controller doesn't really care about address. Maybe we could just have null state for desinationAddress:

empty string = missing address
null/undefined = invalid address
anything else valid address

wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mpetrunic In general, I like the idea as I also would like to avoid sending the state back to the parent. The only issue I see is that if I enter the substrate address when the EVM will be expected, I will have an error on the field level, but the button will have an "enabled" state, which is incorrect. So I will call "validateAddress" one more time. I pushed the code with the changes


public sourceNetwork?: Domain;
public destinationNetwork?: Domain;
Expand Down Expand Up @@ -110,6 +112,7 @@ export class FungibleTokenTransferController implements ReactiveController {
this.sourceNetwork = undefined;
}
this.destinationNetwork = undefined;
this.selectedResource = undefined;
this.pendingEvmApprovalTransactions = [];
this.pendingEvmTransferTransaction = undefined;
this.destinationAddress = '';
Expand Down Expand Up @@ -164,32 +167,37 @@ export class FungibleTokenTransferController implements ReactiveController {
this.host.requestUpdate();
};

onDestinationAddressChange = (address: string): void => {
onDestinationAddressChange = (
address: string,
errorMessage: string | null
): void => {
this.destinationAddress = address;
this.invalidDestinationAddressErrorMessage = errorMessage;

if (this.destinationAddress.length === 0) {
this.pendingEvmApprovalTransactions = [];
this.pendingEvmTransferTransaction = undefined;
this.invalidDestinationAddressErrorMessage = null;
}
void this.buildTransactions();
this.host.requestUpdate();
};

getTransferState(): FungibleTransferState {
// Enabled state
if (this.transferTransactionId) {
return FungibleTransferState.COMPLETED;
}

// Loading states
if (this.waitingUserConfirmation) {
return FungibleTransferState.WAITING_USER_CONFIRMATION;
}
if (this.waitingTxExecution) {
return FungibleTransferState.WAITING_TX_EXECUTION;
}
if (this.pendingEvmApprovalTransactions.length > 0) {
return FungibleTransferState.PENDING_APPROVALS;
}
if (this.pendingEvmTransferTransaction) {
return FungibleTransferState.PENDING_TRANSFER;
}

// Error States
if (!this.sourceNetwork) {
return FungibleTransferState.MISSING_SOURCE_NETWORK;
}
Expand All @@ -199,12 +207,19 @@ export class FungibleTokenTransferController implements ReactiveController {
if (!this.selectedResource) {
return FungibleTransferState.MISSING_RESOURCE;
}

if (this.invalidDestinationAddressErrorMessage?.length) {
return FungibleTransferState.WRONG_DESTINATION_ADDRESS;
}

if (this.resourceAmount.eq(0)) {
return FungibleTransferState.MISSING_RESOURCE_AMOUNT;
}
if (this.destinationAddress === '') {
return FungibleTransferState.MISSING_DESTINATION_ADDRESS;
}

// Enabled States
if (
!this.walletContext.value?.evmWallet &&
!this.walletContext.value?.substrateWallet
Expand All @@ -218,6 +233,14 @@ export class FungibleTokenTransferController implements ReactiveController {
) {
return FungibleTransferState.WRONG_CHAIN;
}

if (this.pendingEvmApprovalTransactions.length > 0) {
return FungibleTransferState.PENDING_APPROVALS;
}
if (this.pendingEvmTransferTransaction) {
return FungibleTransferState.PENDING_TRANSFER;
}

return FungibleTransferState.UNKNOWN;
}

Expand Down
41 changes: 25 additions & 16 deletions packages/widget/tests/unit/components/address-input/address-input.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import { html } from 'lit';
import { Network } from '@buildwithsygma/sygma-sdk-core';
import { AddressInput } from '../../../../src/components';

const errorMessageInvalidAddress = 'invalid Ethereum address';

describe('address-input component', function () {
afterEach(() => {
fixtureCleanup();
Expand Down Expand Up @@ -110,9 +112,13 @@ describe('address-input component', function () {
await listener;

assert.equal(mockAddressChangeHandler.mock.calls.length, 3);
assert.deepEqual(mockAddressChangeHandler.mock.calls[0], ['0x123']);
assert.deepEqual(mockAddressChangeHandler.mock.calls[0], [
'0x123',
errorMessageInvalidAddress
]);
assert.deepEqual(mockAddressChangeHandler.mock.lastCall, [
'0xebFC7A970CAAbC18C8e8b7367147C18FC7585492'
'0xebFC7A970CAAbC18C8e8b7367147C18FC7585492',
null
]);
});

Expand All @@ -137,8 +143,8 @@ describe('address-input component', function () {
await listener;

assert.equal(mockAddressChangeHandler.mock.calls.length, 3);
assert.deepEqual(mockAddressChangeHandler.mock.calls[0], ['']);
assert.deepEqual(mockAddressChangeHandler.mock.calls[1], ['']);
assert.deepEqual(mockAddressChangeHandler.mock.calls[0], ['', null]);
assert.deepEqual(mockAddressChangeHandler.mock.calls[1], ['', null]);

const errorMessage = el.shadowRoot!.querySelector(
'.errorMessage'
Expand All @@ -154,9 +160,10 @@ describe('address-input component', function () {

assert.equal(mockAddressChangeHandler.mock.calls.length, 4);
assert.deepEqual(mockAddressChangeHandler.mock.calls[2], [
'0xebFC7A970CAAbC18C8e8b7367147C18FC7'
'0xebFC7A970CAAbC18C8e8b7367147C18FC7',
errorMessageInvalidAddress
]);
assert.deepEqual(mockAddressChangeHandler.mock.lastCall, ['']);
assert.deepEqual(mockAddressChangeHandler.mock.lastCall, ['', null]);

const errorMessageAfterClean = el.shadowRoot!.querySelector(
'.errorMessage'
Expand Down Expand Up @@ -188,9 +195,10 @@ describe('address-input component', function () {
await listener;

assert.equal(mockAddressChangeHandler.mock.calls.length, 3);
assert.deepEqual(mockAddressChangeHandler.mock.calls[0], ['']);
assert.deepEqual(mockAddressChangeHandler.mock.calls[0], ['', null]);
assert.deepEqual(mockAddressChangeHandler.mock.lastCall, [
'42sydUvocBuEorweEPqxY5vZae1VaTtWoJFiKMrPbRamy2BL'
'42sydUvocBuEorweEPqxY5vZae1VaTtWoJFiKMrPbRamy2BL',
null
]);
});

Expand All @@ -216,9 +224,10 @@ describe('address-input component', function () {
await listener;

assert.equal(mockAddressChangeHandler.mock.calls.length, 3);
assert.deepEqual(mockAddressChangeHandler.mock.calls[0], ['']);
assert.deepEqual(mockAddressChangeHandler.mock.calls[0], ['', null]);
assert.deepEqual(mockAddressChangeHandler.mock.lastCall, [
'0xebFC7A970CAAbC18C8e8b7367147C18FC7585492'
'0xebFC7A970CAAbC18C8e8b7367147C18FC7585492',
null
]);
});

Expand All @@ -245,8 +254,8 @@ describe('address-input component', function () {
await listener;

assert.equal(mockAddressChangeHandler.mock.calls.length, 3);
assert.deepEqual(mockAddressChangeHandler.mock.calls[0], ['']);
assert.deepEqual(mockAddressChangeHandler.mock.calls[1], ['']);
assert.deepEqual(mockAddressChangeHandler.mock.calls[0], ['', null]);
assert.deepEqual(mockAddressChangeHandler.mock.calls[1], ['', null]);

const errorMessage = el.shadowRoot!.querySelector(
'.errorMessage'
Expand Down Expand Up @@ -277,8 +286,8 @@ describe('address-input component', function () {
await listener;

assert.equal(mockAddressChangeHandler.mock.calls.length, 3);
assert.deepEqual(mockAddressChangeHandler.mock.calls[0], ['']);
assert.deepEqual(mockAddressChangeHandler.mock.calls[1], ['']);
assert.deepEqual(mockAddressChangeHandler.mock.calls[0], ['', null]);
assert.deepEqual(mockAddressChangeHandler.mock.calls[1], ['', null]);

const errorMessage = el.shadowRoot!.querySelector(
'.errorMessage'
Expand Down Expand Up @@ -308,8 +317,8 @@ describe('address-input component', function () {
await listener;

assert.equal(mockAddressChangeHandler.mock.calls.length, 3);
assert.deepEqual(mockAddressChangeHandler.mock.calls[0], ['']);
assert.deepEqual(mockAddressChangeHandler.mock.calls[1], ['']);
assert.deepEqual(mockAddressChangeHandler.mock.calls[0], ['', null]);
assert.deepEqual(mockAddressChangeHandler.mock.calls[1], ['', null]);

el.networkType = Network.SUBSTRATE;

Expand Down
Loading