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

Jump to bottom

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

merged 8 commits into from
Apr 19, 2024

Conversation

Lykhoyda
Copy link
Contributor

closes #164

@Lykhoyda Lykhoyda marked this pull request as ready for review April 17, 2024 14:44
@mpetrunic mpetrunic changed the title fix: fix transfer button status fix: bug where user could proceed with invalid destination address Apr 18, 2024
@Lykhoyda Lykhoyda requested a review from mpetrunic April 18, 2024 10:07
@Lykhoyda Lykhoyda requested a review from wainola April 18, 2024 13:49
wainola
wainola previously approved these changes Apr 18, 2024
View reviewed changes
mpetrunic
mpetrunic reviewed Apr 19, 2024
View reviewed changes
packages/widget/src/components/transfer/fungible/transfer-button/transfer-button.ts Outdated Show resolved Hide resolved
packages/widget/src/controllers/transfers/fungible-token-transfer.ts Outdated
@@ -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

@Lykhoyda Lykhoyda requested a review from mpetrunic April 19, 2024 12:21
@MakMuftic MakMuftic requested a review from wainola April 19, 2024 12:32
@Lykhoyda Lykhoyda requested a review from LyonSsS April 19, 2024 12:44
@Lykhoyda Lykhoyda added the QA label Apr 19, 2024
LyonSsS
LyonSsS approved these changes Apr 19, 2024
View reviewed changes
Copy link

@LyonSsS LyonSsS left a comment

Choose a reason for hiding this comment

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

Tested on preview link from PR branch and passed.

@Lykhoyda Lykhoyda merged commit 634a121 into main Apr 19, 2024
4 checks passed
@Lykhoyda Lykhoyda deleted the lykhoyda/fix_transfer_button branch April 19, 2024 13:42
@MakMuftic MakMuftic mentioned this pull request Apr 19, 2024
Merged
mpetrunic pushed a commit that referenced this pull request Apr 22, 2024
@MakMuftic
chore: release main (#60)
31fa5f3 
🤖 I have created a release *beep* *boop*
---


<details><summary>sygmaprotocol-react-widget: 0.1.0</summary>

## 0.1.0 (2024-04-19)


### Features

* base interface implementation for react package
([#61](#61))
([0f38d7c](0f38d7c))
* EmvWallet class base implementation
([74c9649](74c9649))


### Bug Fixes

* fix sygma namespace
([f9ab963](f9ab963))


### Miscellaneous Chores

* **main:** release 0.0.1
([8cff038](8cff038))
* release 0.1.0
([#101](#101))
([c8f1aed](c8f1aed))


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * @buildwithsygma/sygmaprotocol-widget bumped to 0.1.0
</details>

<details><summary>sygmaprotocol-widget: 0.1.0</summary>

##
[0.1.0](sygmaprotocol-widget-v0.1.0...sygmaprotocol-widget-v0.1.0)
(2024-04-19)


### Features

* action buttons v2
([#91](#91))
([5b2fbc5](5b2fbc5))
* add account balance refreshing for erc20 and evm native token
([#88](#88))
([e135edc](e135edc))
* adding token balance to amount component
([c7b6268](c7b6268))
* address to transfer
([#56](#56))
([9fa87ad](9fa87ad))
* allow bridge environment configuration
([#104](#104))
([7dd44e4](7dd44e4))
* amount input styles update
([#77](#77))
([6101005](6101005))
* base implementation of network selector for the widget component
([b04ac49](b04ac49))
* base interface implementation for react package
([#61](#61))
([0f38d7c](0f38d7c))
* base layout for widget + styles
([e259dca](e259dca))
* changes to properties names
([291e753](291e753))
* creation of mixin for property declaration and using component to
render view
([d22772e](d22772e))
* EmvWallet class base implementation
([74c9649](74c9649))
* evm and substrate wallet connection
([#72](#72))
([ccf2049](ccf2049))
* evm token transfer logic
([#95](#95))
([7ae8547](7ae8547))
* externalize wallet connect connection through props
([#94](#94))
([cc8617d](cc8617d))
* improvements on selector implementation and misc changes
([940317f](940317f))
* network selector component
([#70](#70))
([003416e](003416e))
* overlay
([#79](#79))
([751ddb1](751ddb1))
* remove wrong entrypoint for the widget
([9e0007e](9e0007e))
* removing abi definition, calling sdk function to get balance and
updating package.json file
([95b0653](95b0653))
* renaming files, improvements over style and adding the components into
the main layout
([edd4e35](edd4e35))
* reset fields after transfer is complete
([#163](#163))
([762e95a](762e95a)),
closes [#133](#133)
* SDK Manager package
([#25](#25))
([f9a8793](f9a8793))
* set destination address for EVM
([#129](#129))
([679346a](679346a))
* small modifications to amount component and connect component
([1fda019](1fda019))
* transfer status
([#89](#89))
([76c0265](76c0265))
* upon clicking on max, render max value and trigger change event
([24746f5](24746f5))
* utils functions, styles on diff file and rendering icons per chain id
([a2bc154](a2bc154))


### Bug Fixes

* add css reset
([#75](#75))
([d512f58](d512f58))
* address input not submitted on paste
([#108](#108))
([9acf7f0](9acf7f0))
* amount selector prefilled with zero
([#109](#109))
([b178429](b178429))
* bug where user could proceed with invalid destination address
([#176](#176))
([634a121](634a121)),
closes [#164](#164)
* clear wallet context on disconnect
([#152](#152))
([4a497e2](4a497e2))
* easier amount input
([#159](#159))
([8d6620e](8d6620e))
* error where start new transfer isn't working
([#126](#126))
([ca4ece0](ca4ece0))
* evm amount is now calculated as user input minus the fee
([#143](#143))
([d29771e](d29771e))
* fix sygma namespace
([f9ab963](f9ab963))
* incorrect transfer state when disconnecting wallet after transfer
completed
([#130](#130))
([359e6df](359e6df))
* input type and conditional render for token icon
([25d7db3](25d7db3))
* linter issue with style module
([edc34bb](edc34bb))
* max button rounds value
([#137](#137))
([01901e6](01901e6))
* misc fixes with icons
([#85](#85))
([fedbcd9](fedbcd9))
* prevent new lines and spaces in address input
([#81](#81))
([44a318e](44a318e))
* re-validate on account balance change
([#107](#107))
([7935f81](7935f81))
* removing log
([71f42d6](71f42d6))
* specific dispatchers either for network or token selector
([1ba9de3](1ba9de3))
* substrate address not validated bug
([#128](#128))
([74191b1](74191b1))
* switch networks and misc fixes
([#127](#127))
([db9ad8a](db9ad8a))
* Switching to a different TOKEN mapping for "From - TO" relationship is
not updated in UI if amount is inserted
([#160](#160))
([e2d16fd](e2d16fd))
* update sygma sdk version
([#82](#82))
([6d8447a](6d8447a))
* widget cloudflare deploy
([#68](#68))
([fe6c36f](fe6c36f))


### Miscellaneous Chores

* **main:** release 0.0.1
([8cff038](8cff038))
* release 0.1.0
([#101](#101))
([c8f1aed](c8f1aed))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
wainola pushed a commit that referenced this pull request Apr 22, 2024
@Lykhoyda @wainola
fix: bug where user could proceed with invalid destination address (#176
cf15f51 
)

closes #164
wainola pushed a commit that referenced this pull request Apr 22, 2024
@MakMuftic @wainola
chore: release main (#60)
1a0923b 
🤖 I have created a release *beep* *boop*
---


<details><summary>sygmaprotocol-react-widget: 0.1.0</summary>

## 0.1.0 (2024-04-19)


### Features

* base interface implementation for react package
([#61](#61))
([0f38d7c](0f38d7c))
* EmvWallet class base implementation
([74c9649](74c9649))


### Bug Fixes

* fix sygma namespace
([f9ab963](f9ab963))


### Miscellaneous Chores

* **main:** release 0.0.1
([8cff038](8cff038))
* release 0.1.0
([#101](#101))
([c8f1aed](c8f1aed))


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * @buildwithsygma/sygmaprotocol-widget bumped to 0.1.0
</details>

<details><summary>sygmaprotocol-widget: 0.1.0</summary>

##
[0.1.0](sygmaprotocol-widget-v0.1.0...sygmaprotocol-widget-v0.1.0)
(2024-04-19)


### Features

* action buttons v2
([#91](#91))
([5b2fbc5](5b2fbc5))
* add account balance refreshing for erc20 and evm native token
([#88](#88))
([e135edc](e135edc))
* adding token balance to amount component
([c7b6268](c7b6268))
* address to transfer
([#56](#56))
([9fa87ad](9fa87ad))
* allow bridge environment configuration
([#104](#104))
([7dd44e4](7dd44e4))
* amount input styles update
([#77](#77))
([6101005](6101005))
* base implementation of network selector for the widget component
([b04ac49](b04ac49))
* base interface implementation for react package
([#61](#61))
([0f38d7c](0f38d7c))
* base layout for widget + styles
([e259dca](e259dca))
* changes to properties names
([291e753](291e753))
* creation of mixin for property declaration and using component to
render view
([d22772e](d22772e))
* EmvWallet class base implementation
([74c9649](74c9649))
* evm and substrate wallet connection
([#72](#72))
([ccf2049](ccf2049))
* evm token transfer logic
([#95](#95))
([7ae8547](7ae8547))
* externalize wallet connect connection through props
([#94](#94))
([cc8617d](cc8617d))
* improvements on selector implementation and misc changes
([940317f](940317f))
* network selector component
([#70](#70))
([003416e](003416e))
* overlay
([#79](#79))
([751ddb1](751ddb1))
* remove wrong entrypoint for the widget
([9e0007e](9e0007e))
* removing abi definition, calling sdk function to get balance and
updating package.json file
([95b0653](95b0653))
* renaming files, improvements over style and adding the components into
the main layout
([edd4e35](edd4e35))
* reset fields after transfer is complete
([#163](#163))
([762e95a](762e95a)),
closes [#133](#133)
* SDK Manager package
([#25](#25))
([f9a8793](f9a8793))
* set destination address for EVM
([#129](#129))
([679346a](679346a))
* small modifications to amount component and connect component
([1fda019](1fda019))
* transfer status
([#89](#89))
([76c0265](76c0265))
* upon clicking on max, render max value and trigger change event
([24746f5](24746f5))
* utils functions, styles on diff file and rendering icons per chain id
([a2bc154](a2bc154))


### Bug Fixes

* add css reset
([#75](#75))
([d512f58](d512f58))
* address input not submitted on paste
([#108](#108))
([9acf7f0](9acf7f0))
* amount selector prefilled with zero
([#109](#109))
([b178429](b178429))
* bug where user could proceed with invalid destination address
([#176](#176))
([634a121](634a121)),
closes [#164](#164)
* clear wallet context on disconnect
([#152](#152))
([4a497e2](4a497e2))
* easier amount input
([#159](#159))
([8d6620e](8d6620e))
* error where start new transfer isn't working
([#126](#126))
([ca4ece0](ca4ece0))
* evm amount is now calculated as user input minus the fee
([#143](#143))
([d29771e](d29771e))
* fix sygma namespace
([f9ab963](f9ab963))
* incorrect transfer state when disconnecting wallet after transfer
completed
([#130](#130))
([359e6df](359e6df))
* input type and conditional render for token icon
([25d7db3](25d7db3))
* linter issue with style module
([edc34bb](edc34bb))
* max button rounds value
([#137](#137))
([01901e6](01901e6))
* misc fixes with icons
([#85](#85))
([fedbcd9](fedbcd9))
* prevent new lines and spaces in address input
([#81](#81))
([44a318e](44a318e))
* re-validate on account balance change
([#107](#107))
([7935f81](7935f81))
* removing log
([71f42d6](71f42d6))
* specific dispatchers either for network or token selector
([1ba9de3](1ba9de3))
* substrate address not validated bug
([#128](#128))
([74191b1](74191b1))
* switch networks and misc fixes
([#127](#127))
([db9ad8a](db9ad8a))
* Switching to a different TOKEN mapping for "From - TO" relationship is
not updated in UI if amount is inserted
([#160](#160))
([e2d16fd](e2d16fd))
* update sygma sdk version
([#82](#82))
([6d8447a](6d8447a))
* widget cloudflare deploy
([#68](#68))
([fe6c36f](fe6c36f))


### Miscellaneous Chores

* **main:** release 0.0.1
([8cff038](8cff038))
* release 0.1.0
([#101](#101))
([c8f1aed](c8f1aed))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LyonSsS LyonSsS LyonSsS approved these changes

@wainola wainola wainola approved these changes

@mpetrunic mpetrunic mpetrunic approved these changes

@saadahmsiddiqui saadahmsiddiqui Awaiting requested review from saadahmsiddiqui

Assignees
No one assigned
Labels
QA
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Address validation, still allows transfer to move forward if address format is incorrect
4 participants
@Lykhoyda @mpetrunic @wainola @LyonSsS