Fix RSK EIP1191 checksum issues #4151
Conversation
7alip
changed the title
| @@ -154,7 +155,7 @@ const FormSchema = object().shape({ | |||
| address: object({ | |||
| value: string().test( | |||
| 'check-eth-address', | |||
| translateRaw('TO_FIELD_ERROR'), | |||
| translateRaw('TO_FIELD_ERROR', { $network: DEFAULT_NETWORK }), | |||
There was a problem hiding this comment.
I provided network to avoid it to be displayed as Enter a valid $network address, ENS name, or blockchain domain
There was a problem hiding this comment.
Couldn't you move the schema inside the component so it can access the network prop and use that instead?
|
/ok-to-test |
Codecov Report
@@ Coverage Diff @@
## master #4151 +/- ##
==========================================
- Coverage 77.43% 77.42% -0.02%
==========================================
Files 551 551
Lines 12016 12018 +2
Branches 3197 3197
==========================================
Hits 9305 9305
- Misses 2678 2680 +2
Partials 33 33
Continue to review full report at Codecov.
|
|
[sc-7907] |
|
This pull request has been linked to Shortcut Story #7907: Fix address validation for RSK network in custom token form. |
| address: string, | ||
| resolutionErr: ResolutionError | undefined | ||
| resolutionErr: ResolutionError | undefined, | ||
| network: Network = NETWORKS_CONFIG.Ethereum |
There was a problem hiding this comment.
Would prefer having network be undefined by default here. You can use ?? DEFAULT_NETWORK in cases where it is undefined. NETWORK_CONFIG is not the correct type.
| @@ -154,7 +155,7 @@ const FormSchema = object().shape({ | |||
| address: object({ | |||
| value: string().test( | |||
| 'check-eth-address', | |||
| translateRaw('TO_FIELD_ERROR'), | |||
| translateRaw('TO_FIELD_ERROR', { $network: DEFAULT_NETWORK }), | |||
There was a problem hiding this comment.
Couldn't you move the schema inside the component so it can access the network prop and use that instead?
| schema.test('valid', translateRaw('TO_FIELD_ERROR', { $network: network.name }), function ( | ||
| value | ||
| ) { | ||
| return value && value.value && isValidAddress(value.value, network.chainId); |
There was a problem hiding this comment.
This looks to work, but I am not sure it will be possible to send without fixing the issues in Ethers. Balance scanning etc seems to be failing on RSK right now. Have you talked to ricmoo or is there an open issue?
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
Don't close. @7alip have you looked more at this? |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Description
I created new PR according to requesting changes in #4139
Changes
isValidETHRecipientAddresswithisValidRecipientAddresswhich now takes chainId parameter.networkparameter toTO_FIELD_ERRORtranslation.providerHandlerto find a better solution.This changes are supposed to fix checksum errors in the following scenarios:
Issues
There are more scenarios where EIP1191 checksum validation needs to be fixed.
We discussed for the better solution as to have support for it in Ethers.js.