From f63c1ddf7637d17ac761d5b5fb897d5b0a322b4b Mon Sep 17 00:00:00 2001 From: Xiaoming Wang Date: Tue, 21 Nov 2023 03:35:30 +0800 Subject: [PATCH] fix: 7862 invalid address error (#7881) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** This PR was created to fix the issue [#7862], following code change has been done: 1. Change the `getKeyringByAddress()` method to return undefined when address is not valid hex address (e.g. ENS domain name) 2. Fix the existing unit tests to reflect above changes in `address/index.test.ts` file 3. add extra unit tests to make sure function will return undefined when address is not valid hex address (e.g. ENS domain) ## **Related issues** Fixes: [#7862] ## **Manual testing steps** Please refer to following videos for detail how to test. ## **Screenshots/Recordings** ### **Before** https://github.com/MetaMask/metamask-mobile/assets/7315988/2c61515b-df26-460f-b2da-93fe6902e004 ### **After** https://github.com/MetaMask/metamask-mobile/assets/7315988/f8cf64bb-e323-452f-99be-0c3ffc99592f - [x] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've clearly explained what problem this PR is solving and how it is solved. - [x] I've linked related issues - [x] I've included manual testing steps - [x] I've included screenshots/recordings if applicable - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. - [x] I’ve properly set the pull request status: - [x] In case it's not yet "ready for review", I've set it to "draft". - [x] In case it's "ready for review", I've changed it from "draft" to "non-draft". ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: sethkfman <10342624+sethkfman@users.noreply.github.com> Co-authored-by: Gustavo Antunes <17601467+gantunesr@users.noreply.github.com> --- app/util/address/index.js | 4 ++-- app/util/address/index.test.ts | 9 +++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/app/util/address/index.js b/app/util/address/index.js index 8c70e79c973..d8527bbdc1b 100644 --- a/app/util/address/index.js +++ b/app/util/address/index.js @@ -171,11 +171,11 @@ export function isQRHardwareAccount(address) { * get address's kerying * * @param {String} address - String corresponding to an address - * @returns {Keyring} - Returns address's account keyriong + * @returns {Keyring | undefined} - Returns the keyring of the provided address if keyring found, otherwise returns undefined */ export function getKeyringByAddress(address) { if (!isValidHexAddress(address)) { - throw new Error(`Invalid address: ${address}`); + return undefined; } const { KeyringController } = Engine.context; const { keyrings } = KeyringController.state; diff --git a/app/util/address/index.test.ts b/app/util/address/index.test.ts index 63d5fc18180..e934a63b697 100644 --- a/app/util/address/index.test.ts +++ b/app/util/address/index.test.ts @@ -247,10 +247,11 @@ describe('isQRHardwareAccount', () => { }); }); describe('getKeyringByAddress', () => { - it('should throw an error if argument address is undefined', () => { - expect(() => getKeyringByAddress(undefined as any)).toThrow( - 'Invalid address: undefined', - ); + it('should return undefined if argument address is undefined', () => { + expect(getKeyringByAddress(undefined as any)).toBeUndefined(); + }); + it('should return undefined if argument address is not hex address', () => { + expect(getKeyringByAddress('ens.eth')).toBeUndefined(); }); it('should return address if found', () => { expect(