From c98f2c74f5983736dedb960a5bf2ce57d9a3f99c Mon Sep 17 00:00:00 2001 From: khanti42 Date: Mon, 17 Jun 2024 13:02:51 +0200 Subject: [PATCH] fix: ensure account deployment for executTxn in get-starknet (#250) * feat: add webpack.config.dev.js for local serving of remoteEntry.js * chore: lint and optional env variable for SNAP_ID * fix: update get-starknet/webpack-dev-server dep to address security vulnerability * fix: remoteEntry.js serving done on static file from dist/webpack folder * chore: added .env.sample example file * feat: optional PUBLIC_PATH env variable to produce local remoteEntry.js * chore: updated README.md * fix: env variable for local environment resolved at build time * fix: check and deploy on executeTxn * fix: isAccountDeployed requires publicKey * feat: add waiting mode for create account * feat: wait for tx in executTxn if account not deployed * test: adapted executeTxn tests to account for accountDeployment feature * style: prettier * style: lint * fix: waitForDeployment * test: fix executeTxn * fix: lint * fix: add waitForTransaction to maximize chance of success * fix: snap.manifest.json endowment:rpc * fix: uses estimateFeeBulk directly * style: remove console.log * test: executeTxn tests with account deployed or not * test: improve coverage and readability * test: rollback to match MM style --------- Co-authored-by: stanleyyuen <102275989+stanleyyconsensys@users.noreply.github.com> --- packages/starknet-snap/snap.manifest.json | 2 +- packages/starknet-snap/src/executeTxn.ts | 102 ++++++++++++-- packages/starknet-snap/test/constants.test.ts | 6 +- .../starknet-snap/test/src/executeTxn.test.ts | 133 +++++++++++++++++- yarn.lock | 4 +- 5 files changed, 219 insertions(+), 28 deletions(-) diff --git a/packages/starknet-snap/snap.manifest.json b/packages/starknet-snap/snap.manifest.json index f5285026..5d11164d 100644 --- a/packages/starknet-snap/snap.manifest.json +++ b/packages/starknet-snap/snap.manifest.json @@ -7,7 +7,7 @@ "url": "https://github.com/ConsenSys/starknet-snap.git" }, "source": { - "shasum": "vVFyA2r3PuADhGuwrnvREDPCLnSidbQzT3lxf9uzvOA=", + "shasum": "n98JYVZFBDx4LYHqusYAftu/diScJU/ePb8FTYHZXsU=", "location": { "npm": { "filePath": "dist/bundle.js", diff --git a/packages/starknet-snap/src/executeTxn.ts b/packages/starknet-snap/src/executeTxn.ts index 1b848153..6b52c5a6 100644 --- a/packages/starknet-snap/src/executeTxn.ts +++ b/packages/starknet-snap/src/executeTxn.ts @@ -1,28 +1,96 @@ -import { toJson } from './utils/serializer'; -import { getNetworkFromChainId, getTxnSnapTxt } from './utils/snapUtils'; -import { getKeysFromAddress, executeTxn as executeTxnUtil } from './utils/starknetUtils'; +import { Invocations, TransactionType } from 'starknet'; +import { getNetworkFromChainId, getTxnSnapTxt, addDialogTxt } from './utils/snapUtils'; +import { + getKeysFromAddress, + executeTxn as executeTxnUtil, + isAccountDeployed, + estimateFeeBulk, + getAccContractAddressAndCallData, + addFeesFromAllTransactions, +} from './utils/starknetUtils'; import { ApiParams, ExecuteTxnRequestParams } from './types/snapApi'; +import { createAccount } from './createAccount'; import { DialogType } from '@metamask/rpc-methods'; -import { heading, panel } from '@metamask/snaps-sdk'; +import { heading, panel, divider } from '@metamask/snaps-sdk'; import { logger } from './utils/logger'; +import { PROXY_CONTRACT_HASH } from './utils/constants'; export async function executeTxn(params: ApiParams) { try { const { state, keyDeriver, requestParams, wallet } = params; const requestParamsObj = requestParams as ExecuteTxnRequestParams; - - logger.log(`executeTxn params: ${toJson(requestParamsObj, 2)}}`); - const senderAddress = requestParamsObj.senderAddress; const network = getNetworkFromChainId(state, requestParamsObj.chainId); - const { privateKey: senderPrivateKey } = await getKeysFromAddress(keyDeriver, network, state, senderAddress); + const { + privateKey: senderPrivateKey, + publicKey, + addressIndex, + } = await getKeysFromAddress(keyDeriver, network, state, senderAddress); - const snapComponents = getTxnSnapTxt( - senderAddress, + const txnInvocationArray = Array.isArray(requestParamsObj.txnInvocation) + ? requestParamsObj.txnInvocation + : [requestParamsObj.txnInvocation]; + const bulkTransactions: Invocations = txnInvocationArray.map((ele) => ({ + type: TransactionType.INVOKE, + payload: ele, + })); + const accountDeployed = await isAccountDeployed(network, publicKey); + if (!accountDeployed) { + const { callData } = getAccContractAddressAndCallData(publicKey); + const deployAccountpayload = { + classHash: PROXY_CONTRACT_HASH, + contractAddress: senderAddress, + constructorCalldata: callData, + addressSalt: publicKey, + }; + + bulkTransactions.unshift({ + type: TransactionType.DEPLOY_ACCOUNT, + payload: deployAccountpayload, + }); + } + + const fees = await estimateFeeBulk( network, - requestParamsObj.txnInvocation, - requestParamsObj.abis, - requestParamsObj.invocationsDetails, + senderAddress, + senderPrivateKey, + bulkTransactions, + requestParamsObj.invocationsDetails ? requestParamsObj.invocationsDetails : undefined, + ); + const estimateFeeResp = addFeesFromAllTransactions(fees); + + const maxFee = estimateFeeResp.suggestedMaxFee.toString(10); + logger.log(`MaxFee: ${maxFee}`); + + let snapComponents = []; + let createAccountApiParams: ApiParams; + if (!accountDeployed) { + snapComponents.push(heading(`The account will be deployed`)); + addDialogTxt(snapComponents, 'Address', senderAddress); + addDialogTxt(snapComponents, 'Public Key', publicKey); + addDialogTxt(snapComponents, 'Address Index', addressIndex.toString()); + snapComponents.push(divider()); + createAccountApiParams = { + state, + wallet: params.wallet, + saveMutex: params.saveMutex, + keyDeriver, + requestParams: { + addressIndex, + deploy: true, + chainId: requestParamsObj.chainId, + }, + } as ApiParams; + } + + snapComponents = snapComponents.concat( + getTxnSnapTxt( + senderAddress, + network, + requestParamsObj.txnInvocation, + requestParamsObj.abis, + requestParamsObj.invocationsDetails, + ), ); const response = await wallet.request({ @@ -32,16 +100,20 @@ export async function executeTxn(params: ApiParams) { content: panel([heading('Do you want to sign this transaction(s)?'), ...snapComponents]), }, }); - if (!response) return false; + if (!accountDeployed) { + await createAccount(createAccountApiParams, true, true); + } + const nonceSendTransaction = accountDeployed ? undefined : 1; + return await executeTxnUtil( network, senderAddress, senderPrivateKey, requestParamsObj.txnInvocation, requestParamsObj.abis, - requestParamsObj.invocationsDetails, + { maxFee, nonce: nonceSendTransaction }, ); } catch (err) { logger.error(`Problem found: ${err}`); diff --git a/packages/starknet-snap/test/constants.test.ts b/packages/starknet-snap/test/constants.test.ts index cb406abb..a5cdf30e 100644 --- a/packages/starknet-snap/test/constants.test.ts +++ b/packages/starknet-snap/test/constants.test.ts @@ -99,8 +99,8 @@ export const signature4SignMessageWithUnfoundAddress = [ ]; export const signature3 = [ - '607985888254383597713678062496326274484078117880260232069103402822721981909', - '2165861203006010568296813740808310355035386348183576985427784501900302491063', + '1256731126561482761572869006981488976832873819808625500210956924080068064716', + '1621003388569060642997584706464972362757030330535363446245801982840065859049', ]; // Derived from seed phrase: "dog simple gown ankle release anger local pulp rose river approve miracle" export const bip44Entropy: JsonBIP44CoinTypeNode = { @@ -135,7 +135,7 @@ export const createAccountProxyMainnetResp2 = { export const createAccountProxyResp = { transaction_hash: '0x3b690b4c9dd639881a46f6a344ee90254562175ed7a7f5a028f69b8c32ccb47', - contract_address: '0x4b36a2b0a1e9d2af3416914798de776e37d9e0ab9a50d2dec30485dca64bb8', + contract_address: '0x04882a372da3dfe1c53170ad75893832469bf87b62b13e84662565c4a88f25cd', }; export const createAccountFailedProxyResp = { diff --git a/packages/starknet-snap/test/src/executeTxn.test.ts b/packages/starknet-snap/test/src/executeTxn.test.ts index c2768cc4..9c579be2 100644 --- a/packages/starknet-snap/test/src/executeTxn.test.ts +++ b/packages/starknet-snap/test/src/executeTxn.test.ts @@ -6,10 +6,18 @@ import * as utils from '../../src/utils/starknetUtils'; import { executeTxn } from '../../src/executeTxn'; import { SnapState } from '../../src/types/snapState'; import { STARKNET_MAINNET_NETWORK, STARKNET_SEPOLIA_TESTNET_NETWORK } from '../../src/utils/constants'; -import { createAccountProxyTxn, getBip44EntropyStub, account1 } from '../constants.test'; +import { + createAccountProxyTxn, + estimateDeployFeeResp, + getBip44EntropyStub, + account1, + estimateFeeResp, +} from '../constants.test'; +import * as createAccountUtils from '../../src/createAccount'; import { getAddressKeyDeriver } from '../../src/utils/keyPair'; import { Mutex } from 'async-mutex'; import { ApiParams, ExecuteTxnRequestParams } from '../../src/types/snapApi'; +import { GetTransactionReceiptResponse } from 'starknet'; chai.use(sinonChai); const sandbox = sinon.createSandbox(); @@ -18,7 +26,7 @@ describe('Test function: executeTxn', function () { this.timeout(10000); const walletStub = new WalletMock(); const state: SnapState = { - accContracts: [], + accContracts: [account1], erc20Tokens: [], networks: [STARKNET_MAINNET_NETWORK, STARKNET_SEPOLIA_TESTNET_NETWORK], transactions: [], @@ -47,9 +55,22 @@ describe('Test function: executeTxn', function () { walletStub.rpcStubs.snap_getBip44Entropy.callsFake(getBip44EntropyStub); apiParams.keyDeriver = await getAddressKeyDeriver(walletStub); apiParams.requestParams = requestObject; + sandbox.stub(utils, 'estimateFeeBulk').callsFake(async () => { + return [estimateFeeResp]; + }); + sandbox.stub(utils, 'estimateFee').callsFake(async () => { + return estimateFeeResp; + }); + sandbox.stub(utils, 'estimateAccountDeployFee').callsFake(async () => { + return estimateDeployFeeResp; + }); + sandbox.stub(utils, 'getSigner').callsFake(async () => { + return account1.publicKey; + }); sandbox.useFakeTimers(createAccountProxyTxn.timestamp); walletStub.rpcStubs.snap_dialog.resolves(true); walletStub.rpcStubs.snap_manageState.resolves(state); + sandbox.stub(utils, 'waitForTransaction').resolves({} as unknown as GetTransactionReceiptResponse); }); afterEach(function () { @@ -58,7 +79,99 @@ describe('Test function: executeTxn', function () { apiParams.requestParams = requestObject; }); - it('should executeTxn correctly', async function () { + it('should executeTxn correctly and deploy an account', async function () { + sandbox.stub(utils, 'isAccountDeployed').resolves(false); + const createAccountStub = sandbox.stub(createAccountUtils, 'createAccount').resolvesThis(); + const stub = sandbox.stub(utils, 'executeTxn').resolves({ + transaction_hash: 'transaction_hash', + }); + const result = await executeTxn(apiParams); + const { privateKey } = await utils.getKeysFromAddress( + apiParams.keyDeriver, + STARKNET_MAINNET_NETWORK, + state, + account1.address, + ); + expect(result).to.eql({ + transaction_hash: 'transaction_hash', + }); + expect(stub).to.have.been.calledOnceWith( + STARKNET_MAINNET_NETWORK, + account1.address, + privateKey, + { + entrypoint: 'transfer', + calldata: ['0', '0', '0'], + contractAddress: createAccountProxyTxn.contractAddress, + }, + undefined, + { maxFee: '22702500105945', nonce: 1 }, + ); + expect(createAccountStub).to.have.been.calledOnceWith(sinon.match.any, true, true); + }); + + it('should executeTxn multiple and deploy an account', async function () { + sandbox.stub(utils, 'isAccountDeployed').resolves(false); + const createAccountStub = sandbox.stub(createAccountUtils, 'createAccount').resolvesThis(); + const stub = sandbox.stub(utils, 'executeTxn').resolves({ + transaction_hash: 'transaction_hash', + }); + apiParams.requestParams = { + chainId: STARKNET_MAINNET_NETWORK.chainId, + senderAddress: account1.address, + txnInvocation: [ + { + entrypoint: 'transfer', + calldata: ['0', '0', '0'], + contractAddress: createAccountProxyTxn.contractAddress, + }, + { + entrypoint: 'transfer2', + calldata: ['0', '0', '0'], + contractAddress: createAccountProxyTxn.contractAddress, + }, + ], + invocationsDetails: { + maxFee: 100, + }, + }; + const result = await executeTxn(apiParams); + const { privateKey } = await utils.getKeysFromAddress( + apiParams.keyDeriver, + STARKNET_MAINNET_NETWORK, + state, + account1.address, + ); + + expect(result).to.eql({ + transaction_hash: 'transaction_hash', + }); + expect(stub).to.have.been.calledOnce; + expect(stub).to.have.been.calledWith( + STARKNET_MAINNET_NETWORK, + account1.address, + privateKey, + [ + { + entrypoint: 'transfer', + calldata: ['0', '0', '0'], + contractAddress: createAccountProxyTxn.contractAddress, + }, + { + entrypoint: 'transfer2', + calldata: ['0', '0', '0'], + contractAddress: createAccountProxyTxn.contractAddress, + }, + ], + undefined, + { maxFee: '22702500105945', nonce: 1 }, + ); + expect(createAccountStub).to.have.been.calledOnceWith(sinon.match.any, true, true); + }); + + it('should executeTxn and not deploy an account', async function () { + const createAccountStub = sandbox.stub(createAccountUtils, 'createAccount'); + sandbox.stub(utils, 'isAccountDeployed').resolves(true); const stub = sandbox.stub(utils, 'executeTxn').resolves({ transaction_hash: 'transaction_hash', }); @@ -84,11 +197,14 @@ describe('Test function: executeTxn', function () { contractAddress: createAccountProxyTxn.contractAddress, }, undefined, - { maxFee: 100 }, + { maxFee: '22702500105945', nonce: undefined }, ); + expect(createAccountStub).to.not.have.been.called; }); - it('should executeTxn multiple', async function () { + it('should executeTxn multiple and not deploy an account', async function () { + const createAccountStub = sandbox.stub(createAccountUtils, 'createAccount'); + sandbox.stub(utils, 'isAccountDeployed').resolves(true); const stub = sandbox.stub(utils, 'executeTxn').resolves({ transaction_hash: 'transaction_hash', }); @@ -140,11 +256,13 @@ describe('Test function: executeTxn', function () { }, ], undefined, - { maxFee: 100 }, + { maxFee: '22702500105945', nonce: undefined }, ); + expect(createAccountStub).to.not.have.been.called; }); it('should throw error if executeTxn fail', async function () { + sandbox.stub(utils, 'isAccountDeployed').resolves(true); const stub = sandbox.stub(utils, 'executeTxn').rejects('error'); const { privateKey } = await utils.getKeysFromAddress( apiParams.keyDeriver, @@ -170,7 +288,7 @@ describe('Test function: executeTxn', function () { contractAddress: createAccountProxyTxn.contractAddress, }, undefined, - { maxFee: 100 }, + { maxFee: '22702500105945', nonce: undefined }, ); } }); @@ -180,6 +298,7 @@ describe('Test function: executeTxn', function () { const stub = sandbox.stub(utils, 'executeTxn').resolves({ transaction_hash: 'transaction_hash', }); + const result = await executeTxn(apiParams); expect(result).to.equal(false); expect(stub).to.have.been.not.called; diff --git a/yarn.lock b/yarn.lock index 27a2cd54..a67a6192 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3248,7 +3248,7 @@ __metadata: "@consensys/starknet-snap@file:../starknet-snap::locator=wallet-ui%40workspace%3Apackages%2Fwallet-ui": version: 2.7.0 - resolution: "@consensys/starknet-snap@file:../starknet-snap#../starknet-snap::hash=099522&locator=wallet-ui%40workspace%3Apackages%2Fwallet-ui" + resolution: "@consensys/starknet-snap@file:../starknet-snap#../starknet-snap::hash=da052f&locator=wallet-ui%40workspace%3Apackages%2Fwallet-ui" dependencies: "@metamask/snaps-sdk": 3.0.1 async-mutex: ^0.3.2 @@ -3257,7 +3257,7 @@ __metadata: ethers: ^5.5.1 starknet: 6.7.0 starknet_v4.22.0: "npm:starknet@4.22.0" - checksum: a53e3b5b9b53448473b4b362ebdda3d1ecb97321baa7db52393c0b43e9454ccb81d641bd2a86956896a591c23f237566d4c131034806c2609de79f9f1a12ba41 + checksum: d9b1b74d87bd0062457119a8a47fde7ca7914e5e235d2bda51b6f99f74f6a621d6134845518e0c4c969cc84eeaca24bc0cf4ee91245a69c98dc4e4a0a955caab languageName: node linkType: hard