Skip to content

Commit

Permalink
fix: remove get balance from createAccount (#272)
Browse files Browse the repository at this point in the history 
* fix: remove get balance from createAccount

* fix: remove unuse failure reason
  • Loading branch information
@stanleyyconsensys
stanleyyconsensys authored Jul 2, 2024
1 parent 762b536 commit 02b92f9
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 196 deletions.
57 changes: 4 additions & 53 deletions packages/starknet-snap/src/createAccount.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,9 @@ import {
getKeysFromAddressIndex,
getAccContractAddressAndCallData,
deployAccount,
getBalance,
estimateAccountDeployFee,
isAccountDeployed,
waitForTransaction,
} from './utils/starknetUtils';
import {
getEtherErc20Token,
getNetworkFromChainId,
getValidNumber,
upsertAccount,
Expand All @@ -18,10 +14,7 @@ import {
} from './utils/snapUtils';
import { AccContract, VoyagerTransactionType, Transaction, TransactionStatus } from './types/snapState';
import { ApiParams, CreateAccountRequestParams } from './types/snapApi';
import { EstimateFee, num } from 'starknet';
import { ethers } from 'ethers';
import { DialogType } from '@metamask/rpc-methods';
import { heading, panel, text } from '@metamask/snaps-sdk';
import { heading, panel, text, DialogType } from '@metamask/snaps-sdk';
import { logger } from './utils/logger';

/**
Expand Down Expand Up @@ -51,9 +44,6 @@ export async function createAccount(params: ApiParams, silentMode = false, waitM
`createAccount:\ncontractAddress = ${contractAddress}\npublicKey = ${publicKey}\naddressIndex = ${addressIndexInUsed}`,
);

let failureReason = '';
let estimateDeployFee: EstimateFee;

if (deploy) {
if (!silentMode) {
const components = [];
Expand All @@ -78,47 +68,8 @@ export async function createAccount(params: ApiParams, silentMode = false, waitM
};
}

const signerAssigned = await isAccountDeployed(network, contractAddress);

if (!signerAssigned) {
try {
const balance = await getBalance(
getEtherErc20Token(state, network.chainId)?.address,
num.toBigInt(contractAddress).toString(10),
network,
);
logger.log(`createAccount:\ngetBalanceResp: ${balance}`);
estimateDeployFee = await estimateAccountDeployFee(
network,
contractAddress,
contractCallData,
publicKey,
privateKey,
);
logger.log(`createAccount:\nestimateDeployFee: ${toJson(estimateDeployFee)}`);
if (Number(balance) < Number(estimateDeployFee.suggestedMaxFee)) {
const gasFeeStr = ethers.utils.formatUnits(estimateDeployFee.suggestedMaxFee.toString(10), 18);
const gasFeeFloat = parseFloat(gasFeeStr).toFixed(6); // 6 decimal places for ether
const gasFeeInEther = Number(gasFeeFloat) === 0 ? '0.000001' : gasFeeFloat;
failureReason = `The account address needs to hold at least ${gasFeeInEther} ETH for deploy fee`;
}
} catch (err) {
failureReason = 'The account address ETH balance cannot be read';
logger.error(`createAccount: failed to read the ETH balance of ${contractAddress}: ${err}`);
}
}

const deployResp = await deployAccount(
network,
contractAddress,
contractCallData,
publicKey,
privateKey,
undefined,
{
maxFee: estimateDeployFee?.suggestedMaxFee,
},
);
// Deploy account will auto estimate the fee from the network if not provided
const deployResp = await deployAccount(network, contractAddress, contractCallData, publicKey, privateKey);

if (deployResp.contract_address && deployResp.transaction_hash) {
const userAccount: AccContract = {
Expand All @@ -144,7 +95,7 @@ export async function createAccount(params: ApiParams, silentMode = false, waitM
finalityStatus: TransactionStatus.RECEIVED,
executionStatus: TransactionStatus.RECEIVED,
status: '',
failureReason,
failureReason: '',
eventIds: [],
timestamp: Math.floor(Date.now() / 1000),
};
Expand Down
145 changes: 5 additions & 140 deletions packages/starknet-snap/test/src/createAccount.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ describe('Test function: createAccount', function () {
});

it('should only return derived address without sending deploy txn correctly in mainnet if deploy is false', async function () {
sandbox.stub(utils, 'isAccountDeployed').resolves(false);
const requestObject: CreateAccountRequestParams = {
chainId: STARKNET_MAINNET_NETWORK.chainId,
};
Expand All @@ -94,9 +93,7 @@ describe('Test function: createAccount', function () {
sandbox.stub(utils, 'callContract').callsFake(async () => {
return getBalanceResp;
});
sandbox.stub(utils, 'estimateAccountDeployFee').callsFake(async () => {
return estimateDeployFeeResp;
});

const requestObject: CreateAccountRequestParams = {
chainId: STARKNET_MAINNET_NETWORK.chainId,
deploy: true,
Expand All @@ -111,13 +108,7 @@ describe('Test function: createAccount', function () {
sandbox.stub(utils, 'deployAccount').callsFake(async () => {
return createAccountProxyMainnetResp;
});
sandbox.stub(utils, 'isAccountDeployed').resolves(false);
sandbox.stub(utils, 'getBalance').callsFake(async () => {
return getBalanceResp[0];
});
sandbox.stub(utils, 'estimateAccountDeployFee').callsFake(async () => {
return estimateDeployFeeResp;
});

const requestObject: CreateAccountRequestParams = {
chainId: STARKNET_MAINNET_NETWORK.chainId,
deploy: true,
Expand Down Expand Up @@ -146,13 +137,7 @@ describe('Test function: createAccount', function () {
sandbox.stub(utils, 'deployAccount').callsFake(async () => {
return createAccountProxyMainnetResp2;
});
sandbox.stub(utils, 'isAccountDeployed').resolves(false);
sandbox.stub(utils, 'getBalance').callsFake(async () => {
return getBalanceResp[0];
});
sandbox.stub(utils, 'estimateAccountDeployFee').callsFake(async () => {
return estimateDeployFeeResp;
});

const requestObject: CreateAccountRequestParams = {
chainId: STARKNET_MAINNET_NETWORK.chainId,
addressIndex: 1,
Expand Down Expand Up @@ -181,13 +166,7 @@ describe('Test function: createAccount', function () {
sandbox.stub(utils, 'deployAccount').callsFake(async () => {
return createAccountProxyResp;
});
sandbox.stub(utils, 'isAccountDeployed').resolves(false);
sandbox.stub(utils, 'getBalance').callsFake(async () => {
return getBalanceResp[0];
});
sandbox.stub(utils, 'estimateAccountDeployFee').callsFake(async () => {
return estimateDeployFeeResp;
});

const requestObject: CreateAccountRequestParams = { deploy: true };
apiParams.requestParams = requestObject;
const result = await createAccount(apiParams, true);
Expand Down Expand Up @@ -226,98 +205,11 @@ describe('Test function: createAccount', function () {
expect(state.transactions.length).to.be.eq(0);
});

it('should not create any user account with proxy in state in SN_SEPOLIA if account already deployed', async function () {
sandbox.stub(utils, 'deployAccount').callsFake(async () => {
return createAccountProxyResp;
});
sandbox.stub(utils, 'isAccountDeployed').resolves(true);
sandbox.stub(utils, 'getBalance').callsFake(async () => {
return getBalanceResp[0];
});
sandbox.stub(utils, 'estimateAccountDeployFee').callsFake(async () => {
return estimateDeployFeeResp;
});
const requestObject: CreateAccountRequestParams = { deploy: true };
apiParams.requestParams = requestObject;
const result = await createAccount(apiParams);
expect(walletStub.rpcStubs.snap_manageState).to.have.been.callCount(4);
expect(result.address).to.be.eq(createAccountProxyResp.contract_address);
expect(result.transaction_hash).to.be.eq(createAccountProxyResp.transaction_hash);
expect(state.accContracts.length).to.be.eq(1);
expect(state.transactions.length).to.be.eq(1);
});

it('should not create any user account with proxy in state in SN_SEPOLIA if account does not have enough ETH balance', async function () {
sandbox.stub(utils, 'deployAccount').callsFake(async () => {
return createAccountProxyResp;
});
sandbox.stub(utils, 'isAccountDeployed').resolves(false);
sandbox.stub(utils, 'getBalance').callsFake(async () => {
return getBalanceResp[0];
});
sandbox.stub(utils, 'estimateAccountDeployFee').callsFake(async () => {
return estimateDeployFeeResp2;
});
const requestObject: CreateAccountRequestParams = { deploy: true };
apiParams.requestParams = requestObject;
const result = await createAccount(apiParams);
expect(walletStub.rpcStubs.snap_manageState).to.have.been.callCount(4);
expect(result.address).to.be.eq(createAccountProxyResp.contract_address);
expect(result.transaction_hash).to.be.eq(createAccountProxyResp.transaction_hash);
expect(state.accContracts.length).to.be.eq(1);
expect(state.transactions.length).to.be.eq(1);
});

it('should not create any user account with proxy in state in SN_SEPOLIA if account does not have enough ETH balance for suggestedMaxFee > 0.000001 ETH', async function () {
sandbox.stub(utils, 'deployAccount').callsFake(async () => {
return createAccountProxyResp;
});
sandbox.stub(utils, 'isAccountDeployed').resolves(false);
sandbox.stub(utils, 'getBalance').callsFake(async () => {
return getBalanceResp[0];
});
sandbox.stub(utils, 'estimateAccountDeployFee').callsFake(async () => {
return estimateDeployFeeResp3;
});
const requestObject: CreateAccountRequestParams = { deploy: true };
apiParams.requestParams = requestObject;
const result = await createAccount(apiParams);
expect(walletStub.rpcStubs.snap_manageState).to.have.been.callCount(4);
expect(result.address).to.be.eq(createAccountProxyResp.contract_address);
expect(result.transaction_hash).to.be.eq(createAccountProxyResp.transaction_hash);
expect(state.accContracts.length).to.be.eq(1);
expect(state.transactions.length).to.be.eq(1);
});

it('should not create any user account with proxy in state in SN_SEPOLIA if get account ETH balance throws error', async function () {
sandbox.stub(utils, 'deployAccount').callsFake(async () => {
return createAccountProxyResp;
});
sandbox.stub(utils, 'isAccountDeployed').resolves(false);
sandbox.stub(utils, 'getBalance').throws(new Error());
sandbox.stub(utils, 'estimateAccountDeployFee').callsFake(async () => {
return estimateDeployFeeResp2;
});
const requestObject: CreateAccountRequestParams = { deploy: true };
apiParams.requestParams = requestObject;
const result = await createAccount(apiParams);
expect(walletStub.rpcStubs.snap_manageState).to.have.been.callCount(4);
expect(result.address).to.be.eq(createAccountProxyResp.contract_address);
expect(result.transaction_hash).to.be.eq(createAccountProxyResp.transaction_hash);
expect(state.accContracts.length).to.be.eq(1);
expect(state.transactions.length).to.be.eq(1);
});

it('should skip upsert account and transaction if deployTxn response code has no transaction_hash in SN_SEPOLIA', async function () {
sandbox.stub(utils, 'deployAccount').callsFake(async () => {
return createAccountFailedProxyResp;
});
sandbox.stub(utils, 'isAccountDeployed').resolves(false);
sandbox.stub(utils, 'callContract').resolves(getBalanceResp);
sandbox.stub(utils, 'getSigner').throws(new Error());
sandbox.stub(utils, 'estimateAccountDeployFee').callsFake(async () => {
return estimateDeployFeeResp;
});

const requestObject: CreateAccountRequestParams = { deploy: true };
apiParams.requestParams = requestObject;
const result = await createAccount(apiParams);
Expand All @@ -333,30 +225,7 @@ describe('Test function: createAccount', function () {
sandbox.stub(utils, 'deployAccount').callsFake(async () => {
return createAccountProxyResp;
});
sandbox.stub(utils, 'isAccountDeployed').resolves(false);
sandbox.stub(utils, 'callContract').resolves(getBalanceResp);
sandbox.stub(utils, 'getSigner').throws(new Error());
sandbox.stub(utils, 'estimateAccountDeployFee').callsFake(async () => {
return estimateDeployFeeResp;
});
const requestObject: CreateAccountRequestParams = { deploy: true };
apiParams.requestParams = requestObject;

let result;
try {
await createAccount(apiParams);
} catch (err) {
result = err;
} finally {
expect(result).to.be.an('Error');
}
});

it('should throw error if isAccountDeployed failed', async function () {
const isAccountAddressDeployedStub = sandbox.stub(utils, 'isAccountDeployed').throws(new Error());
const deployAccountStub = sandbox.stub(utils, 'deployAccount');
const estimateAccountDeployFeeStub = sandbox.stub(utils, 'estimateAccountDeployFee');
const getBalanceStub = sandbox.stub(utils, 'getBalance');
const requestObject: CreateAccountRequestParams = { deploy: true };
apiParams.requestParams = requestObject;

Expand All @@ -366,10 +235,6 @@ describe('Test function: createAccount', function () {
} catch (err) {
result = err;
} finally {
expect(isAccountAddressDeployedStub).to.have.been.callCount(1);
expect(deployAccountStub).to.have.been.callCount(0);
expect(estimateAccountDeployFeeStub).to.have.been.callCount(0);
expect(getBalanceStub).to.have.been.callCount(0);
expect(result).to.be.an('Error');
}
});
Expand Down
6 changes: 3 additions & 3 deletions yarn.lock
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3246,8 +3246,8 @@ __metadata:
linkType: soft

"@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=64cb92&locator=wallet-ui%40workspace%3Apackages%2Fwallet-ui"
version: 2.8.0
resolution: "@consensys/starknet-snap@file:../starknet-snap#../starknet-snap::hash=511eed&locator=wallet-ui%40workspace%3Apackages%2Fwallet-ui"
dependencies:
"@metamask/snaps-sdk": 3.0.1
async-mutex: ^0.3.2
Expand All @@ -3256,7 +3256,7 @@ __metadata:
ethers: ^5.5.1
starknet: 6.7.0
starknet_v4.22.0: "npm:[email protected]"
checksum: 12c71045fd6bd5d137d51a9fb9a92b3aae206b7b1b1a036bd8fe4ddbf953d94967be56ecb170c4e871a30fa065ba64571dca714180522b89b5a9bf4cc6476b41
checksum: 1a75cd48673a4be804d249ec3356ced4aeba9c2d08d6c9ccd98ac1d13aa3874109b2ba3d0c0adeb90db2046bbe0134468666612405975f8ad822417fc736e10f
languageName: node
linkType: hard

Expand Down

0 comments on commit 02b92f9

Please sign in to comment.