From 5abb39195a662de2aab9ec3f52916300a6e89ce5 Mon Sep 17 00:00:00 2001 From: Stanley Yuen <102275989+stanleyyconsensys@users.noreply.github.com> Date: Wed, 31 Jul 2024 17:25:59 +0800 Subject: [PATCH 01/11] refactor: add error utils (#307) * chore: add error utils * chore: add error utils * chore: remove unuse method --- .../starknet-snap/src/utils/error.test.ts | 55 +++++++++++++++++++ packages/starknet-snap/src/utils/error.ts | 48 ++++++++++++++++ 2 files changed, 103 insertions(+) create mode 100644 packages/starknet-snap/src/utils/error.test.ts create mode 100644 packages/starknet-snap/src/utils/error.ts diff --git a/packages/starknet-snap/src/utils/error.test.ts b/packages/starknet-snap/src/utils/error.test.ts new file mode 100644 index 00000000..140bb608 --- /dev/null +++ b/packages/starknet-snap/src/utils/error.test.ts @@ -0,0 +1,55 @@ +import { + MethodNotFoundError, + UserRejectedRequestError, + MethodNotSupportedError, + ParseError, + ResourceNotFoundError, + ResourceUnavailableError, + TransactionRejected, + ChainDisconnectedError, + DisconnectedError, + UnauthorizedError, + UnsupportedMethodError, + InternalError, + InvalidInputError, + InvalidParamsError, + InvalidRequestError, + LimitExceededError, + SnapError, +} from '@metamask/snaps-sdk'; + +import { isSnapRpcError } from './error'; + +describe('isSnapRpcError', () => { + it('returns true for a Snap RPC error', () => { + const snapErrors = [ + SnapError, + MethodNotFoundError, + UserRejectedRequestError, + MethodNotSupportedError, + ParseError, + ResourceNotFoundError, + ResourceUnavailableError, + TransactionRejected, + ChainDisconnectedError, + DisconnectedError, + UnauthorizedError, + UnsupportedMethodError, + InternalError, + InvalidInputError, + InvalidParamsError, + InvalidRequestError, + LimitExceededError, + ]; + + for (const ErrorCtor of snapErrors) { + const error = new ErrorCtor('snap error message'); + expect(isSnapRpcError(error)).toBe(true); + } + }); + + it('returns false for a non-Snap RPC error', () => { + const error = new Error('error message'); + expect(isSnapRpcError(error)).toBe(false); + }); +}); diff --git a/packages/starknet-snap/src/utils/error.ts b/packages/starknet-snap/src/utils/error.ts new file mode 100644 index 00000000..2bdfebcf --- /dev/null +++ b/packages/starknet-snap/src/utils/error.ts @@ -0,0 +1,48 @@ +import { + MethodNotFoundError, + UserRejectedRequestError, + MethodNotSupportedError, + ParseError, + ResourceNotFoundError, + ResourceUnavailableError, + TransactionRejected, + ChainDisconnectedError, + DisconnectedError, + UnauthorizedError, + UnsupportedMethodError, + InternalError, + InvalidInputError, + InvalidParamsError, + InvalidRequestError, + LimitExceededError, + SnapError, +} from '@metamask/snaps-sdk'; + +/** + * Determines if the given error is a Snap RPC error. + * + * @param error - The error instance to be checked. + * @returns A boolean indicating whether the error is a Snap RPC error. + */ +export function isSnapRpcError(error: Error): boolean { + const errors = [ + SnapError, + MethodNotFoundError, + UserRejectedRequestError, + MethodNotSupportedError, + ParseError, + ResourceNotFoundError, + ResourceUnavailableError, + TransactionRejected, + ChainDisconnectedError, + DisconnectedError, + UnauthorizedError, + UnsupportedMethodError, + InternalError, + InvalidInputError, + InvalidParamsError, + InvalidRequestError, + LimitExceededError, + ]; + return errors.some((errType) => error instanceof errType); +} From fd235444952de446d8d6ea108066ddf623fa43a6 Mon Sep 17 00:00:00 2001 From: Stanley Yuen <102275989+stanleyyconsensys@users.noreply.github.com> Date: Wed, 31 Jul 2024 17:27:13 +0800 Subject: [PATCH 02/11] chore: add snap utils (#306) --- packages/starknet-snap/src/utils/snap.test.ts | 101 ++++++++++++++++++ packages/starknet-snap/src/utils/snap.ts | 77 +++++++++++++ 2 files changed, 178 insertions(+) create mode 100644 packages/starknet-snap/src/utils/snap.test.ts create mode 100644 packages/starknet-snap/src/utils/snap.ts diff --git a/packages/starknet-snap/src/utils/snap.test.ts b/packages/starknet-snap/src/utils/snap.test.ts new file mode 100644 index 00000000..383ed374 --- /dev/null +++ b/packages/starknet-snap/src/utils/snap.test.ts @@ -0,0 +1,101 @@ +import type { Component } from '@metamask/snaps-sdk'; +import { heading, panel, text, divider, row } from '@metamask/snaps-sdk'; + +import * as snapUtil from './snap'; + +jest.mock('@metamask/key-tree', () => ({ + getBIP44AddressKeyDeriver: jest.fn(), +})); + +describe('getBip44Deriver', () => { + it('gets bip44 deriver', async () => { + const spy = jest.spyOn(snapUtil.getProvider(), 'request'); + + await snapUtil.getBip44Deriver(); + + expect(spy).toHaveBeenCalledWith({ + method: 'snap_getBip44Entropy', + params: { + coinType: 9004, + }, + }); + }); +}); + +describe('confirmDialog', () => { + it('calls snap_dialog', async () => { + const spy = jest.spyOn(snapUtil.getProvider(), 'request'); + const components: Component[] = [ + heading('header'), + text('subHeader'), + divider(), + row('Label1', text('Value1')), + text('**Label2**:'), + row('SubLabel1', text('SubValue1')), + ]; + + await snapUtil.confirmDialog(components); + + expect(spy).toHaveBeenCalledWith({ + method: 'snap_dialog', + params: { + type: 'confirmation', + content: panel(components), + }, + }); + }); +}); + +describe('getStateData', () => { + it('gets state data', async () => { + const spy = jest.spyOn(snapUtil.getProvider(), 'request'); + const testcase = { + state: { + transaction: [ + { + txHash: 'hash', + chainId: 'chainId', + }, + ], + }, + }; + + spy.mockResolvedValue(testcase.state); + const result = await snapUtil.getStateData(); + + expect(spy).toHaveBeenCalledWith({ + method: 'snap_manageState', + params: { + operation: 'get', + }, + }); + + expect(result).toStrictEqual(testcase.state); + }); +}); + +describe('setStateData', () => { + it('sets state data', async () => { + const spy = jest.spyOn(snapUtil.getProvider(), 'request'); + const testcase = { + state: { + transaction: [ + { + txHash: 'hash', + chainId: 'chainId', + }, + ], + }, + }; + + await snapUtil.setStateData(testcase.state); + + expect(spy).toHaveBeenCalledWith({ + method: 'snap_manageState', + params: { + operation: 'update', + newState: testcase.state, + }, + }); + }); +}); diff --git a/packages/starknet-snap/src/utils/snap.ts b/packages/starknet-snap/src/utils/snap.ts new file mode 100644 index 00000000..b2dd276c --- /dev/null +++ b/packages/starknet-snap/src/utils/snap.ts @@ -0,0 +1,77 @@ +import type { BIP44AddressKeyDeriver } from '@metamask/key-tree'; +import { getBIP44AddressKeyDeriver } from '@metamask/key-tree'; +import type { Component, DialogResult, Json } from '@metamask/snaps-sdk'; +import { panel, type SnapsProvider } from '@metamask/snaps-sdk'; + +declare const snap: SnapsProvider; + +/** + * Retrieves the current SnapsProvider. + * + * @returns The current SnapsProvider. + */ +export function getProvider(): SnapsProvider { + return snap; +} + +/** + * Retrieves a BIP44AddressKeyDeriver object. + * + * @returns A Promise that resolves to a BIP44AddressKeyDeriver object. + */ +export async function getBip44Deriver(): Promise { + const bip44Node = await snap.request({ + method: 'snap_getBip44Entropy', + params: { + coinType: 9004, + }, + }); + return getBIP44AddressKeyDeriver(bip44Node); +} + +/** + * Displays a confirmation dialog with the specified components. + * + * @param components - An array of components to display in the dialog. + * @returns A Promise that resolves to the result of the dialog. + */ +export async function confirmDialog( + components: Component[], +): Promise { + return snap.request({ + method: 'snap_dialog', + params: { + type: 'confirmation', + content: panel(components), + }, + }); +} + +/** + * Retrieves the current state data. + * + * @returns A Promise that resolves to the current state data. + */ +export async function getStateData(): Promise { + return (await snap.request({ + method: 'snap_manageState', + params: { + operation: 'get', + }, + })) as unknown as State; +} + +/** + * Sets the current state data to the specified data. + * + * @param data - The new state data to set. + */ +export async function setStateData(data: State) { + await snap.request({ + method: 'snap_manageState', + params: { + operation: 'update', + newState: data as unknown as Record, + }, + }); +} From 5a501f9aae7c3cdf041f479eac38f4a1e82855e9 Mon Sep 17 00:00:00 2001 From: khanti42 Date: Wed, 31 Jul 2024 23:01:54 +0200 Subject: [PATCH 03/11] fix: allow multiple consecutive transactions in estimateFee(Bulk) (#289) * fix: allow multiple consecutive pending transactions in estimateFee and estimateFeeBulk * fix: remove try catch block. Just check if nonce provided * test: estimateFee.test.ts should rely only on estimateFeeBulk * chore: lint + prettier * refactor: function account nounce on block naming * refactor: removed gasConsumed and gasPrice in estamiteFee * fix: nonce undefined * revert: yarn.lock and snap.manifest.json from main * fix: use BlockIdentifierEnum * fix: update packages/starknet-snap/src/utils/starknetUtils.ts Co-authored-by: Stanley Yuen <102275989+stanleyyconsensys@users.noreply.github.com> * feat: sets blockIdentifier on getProvider no need for getAccountNonceOnLatest method * chore: lint + prettier * fix: pending/latest balance * revert: getErc20TokenBalance from main --------- Co-authored-by: Stanley Yuen <102275989+stanleyyconsensys@users.noreply.github.com> --- packages/starknet-snap/src/estimateFee.ts | 49 ++++++------------- .../starknet-snap/src/utils/starknetUtils.ts | 21 ++++++-- .../test/src/estimateFee.test.ts | 14 ++---- .../test/src/sendTransaction.test.ts | 2 - 4 files changed, 36 insertions(+), 50 deletions(-) diff --git a/packages/starknet-snap/src/estimateFee.ts b/packages/starknet-snap/src/estimateFee.ts index 8d125547..b3b391f7 100644 --- a/packages/starknet-snap/src/estimateFee.ts +++ b/packages/starknet-snap/src/estimateFee.ts @@ -1,4 +1,4 @@ -import type { Invocations } from 'starknet'; +import type { EstimateFee, Invocations } from 'starknet'; import { TransactionType } from 'starknet'; import type { @@ -14,7 +14,6 @@ import { validateAndParseAddress, getKeysFromAddress, getCallDataArray, - estimateFee as estimateFeeUtil, getAccContractAddressAndCallData, estimateFeeBulk, addFeesFromAllTransactions, @@ -108,44 +107,26 @@ export async function estimateFee(params: ApiParamsWithKeyDeriver) { ]; } - let estimateFeeResp; - - if (accountDeployed) { - // This condition branch will be removed later when starknet.js - // supports estimateFeeBulk in rpc mode - estimateFeeResp = await estimateFeeUtil( - network, - senderAddress, - senderPrivateKey, - txnInvocation, - ); - logger.log( - `estimateFee:\nestimateFeeUtil estimateFeeResp: ${toJson( - estimateFeeResp, - )}`, - ); - } else { - const estimateBulkFeeResp = await estimateFeeBulk( - network, - senderAddress, - senderPrivateKey, - bulkTransactions, - ); - logger.log( - `estimateFee:\nestimateFeeBulk estimateBulkFeeResp: ${toJson( - estimateBulkFeeResp, - )}`, - ); - estimateFeeResp = addFeesFromAllTransactions(estimateBulkFeeResp); - } + const estimateBulkFeeResp = await estimateFeeBulk( + network, + senderAddress, + senderPrivateKey, + bulkTransactions, + ); + logger.log( + `estimateFee:\nestimateFeeBulk estimateBulkFeeResp: ${toJson( + estimateBulkFeeResp, + )}`, + ); + const estimateFeeResp = addFeesFromAllTransactions( + estimateBulkFeeResp, + ) as EstimateFee; logger.log(`estimateFee:\nestimateFeeResp: ${toJson(estimateFeeResp)}`); const resp = { suggestedMaxFee: estimateFeeResp.suggestedMaxFee.toString(10), overallFee: estimateFeeResp.overall_fee.toString(10), - gasConsumed: estimateFeeResp.gas_consumed?.toString(10) ?? '0', - gasPrice: estimateFeeResp.gas_price?.toString(10) ?? '0', unit: 'wei', includeDeploy: !accountDeployed, }; diff --git a/packages/starknet-snap/src/utils/starknetUtils.ts b/packages/starknet-snap/src/utils/starknetUtils.ts index 08c7a175..829043db 100644 --- a/packages/starknet-snap/src/utils/starknetUtils.ts +++ b/packages/starknet-snap/src/utils/starknetUtils.ts @@ -112,11 +112,17 @@ export const getCallDataArray = (callDataStr: string): string[] => { .filter((data) => data.length > 0); }; -export const getProvider = (network: Network): ProviderInterface => { +export const getProvider = ( + network: Network, + blockIdentifier?: BlockIdentifierEnum, +): ProviderInterface => { let providerParam: ProviderOptions = {}; providerParam = { nodeUrl: getRPCUrl(network.chainId), }; + if (blockIdentifier) { + providerParam.blockIdentifier = blockIdentifier; + } return new Provider(providerParam); }; @@ -125,8 +131,9 @@ export const getAccountInstance = ( userAddress: string, privateKey: string | Uint8Array, cairoVersion?: CairoVersion, + blockIdentifier?: BlockIdentifierEnum, ): Account => { - const provider = getProvider(network); + const provider = getProvider(network, blockIdentifier); return new Account( provider, userAddress, @@ -196,11 +203,14 @@ export const estimateFee = async ( cairoVersion?: CairoVersion, invocationsDetails?: UniversalDetails, ): Promise => { - return getAccountInstance( + // We force block identifier to latest to avoid issues estimating fees on + // the pending block, that can fail if there are already transactions in the pending state. + return await getAccountInstance( network, senderAddress, privateKey, cairoVersion, + BlockIdentifierEnum.Latest, ).estimateInvokeFee(txnInvocation, { ...invocationsDetails, skipValidate: false, @@ -216,11 +226,14 @@ export const estimateFeeBulk = async ( invocationsDetails?: UniversalDetails, cairoVersion?: CairoVersion, ): Promise => { - return getAccountInstance( + // We force block identifier to latest to avoid issues estimating fees on + // the pending block, that can fail if there are already transactions in the pending state. + return await getAccountInstance( network, senderAddress, privateKey, cairoVersion, + BlockIdentifierEnum.Latest, ).estimateFeeBulk(txnInvocation, { ...invocationsDetails, skipValidate: false, diff --git a/packages/starknet-snap/test/src/estimateFee.test.ts b/packages/starknet-snap/test/src/estimateFee.test.ts index 488de399..084de926 100644 --- a/packages/starknet-snap/test/src/estimateFee.test.ts +++ b/packages/starknet-snap/test/src/estimateFee.test.ts @@ -161,7 +161,6 @@ describe('Test function: estimateFee', function () { describe('when account is not require upgrade', function () { let estimateFeeBulkStub: sinon.SinonStub; - let estimateFeeStub: sinon.SinonStub; beforeEach(async function () { sandbox.stub(utils, 'isUpgradeRequired').resolves(false); @@ -173,28 +172,25 @@ describe('Test function: estimateFee', function () { describe('when account is deployed', function () { beforeEach(async function () { - estimateFeeBulkStub = sandbox.stub(utils, 'estimateFeeBulk'); + estimateFeeBulkStub = sandbox + .stub(utils, 'estimateFeeBulk') + .resolves([estimateFeeResp]); sandbox .stub(utils, 'validateAccountRequireUpgradeOrDeploy') .resolvesThis(); }); it('should estimate the fee correctly', async function () { - estimateFeeStub = sandbox - .stub(utils, 'estimateFee') - .resolves(estimateFeeResp); const result = await estimateFee(apiParams); expect(result.suggestedMaxFee).to.be.eq( estimateFeeResp.suggestedMaxFee.toString(10), ); - expect(estimateFeeStub).callCount(1); - expect(estimateFeeBulkStub).callCount(0); + expect(estimateFeeBulkStub).callCount(1); }); }); describe('when account is not deployed', function () { beforeEach(async function () { - estimateFeeStub = sandbox.stub(utils, 'estimateFee'); sandbox .stub(utils, 'validateAccountRequireUpgradeOrDeploy') .resolvesThis(); @@ -246,7 +242,6 @@ describe('Test function: estimateFee', function () { expect(result.suggestedMaxFee).to.be.eq( expectedSuggestedMaxFee.toString(10), ); - expect(estimateFeeStub).callCount(0); expect(estimateFeeBulkStub).callCount(1); expect(estimateFeeBulkStub).to.be.calledWith( STARKNET_SEPOLIA_TESTNET_NETWORK, @@ -269,7 +264,6 @@ describe('Test function: estimateFee', function () { result = err; } finally { expect(result).to.be.an('Error'); - expect(estimateFeeStub).callCount(0); expect(estimateFeeBulkStub).callCount(1); } }); diff --git a/packages/starknet-snap/test/src/sendTransaction.test.ts b/packages/starknet-snap/test/src/sendTransaction.test.ts index a0e05a0d..764e45d6 100644 --- a/packages/starknet-snap/test/src/sendTransaction.test.ts +++ b/packages/starknet-snap/test/src/sendTransaction.test.ts @@ -242,8 +242,6 @@ describe('Test function: sendTransaction', function () { sandbox.stub(estimateFeeSnap, 'estimateFee').resolves({ suggestedMaxFee: estimateFeeResp.suggestedMaxFee.toString(10), overallFee: estimateFeeResp.overall_fee.toString(10), - gasConsumed: '0', - gasPrice: '0', unit: 'wei', includeDeploy: true, }); From b65a2fc2717e265c13fd04e755d602e1372ed8a5 Mon Sep 17 00:00:00 2001 From: Stanley Yuen <102275989+stanleyyconsensys@users.noreply.github.com> Date: Thu, 1 Aug 2024 11:16:21 +0800 Subject: [PATCH 04/11] chore: add mock for snap helper (#309) Co-authored-by: khanti42 --- packages/starknet-snap/src/utils/__mocks__/snap.ts | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 packages/starknet-snap/src/utils/__mocks__/snap.ts diff --git a/packages/starknet-snap/src/utils/__mocks__/snap.ts b/packages/starknet-snap/src/utils/__mocks__/snap.ts new file mode 100644 index 00000000..da3f01cd --- /dev/null +++ b/packages/starknet-snap/src/utils/__mocks__/snap.ts @@ -0,0 +1,9 @@ +export const getProvider = jest.fn(); + +export const getBip44Deriver = jest.fn(); + +export const confirmDialog = jest.fn(); + +export const getStateData = jest.fn(); + +export const setStateData = jest.fn(); From 1ad58a040611ac25d204ffd8b3c51063840c15f1 Mon Sep 17 00:00:00 2001 From: khanti42 Date: Mon, 12 Aug 2024 12:12:11 +0200 Subject: [PATCH 05/11] chore: return spendable and actual balance in rpc api `getErc20Balance` (#308) * fix: allow multiple consecutive pending transactions in estimateFee and estimateFeeBulk * fix: remove try catch block. Just check if nonce provided * test: estimateFee.test.ts should rely only on estimateFeeBulk * chore: lint + prettier * refactor: function account nounce on block naming * refactor: removed gasConsumed and gasPrice in estamiteFee * fix: nonce undefined * revert: yarn.lock and snap.manifest.json from main * fix: use BlockIdentifierEnum * fix: update packages/starknet-snap/src/utils/starknetUtils.ts Co-authored-by: Stanley Yuen <102275989+stanleyyconsensys@users.noreply.github.com> * feat: sets blockIdentifier on getProvider no need for getAccountNonceOnLatest method * chore: lint + prettier * fix: pending/latest balance * feat: support spendable balance in case of unconfirmed amount * revert: wallet-ui changes * test: update getErc20TokenBalance.test.ts * feat: let consumer app decide the logic to use to display balance * chore: lint + prettier --------- Co-authored-by: Stanley Yuen <102275989+stanleyyconsensys@users.noreply.github.com> --- .../starknet-snap/src/getErc20TokenBalance.ts | 31 +++++++++---------- .../test/src/getErc20TokenBalance.test.ts | 15 +++++++-- .../wallet-ui/src/services/useStarkNetSnap.ts | 2 +- 3 files changed, 28 insertions(+), 20 deletions(-) diff --git a/packages/starknet-snap/src/getErc20TokenBalance.ts b/packages/starknet-snap/src/getErc20TokenBalance.ts index aaad6e58..7d6130a2 100644 --- a/packages/starknet-snap/src/getErc20TokenBalance.ts +++ b/packages/starknet-snap/src/getErc20TokenBalance.ts @@ -6,11 +6,7 @@ import { BlockIdentifierEnum } from './utils/constants'; import { logger } from './utils/logger'; import { toJson } from './utils/serializer'; import { getNetworkFromChainId } from './utils/snapUtils'; -import { - getBalance, - isAccountDeployed, - validateAndParseAddress, -} from './utils/starknetUtils'; +import { getBalance, validateAndParseAddress } from './utils/starknetUtils'; /** * @@ -53,23 +49,26 @@ export async function getErc20TokenBalance(params: ApiParams) { `getErc20Balance:\nerc20Address: ${erc20Address}\nuserAddress: ${userAddress}`, ); - // For deployed accounts, use the PENDING block to show balance updates as soon as possible, - // facilitating concurrent transactions without delays. - // For non-deployed accounts, use the LATEST block to avoid displaying non-zero balances - // from pending transactions, because confirmed non-zero balance is required for deployment. - const blockIdentifier = (await isAccountDeployed(network, userAddress)) - ? BlockIdentifierEnum.Pending - : BlockIdentifierEnum.Latest; - const balance = await getBalance( + const balanceLatest = await getBalance( userAddress, erc20Address, network, - blockIdentifier, + BlockIdentifierEnum.Latest, + ); + const balancePending = await getBalance( + userAddress, + erc20Address, + network, + BlockIdentifierEnum.Pending, ); - logger.log(`getErc20Balance:\nresp: ${toJson(balance)}`); + const resp = { + balancePending, + balanceLatest, + }; + logger.log(`getErc20Balance:\nresp: ${toJson(resp)}`); - return balance; + return resp; } catch (error) { logger.error(`Problem found:`, error); throw error; diff --git a/packages/starknet-snap/test/src/getErc20TokenBalance.test.ts b/packages/starknet-snap/test/src/getErc20TokenBalance.test.ts index 3affc43f..bb850f94 100644 --- a/packages/starknet-snap/test/src/getErc20TokenBalance.test.ts +++ b/packages/starknet-snap/test/src/getErc20TokenBalance.test.ts @@ -51,7 +51,10 @@ describe('Test function: getErc20TokenBalance', function () { }; apiParams.requestParams = requestObject; const result = await getErc20TokenBalance(apiParams); - expect(result).to.be.eq('0x64a'); + expect(result).to.be.eql({ + balanceLatest: hexAmount, + balancePending: hexAmount, + }); }); it('should get ERC-20 token balance with BlockIdentifier pending if the account is deployed', async function () { @@ -69,7 +72,10 @@ describe('Test function: getErc20TokenBalance', function () { }; apiParams.requestParams = requestObject; const result = await getErc20TokenBalance(apiParams); - expect(result).to.be.eq('0x64a'); + expect(result).to.be.eql({ + balanceLatest: hexAmount, + balancePending: hexAmount, + }); expect(stub).to.have.been.calledWith( requestObject.userAddress, requestObject.tokenAddress, @@ -93,7 +99,10 @@ describe('Test function: getErc20TokenBalance', function () { }; apiParams.requestParams = requestObject; const result = await getErc20TokenBalance(apiParams); - expect(result).to.be.eq('0x64a'); + expect(result).to.be.eql({ + balanceLatest: hexAmount, + balancePending: hexAmount, + }); expect(stub).to.have.been.calledWith( requestObject.userAddress, requestObject.tokenAddress, diff --git a/packages/wallet-ui/src/services/useStarkNetSnap.ts b/packages/wallet-ui/src/services/useStarkNetSnap.ts index f672e522..8599c416 100644 --- a/packages/wallet-ui/src/services/useStarkNetSnap.ts +++ b/packages/wallet-ui/src/services/useStarkNetSnap.ts @@ -694,7 +694,7 @@ export const useStarkNetSnap = () => { }, }, }); - return response; + return response.balanceLatest; } catch (err) { //eslint-disable-next-line no-console console.error(err); From 6744d180d099454a8f4081edd7cdc6d497e9af8e Mon Sep 17 00:00:00 2001 From: Stanley Yuen <102275989+stanleyyconsensys@users.noreply.github.com> Date: Mon, 12 Aug 2024 18:26:14 +0800 Subject: [PATCH 06/11] chore: add merge queue (#314) --- .github/workflows/main.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 1bfbbad1..9fbad635 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -4,6 +4,7 @@ on: push: branches: [main] pull_request: + merge_group: jobs: check-workflows: From ad12592799b314ed89d0595d40f3eeecf6e536ab Mon Sep 17 00:00:00 2001 From: Stanley Yuen <102275989+stanleyyconsensys@users.noreply.github.com> Date: Mon, 12 Aug 2024 18:42:33 +0800 Subject: [PATCH 07/11] chore: decrease test coverage limit (#313) --- packages/starknet-snap/package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/starknet-snap/package.json b/packages/starknet-snap/package.json index dc96fec7..4f9b7942 100644 --- a/packages/starknet-snap/package.json +++ b/packages/starknet-snap/package.json @@ -28,8 +28,8 @@ "serve": "mm-snap serve", "start": "mm-snap watch", "test": "yarn run test:unit && yarn run cover:report && yarn run jest", - "test:unit": "nyc --check-coverage --statements 70 --branches 70 --functions 70 --lines 70 mocha --colors -r ts-node/register \"test/**/*.test.ts\"", - "test:unit:one": "nyc --check-coverage --statements 70 --branches 70 --functions 70 --lines 70 mocha --colors -r ts-node/register" + "test:unit": "nyc --check-coverage --statements 50 --branches 50 --functions 50 --lines 50 mocha --colors -r ts-node/register \"test/**/*.test.ts\"", + "test:unit:one": "nyc --check-coverage --statements 50 --branches 50 --functions 50 --lines 50 mocha --colors -r ts-node/register" }, "nyc": { "exclude": [ From 45c127c28bd7f5070588fcc10e2a41ce744f7f29 Mon Sep 17 00:00:00 2001 From: Stanley Yuen <102275989+stanleyyconsensys@users.noreply.github.com> Date: Mon, 12 Aug 2024 18:42:47 +0800 Subject: [PATCH 08/11] chore: refactor `showAccountRequireUpgradeOrDeployModal ` (#311) --- packages/starknet-snap/src/declareContract.ts | 14 +- packages/starknet-snap/src/estimateFee.ts | 9 +- packages/starknet-snap/src/executeTxn.ts | 14 +- .../starknet-snap/src/extractPrivateKey.ts | 11 +- .../starknet-snap/src/extractPublicKey.ts | 11 +- .../src/signDeclareTransaction.ts | 14 +- .../src/signDeployAccountTransaction.ts | 14 +- packages/starknet-snap/src/signMessage.ts | 14 +- packages/starknet-snap/src/signTransaction.ts | 19 +-- .../starknet-snap/src/utils/__mocks__/snap.ts | 2 + .../starknet-snap/src/utils/exceptions.ts | 18 ++- packages/starknet-snap/src/utils/snap.test.ts | 24 ++++ packages/starknet-snap/src/utils/snap.ts | 22 ++- .../starknet-snap/src/utils/snapUtils.test.ts | 131 ++++++++++++++++++ packages/starknet-snap/src/utils/snapUtils.ts | 108 ++++++++------- .../starknet-snap/src/utils/starknetUtils.ts | 6 +- .../starknet-snap/src/verifySignedMessage.ts | 10 +- .../test/src/declareContract.test.ts | 65 --------- .../starknet-snap/test/src/executeTxn.test.ts | 51 ------- .../test/src/signDeclareTransaction.test.ts | 64 --------- .../src/signDeployAccountTransaction.test.ts | 64 --------- .../test/src/signMessage.test.ts | 62 --------- .../test/src/signTransaction.test.ts | 64 --------- .../test/src/verifySignedMessage.test.ts | 27 ---- 24 files changed, 291 insertions(+), 547 deletions(-) create mode 100644 packages/starknet-snap/src/utils/snapUtils.test.ts diff --git a/packages/starknet-snap/src/declareContract.ts b/packages/starknet-snap/src/declareContract.ts index 297920c8..583e244e 100644 --- a/packages/starknet-snap/src/declareContract.ts +++ b/packages/starknet-snap/src/declareContract.ts @@ -9,12 +9,11 @@ import { toJson } from './utils/serializer'; import { getNetworkFromChainId, getDeclareSnapTxt, - showAccountRequireUpgradeOrDeployModal, + verifyIfAccountNeedUpgradeOrDeploy, } from './utils/snapUtils'; import { getKeysFromAddress, declareContract as declareContractUtil, - validateAccountRequireUpgradeOrDeploy, } from './utils/starknetUtils'; /** @@ -38,16 +37,7 @@ export async function declareContract(params: ApiParamsWithKeyDeriver) { senderAddress, ); - try { - await validateAccountRequireUpgradeOrDeploy( - network, - senderAddress, - publicKey, - ); - } catch (validateError) { - await showAccountRequireUpgradeOrDeployModal(wallet, validateError); - throw validateError; - } + await verifyIfAccountNeedUpgradeOrDeploy(network, senderAddress, publicKey); const snapComponents = getDeclareSnapTxt( senderAddress, diff --git a/packages/starknet-snap/src/estimateFee.ts b/packages/starknet-snap/src/estimateFee.ts index b3b391f7..9f83f42a 100644 --- a/packages/starknet-snap/src/estimateFee.ts +++ b/packages/starknet-snap/src/estimateFee.ts @@ -8,9 +8,11 @@ import type { import { ACCOUNT_CLASS_HASH } from './utils/constants'; import { logger } from './utils/logger'; import { toJson } from './utils/serializer'; -import { getNetworkFromChainId } from './utils/snapUtils'; import { - validateAccountRequireUpgradeOrDeploy, + getNetworkFromChainId, + verifyIfAccountNeedUpgradeOrDeploy, +} from './utils/snapUtils'; +import { validateAndParseAddress, getKeysFromAddress, getCallDataArray, @@ -64,10 +66,11 @@ export async function estimateFee(params: ApiParamsWithKeyDeriver) { const { privateKey: senderPrivateKey, publicKey } = await getKeysFromAddress(keyDeriver, network, state, senderAddress); - await validateAccountRequireUpgradeOrDeploy( + await verifyIfAccountNeedUpgradeOrDeploy( network, senderAddress, publicKey, + false, ); const txnInvocation = { diff --git a/packages/starknet-snap/src/executeTxn.ts b/packages/starknet-snap/src/executeTxn.ts index e5698128..0bfce333 100644 --- a/packages/starknet-snap/src/executeTxn.ts +++ b/packages/starknet-snap/src/executeTxn.ts @@ -14,7 +14,7 @@ import { getNetworkFromChainId, getTxnSnapTxt, addDialogTxt, - showAccountRequireUpgradeOrDeployModal, + verifyIfAccountNeedUpgradeOrDeploy, } from './utils/snapUtils'; import { getKeysFromAddress, @@ -23,7 +23,6 @@ import { estimateFeeBulk, getAccContractAddressAndCallData, addFeesFromAllTransactions, - validateAccountRequireUpgradeOrDeploy, } from './utils/starknetUtils'; /** @@ -42,16 +41,7 @@ export async function executeTxn(params: ApiParamsWithKeyDeriver) { addressIndex, } = await getKeysFromAddress(keyDeriver, network, state, senderAddress); - try { - await validateAccountRequireUpgradeOrDeploy( - network, - senderAddress, - publicKey, - ); - } catch (validateError) { - await showAccountRequireUpgradeOrDeployModal(wallet, validateError); - throw validateError; - } + await verifyIfAccountNeedUpgradeOrDeploy(network, senderAddress, publicKey); const txnInvocationArray = Array.isArray(requestParamsObj.txnInvocation) ? requestParamsObj.txnInvocation diff --git a/packages/starknet-snap/src/extractPrivateKey.ts b/packages/starknet-snap/src/extractPrivateKey.ts index 439a71bb..69739c2e 100644 --- a/packages/starknet-snap/src/extractPrivateKey.ts +++ b/packages/starknet-snap/src/extractPrivateKey.ts @@ -6,13 +6,14 @@ import type { } from './types/snapApi'; import { logger } from './utils/logger'; import { toJson } from './utils/serializer'; -import { getNetworkFromChainId } from './utils/snapUtils'; +import { + getNetworkFromChainId, + verifyIfAccountNeedUpgradeOrDeploy, +} from './utils/snapUtils'; import { validateAndParseAddress, getKeysFromAddress, - validateAccountRequireUpgradeOrDeploy, } from './utils/starknetUtils'; - /** * * @param params @@ -43,10 +44,12 @@ export async function extractPrivateKey(params: ApiParamsWithKeyDeriver) { state, userAddress, ); - await validateAccountRequireUpgradeOrDeploy( + + await verifyIfAccountNeedUpgradeOrDeploy( network, userAddress, publicKey, + false, ); const response = await wallet.request({ diff --git a/packages/starknet-snap/src/extractPublicKey.ts b/packages/starknet-snap/src/extractPublicKey.ts index d05e7fa1..2d90fd86 100644 --- a/packages/starknet-snap/src/extractPublicKey.ts +++ b/packages/starknet-snap/src/extractPublicKey.ts @@ -6,10 +6,13 @@ import type { } from './types/snapApi'; import { logger } from './utils/logger'; import { toJson } from './utils/serializer'; -import { getAccount, getNetworkFromChainId } from './utils/snapUtils'; +import { + getAccount, + getNetworkFromChainId, + verifyIfAccountNeedUpgradeOrDeploy, +} from './utils/snapUtils'; import { validateAndParseAddress, - validateAccountRequireUpgradeOrDeploy, getKeysFromAddress, } from './utils/starknetUtils'; @@ -48,10 +51,12 @@ export async function extractPublicKey(params: ApiParamsWithKeyDeriver) { state, userAddress, ); - await validateAccountRequireUpgradeOrDeploy( + + await verifyIfAccountNeedUpgradeOrDeploy( network, userAddress, publicKey, + false, ); let userPublicKey; diff --git a/packages/starknet-snap/src/signDeclareTransaction.ts b/packages/starknet-snap/src/signDeclareTransaction.ts index a346d94e..3c627d63 100644 --- a/packages/starknet-snap/src/signDeclareTransaction.ts +++ b/packages/starknet-snap/src/signDeclareTransaction.ts @@ -10,12 +10,11 @@ import { toJson } from './utils/serializer'; import { getNetworkFromChainId, getSignTxnTxt, - showAccountRequireUpgradeOrDeployModal, + verifyIfAccountNeedUpgradeOrDeploy, } from './utils/snapUtils'; import { getKeysFromAddress, signDeclareTransaction as signDeclareTransactionUtil, - validateAccountRequireUpgradeOrDeploy, } from './utils/starknetUtils'; /** @@ -38,16 +37,7 @@ export async function signDeclareTransaction( signerAddress, ); - try { - await validateAccountRequireUpgradeOrDeploy( - network, - signerAddress, - publicKey, - ); - } catch (validateError) { - await showAccountRequireUpgradeOrDeployModal(wallet, validateError); - throw validateError; - } + await verifyIfAccountNeedUpgradeOrDeploy(network, signerAddress, publicKey); logger.log( `signDeclareTransaction params: ${toJson( diff --git a/packages/starknet-snap/src/signDeployAccountTransaction.ts b/packages/starknet-snap/src/signDeployAccountTransaction.ts index 478731d7..222a323f 100644 --- a/packages/starknet-snap/src/signDeployAccountTransaction.ts +++ b/packages/starknet-snap/src/signDeployAccountTransaction.ts @@ -10,12 +10,11 @@ import { toJson } from './utils/serializer'; import { getNetworkFromChainId, getSignTxnTxt, - showAccountRequireUpgradeOrDeployModal, + verifyIfAccountNeedUpgradeOrDeploy, } from './utils/snapUtils'; import { getKeysFromAddress, signDeployAccountTransaction as signDeployAccountTransactionUtil, - validateAccountRequireUpgradeOrDeploy, } from './utils/starknetUtils'; /** @@ -38,16 +37,7 @@ export async function signDeployAccountTransaction( signerAddress, ); - try { - await validateAccountRequireUpgradeOrDeploy( - network, - signerAddress, - publicKey, - ); - } catch (validateError) { - await showAccountRequireUpgradeOrDeployModal(wallet, validateError); - throw validateError; - } + await verifyIfAccountNeedUpgradeOrDeploy(network, signerAddress, publicKey); logger.log( `signDeployAccountTransaction params: ${toJson( diff --git a/packages/starknet-snap/src/signMessage.ts b/packages/starknet-snap/src/signMessage.ts index b0a8ac83..4f61714a 100644 --- a/packages/starknet-snap/src/signMessage.ts +++ b/packages/starknet-snap/src/signMessage.ts @@ -9,13 +9,12 @@ import { toJson } from './utils/serializer'; import { getNetworkFromChainId, addDialogTxt, - showAccountRequireUpgradeOrDeployModal, + verifyIfAccountNeedUpgradeOrDeploy, } from './utils/snapUtils'; import { signMessage as signMessageUtil, getKeysFromAddress, validateAndParseAddress, - validateAccountRequireUpgradeOrDeploy, } from './utils/starknetUtils'; /** @@ -52,16 +51,7 @@ export async function signMessage(params: ApiParamsWithKeyDeriver) { ); } - try { - await validateAccountRequireUpgradeOrDeploy( - network, - signerAddress, - publicKey, - ); - } catch (validateError) { - await showAccountRequireUpgradeOrDeployModal(wallet, validateError); - throw validateError; - } + await verifyIfAccountNeedUpgradeOrDeploy(network, signerAddress, publicKey); const components = []; addDialogTxt(components, 'Message', toJson(typedDataMessage)); diff --git a/packages/starknet-snap/src/signTransaction.ts b/packages/starknet-snap/src/signTransaction.ts index 3149579e..72876068 100644 --- a/packages/starknet-snap/src/signTransaction.ts +++ b/packages/starknet-snap/src/signTransaction.ts @@ -10,13 +10,9 @@ import { toJson } from './utils/serializer'; import { getNetworkFromChainId, getSignTxnTxt, - showAccountRequireUpgradeOrDeployModal, + verifyIfAccountNeedUpgradeOrDeploy, } from './utils/snapUtils'; -import { - getKeysFromAddress, - signTransactions, - validateAccountRequireUpgradeOrDeploy, -} from './utils/starknetUtils'; +import { getKeysFromAddress, signTransactions } from './utils/starknetUtils'; /** * @@ -37,16 +33,7 @@ export async function signTransaction( signerAddress, ); - try { - await validateAccountRequireUpgradeOrDeploy( - network, - signerAddress, - publicKey, - ); - } catch (validateError) { - await showAccountRequireUpgradeOrDeployModal(wallet, validateError); - throw validateError; - } + await verifyIfAccountNeedUpgradeOrDeploy(network, signerAddress, publicKey); logger.log( `signTransaction params: ${toJson(requestParamsObj.transactions, 2)}}`, diff --git a/packages/starknet-snap/src/utils/__mocks__/snap.ts b/packages/starknet-snap/src/utils/__mocks__/snap.ts index da3f01cd..75f5e80b 100644 --- a/packages/starknet-snap/src/utils/__mocks__/snap.ts +++ b/packages/starknet-snap/src/utils/__mocks__/snap.ts @@ -4,6 +4,8 @@ export const getBip44Deriver = jest.fn(); export const confirmDialog = jest.fn(); +export const alertDialog = jest.fn(); + export const getStateData = jest.fn(); export const setStateData = jest.fn(); diff --git a/packages/starknet-snap/src/utils/exceptions.ts b/packages/starknet-snap/src/utils/exceptions.ts index 1fc757f0..d91b7d14 100644 --- a/packages/starknet-snap/src/utils/exceptions.ts +++ b/packages/starknet-snap/src/utils/exceptions.ts @@ -1,3 +1,17 @@ -export class UpgradeRequiredError extends Error {} +import { SnapError } from '@metamask/snaps-sdk'; -export class DeployRequiredError extends Error {} +// Extend SnapError to allow error message visible to client +export class UpgradeRequiredError extends SnapError { + constructor(message?: string) { + super(message ?? 'Upgrade required'); + } +} + +export class DeployRequiredError extends SnapError { + constructor(message?: string) { + super( + message ?? + 'Cairo 0 contract address balance is not empty, deploy required', + ); + } +} diff --git a/packages/starknet-snap/src/utils/snap.test.ts b/packages/starknet-snap/src/utils/snap.test.ts index 383ed374..8fa99749 100644 --- a/packages/starknet-snap/src/utils/snap.test.ts +++ b/packages/starknet-snap/src/utils/snap.test.ts @@ -46,6 +46,30 @@ describe('confirmDialog', () => { }); }); +describe('alertDialog', () => { + it('calls snap_dialog', async () => { + const spy = jest.spyOn(snapUtil.getProvider(), 'request'); + const components: Component[] = [ + heading('header'), + text('subHeader'), + divider(), + row('Label1', text('Value1')), + text('**Label2**:'), + row('SubLabel1', text('SubValue1')), + ]; + + await snapUtil.alertDialog(components); + + expect(spy).toHaveBeenCalledWith({ + method: 'snap_dialog', + params: { + type: 'alert', + content: panel(components), + }, + }); + }); +}); + describe('getStateData', () => { it('gets state data', async () => { const spy = jest.spyOn(snapUtil.getProvider(), 'request'); diff --git a/packages/starknet-snap/src/utils/snap.ts b/packages/starknet-snap/src/utils/snap.ts index b2dd276c..49856f57 100644 --- a/packages/starknet-snap/src/utils/snap.ts +++ b/packages/starknet-snap/src/utils/snap.ts @@ -1,7 +1,7 @@ import type { BIP44AddressKeyDeriver } from '@metamask/key-tree'; import { getBIP44AddressKeyDeriver } from '@metamask/key-tree'; import type { Component, DialogResult, Json } from '@metamask/snaps-sdk'; -import { panel, type SnapsProvider } from '@metamask/snaps-sdk'; +import { DialogType, panel, type SnapsProvider } from '@metamask/snaps-sdk'; declare const snap: SnapsProvider; @@ -41,7 +41,25 @@ export async function confirmDialog( return snap.request({ method: 'snap_dialog', params: { - type: 'confirmation', + type: DialogType.Confirmation, + content: panel(components), + }, + }); +} + +/** + * Displays a alert dialog with the specified components. + * + * @param components - An array of components to display in the dialog. + * @returns A Promise that resolves to the result of the dialog. + */ +export async function alertDialog( + components: Component[], +): Promise { + return snap.request({ + method: 'snap_dialog', + params: { + type: DialogType.Alert, content: panel(components), }, }); diff --git a/packages/starknet-snap/src/utils/snapUtils.test.ts b/packages/starknet-snap/src/utils/snapUtils.test.ts new file mode 100644 index 00000000..442fc06e --- /dev/null +++ b/packages/starknet-snap/src/utils/snapUtils.test.ts @@ -0,0 +1,131 @@ +import { constants } from 'starknet'; + +import { generateAccounts } from '../../test/utils'; +import { STARKNET_SEPOLIA_TESTNET_NETWORK } from './constants'; +import { DeployRequiredError, UpgradeRequiredError } from './exceptions'; +import * as snapHelper from './snap'; +import { verifyIfAccountNeedUpgradeOrDeploy } from './snapUtils'; +import * as starknetUtils from './starknetUtils'; + +jest.mock('./snap'); +jest.mock('./logger'); + +describe('verifyIfAccountNeedUpgradeOrDeploy', () => { + const network = STARKNET_SEPOLIA_TESTNET_NETWORK; + + const mockAcccount = async () => { + const accounts = await generateAccounts( + constants.StarknetChainId.SN_SEPOLIA, + 1, + ); + return accounts[0]; + }; + + const prepareMock = () => { + const verifyIfAccountNeedUpgradeOrDeploySpy = jest.spyOn( + starknetUtils, + 'validateAccountRequireUpgradeOrDeploy', + ); + + const alertDialogSpy = jest.spyOn(snapHelper, 'alertDialog'); + + verifyIfAccountNeedUpgradeOrDeploySpy.mockReturnThis(); + alertDialogSpy.mockReturnThis(); + + return { + verifyIfAccountNeedUpgradeOrDeploySpy, + alertDialogSpy, + }; + }; + + it('does not throw error if the account does not required to upgrade or deploy', async () => { + const account = await mockAcccount(); + const { alertDialogSpy } = prepareMock(); + + await verifyIfAccountNeedUpgradeOrDeploy( + network, + account.address, + account.publicKey, + ); + + expect(alertDialogSpy).not.toHaveBeenCalled(); + }); + + it.each([ + { + action: 'upgrade', + error: new UpgradeRequiredError(), + }, + { + action: 'deploy', + error: new DeployRequiredError(), + }, + ])( + 'throws error but does not render alert dialog if the account required $action and `showAlert` is false', + async (testData: { error: Error }) => { + const account = await mockAcccount(); + const { verifyIfAccountNeedUpgradeOrDeploySpy, alertDialogSpy } = + prepareMock(); + verifyIfAccountNeedUpgradeOrDeploySpy.mockRejectedValue(testData.error); + + await expect( + verifyIfAccountNeedUpgradeOrDeploy( + network, + account.address, + account.publicKey, + false, + ), + ).rejects.toThrow(testData.error); + + expect(alertDialogSpy).not.toHaveBeenCalled(); + }, + ); + + it('throws error and does not render alert dialog if the error is not either UpgradeRequiredError or DeployRequiredError', async () => { + const account = await mockAcccount(); + const { verifyIfAccountNeedUpgradeOrDeploySpy } = prepareMock(); + verifyIfAccountNeedUpgradeOrDeploySpy.mockRejectedValue( + new Error('Internal Error'), + ); + + await expect( + verifyIfAccountNeedUpgradeOrDeploy( + network, + account.address, + account.publicKey, + ), + ).rejects.toThrow('Internal Error'); + + expect(snapHelper.alertDialog).not.toHaveBeenCalled(); + }); + + it.each([ + { + action: 'upgrade', + error: new UpgradeRequiredError(), + }, + { + action: 'deploy', + error: new DeployRequiredError(), + }, + ])( + 'throws error and renders alert dialog if the account required $action and `showAlert` is true', + async (testData: { error: Error }) => { + const account = await mockAcccount(); + const { verifyIfAccountNeedUpgradeOrDeploySpy, alertDialogSpy } = + prepareMock(); + verifyIfAccountNeedUpgradeOrDeploySpy.mockRejectedValue(testData.error); + + await expect( + verifyIfAccountNeedUpgradeOrDeploy( + network, + account.address, + account.publicKey, + true, + ), + ).rejects.toThrow(testData.error); + + expect(alertDialogSpy).toHaveBeenCalled(); + }, + ); +}); diff --git a/packages/starknet-snap/src/utils/snapUtils.ts b/packages/starknet-snap/src/utils/snapUtils.ts index 0ecc7d90..4aa38a44 100644 --- a/packages/starknet-snap/src/utils/snapUtils.ts +++ b/packages/starknet-snap/src/utils/snapUtils.ts @@ -1,11 +1,5 @@ import type { Component, Json, SnapsProvider } from '@metamask/snaps-sdk'; -import { - text, - copyable, - panel, - heading, - DialogType, -} from '@metamask/snaps-sdk'; +import { text, copyable, heading } from '@metamask/snaps-sdk'; import type { Mutex } from 'async-mutex'; import convert from 'ethereum-unit-converter'; import { num as numUtils, constants } from 'starknet'; @@ -43,7 +37,11 @@ import { import { DeployRequiredError, UpgradeRequiredError } from './exceptions'; import { logger } from './logger'; import { toJson } from './serializer'; -import { validateAndParseAddress } from './starknetUtils'; +import { alertDialog } from './snap'; +import { + validateAccountRequireUpgradeOrDeploy, + validateAndParseAddress, +} from './starknetUtils'; import { filterTransactions, TimestampFilter, @@ -1184,61 +1182,65 @@ export function dappUrl(envt: string) { } /** - * - * @param wallet + * Displays a modal to the user requesting them to upgrade their account. */ -export async function showUpgradeRequestModal(wallet) { - await wallet.request({ - method: 'snap_dialog', - params: { - type: DialogType.Alert, - content: panel([ - heading('Account Upgrade Mandatory!'), - text( - `Visit the [companion dapp for Starknet](${dappUrl( - // eslint-disable-next-line no-restricted-globals - process.env.SNAP_ENV as unknown as string, - )}) and click “Upgrade”.\nThank you!`, - ), - ]), - }, - }); +export async function showUpgradeRequestModal() { + await alertDialog([ + heading('Account Upgrade Mandatory!'), + text( + `Visit the [companion dapp for Starknet](${dappUrl( + // eslint-disable-next-line no-restricted-globals + process.env.SNAP_ENV as unknown as string, + )}) and click “Upgrade”.\nThank you!`, + ), + ]); } /** - * - * @param wallet + * Displays a modal to the user requesting them to deploy their account. */ -export async function showDeployRequestModal(wallet) { - await wallet.request({ - method: 'snap_dialog', - params: { - type: DialogType.Alert, - content: panel([ - heading('Account Deployment Mandatory!'), - text( - `Visit the [companion dapp for Starknet](${dappUrl( - // eslint-disable-next-line no-restricted-globals - process.env.SNAP_ENV as unknown as string, - )}) to deploy your account.\nThank you!`, - ), - ]), - }, - }); +export async function showDeployRequestModal() { + await alertDialog([ + heading('Account Deployment Mandatory!'), + text( + `Visit the [companion dapp for Starknet](${dappUrl( + // eslint-disable-next-line no-restricted-globals + process.env.SNAP_ENV as unknown as string, + )}) to deploy your account.\nThank you!`, + ), + ]); } /** + * Verifies whether the account needs to be upgraded or deployed and throws an error if necessary. * - * @param wallet - * @param error + * @param network - The network object. + * @param address - The account address. + * @param publicKey - The public key of the account address. + * @param [showAlert] - The flag to show an alert modal; true will show the modal, false will not. + * @throws {DeployRequiredError} If the account needs to be deployed. + * @throws {UpgradeRequiredError} If the account needs to be upgraded. */ -export async function showAccountRequireUpgradeOrDeployModal( - wallet, - error: DeployRequiredError | UpgradeRequiredError, +export async function verifyIfAccountNeedUpgradeOrDeploy( + network: Network, + address: string, + publicKey: string, + showAlert = true, ) { - if (error instanceof DeployRequiredError) { - await showDeployRequestModal(wallet); - } else if (error instanceof UpgradeRequiredError) { - await showUpgradeRequestModal(wallet); + try { + await validateAccountRequireUpgradeOrDeploy(network, address, publicKey); + } catch (error) { + if (error instanceof DeployRequiredError) { + showAlert && (await showDeployRequestModal()); + } else if (error instanceof UpgradeRequiredError) { + showAlert && (await showUpgradeRequestModal()); + } else { + logger.warn( + 'Unexpected Error, neither DeployRequiredError or UpgradeRequiredError', + error, + ); + } + + throw error; } } diff --git a/packages/starknet-snap/src/utils/starknetUtils.ts b/packages/starknet-snap/src/utils/starknetUtils.ts index 829043db..dd31f635 100644 --- a/packages/starknet-snap/src/utils/starknetUtils.ts +++ b/packages/starknet-snap/src/utils/starknetUtils.ts @@ -1154,10 +1154,8 @@ export const validateAccountRequireUpgradeOrDeploy = async ( pubKey: string, ) => { if (await isUpgradeRequired(network, address)) { - throw new UpgradeRequiredError('Upgrade required'); + throw new UpgradeRequiredError(); } else if (await isDeployRequired(network, address, pubKey)) { - throw new DeployRequiredError( - `Cairo 0 contract address ${address} balance is not empty, deploy required`, - ); + throw new DeployRequiredError(); } }; diff --git a/packages/starknet-snap/src/verifySignedMessage.ts b/packages/starknet-snap/src/verifySignedMessage.ts index e35a7883..9865d3cd 100644 --- a/packages/starknet-snap/src/verifySignedMessage.ts +++ b/packages/starknet-snap/src/verifySignedMessage.ts @@ -5,12 +5,14 @@ import type { } from './types/snapApi'; import { logger } from './utils/logger'; import { toJson } from './utils/serializer'; -import { getNetworkFromChainId } from './utils/snapUtils'; +import { + getNetworkFromChainId, + verifyIfAccountNeedUpgradeOrDeploy, +} from './utils/snapUtils'; import { verifyTypedDataMessageSignature, getFullPublicKeyPairFromPrivateKey, getKeysFromAddress, - validateAccountRequireUpgradeOrDeploy, validateAndParseAddress, } from './utils/starknetUtils'; @@ -53,10 +55,12 @@ export async function verifySignedMessage(params: ApiParamsWithKeyDeriver) { const { privateKey: signerPrivateKey, publicKey } = await getKeysFromAddress(keyDeriver, network, state, verifySignerAddress); - await validateAccountRequireUpgradeOrDeploy( + + await verifyIfAccountNeedUpgradeOrDeploy( network, verifySignerAddress, publicKey, + false, ); const fullPublicKey = getFullPublicKeyPairFromPrivateKey(signerPrivateKey); diff --git a/packages/starknet-snap/test/src/declareContract.test.ts b/packages/starknet-snap/test/src/declareContract.test.ts index bb894752..137aaae2 100644 --- a/packages/starknet-snap/test/src/declareContract.test.ts +++ b/packages/starknet-snap/test/src/declareContract.test.ts @@ -18,7 +18,6 @@ import { import { getAddressKeyDeriver } from '../../src/utils/keyPair'; import { Mutex } from 'async-mutex'; import { - ApiParams, ApiParamsWithKeyDeriver, DeclareContractRequestParams, } from '../../src/types/snapApi'; @@ -65,16 +64,6 @@ describe('Test function: declareContract', function () { sandbox.useFakeTimers(createAccountProxyTxn.timestamp); walletStub.rpcStubs.snap_dialog.resolves(true); walletStub.rpcStubs.snap_manageState.resolves(state); - - sandbox - .stub(snapsUtil, 'showAccountRequireUpgradeOrDeployModal') - .callsFake(async (wallet, e) => { - if (e instanceof DeployRequiredError) { - await snapsUtil.showDeployRequestModal(wallet); - } else if (e instanceof UpgradeRequiredError) { - await snapsUtil.showUpgradeRequestModal(wallet); - } - }); }); afterEach(function () { @@ -82,60 +71,6 @@ describe('Test function: declareContract', function () { sandbox.restore(); }); - it('should 1) throw an error and 2) show upgrade modal if account upgrade required', async function () { - const validateAccountRequireUpgradeOrDeployStub = sandbox - .stub(utils, 'validateAccountRequireUpgradeOrDeploy') - .throws(new UpgradeRequiredError('Upgrade Required')); - const showUpgradeRequestModalStub = sandbox - .stub(snapsUtil, 'showUpgradeRequestModal') - .resolves(); - let result; - try { - result = await declareContract(apiParams); - } catch (err) { - result = err; - } finally { - expect( - validateAccountRequireUpgradeOrDeployStub, - ).to.have.been.calledOnceWith( - STARKNET_SEPOLIA_TESTNET_NETWORK, - account1.address, - account1.publicKey, - ); - expect(showUpgradeRequestModalStub).to.have.been.calledOnce; - expect(result).to.be.an('Error'); - } - }); - - it('should 1) throw an error and 2) show deploy modal if account deployed required', async function () { - const validateAccountRequireUpgradeOrDeployStub = sandbox - .stub(utils, 'validateAccountRequireUpgradeOrDeploy') - .throws( - new DeployRequiredError( - `Cairo 0 contract address ${account1.address} balance is not empty, deploy required`, - ), - ); - const showDeployRequestModalStub = sandbox - .stub(snapsUtil, 'showDeployRequestModal') - .resolves(); - let result; - try { - result = await declareContract(apiParams); - } catch (err) { - result = err; - } finally { - expect( - validateAccountRequireUpgradeOrDeployStub, - ).to.have.been.calledOnceWith( - STARKNET_SEPOLIA_TESTNET_NETWORK, - account1.address, - account1.publicKey, - ); - expect(showDeployRequestModalStub).to.have.been.calledOnce; - expect(result).to.be.an('Error'); - } - }); - it('should declareContract correctly', async function () { sandbox.stub(utils, 'validateAccountRequireUpgradeOrDeploy').resolvesThis(); const declareContractStub = sandbox diff --git a/packages/starknet-snap/test/src/executeTxn.test.ts b/packages/starknet-snap/test/src/executeTxn.test.ts index 00f85e95..68971955 100644 --- a/packages/starknet-snap/test/src/executeTxn.test.ts +++ b/packages/starknet-snap/test/src/executeTxn.test.ts @@ -84,15 +84,6 @@ describe('Test function: executeTxn', function () { sandbox .stub(utils, 'waitForTransaction') .resolves({} as unknown as GetTransactionReceiptResponse); - sandbox - .stub(snapsUtil, 'showAccountRequireUpgradeOrDeployModal') - .callsFake(async (wallet, e) => { - if (e instanceof DeployRequiredError) { - await snapsUtil.showDeployRequestModal(wallet); - } else if (e instanceof UpgradeRequiredError) { - await snapsUtil.showUpgradeRequestModal(wallet); - } - }); }); afterEach(function () { @@ -101,48 +92,6 @@ describe('Test function: executeTxn', function () { apiParams.requestParams = requestObject; }); - it('should 1) throw an error and 2) show upgrade modal if account upgrade required', async function () { - const validateAccountRequireUpgradeOrDeployStub = sandbox - .stub(utils, 'validateAccountRequireUpgradeOrDeploy') - .throws(new UpgradeRequiredError('Upgrade Required')); - const showUpgradeRequestModalStub = sandbox - .stub(snapsUtil, 'showUpgradeRequestModal') - .resolves(); - let result; - try { - result = await executeTxn(apiParams); - } catch (err) { - result = err; - } finally { - expect(validateAccountRequireUpgradeOrDeployStub).to.have.been.calledOnce; - expect(showUpgradeRequestModalStub).to.have.been.calledOnce; - expect(result).to.be.an('Error'); - } - }); - - it('should 1) throw an error and 2) show deploy modal if account deployed required', async function () { - const validateAccountRequireUpgradeOrDeployStub = sandbox - .stub(utils, 'validateAccountRequireUpgradeOrDeploy') - .throws( - new DeployRequiredError( - `Cairo 0 contract address ${account1.address} balance is not empty, deploy required`, - ), - ); - const showDeployRequestModalStub = sandbox - .stub(snapsUtil, 'showDeployRequestModal') - .resolves(); - let result; - try { - result = await executeTxn(apiParams); - } catch (err) { - result = err; - } finally { - expect(validateAccountRequireUpgradeOrDeployStub).to.have.been.calledOnce; - expect(showDeployRequestModalStub).to.have.been.calledOnce; - expect(result).to.be.an('Error'); - } - }); - it('should executeTxn correctly and deploy an account', async function () { sandbox.stub(utils, 'validateAccountRequireUpgradeOrDeploy').resolvesThis(); sandbox.stub(utils, 'isAccountDeployed').resolves(false); diff --git a/packages/starknet-snap/test/src/signDeclareTransaction.test.ts b/packages/starknet-snap/test/src/signDeclareTransaction.test.ts index d36ba3d6..e7d0689e 100644 --- a/packages/starknet-snap/test/src/signDeclareTransaction.test.ts +++ b/packages/starknet-snap/test/src/signDeclareTransaction.test.ts @@ -71,15 +71,6 @@ describe('Test function: signDeclareTransaction', function () { sandbox.useFakeTimers(createAccountProxyTxn.timestamp); walletStub.rpcStubs.snap_dialog.resolves(true); walletStub.rpcStubs.snap_manageState.resolves(state); - sandbox - .stub(snapsUtil, 'showAccountRequireUpgradeOrDeployModal') - .callsFake(async (wallet, e) => { - if (e instanceof DeployRequiredError) { - await snapsUtil.showDeployRequestModal(wallet); - } else if (e instanceof UpgradeRequiredError) { - await snapsUtil.showUpgradeRequestModal(wallet); - } - }); }); afterEach(function () { @@ -96,61 +87,6 @@ describe('Test function: signDeclareTransaction', function () { expect(result).to.be.eql(signature3); }); - it('should 1) throw an error and 2) show upgrade modal if account upgrade required', async function () { - const validateAccountRequireUpgradeOrDeployStub = sandbox - .stub(utils, 'validateAccountRequireUpgradeOrDeploy') - .throws(new UpgradeRequiredError('Upgrade Required')); - const showUpgradeRequestModalStub = sandbox - .stub(snapsUtil, 'showUpgradeRequestModal') - .resolves(); - let result; - try { - result = await signDeclareTransaction(apiParams); - } catch (err) { - result = err; - } finally { - expect( - validateAccountRequireUpgradeOrDeployStub, - ).to.have.been.calledOnceWith( - STARKNET_SEPOLIA_TESTNET_NETWORK, - account1.address, - account1.publicKey, - ); - expect(showUpgradeRequestModalStub).to.have.been.calledOnce; - expect(result).to.be.an('Error'); - expect(result.message).to.equal('Upgrade Required'); - } - }); - - it('should 1) throw an error and 2) show deploy modal if account deployed required', async function () { - const validateAccountRequireUpgradeOrDeployStub = sandbox - .stub(utils, 'validateAccountRequireUpgradeOrDeploy') - .throws( - new DeployRequiredError( - `Cairo 0 contract address ${account1.address} balance is not empty, deploy required`, - ), - ); - const showDeployRequestModalStub = sandbox - .stub(snapsUtil, 'showDeployRequestModal') - .resolves(); - let result; - try { - result = await signDeclareTransaction(apiParams); - } catch (err) { - result = err; - } finally { - expect( - validateAccountRequireUpgradeOrDeployStub, - ).to.have.been.calledOnceWith( - STARKNET_SEPOLIA_TESTNET_NETWORK, - account1.address, - account1.publicKey, - ); - expect(showDeployRequestModalStub).to.have.been.calledOnce; - expect(result).to.be.an('Error'); - } - }); - it('should throw error if signDeclareTransaction fail', async function () { sandbox.stub(utils, 'validateAccountRequireUpgradeOrDeploy').resolvesThis(); sandbox.stub(utils, 'signDeclareTransaction').throws(new Error()); diff --git a/packages/starknet-snap/test/src/signDeployAccountTransaction.test.ts b/packages/starknet-snap/test/src/signDeployAccountTransaction.test.ts index 66b4fcd1..853413c4 100644 --- a/packages/starknet-snap/test/src/signDeployAccountTransaction.test.ts +++ b/packages/starknet-snap/test/src/signDeployAccountTransaction.test.ts @@ -74,15 +74,6 @@ describe('Test function: signDeployAccountTransaction', function () { sandbox.useFakeTimers(createAccountProxyTxn.timestamp); walletStub.rpcStubs.snap_dialog.resolves(true); walletStub.rpcStubs.snap_manageState.resolves(state); - sandbox - .stub(snapsUtil, 'showAccountRequireUpgradeOrDeployModal') - .callsFake(async (wallet, e) => { - if (e instanceof DeployRequiredError) { - await snapsUtil.showDeployRequestModal(wallet); - } else if (e instanceof UpgradeRequiredError) { - await snapsUtil.showUpgradeRequestModal(wallet); - } - }); }); afterEach(function () { @@ -99,61 +90,6 @@ describe('Test function: signDeployAccountTransaction', function () { expect(result).to.be.eql(signature3); }); - it('should 1) throw an error and 2) show upgrade modal if account upgrade required', async function () { - const validateAccountRequireUpgradeOrDeployStub = sandbox - .stub(utils, 'validateAccountRequireUpgradeOrDeploy') - .throws(new UpgradeRequiredError('Upgrade Required')); - const showUpgradeRequestModalStub = sandbox - .stub(snapsUtil, 'showUpgradeRequestModal') - .resolves(); - let result; - try { - result = await signDeployAccountTransaction(apiParams); - } catch (err) { - result = err; - } finally { - expect( - validateAccountRequireUpgradeOrDeployStub, - ).to.have.been.calledOnceWith( - STARKNET_SEPOLIA_TESTNET_NETWORK, - account1.address, - account1.publicKey, - ); - expect(showUpgradeRequestModalStub).to.have.been.calledOnce; - expect(result).to.be.an('Error'); - expect(result.message).to.equal('Upgrade Required'); - } - }); - - it('should 1) throw an error and 2) show deploy modal if account deployed required', async function () { - const validateAccountRequireUpgradeOrDeployStub = sandbox - .stub(utils, 'validateAccountRequireUpgradeOrDeploy') - .throws( - new DeployRequiredError( - `Cairo 0 contract address ${account1.address} balance is not empty, deploy required`, - ), - ); - const showDeployRequestModalStub = sandbox - .stub(snapsUtil, 'showDeployRequestModal') - .resolves(); - let result; - try { - result = await signDeployAccountTransaction(apiParams); - } catch (err) { - result = err; - } finally { - expect( - validateAccountRequireUpgradeOrDeployStub, - ).to.have.been.calledOnceWith( - STARKNET_SEPOLIA_TESTNET_NETWORK, - account1.address, - account1.publicKey, - ); - expect(showDeployRequestModalStub).to.have.been.calledOnce; - expect(result).to.be.an('Error'); - } - }); - it('should throw error if signDeployAccountTransaction fail', async function () { sandbox.stub(utils, 'validateAccountRequireUpgradeOrDeploy').resolvesThis(); sandbox.stub(utils, 'signDeployAccountTransaction').throws(new Error()); diff --git a/packages/starknet-snap/test/src/signMessage.test.ts b/packages/starknet-snap/test/src/signMessage.test.ts index a00a89b5..8d26f526 100644 --- a/packages/starknet-snap/test/src/signMessage.test.ts +++ b/packages/starknet-snap/test/src/signMessage.test.ts @@ -148,68 +148,6 @@ describe('Test function: signMessage', function () { }); }); - describe('when account require upgrade', function () { - let validateAccountRequireUpgradeOrDeployStub: sinon.SinonStub; - beforeEach(async function () { - validateAccountRequireUpgradeOrDeployStub = sandbox - .stub(utils, 'validateAccountRequireUpgradeOrDeploy') - .throws(new UpgradeRequiredError('Upgrade Required')); - }); - - it('should throw error if upgrade required', async function () { - let result; - try { - result = await signMessage(apiParams); - } catch (err) { - result = err; - } finally { - expect( - validateAccountRequireUpgradeOrDeployStub, - ).to.have.been.calledOnceWith( - STARKNET_SEPOLIA_TESTNET_NETWORK, - account1.address, - account1.publicKey, - ); - expect(result).to.be.an('Error'); - expect(result.message).to.equal('Upgrade Required'); - } - }); - }); - - describe('when account require deploy', function () { - let validateAccountRequireUpgradeOrDeployStub: sinon.SinonStub; - beforeEach(async function () { - validateAccountRequireUpgradeOrDeployStub = sandbox - .stub(utils, 'validateAccountRequireUpgradeOrDeploy') - .throws( - new DeployRequiredError( - `Cairo 0 contract address ${account1.address} balance is not empty, deploy required`, - ), - ); - }); - - it('should throw error if deploy required', async function () { - let result; - try { - result = await signMessage(apiParams); - } catch (err) { - result = err; - } finally { - expect( - validateAccountRequireUpgradeOrDeployStub, - ).to.have.been.calledOnceWith( - STARKNET_SEPOLIA_TESTNET_NETWORK, - account1.address, - account1.publicKey, - ); - expect(result).to.be.an('Error'); - expect(result.message).to.equal( - `Cairo 0 contract address ${account1.address} balance is not empty, deploy required`, - ); - } - }); - }); - describe('when account is not require upgrade', function () { beforeEach(async function () { apiParams.requestParams = { diff --git a/packages/starknet-snap/test/src/signTransaction.test.ts b/packages/starknet-snap/test/src/signTransaction.test.ts index b8a6167a..648bac3b 100644 --- a/packages/starknet-snap/test/src/signTransaction.test.ts +++ b/packages/starknet-snap/test/src/signTransaction.test.ts @@ -82,15 +82,6 @@ describe('Test function: signTransaction', function () { sandbox.useFakeTimers(createAccountProxyTxn.timestamp); walletStub.rpcStubs.snap_dialog.resolves(true); walletStub.rpcStubs.snap_manageState.resolves(state); - sandbox - .stub(snapsUtil, 'showAccountRequireUpgradeOrDeployModal') - .callsFake(async (wallet, e) => { - if (e instanceof DeployRequiredError) { - await snapsUtil.showDeployRequestModal(wallet); - } else if (e instanceof UpgradeRequiredError) { - await snapsUtil.showUpgradeRequestModal(wallet); - } - }); }); afterEach(function () { @@ -106,61 +97,6 @@ describe('Test function: signTransaction', function () { expect(result).to.be.eql(signature3); }); - it('should 1) throw an error and 2) show upgrade modal if account upgrade required', async function () { - const validateAccountRequireUpgradeOrDeployStub = sandbox - .stub(utils, 'validateAccountRequireUpgradeOrDeploy') - .throws(new UpgradeRequiredError('Upgrade Required')); - const showUpgradeRequestModalStub = sandbox - .stub(snapsUtil, 'showUpgradeRequestModal') - .resolves(); - let result; - try { - result = await signTransaction(apiParams); - } catch (err) { - result = err; - } finally { - expect( - validateAccountRequireUpgradeOrDeployStub, - ).to.have.been.calledOnceWith( - STARKNET_SEPOLIA_TESTNET_NETWORK, - account1.address, - account1.publicKey, - ); - expect(showUpgradeRequestModalStub).to.have.been.calledOnce; - expect(result).to.be.an('Error'); - expect(result.message).to.equal('Upgrade Required'); - } - }); - - it('should 1) throw an error and 2) show deploy modal if account deployed required', async function () { - const validateAccountRequireUpgradeOrDeployStub = sandbox - .stub(utils, 'validateAccountRequireUpgradeOrDeploy') - .throws( - new DeployRequiredError( - `Cairo 0 contract address ${account1.address} balance is not empty, deploy required`, - ), - ); - const showDeployRequestModalStub = sandbox - .stub(snapsUtil, 'showDeployRequestModal') - .resolves(); - let result; - try { - result = await signTransaction(apiParams); - } catch (err) { - result = err; - } finally { - expect( - validateAccountRequireUpgradeOrDeployStub, - ).to.have.been.calledOnceWith( - STARKNET_SEPOLIA_TESTNET_NETWORK, - account1.address, - account1.publicKey, - ); - expect(showDeployRequestModalStub).to.have.been.calledOnce; - expect(result).to.be.an('Error'); - } - }); - it('should throw error if signTransaction fail', async function () { sandbox.stub(utils, 'validateAccountRequireUpgradeOrDeploy').resolvesThis(); sandbox.stub(utils, 'signTransactions').throws(new Error()); diff --git a/packages/starknet-snap/test/src/verifySignedMessage.test.ts b/packages/starknet-snap/test/src/verifySignedMessage.test.ts index edfc2aec..5273b1d5 100644 --- a/packages/starknet-snap/test/src/verifySignedMessage.test.ts +++ b/packages/starknet-snap/test/src/verifySignedMessage.test.ts @@ -119,33 +119,6 @@ describe('Test function: verifySignedMessage', function () { }); }); - describe('when account require upgrade', function () { - let validateAccountRequireUpgradeOrDeployStub: sinon.SinonStub; - beforeEach(async function () { - validateAccountRequireUpgradeOrDeployStub = sandbox - .stub(utils, 'validateAccountRequireUpgradeOrDeploy') - .throws(new UpgradeRequiredError('Upgrade Required')); - }); - - it('should throw error if upgrade required', async function () { - let result; - try { - result = await verifySignedMessage(apiParams); - } catch (err) { - result = err; - } finally { - expect( - validateAccountRequireUpgradeOrDeployStub, - ).to.have.been.calledOnceWith( - STARKNET_SEPOLIA_TESTNET_NETWORK, - account1.address, - account1.publicKey, - ); - expect(result).to.be.an('Error'); - } - }); - }); - describe('when account is not require upgrade', function () { beforeEach(async function () { sandbox From c7facfcd57545c5aa59df1caa1d3e34f06d79663 Mon Sep 17 00:00:00 2001 From: Stanley Yuen <102275989+stanleyyconsensys@users.noreply.github.com> Date: Mon, 12 Aug 2024 21:06:42 +0800 Subject: [PATCH 09/11] refactor: revamp rpc api `starkNet_signMessage` (#312) * chore: revamp rpc sign message * chore: updata superstruct * chore: update signMessage test * chore: refactor `showAccountRequireUpgradeOrDeployModal ` * chore: update lint style * chore: remove un used test * chore: update test folder * fix: correct typo in file path --------- Co-authored-by: Florin Dzeladini --- .../__tests__/fixture/typedDataExample.json | 35 +++ packages/starknet-snap/src/index.ts | 5 +- .../starknet-snap/src/signMessage.test.ts | 167 ++++++++++++ packages/starknet-snap/src/signMessage.ts | 175 +++++++----- packages/starknet-snap/src/utils/index.ts | 8 + .../src/utils/superstruct.test.ts | 108 +++++++- .../starknet-snap/src/utils/superstruct.ts | 70 ++++- .../test/src/signMessage.test.ts | 255 ------------------ 8 files changed, 499 insertions(+), 324 deletions(-) create mode 100644 packages/starknet-snap/src/__tests__/fixture/typedDataExample.json create mode 100644 packages/starknet-snap/src/signMessage.test.ts create mode 100644 packages/starknet-snap/src/utils/index.ts delete mode 100644 packages/starknet-snap/test/src/signMessage.test.ts diff --git a/packages/starknet-snap/src/__tests__/fixture/typedDataExample.json b/packages/starknet-snap/src/__tests__/fixture/typedDataExample.json new file mode 100644 index 00000000..d8bb55b9 --- /dev/null +++ b/packages/starknet-snap/src/__tests__/fixture/typedDataExample.json @@ -0,0 +1,35 @@ +{ + "types": { + "StarkNetDomain": [ + { "name": "name", "type": "felt" }, + { "name": "version", "type": "felt" }, + { "name": "chainId", "type": "felt" } + ], + "Person": [ + { "name": "name", "type": "felt" }, + { "name": "wallet", "type": "felt" } + ], + "Mail": [ + { "name": "from", "type": "Person" }, + { "name": "to", "type": "Person" }, + { "name": "contents", "type": "felt" } + ] + }, + "primaryType": "Mail", + "domain": { + "name": "Starknet Mail", + "version": "1", + "chainId": 1 + }, + "message": { + "from": { + "name": "Cow", + "wallet": "0xCD2a3d9F938E13CD947Ec05AbC7FE734Df8DD826" + }, + "to": { + "name": "Bob", + "wallet": "0xbBbBBBBbbBBBbbbBbbBbbbbBBbBbbbbBbBbbBBbB" + }, + "contents": "Hello, Bob!" + } +} diff --git a/packages/starknet-snap/src/index.ts b/packages/starknet-snap/src/index.ts index dfaa4e4e..af29e6d8 100644 --- a/packages/starknet-snap/src/index.ts +++ b/packages/starknet-snap/src/index.ts @@ -40,6 +40,7 @@ import { recoverAccounts } from './recoverAccounts'; import { sendTransaction } from './sendTransaction'; import { signDeclareTransaction } from './signDeclareTransaction'; import { signDeployAccountTransaction } from './signDeployAccountTransaction'; +import type { SignMessageParams } from './signMessage'; import { signMessage } from './signMessage'; import { signTransaction } from './signTransaction'; import { switchNetwork } from './switchNetwork'; @@ -170,9 +171,9 @@ export const onRpcRequest: OnRpcRequestHandler = async ({ request }) => { ); case 'starkNet_signMessage': - apiParams.keyDeriver = await getAddressKeyDeriver(snap); return await signMessage( - apiParams as unknown as ApiParamsWithKeyDeriver, + apiParams.requestParams as unknown as SignMessageParams, + state, ); case 'starkNet_signTransaction': diff --git a/packages/starknet-snap/src/signMessage.test.ts b/packages/starknet-snap/src/signMessage.test.ts new file mode 100644 index 00000000..5810620d --- /dev/null +++ b/packages/starknet-snap/src/signMessage.test.ts @@ -0,0 +1,167 @@ +import { + InvalidParamsError, + UserRejectedRequestError, +} from '@metamask/snaps-sdk'; +import { constants } from 'starknet'; + +import type { StarknetAccount } from '../test/utils'; +import { generateAccounts } from '../test/utils'; +import typedDataExample from './__tests__/fixture/typedDataExample.json'; +import type { SignMessageParams } from './signMessage'; +import { signMessage } from './signMessage'; +import type { SnapState } from './types/snapState'; +import { toJson } from './utils'; +import { STARKNET_SEPOLIA_TESTNET_NETWORK } from './utils/constants'; +import * as snapHelper from './utils/snap'; +import * as snapUtils from './utils/snapUtils'; +import * as starknetUtils from './utils/starknetUtils'; + +jest.mock('./utils/snap'); +jest.mock('./utils/logger'); + +describe('signMessage', function () { + const state: SnapState = { + accContracts: [], + erc20Tokens: [], + networks: [STARKNET_SEPOLIA_TESTNET_NETWORK], + transactions: [], + }; + + const mockAccount = async (network: constants.StarknetChainId) => { + const accounts = await generateAccounts(network, 1); + return accounts[0]; + }; + + const prepareSignMessageMock = async (account: StarknetAccount) => { + const verifyIfAccountNeedUpgradeOrDeploySpy = jest.spyOn( + snapUtils, + 'verifyIfAccountNeedUpgradeOrDeploy', + ); + const getKeysFromAddressSpy = jest.spyOn( + starknetUtils, + 'getKeysFromAddress', + ); + const confirmDialogSpy = jest.spyOn(snapHelper, 'confirmDialog'); + + getKeysFromAddressSpy.mockResolvedValue({ + privateKey: account.privateKey, + publicKey: account.publicKey, + addressIndex: account.addressIndex, + derivationPath: account.derivationPath as unknown as any, + }); + + verifyIfAccountNeedUpgradeOrDeploySpy.mockReturnThis(); + confirmDialogSpy.mockResolvedValue(true); + + return { + getKeysFromAddressSpy, + verifyIfAccountNeedUpgradeOrDeploySpy, + confirmDialogSpy, + }; + }; + + it('signs message correctly', async function () { + const account = await mockAccount(constants.StarknetChainId.SN_SEPOLIA); + + await prepareSignMessageMock(account); + + const expectedResult = await starknetUtils.signMessage( + account.privateKey, + typedDataExample, + account.address, + ); + + const request = { + chainId: constants.StarknetChainId.SN_SEPOLIA, + signerAddress: account.address, + typedDataMessage: typedDataExample, + }; + const result = await signMessage(request, state); + + expect(result).toStrictEqual(expectedResult); + }); + + it('renders confirmation dialog', async function () { + const account = await mockAccount(constants.StarknetChainId.SN_SEPOLIA); + + const { confirmDialogSpy } = await prepareSignMessageMock(account); + + const request = { + chainId: constants.StarknetChainId.SN_SEPOLIA, + signerAddress: account.address, + typedDataMessage: typedDataExample, + enableAuthorize: true, + }; + + await signMessage(request, state); + + const calls = confirmDialogSpy.mock.calls[0][0]; + expect(calls).toStrictEqual([ + { type: 'heading', value: 'Do you want to sign this message?' }, + { + type: 'row', + label: 'Message', + value: { + value: toJson(typedDataExample), + markdown: false, + type: 'text', + }, + }, + { + type: 'row', + label: 'Signer Address', + value: { + value: account.address, + markdown: false, + type: 'text', + }, + }, + ]); + }); + + it('throws `UserRejectedRequestError` if user denied the operation', async function () { + const account = await mockAccount(constants.StarknetChainId.SN_SEPOLIA); + + const { confirmDialogSpy } = await prepareSignMessageMock(account); + + confirmDialogSpy.mockResolvedValue(false); + + const request = { + chainId: constants.StarknetChainId.SN_SEPOLIA, + signerAddress: account.address, + typedDataMessage: typedDataExample, + enableAuthorize: true, + }; + + await expect(signMessage(request, state)).rejects.toThrow( + UserRejectedRequestError, + ); + }); + + it('throws `InvalidParamsError` when request parameter is not correct', async function () { + const request = { + chainId: STARKNET_SEPOLIA_TESTNET_NETWORK.chainId, + }; + await expect( + signMessage(request as unknown as SignMessageParams, state), + ).rejects.toThrow(InvalidParamsError); + }); + + it('throws `Failed to sign the message` if another error was thrown', async function () { + const account = await mockAccount(constants.StarknetChainId.SN_SEPOLIA); + + const { getKeysFromAddressSpy } = await prepareSignMessageMock(account); + + getKeysFromAddressSpy.mockRejectedValue(new Error('some error')); + + const request = { + chainId: constants.StarknetChainId.SN_SEPOLIA, + signerAddress: account.address, + typedDataMessage: typedDataExample, + }; + + await expect(signMessage(request, state)).rejects.toThrow( + 'Failed to sign the message', + ); + }); +}); diff --git a/packages/starknet-snap/src/signMessage.ts b/packages/starknet-snap/src/signMessage.ts index 4f61714a..b4b79308 100644 --- a/packages/starknet-snap/src/signMessage.ts +++ b/packages/starknet-snap/src/signMessage.ts @@ -1,92 +1,139 @@ -import { heading, panel, DialogType } from '@metamask/snaps-sdk'; - -import type { - ApiParamsWithKeyDeriver, - SignMessageRequestParams, -} from './types/snapApi'; -import { logger } from './utils/logger'; -import { toJson } from './utils/serializer'; +import type { Component } from '@metamask/snaps-sdk'; +import { + heading, + row, + text, + UserRejectedRequestError, +} from '@metamask/snaps-sdk'; +import type { Infer } from 'superstruct'; +import { array, object, string, assign } from 'superstruct'; + +import type { SnapState } from './types/snapState'; +import { + confirmDialog, + getBip44Deriver, + isSnapRpcError, + AddressStruct, + logger, + toJson, + validateRequest, + validateResponse, + TypeDataStruct, + AuthorizableStruct, + BaseRequestStruct, +} from './utils'; import { getNetworkFromChainId, - addDialogTxt, verifyIfAccountNeedUpgradeOrDeploy, } from './utils/snapUtils'; import { signMessage as signMessageUtil, getKeysFromAddress, - validateAndParseAddress, } from './utils/starknetUtils'; +export const SignMessageRequestStruct = assign( + object({ + signerAddress: AddressStruct, + typedDataMessage: TypeDataStruct, + }), + AuthorizableStruct, + BaseRequestStruct, +); + +export const SignMessageResponseStruct = array(string()); + +export type SignMessageParams = Infer; + +export type SignMessageResponse = Infer; + /** + * Signs a message. * - * @param params + * @param requestParams - The request parameters of the sign message request. + * @param state - The current state of the snap. */ -export async function signMessage(params: ApiParamsWithKeyDeriver) { +export async function signMessage( + requestParams: SignMessageParams, + // TODO: the state should be re-fetching in the rpc, rather than pass in to avoid object mutation. we will refactor it with a proper state management. + state: SnapState, +) { try { - const { state, wallet, keyDeriver, requestParams } = params; - const requestParamsObj = requestParams as SignMessageRequestParams; - const { signerAddress } = requestParamsObj; - const { typedDataMessage } = requestParamsObj; - const network = getNetworkFromChainId(state, requestParamsObj.chainId); - - logger.log( - `signMessage:\nsignerAddress: ${signerAddress}\ntypedDataMessage: ${toJson( - typedDataMessage, - )}`, - ); + validateRequest(requestParams, SignMessageRequestStruct); - try { - validateAndParseAddress(signerAddress); - } catch (error) { - throw new Error(`The given signer address is invalid: ${signerAddress}`); - } + const deriver = await getBip44Deriver(); + const { signerAddress, typedDataMessage, chainId, enableAuthorize } = + requestParams; + // TODO: getNetworkFromChainId is not needed, as the network doesnt need to fetch from state + const network = getNetworkFromChainId(state, chainId); - const { privateKey: signerPrivateKey, publicKey } = - await getKeysFromAddress(keyDeriver, network, state, signerAddress); - if (!signerAddress) { - throw new Error( - `The given signer address need to be non-empty string, got: ${toJson( - signerAddress, - )}`, - ); - } + const { privateKey, publicKey } = await getKeysFromAddress( + deriver, + network, + state, + signerAddress, + ); await verifyIfAccountNeedUpgradeOrDeploy(network, signerAddress, publicKey); - const components = []; - addDialogTxt(components, 'Message', toJson(typedDataMessage)); - addDialogTxt(components, 'Signer Address', signerAddress); - - if (requestParamsObj.enableAuthorize) { - const response = await wallet.request({ - method: 'snap_dialog', - params: { - type: DialogType.Confirmation, - content: panel([ - heading('Do you want to sign this message?'), - ...components, - ]), - }, - }); - - if (!response) { - return false; - } + if ( + // Get Starknet expected not to show the confirm dialog, therefore, `enableAuthorize` will set to false to bypass the confirmation + // TODO: enableAuthorize should set default to true + enableAuthorize && + !(await getSignMessageConsensus(typedDataMessage, signerAddress)) + ) { + throw new UserRejectedRequestError() as unknown as Error; } - const typedDataSignature = signMessageUtil( - signerPrivateKey, + const response = await signMessageUtil( + privateKey, typedDataMessage, signerAddress, ); - logger.log( - `signMessage:\ntypedDataSignature: ${toJson(typedDataSignature)}`, - ); + validateResponse(response, SignMessageResponseStruct); - return typedDataSignature; + return response; } catch (error) { - logger.error(`Problem found:`, error); - throw error; + logger.error('Failed to sign the message', error); + + if (isSnapRpcError(error)) { + throw error as unknown as Error; + } + + throw new Error('Failed to sign the message'); } } + +/** + * Gets the consensus to sign a message. + * + * @param typedDataMessage - The type data to sign. + * @param signerAddress - The address of the signer. + */ +export async function getSignMessageConsensus( + typedDataMessage: Infer, + signerAddress: string, +) { + const components: Component[] = []; + components.push(heading('Do you want to sign this message?')); + components.push( + row( + 'Message', + text({ + value: toJson(typedDataMessage), + markdown: false, + }), + ), + ); + components.push( + row( + 'Signer Address', + text({ + value: signerAddress, + markdown: false, + }), + ), + ); + + return await confirmDialog(components); +} diff --git a/packages/starknet-snap/src/utils/index.ts b/packages/starknet-snap/src/utils/index.ts new file mode 100644 index 00000000..21bcea3b --- /dev/null +++ b/packages/starknet-snap/src/utils/index.ts @@ -0,0 +1,8 @@ +export * from './error'; +export * from './rpc'; +export * from './snap'; +export * from './serializer'; +export * from './superstruct'; +export * from './logger'; +export * from './formatterUtils'; +// TODO: add other utils diff --git a/packages/starknet-snap/src/utils/superstruct.test.ts b/packages/starknet-snap/src/utils/superstruct.test.ts index 2e183aae..d47cf77c 100644 --- a/packages/starknet-snap/src/utils/superstruct.test.ts +++ b/packages/starknet-snap/src/utils/superstruct.test.ts @@ -1,6 +1,14 @@ +import { constants } from 'starknet'; import { StructError, assert } from 'superstruct'; -import { AddressStruct } from './superstruct'; +import typedDataExample from '../__tests__/fixture/typedDataExample.json'; +import { + AddressStruct, + AuthorizableStruct, + BaseRequestStruct, + ChainIdStruct, + TypeDataStruct, +} from './superstruct'; describe('AddressStruct', () => { it.each([ @@ -19,3 +27,101 @@ describe('AddressStruct', () => { expect(() => assert(address, AddressStruct)).toThrow(StructError); }); }); + +describe('ChainIdStruct', () => { + it.each(Object.values(constants.StarknetChainId))( + 'does not throw error if the chain id is valid - %s', + (chainId) => { + expect(() => assert(chainId, ChainIdStruct)).not.toThrow(); + }, + ); + + it('throws error if the chain id is invalid', () => { + expect(() => assert('0x', ChainIdStruct)).toThrow(StructError); + }); +}); + +describe('TypeDataStruct', () => { + it('does not throw error if the type data is valid', () => { + expect(() => assert(typedDataExample, TypeDataStruct)).not.toThrow(); + }); + + it('throws error if the type data is invalid', () => { + expect(() => + assert( + { + ...typedDataExample, + domain: { + name: 1, + }, + }, + TypeDataStruct, + ), + ).toThrow(StructError); + }); +}); + +describe('AuthorizableStruct', () => { + it('does not throw error if `enableAuthorize` is true', () => { + expect(() => + assert( + { + enableAuthorize: true, + }, + AuthorizableStruct, + ), + ).not.toThrow(); + }); + + it('does not throw error if `enableAuthorize` is omit', () => { + expect(() => assert({}, AuthorizableStruct)).not.toThrow(); + }); + + it('throws error if the type data is invalid', () => { + expect(() => + assert( + { + enable: true, + }, + AuthorizableStruct, + ), + ).toThrow(StructError); + }); +}); + +describe('BaseRequestStruct', () => { + it('does not throw error if data is valid', () => { + expect(() => + assert( + { + chainId: constants.StarknetChainId.SN_SEPOLIA.toString(), + debugLevel: 'ALL', + }, + BaseRequestStruct, + ), + ).not.toThrow(); + }); + + it('does not throw error if `debugLevel` is omit', () => { + expect(() => + assert( + { + chainId: constants.StarknetChainId.SN_SEPOLIA.toString(), + }, + BaseRequestStruct, + ), + ).not.toThrow(); + }); + + it('throws error if `debugLevel` is invalid', () => { + expect(() => + assert( + { + chainId: constants.StarknetChainId.SN_SEPOLIA.toString(), + debugLevel: 'Invalid-Debug-Level', + }, + BaseRequestStruct, + ), + ).toThrow(StructError); + }); +}); diff --git a/packages/starknet-snap/src/utils/superstruct.ts b/packages/starknet-snap/src/utils/superstruct.ts index 5e214e34..e19efa66 100644 --- a/packages/starknet-snap/src/utils/superstruct.ts +++ b/packages/starknet-snap/src/utils/superstruct.ts @@ -1,5 +1,20 @@ -import { validateAndParseAddress } from 'starknet'; -import { refine, string } from 'superstruct'; +import { union } from '@metamask/snaps-sdk'; +import { constants, validateAndParseAddress } from 'starknet'; +import { + assert, + boolean, + enums, + object, + optional, + record, + refine, + string, + unknown, + number, + array, +} from 'superstruct'; + +import { LogLevel } from './logger'; export const AddressStruct = refine( string(), @@ -20,3 +35,54 @@ export const AddressStruct = refine( return true; }, ); + +export const ChainIdStruct = refine( + string(), + 'ChainIdStruct', + (value: string) => { + assert(value, enums(Object.values(constants.StarknetChainId))); + return true; + }, +); + +export const TypeDataStarknetTypeStruct = union([ + object({ + name: string(), + type: enums(['enum']), + contains: string(), + }), + object({ + name: string(), + type: enums(['merkletree']), + contains: string(), + }), + object({ + name: string(), + type: string(), + }), +]); + +export const TypeDataStarknetDomainStruct = object({ + name: optional(string()), + version: optional(string()), + chainId: optional(union([string(), number()])), + revision: optional(union([string(), number()])), +}); + +export const TypeDataStruct = object({ + types: record(string(), array(TypeDataStarknetTypeStruct)), + primaryType: string(), + domain: TypeDataStarknetDomainStruct, + message: record(string(), unknown()), +}); + +export const AuthorizableStruct = object({ + // TODO: the enableAuthorize should default to true + enableAuthorize: optional(boolean()), +}); + +export const BaseRequestStruct = object({ + chainId: ChainIdStruct, + // TODO: the debug level should be set by snap rather than pass in from request. + debugLevel: optional(enums(Object.keys(LogLevel))), +}); diff --git a/packages/starknet-snap/test/src/signMessage.test.ts b/packages/starknet-snap/test/src/signMessage.test.ts deleted file mode 100644 index 8d26f526..00000000 --- a/packages/starknet-snap/test/src/signMessage.test.ts +++ /dev/null @@ -1,255 +0,0 @@ -import chai, { expect } from 'chai'; -import sinon from 'sinon'; -import sinonChai from 'sinon-chai'; -import { WalletMock } from '../wallet.mock.test'; -import { SnapState } from '../../src/types/snapState'; -import { signMessage } from '../../src/signMessage'; -import typedDataExample from '../../src/typedData/typedDataExample.json'; -import { STARKNET_SEPOLIA_TESTNET_NETWORK } from '../../src/utils/constants'; -import { - account1, - Cairo1Account1, - getBip44EntropyStub, - signature4SignMessageWithUnfoundAddress, - unfoundUserAddress, - signature4Cairo1SignMessage, -} from '../constants.test'; -import { getAddressKeyDeriver } from '../../src/utils/keyPair'; -import * as utils from '../../src/utils/starknetUtils'; -import { Mutex } from 'async-mutex'; -import { - ApiParamsWithKeyDeriver, - SignMessageRequestParams, -} from '../../src/types/snapApi'; -import { - DeployRequiredError, - UpgradeRequiredError, -} from '../../src/utils/exceptions'; - -chai.use(sinonChai); -const sandbox = sinon.createSandbox(); - -describe('Test function: signMessage', function () { - this.timeout(5000); - const walletStub = new WalletMock(); - const state: SnapState = { - accContracts: [], - erc20Tokens: [], - networks: [STARKNET_SEPOLIA_TESTNET_NETWORK], - transactions: [], - }; - let apiParams: ApiParamsWithKeyDeriver; - - const requestObject: SignMessageRequestParams = { - chainId: STARKNET_SEPOLIA_TESTNET_NETWORK.chainId, - signerAddress: account1.address, - typedDataMessage: typedDataExample, - enableAuthorize: true, - }; - - beforeEach(async function () { - walletStub.rpcStubs.snap_getBip44Entropy.callsFake(getBip44EntropyStub); - apiParams = { - state, - requestParams: requestObject, - wallet: walletStub, - saveMutex: new Mutex(), - keyDeriver: await getAddressKeyDeriver(walletStub), - }; - walletStub.rpcStubs.snap_dialog.resolves(true); - }); - - afterEach(function () { - walletStub.reset(); - sandbox.restore(); - }); - - describe('when request param validation fail', function () { - let invalidRequest = Object.assign({}, requestObject); - - afterEach(async function () { - invalidRequest = Object.assign({}, requestObject); - apiParams.requestParams = requestObject; - }); - - it('should throw an error if the signerAddress is undefined', async function () { - invalidRequest.signerAddress = undefined as unknown as string; - apiParams.requestParams = invalidRequest; - let result; - try { - result = await signMessage(apiParams); - } catch (err) { - result = err; - } finally { - expect(result).to.be.an('Error'); - } - }); - - it('should throw an error if the signerAddress is an invalid address', async function () { - invalidRequest.signerAddress = 'wrongAddress'; - apiParams.requestParams = invalidRequest; - let result; - try { - result = await signMessage(apiParams); - } catch (err) { - result = err; - } finally { - expect(result).to.be.an('Error'); - } - }); - }); - - describe('when request param validation pass', function () { - beforeEach(async function () { - apiParams.requestParams = Object.assign({}, requestObject); - }); - - afterEach(async function () { - apiParams.requestParams = Object.assign({}, requestObject); - }); - - it('skip dialog if enableAuthorize is false or omit', async function () { - sandbox - .stub(utils, 'validateAccountRequireUpgradeOrDeploy') - .resolvesThis(); - const paramsObject = apiParams.requestParams as SignMessageRequestParams; - - paramsObject.enableAuthorize = false; - await signMessage(apiParams); - expect(walletStub.rpcStubs.snap_dialog).to.have.been.callCount(0); - - paramsObject.enableAuthorize = undefined; - await signMessage(apiParams); - expect(walletStub.rpcStubs.snap_dialog).to.have.been.callCount(0); - - paramsObject.enableAuthorize = true; - }); - - describe('when require upgrade checking fail', function () { - it('should throw error', async function () { - const validateAccountRequireUpgradeOrDeployStub = sandbox - .stub(utils, 'validateAccountRequireUpgradeOrDeploy') - .throws('network error'); - let result; - try { - result = await signMessage(apiParams); - } catch (err) { - result = err; - } finally { - expect( - validateAccountRequireUpgradeOrDeployStub, - ).to.have.been.calledOnceWith( - STARKNET_SEPOLIA_TESTNET_NETWORK, - account1.address, - account1.publicKey, - ); - expect(result).to.be.an('Error'); - } - }); - }); - - describe('when account is not require upgrade', function () { - beforeEach(async function () { - apiParams.requestParams = { - ...apiParams.requestParams, - signerAddress: Cairo1Account1.address, - }; - sandbox - .stub(utils, 'validateAccountRequireUpgradeOrDeploy') - .resolvesThis(); - }); - - it('should sign a message from an user account correctly', async function () { - const result = await signMessage(apiParams); - expect(walletStub.rpcStubs.snap_dialog).to.have.been.calledOnce; - expect(walletStub.rpcStubs.snap_manageState).not.to.have.been.called; - expect(result).to.be.eql(signature4Cairo1SignMessage); - }); - - it('should sign a message from an unfound user account correctly', async function () { - const paramsObject = - apiParams.requestParams as SignMessageRequestParams; - paramsObject.signerAddress = unfoundUserAddress; - const result = await signMessage(apiParams); - expect(walletStub.rpcStubs.snap_dialog).to.have.been.calledOnce; - expect(walletStub.rpcStubs.snap_manageState).not.to.have.been.called; - expect(result).to.be.eql(signature4SignMessageWithUnfoundAddress); - }); - - it('should throw error if getKeysFromAddress failed', async function () { - sandbox.stub(utils, 'getKeysFromAddress').throws(new Error()); - let result; - try { - await signMessage(apiParams); - } catch (err) { - result = err; - } finally { - expect(result).to.be.an('Error'); - } - expect(walletStub.rpcStubs.snap_dialog).to.have.not.been.called; - expect(walletStub.rpcStubs.snap_manageState).not.to.have.been.called; - }); - - it('should return false if the user not confirmed', async function () { - walletStub.rpcStubs.snap_dialog.resolves(false); - const result = await signMessage(apiParams); - expect(walletStub.rpcStubs.snap_dialog).to.have.been.calledOnce; - expect(walletStub.rpcStubs.snap_manageState).not.to.have.been.called; - expect(result).to.be.eql(false); - }); - - it('should throw an error if the signerAddress is undefined', async function () { - const requestObject: SignMessageRequestParams = { - signerAddress: undefined as unknown as string, - typedDataMessage: typedDataExample, - }; - apiParams.requestParams = requestObject; - let result; - try { - result = await signMessage(apiParams); - } catch (err) { - result = err; - } finally { - expect(result).to.be.an('Error'); - } - }); - - it('should throw an error if the signerAddress is an invalid address', async function () { - const invalidAddress = 'wrongAddress'; - const requestObject: SignMessageRequestParams = { - signerAddress: invalidAddress, - typedDataMessage: typedDataExample, - }; - apiParams.requestParams = requestObject; - let result; - try { - result = await signMessage(apiParams); - } catch (err) { - result = err; - } finally { - expect(result).to.be.an('Error'); - } - }); - - it('should skip dialog if enableAuthorize is false', async function () { - const paramsObject = - apiParams.requestParams as SignMessageRequestParams; - paramsObject.enableAuthorize = false; - const result = await signMessage(apiParams); - expect(walletStub.rpcStubs.snap_dialog).to.have.been.callCount(0); - expect(result).to.be.eql(signature4Cairo1SignMessage); - paramsObject.enableAuthorize = true; - }); - - it('should skip dialog if enableAuthorize is omit', async function () { - const paramsObject = - apiParams.requestParams as SignMessageRequestParams; - paramsObject.enableAuthorize = undefined; - const result = await signMessage(apiParams); - expect(walletStub.rpcStubs.snap_dialog).to.have.been.callCount(0); - expect(result).to.be.eql(signature4Cairo1SignMessage); - paramsObject.enableAuthorize = true; - }); - }); - }); -}); From ea87d38e745f9ba93d9b32169dbcfe2759934bcf Mon Sep 17 00:00:00 2001 From: khanti42 Date: Tue, 13 Aug 2024 10:00:44 +0200 Subject: [PATCH 10/11] chore: display both spendable and actual balance in wallet-ui (#310) * fix: allow multiple consecutive pending transactions in estimateFee and estimateFeeBulk * fix: remove try catch block. Just check if nonce provided * test: estimateFee.test.ts should rely only on estimateFeeBulk * chore: lint + prettier * refactor: function account nounce on block naming * refactor: removed gasConsumed and gasPrice in estamiteFee * fix: nonce undefined * revert: yarn.lock and snap.manifest.json from main * fix: use BlockIdentifierEnum * fix: update packages/starknet-snap/src/utils/starknetUtils.ts Co-authored-by: Stanley Yuen <102275989+stanleyyconsensys@users.noreply.github.com> * feat: sets blockIdentifier on getProvider no need for getAccountNonceOnLatest method * chore: lint + prettier * fix: pending/latest balance * feat: support spendable balance in case of unconfirmed amount * revert: wallet-ui changes * feat: wallet-ui support spendable balance in case of unconfirmed amount * test: update getErc20TokenBalance.test.ts * feat: let consumer app decide the logic to use to display balance * chore: lint + prettier * refactor: add getTokenBalanceWithDetails and getTokenBalancesWithDetails helper * fix: address PR feedback and update implementation * fix: token-balance type * fix: address PR feedback and update implementation * fix: address PR feedback and update implementation --------- Co-authored-by: Stanley Yuen <102275989+stanleyyconsensys@users.noreply.github.com> --- .../AssetListItem/AssetListItem.view.tsx | 4 +- .../ui/organism/Header/Header.view.tsx | 6 ++- .../wallet-ui/src/services/useStarkNetSnap.ts | 42 +++++++++---------- packages/wallet-ui/src/types/index.ts | 12 ++++++ packages/wallet-ui/src/utils/utils.ts | 42 ++++++++++++++++++- 5 files changed, 78 insertions(+), 28 deletions(-) diff --git a/packages/wallet-ui/src/components/ui/molecule/AssetsList/AssetListItem/AssetListItem.view.tsx b/packages/wallet-ui/src/components/ui/molecule/AssetsList/AssetListItem/AssetListItem.view.tsx index 0fcb36f7..a7928240 100644 --- a/packages/wallet-ui/src/components/ui/molecule/AssetsList/AssetListItem/AssetListItem.view.tsx +++ b/packages/wallet-ui/src/components/ui/molecule/AssetsList/AssetListItem/AssetListItem.view.tsx @@ -11,7 +11,7 @@ import { Wrapper, } from './AssetListItem.style'; import { DoubleIcons } from 'components/ui/atom/DoubleIcons'; -import { getHumanReadableAmount } from 'utils/utils'; +import { getSpendableTotalBalance } from 'utils/utils'; interface Props extends HTMLAttributes { asset: Erc20TokenBalance; @@ -34,7 +34,7 @@ export const AssetListItemView = ({ - {getHumanReadableAmount(asset)} {asset.symbol} + {getSpendableTotalBalance(asset)} {asset.symbol} diff --git a/packages/wallet-ui/src/components/ui/organism/Header/Header.view.tsx b/packages/wallet-ui/src/components/ui/organism/Header/Header.view.tsx index 4ad17c64..5acd07d3 100644 --- a/packages/wallet-ui/src/components/ui/organism/Header/Header.view.tsx +++ b/packages/wallet-ui/src/components/ui/organism/Header/Header.view.tsx @@ -5,7 +5,7 @@ import { getAmountPrice } from 'utils/utils'; import { Button } from 'components/ui/atom/Button'; import { AssetQuantity } from 'components/ui/molecule/AssetQuantity'; import { PopIn } from 'components/ui/molecule/PopIn'; -import { getHumanReadableAmount } from 'utils/utils'; +import { getSpendableTotalBalance } from 'utils/utils'; import { Buttons, HeaderButton, Wrapper } from './Header.style'; import { ReceiveModal } from './ReceiveModal'; import { SendModal } from './SendModal'; @@ -64,7 +64,9 @@ export const HeaderView = ({ address }: Props) => { { : (acc as Account).upgradeRequired) ?? false; } - const tokenBalances = await Promise.all( - tokens.map(async (token) => { - const accountAddr = Array.isArray(acc) ? acc[0].address : acc.address; - return await getTokenBalance(token.address, accountAddr, chainId); - }), - ); + const accountAddr = Array.isArray(acc) ? acc[0].address : acc.address; - const tokenUSDPrices = await Promise.all( + // Get all tokens balance, USD value, and format them into Erc20TokenBalance type + const tokensWithBalances: Erc20TokenBalance[] = await Promise.all( tokens.map(async (token) => { - return await getAssetPriceUSD(token); + const balance = await getTokenBalance( + token.address, + accountAddr, + chainId, + ); + const usdPrice = await getAssetPriceUSD(token); + return await getTokenBalanceWithDetails(balance, token, usdPrice); }), ); - - const tokensWithBalances = tokens.map((token, index): Erc20TokenBalance => { - return addMissingPropertiesToToken( - token, - tokenBalances[index], - tokenUSDPrices[index], - ); - }); if (networks) { dispatch(setNetworks(networks)); } @@ -625,9 +619,9 @@ export const useStarkNetSnap = () => { chainId, ); const usdPrice = await getAssetPriceUSD(token); - const tokenWithBalance: Erc20TokenBalance = addMissingPropertiesToToken( - token, + const tokenWithBalance: Erc20TokenBalance = getTokenBalanceWithDetails( tokenBalance, + token, usdPrice, ); dispatch(upsertErc20TokenBalance(tokenWithBalance)); @@ -664,9 +658,9 @@ export const useStarkNetSnap = () => { chainId, ); const usdPrice = await getAssetPriceUSD(foundTokenWithBalance); - const tokenWithBalance: Erc20TokenBalance = addMissingPropertiesToToken( - foundTokenWithBalance, + const tokenWithBalance: Erc20TokenBalance = getTokenBalanceWithDetails( tokenBalance, + foundTokenWithBalance, usdPrice, ); dispatch(upsertErc20TokenBalance(tokenWithBalance)); @@ -694,10 +688,14 @@ export const useStarkNetSnap = () => { }, }, }); - return response.balanceLatest; + return response; } catch (err) { //eslint-disable-next-line no-console console.error(err); + return { + balanceLatest: '0x0', + balancePending: '0x0', + }; } }; diff --git a/packages/wallet-ui/src/types/index.ts b/packages/wallet-ui/src/types/index.ts index 4799182b..34f5562e 100644 --- a/packages/wallet-ui/src/types/index.ts +++ b/packages/wallet-ui/src/types/index.ts @@ -13,6 +13,7 @@ export type Network = Pick< export interface Erc20TokenBalance extends Types.Erc20Token { amount: BigNumber; usdPrice?: number; + spendableAmount?: BigNumber; } export type TransactionStatusOptions = | 'Received' @@ -37,7 +38,18 @@ export enum TransactionStatus { // for retrieving txn from Starknet feeder gatew REJECTED = 'REJECTED', } +export enum BalanceType { + Spendable = 'spendable', + Total = 'total', +} + export type { Erc20Token, Transaction, } from '@consensys/starknet-snap/src/types/snapState'; + +// Define the type for your token balances +export interface TokenBalance { + balancePending: BigNumber; + balanceLatest: BigNumber; +} diff --git a/packages/wallet-ui/src/utils/utils.ts b/packages/wallet-ui/src/utils/utils.ts index eb431999..319b4fd6 100644 --- a/packages/wallet-ui/src/utils/utils.ts +++ b/packages/wallet-ui/src/utils/utils.ts @@ -8,7 +8,7 @@ import { TIMEOUT_DURATION, MIN_ACC_CONTRACT_VERSION, } from './constants'; -import { Erc20Token, Erc20TokenBalance } from 'types'; +import { Erc20Token, Erc20TokenBalance, TokenBalance } from 'types'; import { constants } from 'starknet'; export const shortenAddress = (address: string, num = 3) => { @@ -45,11 +45,15 @@ export const isValidAddress = (toCheck: string) => { export const addMissingPropertiesToToken = ( token: Erc20Token, balance?: string, + balanceSpendable?: string, usdPrice?: number, ): Erc20TokenBalance => { return { ...token, amount: ethers.BigNumber.from(balance ? balance : '0x0'), + spendableAmount: ethers.BigNumber.from( + balanceSpendable ? balanceSpendable : '0x0', + ), usdPrice: usdPrice, }; }; @@ -57,7 +61,7 @@ export const addMissingPropertiesToToken = ( export const getHumanReadableAmount = ( asset: Erc20TokenBalance, assetAmount?: string, -) => { +): string => { const amountStr = assetAmount ? assetAmount : ethers.utils.formatUnits(asset.amount, asset.decimals); @@ -79,6 +83,24 @@ export const getHumanReadableAmount = ( return amountStr.substring(0, indexDecimal + firstNonZeroIndex + 3); }; +export const getSpendableTotalBalance = (asset: Erc20TokenBalance): string => { + if (asset.spendableAmount === undefined) { + throw new Error('Spendable amount can not be undefined'); + } + const spendableAmount = getHumanReadableAmount( + asset, + asset.spendableAmount.toString(), + ); + + if (asset.spendableAmount === asset.amount) { + return `${spendableAmount}`; + } + + const totalAmount = getHumanReadableAmount(asset, asset.amount.toString()); + + return `${spendableAmount} (${totalAmount})`; +}; + export const getMaxDecimalsReadable = ( asset: Erc20TokenBalance, assetAmount?: string, @@ -218,3 +240,19 @@ export const shortenDomain = (domain: string, maxLength = 18) => { const shortenedPartLength = maxLength - ellipsis.length; return `${domain.substring(0, shortenedPartLength)}${ellipsis}`; }; + +export function getTokenBalanceWithDetails( + tokenBalance: TokenBalance, + token: Erc20Token, + tokenUSDPrice?: number, +): Erc20TokenBalance { + const { balancePending, balanceLatest } = tokenBalance; + const spendableBalance = + balancePending < balanceLatest ? balancePending : balanceLatest; + return addMissingPropertiesToToken( + token, + balanceLatest.toString(), + spendableBalance.toString(), + tokenUSDPrice, + ); +} From 458fa8a7c4701764582ba0b99bc800168b30c6b5 Mon Sep 17 00:00:00 2001 From: khanti42 Date: Tue, 13 Aug 2024 13:05:18 +0200 Subject: [PATCH 11/11] fix: getSpendableTotalBalance (#316) --- packages/wallet-ui/src/utils/utils.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/wallet-ui/src/utils/utils.ts b/packages/wallet-ui/src/utils/utils.ts index 319b4fd6..4022c692 100644 --- a/packages/wallet-ui/src/utils/utils.ts +++ b/packages/wallet-ui/src/utils/utils.ts @@ -89,14 +89,13 @@ export const getSpendableTotalBalance = (asset: Erc20TokenBalance): string => { } const spendableAmount = getHumanReadableAmount( asset, - asset.spendableAmount.toString(), + ethers.utils.formatUnits(asset.spendableAmount, asset.decimals), ); - - if (asset.spendableAmount === asset.amount) { + if (asset.spendableAmount.eq(asset.amount)) { return `${spendableAmount}`; } - const totalAmount = getHumanReadableAmount(asset, asset.amount.toString()); + const totalAmount = getHumanReadableAmount(asset); return `${spendableAmount} (${totalAmount})`; };