From 3435780ef55dce725ae763d16f8ee3419f2bad15 Mon Sep 17 00:00:00 2001 From: Jeff Smale <6363749+jeffsmale90@users.noreply.github.com> Date: Tue, 18 Feb 2025 16:50:10 +1300 Subject: [PATCH 1/4] Ensure authorization contract address is provided. --- .../src/KeyringController.test.ts | 22 +++++++++++++++++++ .../src/KeyringController.ts | 8 ++++++- packages/keyring-controller/src/constants.ts | 1 + 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/packages/keyring-controller/src/KeyringController.test.ts b/packages/keyring-controller/src/KeyringController.test.ts index 24966a71cbf..a0879f3bd68 100644 --- a/packages/keyring-controller/src/KeyringController.test.ts +++ b/packages/keyring-controller/src/KeyringController.test.ts @@ -1719,6 +1719,28 @@ describe('KeyringController', () => { ); }); }); + + it('should throw error if contract address is not provided', async () => { + await withController(async ({ controller, initialState }) => { + const account = initialState.keyrings[0].accounts[0]; + + for (const contractAddress of [undefined, null] as any as [ + string, + string, + ]) { + await expect( + controller.signEip7702Authorization({ + from: account, + chainId, + contractAddress, + nonce, + }), + ).rejects.toThrow( + 'KeyringController - The EIP-7702 Authorization is invalid. No contract address provided.', + ); + } + }); + }); }); describe('when the keyring for the given address does not support signEip7702Authorization', () => { diff --git a/packages/keyring-controller/src/KeyringController.ts b/packages/keyring-controller/src/KeyringController.ts index 6c194c8ee61..22a0e5f7cb2 100644 --- a/packages/keyring-controller/src/KeyringController.ts +++ b/packages/keyring-controller/src/KeyringController.ts @@ -1217,9 +1217,15 @@ export class KeyringController extends BaseController< | Hex | undefined; + if (contractAddress === undefined) { + throw new Error( + KeyringControllerError.MissingEip7702AuthorizationContractAddress, + ); + } + return await keyring.signEip7702Authorization(from, [ chainId, - contractAddress as Hex, + contractAddress, nonce, ]); } diff --git a/packages/keyring-controller/src/constants.ts b/packages/keyring-controller/src/constants.ts index f7b81828221..abf89373050 100644 --- a/packages/keyring-controller/src/constants.ts +++ b/packages/keyring-controller/src/constants.ts @@ -25,6 +25,7 @@ export enum KeyringControllerError { UnsupportedPatchUserOperation = 'KeyringController - The keyring for the current address does not support the method patchUserOperation.', UnsupportedSignUserOperation = 'KeyringController - The keyring for the current address does not support the method signUserOperation.', UnsupportedVerifySeedPhrase = 'KeyringController - The keyring does not support the method verifySeedPhrase.', + MissingEip7702AuthorizationContractAddress = 'KeyringController - The EIP-7702 Authorization is invalid. No contract address provided.', NoAccountOnKeychain = "KeyringController - The keychain doesn't have accounts.", ControllerLocked = 'KeyringController - The operation cannot be completed while the controller is locked.', MissingCredentials = 'KeyringController - Cannot persist vault without password and encryption key', From 71b41f1781d932709e185fc0d7051ecf51a22b49 Mon Sep 17 00:00:00 2001 From: Jeff Smale <6363749+jeffsmale90@users.noreply.github.com> Date: Tue, 18 Feb 2025 17:04:31 +1300 Subject: [PATCH 2/4] Rename contractAddress to invalidContractAddress to avoid shadowing previous declaration --- packages/keyring-controller/src/KeyringController.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/keyring-controller/src/KeyringController.test.ts b/packages/keyring-controller/src/KeyringController.test.ts index a0879f3bd68..27d762e9402 100644 --- a/packages/keyring-controller/src/KeyringController.test.ts +++ b/packages/keyring-controller/src/KeyringController.test.ts @@ -1724,7 +1724,7 @@ describe('KeyringController', () => { await withController(async ({ controller, initialState }) => { const account = initialState.keyrings[0].accounts[0]; - for (const contractAddress of [undefined, null] as any as [ + for (const invalidContractAddress of [undefined, null] as any as [ string, string, ]) { @@ -1732,7 +1732,7 @@ describe('KeyringController', () => { controller.signEip7702Authorization({ from: account, chainId, - contractAddress, + contractAddress: invalidContractAddress, nonce, }), ).rejects.toThrow( From 3a0699d9d997c141040976c418cc481067523bf6 Mon Sep 17 00:00:00 2001 From: Jeff Smale <6363749+jeffsmale90@users.noreply.github.com> Date: Tue, 18 Feb 2025 21:10:02 +1300 Subject: [PATCH 3/4] Remove explicit any from keyring controller test, to align with linting rules --- packages/keyring-controller/src/KeyringController.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/keyring-controller/src/KeyringController.test.ts b/packages/keyring-controller/src/KeyringController.test.ts index 27d762e9402..b3a006f2fa4 100644 --- a/packages/keyring-controller/src/KeyringController.test.ts +++ b/packages/keyring-controller/src/KeyringController.test.ts @@ -1724,10 +1724,10 @@ describe('KeyringController', () => { await withController(async ({ controller, initialState }) => { const account = initialState.keyrings[0].accounts[0]; - for (const invalidContractAddress of [undefined, null] as any as [ - string, - string, - ]) { + for (const invalidContractAddress of [ + undefined, + null, + ] as unknown as string[]) { await expect( controller.signEip7702Authorization({ from: account, From 200c57dc36d5a9d5664887343a777d2c5ec7270f Mon Sep 17 00:00:00 2001 From: Jeff Smale <6363749+jeffsmale90@users.noreply.github.com> Date: Wed, 19 Feb 2025 19:38:53 +1300 Subject: [PATCH 4/4] Use error message constant in test, and use it.each for test cases --- .../src/KeyringController.test.ts | 23 ++++++++----------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/packages/keyring-controller/src/KeyringController.test.ts b/packages/keyring-controller/src/KeyringController.test.ts index b3a006f2fa4..444761a2a03 100644 --- a/packages/keyring-controller/src/KeyringController.test.ts +++ b/packages/keyring-controller/src/KeyringController.test.ts @@ -1720,27 +1720,24 @@ describe('KeyringController', () => { }); }); - it('should throw error if contract address is not provided', async () => { - await withController(async ({ controller, initialState }) => { - const account = initialState.keyrings[0].accounts[0]; - - for (const invalidContractAddress of [ - undefined, - null, - ] as unknown as string[]) { + it.each([undefined, null])( + 'should throw error if contract address is %s', + async (invalidContractAddress) => { + await withController(async ({ controller, initialState }) => { + const account = initialState.keyrings[0].accounts[0]; await expect( controller.signEip7702Authorization({ from: account, chainId, - contractAddress: invalidContractAddress, + contractAddress: invalidContractAddress as unknown as string, nonce, }), ).rejects.toThrow( - 'KeyringController - The EIP-7702 Authorization is invalid. No contract address provided.', + KeyringControllerError.MissingEip7702AuthorizationContractAddress, ); - } - }); - }); + }); + }, + ); }); describe('when the keyring for the given address does not support signEip7702Authorization', () => {