From 0d095c6bb9375b7753efa2a9903f613b1013584e Mon Sep 17 00:00:00 2001 From: Sergey Kintsel Date: Thu, 13 Jun 2024 12:24:03 +0100 Subject: [PATCH] Fix operations execution for unrevealed accounts --- .../SignTransactionFormPage.tsx | 2 +- src/mocks/helpers.ts | 2 +- src/types/AccountOperations.ts | 3 + src/utils/hooks/setAccountDataHooks.test.ts | 12 +-- src/utils/mnemonic.test.ts | 22 ++-- src/utils/mnemonic.ts | 4 +- src/utils/tezos/estimate.test.ts | 54 +++++++++- src/utils/tezos/estimate.ts | 44 ++++++-- src/utils/tezos/helpers.test.ts | 102 +++++++++++++----- src/utils/tezos/helpers.ts | 18 ++-- src/utils/tzkt/types.ts | 11 +- 11 files changed, 198 insertions(+), 76 deletions(-) diff --git a/src/components/SendFlow/MultisigAccount/SignTransactionFormPage.tsx b/src/components/SendFlow/MultisigAccount/SignTransactionFormPage.tsx index b12d7f8c5c..668bf84517 100644 --- a/src/components/SendFlow/MultisigAccount/SignTransactionFormPage.tsx +++ b/src/components/SendFlow/MultisigAccount/SignTransactionFormPage.tsx @@ -98,7 +98,7 @@ export const SignTransactionFormPage: React.FC> = prop - Approvers + Approvers {signers.map(signer => ( { store.dispatch(accountsActions.addAccount(account)); }; -export const fakeAddressExists = (revealedKeyPairs: { pkh: RawPkh }[]) => (pkh: RawPkh) => +export const fakeIsAccountRevealed = (revealedKeyPairs: { pkh: RawPkh }[]) => (pkh: RawPkh) => Promise.resolve(revealedKeyPairs.map(keyPair => keyPair.pkh).includes(pkh)); diff --git a/src/types/AccountOperations.ts b/src/types/AccountOperations.ts index b23d4db7b7..ad9b70678c 100644 --- a/src/types/AccountOperations.ts +++ b/src/types/AccountOperations.ts @@ -21,6 +21,9 @@ export type ImplicitOperations = { export type AccountOperations = ProposalOperations | ImplicitOperations; export type EstimatedAccountOperations = AccountOperations & { estimates: Estimation[]; + // for some operations taquito automatically adds a reveal operation + // and returns it as the first estimate + revealEstimate?: Estimation; }; export const totalFee = (estimates: Estimation[]): number => diff --git a/src/utils/hooks/setAccountDataHooks.test.ts b/src/utils/hooks/setAccountDataHooks.test.ts index af6a1e6491..035ae87c23 100644 --- a/src/utils/hooks/setAccountDataHooks.test.ts +++ b/src/utils/hooks/setAccountDataHooks.test.ts @@ -12,7 +12,7 @@ import { mockSecretKeyAccount, mockSocialAccount, } from "../../mocks/factories"; -import { addAccount, fakeAddressExists } from "../../mocks/helpers"; +import { addAccount, fakeIsAccountRevealed } from "../../mocks/helpers"; import { mnemonic1 } from "../../mocks/mockMnemonic"; import { act, renderHook } from "../../mocks/testUtils"; import { ImplicitAccount, MnemonicAccount } from "../../types/Account"; @@ -36,7 +36,7 @@ describe("setAccountDataHooks", () => { }); describe("mnemonic accounts", () => { - const addressExistsMock = jest.spyOn(tezosHelpers, "addressExists"); + const isAccountRevealedMock = jest.spyOn(tezosHelpers, "isAccountRevealed"); const getFingerPrintMock = jest.spyOn(tezosHelpers, "getFingerPrint"); const encryptMock = jest.spyOn(functionsToMock, "encrypt"); const decryptMock = jest.spyOn(functionsToMock, "decrypt"); @@ -108,8 +108,8 @@ describe("setAccountDataHooks", () => { await mnemonicAccount(4, `${LABEL_BASE} 5`), await mnemonicAccount(5, `${LABEL_BASE} 6`), ]; - addressExistsMock.mockImplementation( - fakeAddressExists(revealedAccounts.map(account => account.address)) + isAccountRevealedMock.mockImplementation( + fakeIsAccountRevealed(revealedAccounts.map(account => account.address)) ); const { @@ -148,8 +148,8 @@ describe("setAccountDataHooks", () => { await mnemonicAccount(2, `${LABEL_BASE} 5`), ]; // Reveal mnemonic accounts - addressExistsMock.mockImplementation( - fakeAddressExists(expected.map(account => account.address)) + isAccountRevealedMock.mockImplementation( + fakeIsAccountRevealed(expected.map(account => account.address)) ); const { diff --git a/src/utils/mnemonic.test.ts b/src/utils/mnemonic.test.ts index 60577f7c61..afa0925eda 100644 --- a/src/utils/mnemonic.test.ts +++ b/src/utils/mnemonic.test.ts @@ -8,13 +8,13 @@ import { networksActions } from "./redux/slices/networks"; import { store } from "./redux/store"; import * as tezosHelpers from "./tezos/helpers"; import { mockContractContact, mockImplicitContact, mockSocialAccount } from "../mocks/factories"; -import { addAccount, fakeAddressExists } from "../mocks/helpers"; +import { addAccount, fakeIsAccountRevealed } from "../mocks/helpers"; import { mnemonic1 } from "../mocks/mockMnemonic"; import { renderHook } from "../mocks/testUtils"; import { ImplicitAccount } from "../types/Account"; import { MAINNET } from "../types/Network"; -const addressExistsMock = jest.spyOn(tezosHelpers, "addressExists"); +const isAccountRevealedMock = jest.spyOn(tezosHelpers, "isAccountRevealed"); const testPublicKeys = [ { @@ -37,7 +37,7 @@ beforeEach(() => { describe("restoreRevealedPublicKeyPairs", () => { it("restores existing accounts", async () => { - addressExistsMock.mockImplementation(fakeAddressExists(testPublicKeys)); + isAccountRevealedMock.mockImplementation(fakeIsAccountRevealed(testPublicKeys)); const result = await restoreRevealedPublicKeyPairs( mnemonic1, @@ -49,7 +49,7 @@ describe("restoreRevealedPublicKeyPairs", () => { }); it("restores first account if none exists", async () => { - addressExistsMock.mockImplementation(fakeAddressExists([])); + isAccountRevealedMock.mockImplementation(fakeIsAccountRevealed([])); const result = await restoreRevealedPublicKeyPairs( mnemonic1, @@ -61,7 +61,9 @@ describe("restoreRevealedPublicKeyPairs", () => { }); it("stops at first unrevealed account", async () => { - addressExistsMock.mockImplementation(fakeAddressExists([testPublicKeys[0], testPublicKeys[2]])); + isAccountRevealedMock.mockImplementation( + fakeIsAccountRevealed([testPublicKeys[0], testPublicKeys[2]]) + ); const result = await restoreRevealedPublicKeyPairs( mnemonic1, @@ -109,8 +111,8 @@ describe("useRestoreRevealedMnemonicAccounts", () => { derivationPathTemplate: "44'/1729'/?'/0'", }, ]; - addressExistsMock.mockImplementation( - fakeAddressExists(expected.map(account => account.address)) + isAccountRevealedMock.mockImplementation( + fakeIsAccountRevealed(expected.map(account => account.address)) ); const { @@ -127,7 +129,7 @@ describe("useRestoreRevealedMnemonicAccounts", () => { }); it("restores one account if none were revealed", async () => { - addressExistsMock.mockImplementation(fakeAddressExists([])); + isAccountRevealedMock.mockImplementation(fakeIsAccountRevealed([])); const { result: { current: restoreRevealedMnemonicsHook }, @@ -148,7 +150,7 @@ describe("useRestoreRevealedMnemonicAccounts", () => { }); it("sets unique labels for restored accounts", async () => { - addressExistsMock.mockImplementation(fakeAddressExists(testPublicKeys.slice(0, 3))); + isAccountRevealedMock.mockImplementation(fakeIsAccountRevealed(testPublicKeys.slice(0, 3))); store.dispatch(networksActions.setCurrent(MAINNET)); store.dispatch(contactsActions.upsert(mockImplicitContact(1, CUSTOM_LABEL))); store.dispatch(contactsActions.upsert(mockContractContact(0, "ghostnet", `${CUSTOM_LABEL} 4`))); @@ -180,7 +182,7 @@ describe("useRestoreRevealedMnemonicAccounts", () => { }); it("restores existing accounts with a custom derivation path", async () => { - addressExistsMock.mockImplementation(fakeAddressExists(testPublicKeys.slice(0, 2))); + isAccountRevealedMock.mockImplementation(fakeIsAccountRevealed(testPublicKeys.slice(0, 2))); const { result: { current: restoreRevealedMnemonicsHook }, diff --git a/src/utils/mnemonic.ts b/src/utils/mnemonic.ts index 6477c95ca3..97563ee219 100644 --- a/src/utils/mnemonic.ts +++ b/src/utils/mnemonic.ts @@ -3,7 +3,7 @@ import { generateMnemonic } from "bip39"; import { makeDerivationPath } from "./account/derivationPathUtils"; import { makeMnemonicAccount } from "./account/makeMnemonicAccount"; import { useGetNextAvailableAccountLabels } from "./hooks/labelsHooks"; -import { PublicKeyPair, addressExists, derivePublicKeyPair, getFingerPrint } from "./tezos"; +import { PublicKeyPair, derivePublicKeyPair, getFingerPrint, isAccountRevealed } from "./tezos"; import { MnemonicAccount } from "../types/Account"; import { Network } from "../types/Network"; @@ -44,7 +44,7 @@ export const restoreRevealedPublicKeyPairs = async ( mnemonic, makeDerivationPath(derivationPathTemplate, accountIndex) ); - } while (await addressExists(pubKeyPair.pkh, network)); + } while (await isAccountRevealed(pubKeyPair.pkh, network)); return result; }; diff --git a/src/utils/tezos/estimate.test.ts b/src/utils/tezos/estimate.test.ts index 19121bcf72..8e9383b7ca 100644 --- a/src/utils/tezos/estimate.test.ts +++ b/src/utils/tezos/estimate.test.ts @@ -1,5 +1,5 @@ import { estimate, handleTezError } from "./estimate"; -import { addressExists, makeToolkit } from "./helpers"; +import { isAccountRevealed, makeToolkit } from "./helpers"; import { executeParams } from "../../mocks/executeParams"; import { mockImplicitAccount, mockTezOperation } from "../../mocks/factories"; import { makeAccountOperations } from "../../types/AccountOperations"; @@ -7,7 +7,7 @@ import { GHOSTNET } from "../../types/Network"; jest.mock("./helpers", () => ({ ...jest.requireActual("./helpers"), makeToolkit: jest.fn(), - addressExists: jest.fn(), + isAccountRevealed: jest.fn(), })); const accountOperations = makeAccountOperations(mockImplicitAccount(1), mockImplicitAccount(1), [ @@ -17,7 +17,7 @@ const accountOperations = makeAccountOperations(mockImplicitAccount(1), mockImpl describe("estimate", () => { describe("Error handling", () => { it("Catches unrevealed account", async () => { - jest.mocked(addressExists).mockResolvedValue(false); + jest.mocked(isAccountRevealed).mockResolvedValue(false); jest.mocked(makeToolkit).mockImplementation( () => @@ -51,7 +51,7 @@ describe("estimate", () => { }); }); - it("returns fee, storageLimit and gasLimit estimate ", async () => { + it("returns estimated operations", async () => { const estimateResult = [ { burnFeeMutez: 0, @@ -84,4 +84,50 @@ describe("estimate", () => { expect(result).toEqual(processedEstimateResult); }); + + it("can deal with the reveal estimation", async () => { + const estimateResult = [ + { + burnFeeMutez: 0, + consumedMilligas: 1, + gasLimit: 2, + minimalFeeMutez: 3, + operationFeeMutez: 4, + storageLimit: 5, + suggestedFeeMutez: 6, + totalCost: 7, + usingBaseFeeMutez: 8, + }, + { + burnFeeMutez: 0, + consumedMilligas: 168681, + gasLimit: 169, + minimalFeeMutez: 269, + operationFeeMutez: 168.9, + storageLimit: 0, + suggestedFeeMutez: 289, + totalCost: 269, + usingBaseFeeMutez: 269, + }, + ]; + + const processedEstimateResult = { + ...accountOperations, + estimates: [executeParams({ fee: 289, gasLimit: 169 })], + revealEstimate: executeParams({ fee: 7, gasLimit: 2, storageLimit: 5 }), + }; + + jest.mocked(makeToolkit).mockImplementation( + () => + ({ + estimate: { + batch: jest.fn().mockResolvedValue(estimateResult), + }, + }) as any + ); + + const result = await estimate(accountOperations, GHOSTNET); + + expect(result).toEqual(processedEstimateResult); + }); }); diff --git a/src/utils/tezos/estimate.ts b/src/utils/tezos/estimate.ts index 7a08e9bc4d..d391db8580 100644 --- a/src/utils/tezos/estimate.ts +++ b/src/utils/tezos/estimate.ts @@ -1,7 +1,19 @@ -import { addressExists, makeToolkit, operationsToBatchParams } from "./helpers"; +import { Estimate } from "@taquito/taquito"; + +import { isAccountRevealed, makeToolkit, operationsToBatchParams } from "./helpers"; +import { Estimation } from "./types"; import { AccountOperations, EstimatedAccountOperations } from "../../types/AccountOperations"; import { Network } from "../../types/Network"; +/** + * Estimates (and simulates the execution of) the operations. + * + * Note: if + * + * @param operations - operations to be estimated with a particular transaction signer + * @param network - + * @returns operations with their estimated fees, gas and storage limits + */ export const estimate = async ( operations: AccountOperations, network: Network @@ -14,20 +26,20 @@ export const estimate = async ( try { const estimates = await tezosToolkit.estimate.batch(operationsToBatchParams(operations)); + let revealEstimate = undefined; + // taquito automatically adds a reveal operation (sometimes) + // if an account is not revealed yet + if (estimates.length > operations.operations.length) { + revealEstimate = estimateToEstimation(estimates.shift()!); + } + return { ...operations, - estimates: estimates.map(estimate => ({ - storageLimit: estimate.storageLimit, - gasLimit: estimate.gasLimit, - fee: Math.max(estimate.suggestedFeeMutez, estimate.totalCost), - })), + estimates: estimates.map(estimateToEstimation), + revealEstimate, }; - // The way taquito works we need to take the max of suggestedFeeMutez and totalCost - // because the suggestedFeeMutez does not include the storage & execution cost - // and in these cases the totalCost is the one to go (so, for contract calls) - // though totalCost doesn't work well with simple tez transfers and suggestedFeeMutez is more accurate } catch (err: any) { - const isRevealed = await addressExists(operations.signer.address.pkh, network); + const isRevealed = await isAccountRevealed(operations.signer.address.pkh, network); if (!isRevealed) { throw new Error(`Signer address is not revealed on the ${network.name}.`); @@ -39,6 +51,16 @@ export const estimate = async ( } }; +const estimateToEstimation = (estimate: Estimate): Estimation => ({ + storageLimit: estimate.storageLimit, + gasLimit: estimate.gasLimit, + // The way taquito works we need to take the max of suggestedFeeMutez and totalCost + // because the suggestedFeeMutez does not include the storage & execution cost + // and in these cases the totalCost is the one to go (so, for contract calls) + // though totalCost doesn't work well with simple tez transfers and suggestedFeeMutez is more accurate + fee: Math.max(estimate.suggestedFeeMutez, estimate.totalCost), +}); + // Converts a known L1 error message to a more user-friendly one export const handleTezError = (err: Error): string => { if (err.message.includes("subtraction_underflow")) { diff --git a/src/utils/tezos/helpers.test.ts b/src/utils/tezos/helpers.test.ts index 6a333bd971..12dcc1eb5a 100644 --- a/src/utils/tezos/helpers.test.ts +++ b/src/utils/tezos/helpers.test.ts @@ -2,8 +2,8 @@ import { InMemorySigner } from "@taquito/signer"; import axios from "axios"; import { - addressExists, getPublicKeyPairFromSk, + isAccountRevealed, operationToTaquitoOperation, operationsToBatchParams, operationsToWalletParams, @@ -20,7 +20,7 @@ import { mockTezOperation, mockUndelegationOperation, } from "../../mocks/factories"; -import { makeAccountOperations } from "../../types/AccountOperations"; +import { EstimatedAccountOperations, makeAccountOperations } from "../../types/AccountOperations"; import { MAINNET } from "../../types/Network"; import { ContractCall, ContractOrigination } from "../../types/Operation"; jest.mock("@taquito/signer"); @@ -35,32 +35,50 @@ describe("tezos utils helpers", () => { expect(InMemorySigner).toHaveBeenCalledTimes(1); }); - it("addressExists for non empty response", async () => { - const mockResponse = { - data: { - type: "user", - }, - }; - mockedAxios.get.mockResolvedValue(mockResponse); - const result = await addressExists(mockImplicitAddress(0).pkh, MAINNET); - expect(mockedAxios.get).toHaveBeenCalledWith( - `${MAINNET.tzktApiUrl}/v1/accounts/${mockImplicitAddress(0).pkh}` - ); - expect(result).toEqual(true); - }); + describe("isAccountRevealed", () => { + it("returns true for a revealed account", async () => { + const mockResponse = { + data: { + type: "user", + revealed: true, + }, + }; + mockedAxios.get.mockResolvedValue(mockResponse); + const result = await isAccountRevealed(mockImplicitAddress(0).pkh, MAINNET); + expect(mockedAxios.get).toHaveBeenCalledWith( + `${MAINNET.tzktApiUrl}/v1/accounts/${mockImplicitAddress(0).pkh}` + ); + expect(result).toEqual(true); + }); - it("addressExists returns false for empty response", async () => { - const mockResponse = { - data: { - type: "empty", - }, - }; - mockedAxios.get.mockResolvedValue(mockResponse); - const result = await addressExists(mockImplicitAddress(0).pkh, MAINNET); - expect(mockedAxios.get).toHaveBeenCalledWith( - `${MAINNET.tzktApiUrl}/v1/accounts/${mockImplicitAddress(0).pkh}` - ); - expect(result).toEqual(false); + it("returns false for unrevealed account", async () => { + const mockResponse = { + data: { + type: "user", + revealed: false, + }, + }; + mockedAxios.get.mockResolvedValue(mockResponse); + const result = await isAccountRevealed(mockImplicitAddress(0).pkh, MAINNET); + expect(mockedAxios.get).toHaveBeenCalledWith( + `${MAINNET.tzktApiUrl}/v1/accounts/${mockImplicitAddress(0).pkh}` + ); + expect(result).toEqual(false); + }); + + it("returns false for empty response", async () => { + const mockResponse = { + data: { + type: "empty", + }, + }; + mockedAxios.get.mockResolvedValue(mockResponse); + const result = await isAccountRevealed(mockImplicitAddress(0).pkh, MAINNET); + expect(mockedAxios.get).toHaveBeenCalledWith( + `${MAINNET.tzktApiUrl}/v1/accounts/${mockImplicitAddress(0).pkh}` + ); + expect(result).toEqual(false); + }); }); describe("operationToTaquitoOperation", () => { @@ -423,4 +441,34 @@ describe("tezos utils helpers", () => { }); }); }); + + test("operationsToWalletParams", () => { + const operations: EstimatedAccountOperations = { + ...makeAccountOperations(mockImplicitAccount(0), mockImplicitAccount(0), [ + mockTezOperation(0), + mockDelegationOperation(0), + ]), + estimates: [executeParams({ fee: 123 }), executeParams({ fee: 456, storageLimit: 123 })], + }; + + expect(operationsToWalletParams(operations)).toEqual([ + { + amount: 0, + fee: 123, + gasLimit: 0, + kind: "transaction", + mutez: true, + storageLimit: 0, + to: "tz1UZFB9kGauB6F5c2gfJo4hVcvrD8MeJ3Vf", + }, + { + delegate: "tz1UZFB9kGauB6F5c2gfJo4hVcvrD8MeJ3Vf", + fee: 456, + gasLimit: 0, + kind: "delegation", + source: "tz1gUNyn3hmnEWqkusWPzxRaon1cs7ndWh7h", + storageLimit: 123, + }, + ]); + }); }); diff --git a/src/utils/tezos/helpers.ts b/src/utils/tezos/helpers.ts index f371d2e148..4b57e19971 100644 --- a/src/utils/tezos/helpers.ts +++ b/src/utils/tezos/helpers.ts @@ -17,23 +17,19 @@ import { makeMultisigProposeOperation, } from "../../types/Operation"; import { SignerConfig } from "../../types/SignerConfig"; -import { RawTzktGetAddressType } from "../tzkt/types"; +import { RawTzktAccountType } from "../tzkt/types"; export type PublicKeyPair = { pk: string; pkh: string; }; -export const addressExists = async (pkh: string, network: Network): Promise => { - try { - const url = `${network.tzktApiUrl}/v1/accounts/${pkh}`; - const { - data: { type }, - } = await axios.get(url); - return type !== "empty"; - } catch (error: any) { - throw new Error(`Error fetching account from tzkt ${error.message}`); - } +export const isAccountRevealed = async (pkh: string, network: Network): Promise => { + const url = `${network.tzktApiUrl}/v1/accounts/${pkh}`; + const { + data: { type, revealed }, + } = await axios.get<{ type: RawTzktAccountType; revealed: boolean }>(url); + return type !== "empty" && revealed; }; // Temporary solution for generating fingerprint for seedphrase diff --git a/src/utils/tzkt/types.ts b/src/utils/tzkt/types.ts index 56a4e31e24..16a60e0585 100644 --- a/src/utils/tzkt/types.ts +++ b/src/utils/tzkt/types.ts @@ -1,6 +1,11 @@ -export type RawTzktGetAddressType = { - type: "user" | "delegate" | "contract" | "ghost" | "rollup" | "smart_rollup" | "empty"; -}; +export type RawTzktAccountType = + | "user" + | "delegate" + | "contract" + | "ghost" + | "rollup" + | "smart_rollup" + | "empty"; export type RawTzktGetSameMultisigsItem = { address: string;