From 5d569a88ed0605e4f022de5b567acfd5c8fc6f15 Mon Sep 17 00:00:00 2001 From: Stanley Yuen <102275989+stanleyyconsensys@users.noreply.github.com> Date: Mon, 2 Dec 2024 15:57:19 +0800 Subject: [PATCH] chore: revamp RPC `starkNet_getTransactionStatus` (#447) * chore: add chain rpc controller * chore: relocate base rpc controller * chore: revamp getTransactionStatus API * chore: update get transaction status code * chore: remove mocha test * chore: remove get transactions test --- .../starknet-snap/src/getTransactionStatus.ts | 35 --- packages/starknet-snap/src/index.tsx | 7 +- .../src/rpcs/get-transaction-status.test.ts | 74 +++++ .../src/rpcs/get-transaction-status.ts | 69 +++++ packages/starknet-snap/src/rpcs/index.ts | 1 + packages/starknet-snap/src/types/snapApi.ts | 6 +- .../test/src/getTransactionStatus.test.ts | 69 ----- .../test/src/getTransactions.test.ts | 276 ------------------ 8 files changed, 153 insertions(+), 384 deletions(-) delete mode 100644 packages/starknet-snap/src/getTransactionStatus.ts create mode 100644 packages/starknet-snap/src/rpcs/get-transaction-status.test.ts create mode 100644 packages/starknet-snap/src/rpcs/get-transaction-status.ts delete mode 100644 packages/starknet-snap/test/src/getTransactionStatus.test.ts delete mode 100644 packages/starknet-snap/test/src/getTransactions.test.ts diff --git a/packages/starknet-snap/src/getTransactionStatus.ts b/packages/starknet-snap/src/getTransactionStatus.ts deleted file mode 100644 index 998a1165..00000000 --- a/packages/starknet-snap/src/getTransactionStatus.ts +++ /dev/null @@ -1,35 +0,0 @@ -import type { - ApiParams, - GetTransactionStatusRequestParams, -} from './types/snapApi'; -import { logger } from './utils/logger'; -import { toJson } from './utils/serializer'; -import { getNetworkFromChainId } from './utils/snapUtils'; -import * as utils from './utils/starknetUtils'; - -/** - * - * @param params - */ -export async function getTransactionStatus(params: ApiParams) { - try { - const { state, requestParams } = params; - const requestParamsObj = requestParams as GetTransactionStatusRequestParams; - - const { transactionHash } = requestParamsObj; - const network = getNetworkFromChainId(state, requestParamsObj.chainId); - - const getTxnStatusResp = await utils.getTransactionStatus( - transactionHash, - network, - ); - logger.log( - `getTransactionStatus:\ngetTxnStatusResp: ${toJson(getTxnStatusResp)}`, - ); - - return getTxnStatusResp; - } catch (error) { - logger.error(`Problem found:`, error); - throw error; - } -} diff --git a/packages/starknet-snap/src/index.tsx b/packages/starknet-snap/src/index.tsx index af3b7b3f..d43e696a 100644 --- a/packages/starknet-snap/src/index.tsx +++ b/packages/starknet-snap/src/index.tsx @@ -23,7 +23,6 @@ import { getStoredNetworks } from './getStoredNetworks'; import { getStoredTransactions } from './getStoredTransactions'; import { getStoredUserAccounts } from './getStoredUserAccounts'; import { getTransactions } from './getTransactions'; -import { getTransactionStatus } from './getTransactionStatus'; import { getValue } from './getValue'; import { homePageController } from './on-home-page'; import { recoverAccounts } from './recoverAccounts'; @@ -39,6 +38,7 @@ import type { GetDeploymentDataParams, DeclareContractParams, WatchAssetParams, + GetTransactionStatusParams, } from './rpcs'; import { displayPrivateKey, @@ -52,6 +52,7 @@ import { switchNetwork, getDeploymentData, watchAsset, + getTransactionStatus, } from './rpcs'; import { signDeployAccountTransaction } from './signDeployAccountTransaction'; import type { @@ -201,7 +202,9 @@ export const onRpcRequest: OnRpcRequestHandler = async ({ request }) => { return await getErc20TokenBalance(apiParams); case 'starkNet_getTransactionStatus': - return await getTransactionStatus(apiParams); + return await getTransactionStatus.execute( + apiParams.requestParams as unknown as GetTransactionStatusParams, + ); case 'starkNet_getValue': return await getValue(apiParams); diff --git a/packages/starknet-snap/src/rpcs/get-transaction-status.test.ts b/packages/starknet-snap/src/rpcs/get-transaction-status.test.ts new file mode 100644 index 00000000..07e76d46 --- /dev/null +++ b/packages/starknet-snap/src/rpcs/get-transaction-status.test.ts @@ -0,0 +1,74 @@ +import type { constants } from 'starknet'; +import { + TransactionExecutionStatus, + TransactionFinalityStatus, +} from 'starknet'; + +import { mockNetworkStateManager } from '../state/__tests__/helper'; +import type { Network } from '../types/snapState'; +import { STARKNET_SEPOLIA_TESTNET_NETWORK } from '../utils/constants'; +import { InvalidRequestParamsError } from '../utils/exceptions'; +import * as starknetUtils from '../utils/starknetUtils'; +import type { GetTransactionStatusParams } from './get-transaction-status'; +import { getTransactionStatus } from './get-transaction-status'; + +jest.mock('../utils/snap'); +jest.mock('../utils/logger'); + +describe('GetTransactionStatusRpc', () => { + const prepareGetTransactionStatusTest = ({ + network, + status, + }: { + network: Network; + status: { + finalityStatus: TransactionFinalityStatus; + executionStatus: TransactionExecutionStatus; + }; + }) => { + const { getNetworkSpy } = mockNetworkStateManager(network); + + const getTransactionStatusSpy = jest.spyOn( + starknetUtils, + 'getTransactionStatus', + ); + + getTransactionStatusSpy.mockResolvedValue(status); + + return { + getTransactionStatusSpy, + getNetworkSpy, + }; + }; + + it('returns transaction status', async () => { + const network = STARKNET_SEPOLIA_TESTNET_NETWORK; + const transactionHash = + '0x06385d46da9fbed4a5798298b17df069ac5f786e4c9f8f6b81c665540aea245a'; + const expectedResult = { + finalityStatus: TransactionFinalityStatus.ACCEPTED_ON_L1, + executionStatus: TransactionExecutionStatus.SUCCEEDED, + }; + const { getTransactionStatusSpy } = prepareGetTransactionStatusTest({ + network, + status: expectedResult, + }); + + const result = await getTransactionStatus.execute({ + chainId: network.chainId as unknown as constants.StarknetChainId, + transactionHash, + }); + + expect(result).toStrictEqual(expectedResult); + expect(getTransactionStatusSpy).toHaveBeenCalledWith( + transactionHash, + network, + ); + }); + + it('throws `InvalidRequestParamsError` when request parameter is not correct', async () => { + await expect( + getTransactionStatus.execute({} as unknown as GetTransactionStatusParams), + ).rejects.toThrow(InvalidRequestParamsError); + }); +}); diff --git a/packages/starknet-snap/src/rpcs/get-transaction-status.ts b/packages/starknet-snap/src/rpcs/get-transaction-status.ts new file mode 100644 index 00000000..7f8ed16f --- /dev/null +++ b/packages/starknet-snap/src/rpcs/get-transaction-status.ts @@ -0,0 +1,69 @@ +import { HexStruct } from '@metamask/utils'; +import { + TransactionExecutionStatus, + TransactionFinalityStatus, +} from 'starknet'; +import type { Infer } from 'superstruct'; +import { assign, nonempty, object, enums, optional } from 'superstruct'; + +import { BaseRequestStruct } from '../utils'; +import { getTransactionStatus as getTransactionStatusFn } from '../utils/starknetUtils'; +import { ChainRpcController } from './abstract/chain-rpc-controller'; + +export const GetTransactionStatusRequestStruct = assign( + object({ + transactionHash: nonempty(HexStruct), + }), + BaseRequestStruct, +); + +export const GetTransactionStatusResponseStruct = object({ + executionStatus: optional(enums(Object.values(TransactionExecutionStatus))), + finalityStatus: optional(enums(Object.values(TransactionFinalityStatus))), +}); + +export type GetTransactionStatusParams = Infer< + typeof GetTransactionStatusRequestStruct +>; + +export type GetTransactionStatusResponse = Infer< + typeof GetTransactionStatusResponseStruct +>; + +/** + * The RPC handler to get a transaction status by the given transaction hash. + */ +export class GetTransactionStatusRpc extends ChainRpcController< + GetTransactionStatusParams, + GetTransactionStatusResponse +> { + protected requestStruct = GetTransactionStatusRequestStruct; + + protected responseStruct = GetTransactionStatusResponseStruct; + + /** + * Execute the get transaction request handler. + * + * @param params - The parameters of the request. + * @param params.transactionHash - The transaction hash to enquire the transaction status. + * @param params.chainId - The chain id of the transaction. + * @returns A promise that resolves to a GetTransactionStatusResponse object that contains executionStatus and finalityStatus. + */ + async execute( + params: GetTransactionStatusParams, + ): Promise { + return super.execute(params); + } + + protected async handleRequest( + params: GetTransactionStatusParams, + ): Promise { + const { transactionHash } = params; + + const resp = await getTransactionStatusFn(transactionHash, this.network); + + return resp; + } +} + +export const getTransactionStatus = new GetTransactionStatusRpc(); diff --git a/packages/starknet-snap/src/rpcs/index.ts b/packages/starknet-snap/src/rpcs/index.ts index 09e65cf3..609edb5d 100644 --- a/packages/starknet-snap/src/rpcs/index.ts +++ b/packages/starknet-snap/src/rpcs/index.ts @@ -9,3 +9,4 @@ export * from './verify-signature'; export * from './switch-network'; export * from './get-deployment-data'; export * from './watch-asset'; +export * from './get-transaction-status'; diff --git a/packages/starknet-snap/src/types/snapApi.ts b/packages/starknet-snap/src/types/snapApi.ts index 639952de..45cb569d 100644 --- a/packages/starknet-snap/src/types/snapApi.ts +++ b/packages/starknet-snap/src/types/snapApi.ts @@ -7,6 +7,8 @@ import type { EstimateFeeDetails, DeployAccountSignerDetails, constants, + TransactionExecutionStatus, + TransactionFinalityStatus, } from 'starknet'; import type { SnapState, VoyagerTransactionType } from './snapState'; @@ -152,8 +154,8 @@ export type DeclareContractRequestParams = { } & BaseRequestParams; export type RpcV4GetTransactionReceiptResponse = { - execution_status?: string; - finality_status?: string; + execution_status?: TransactionExecutionStatus; + finality_status?: TransactionFinalityStatus; }; export type Authorizable = { diff --git a/packages/starknet-snap/test/src/getTransactionStatus.test.ts b/packages/starknet-snap/test/src/getTransactionStatus.test.ts deleted file mode 100644 index cc3b67fc..00000000 --- a/packages/starknet-snap/test/src/getTransactionStatus.test.ts +++ /dev/null @@ -1,69 +0,0 @@ -import chai, { expect } from 'chai'; -import sinon from 'sinon'; -import sinonChai from 'sinon-chai'; -import { WalletMock } from '../wallet.mock.test'; -import * as utils from '../../src/utils/starknetUtils'; -import { getTransactionStatus } from '../../src/getTransactionStatus'; -import { SnapState } from '../../src/types/snapState'; -import { STARKNET_MAINNET_NETWORK } from '../../src/utils/constants'; -import { getTxnStatusResp } from '../constants.test'; -import { Mutex } from 'async-mutex'; -import { - ApiParams, - GetTransactionStatusRequestParams, -} from '../../src/types/snapApi'; - -chai.use(sinonChai); -const sandbox = sinon.createSandbox(); - -describe('Test function: getTransactionStatus', function () { - const walletStub = new WalletMock(); - const state: SnapState = { - accContracts: [], - erc20Tokens: [], - networks: [STARKNET_MAINNET_NETWORK], - transactions: [], - }; - const apiParams: ApiParams = { - state, - requestParams: {}, - wallet: walletStub, - saveMutex: new Mutex(), - }; - - afterEach(function () { - walletStub.reset(); - sandbox.restore(); - }); - - it('should get the transaction status correctly', async function () { - sandbox.stub(utils, 'getTransactionStatus').callsFake(async () => { - return getTxnStatusResp; - }); - const requestObject: GetTransactionStatusRequestParams = { - transactionHash: - '0x27f204588cadd08a7914f6a9808b34de0cbfc4cb53aa053663e7fd3a34dbc26', - }; - apiParams.requestParams = requestObject; - const result = await getTransactionStatus(apiParams); - expect(result).to.be.eq(getTxnStatusResp); - }); - - it('should throw error if getTransactionStatus failed', async function () { - sandbox.stub(utils, 'getTransactionStatus').throws(new Error()); - const requestObject: GetTransactionStatusRequestParams = { - transactionHash: - '0x27f204588cadd08a7914f6a9808b34de0cbfc4cb53aa053663e7fd3a34dbc26', - }; - apiParams.requestParams = requestObject; - - let result; - try { - await getTransactionStatus(apiParams); - } catch (err) { - result = err; - } finally { - expect(result).to.be.an('Error'); - } - }); -}); diff --git a/packages/starknet-snap/test/src/getTransactions.test.ts b/packages/starknet-snap/test/src/getTransactions.test.ts deleted file mode 100644 index 5688c26c..00000000 --- a/packages/starknet-snap/test/src/getTransactions.test.ts +++ /dev/null @@ -1,276 +0,0 @@ -import chai, { expect } from 'chai'; -import sinon from 'sinon'; -import sinonChai from 'sinon-chai'; -import { WalletMock } from '../wallet.mock.test'; -import * as utils from '../../src/utils/starknetUtils'; -import * as snapUtils from '../../src/utils/snapUtils'; -import { SnapState, Transaction } from '../../src/types/snapState'; -import { - STARKNET_SEPOLIA_TESTNET_NETWORK, - STARKNET_MAINNET_NETWORK, -} from '../../src/utils/constants'; -import { - createAccountProxyTxn, - expectedMassagedTxn4, - expectedMassagedTxn5, - expectedMassagedTxns, - getTxnFromSequencerResp1, - getTxnFromSequencerResp2, - getTxnStatusAcceptL2Resp, - getTxnStatusResp, - getTxnsFromVoyagerResp, - unsettedTransactionInMassagedTxn, - initAccountTxn, - txn1, - txn2, - txn3, - txn4, - txn5, - mainnetTxn1, -} from '../constants.test'; -import { getTransactions, updateStatus } from '../../src/getTransactions'; -import { Mutex } from 'async-mutex'; -import { - ApiParams, - GetTransactionsRequestParams, -} from '../../src/types/snapApi'; -import { GetTransactionResponse, num } from 'starknet'; -import { VoyagerTransactions } from '../../src/types/voyager'; -import { TransactionStatuses } from '../../src/types/starknet'; - -chai.use(sinonChai); -const sandbox = sinon.createSandbox(); -describe('Test function: getTransactions', function () { - const walletStub = new WalletMock(); - let getTransactionStatusStub: sinon.SinonStub; - const state: SnapState = { - accContracts: [], - erc20Tokens: [], - networks: [STARKNET_SEPOLIA_TESTNET_NETWORK, STARKNET_MAINNET_NETWORK], - transactions: [ - { ...unsettedTransactionInMassagedTxn }, - { ...txn1 }, - { ...txn2 }, - { ...txn3 }, - { ...txn4 }, - { ...txn5 }, - { ...mainnetTxn1 }, - { ...createAccountProxyTxn }, - { ...initAccountTxn }, - ], - }; - const apiParams: ApiParams = { - state, - requestParams: {}, - wallet: walletStub, - saveMutex: new Mutex(), - }; - - beforeEach(function () { - sandbox.useFakeTimers(1653553083147); - sandbox.stub(utils, 'getTransactionsFromVoyager').callsFake(async () => { - return getTxnsFromVoyagerResp as unknown as VoyagerTransactions; - }); - sandbox.stub(utils, 'getTransaction').callsFake(async (...args) => { - if (args?.[0] === getTxnsFromVoyagerResp.items[0].hash) { - return getTxnFromSequencerResp1 as unknown as GetTransactionResponse; - } else if (args?.[0] === getTxnsFromVoyagerResp.items[1].hash) { - return getTxnFromSequencerResp2 as unknown as GetTransactionResponse; - } else { - return null as unknown as GetTransactionResponse; - } - }); - getTransactionStatusStub = sandbox - .stub(utils, 'getTransactionStatus') - .callsFake(async (...args) => { - if (args?.[0] === getTxnsFromVoyagerResp.items[0].hash) { - return getTxnStatusResp as unknown as TransactionStatuses; - } else if (args?.[0] === getTxnsFromVoyagerResp.items[1].hash) { - return getTxnStatusResp as unknown as TransactionStatuses; - } else if (args?.[0] === expectedMassagedTxn5.txnHash) { - return undefined as unknown as TransactionStatuses; - } - return getTxnStatusAcceptL2Resp as unknown as TransactionStatuses; - }); - walletStub.rpcStubs.snap_manageState.resolves(state); - }); - - afterEach(function () { - walletStub.reset(); - sandbox.restore(); - }); - - it('should get the transactions from Voyager of testnet correctly', async function () { - const requestObject: GetTransactionsRequestParams = { - senderAddress: txn4.senderAddress, - pageSize: '10', - chainId: STARKNET_SEPOLIA_TESTNET_NETWORK.chainId, - }; - apiParams.requestParams = requestObject; - - const result = await getTransactions(apiParams); - - expect(walletStub.rpcStubs.snap_manageState).to.have.been.called; - expect(result.length).to.be.eq(4); - expect(result).to.be.eql(expectedMassagedTxns); - }); - - it('should merge the transactions stored in snap state correctly', async function () { - const requestObject: GetTransactionsRequestParams = { - senderAddress: txn4.senderAddress, - pageSize: '10', - chainId: STARKNET_SEPOLIA_TESTNET_NETWORK.chainId, - }; - apiParams.requestParams = requestObject; - - const result = await getTransactions(apiParams); - const mergeTxn = result.find( - (e) => - num.toBigInt(e.txnHash) === - num.toBigInt(unsettedTransactionInMassagedTxn.txnHash), - ); - expect(getTransactionStatusStub.callCount).to.be.eq(4); - expect(walletStub.rpcStubs.snap_manageState).to.have.been.called; - expect(mergeTxn).not.to.be.undefined; - if (mergeTxn !== undefined) { - expect(mergeTxn.status).to.be.eq(''); - expect(mergeTxn.finalityStatus).to.be.eq(getTxnStatusResp.finalityStatus); - expect(mergeTxn.executionStatus).to.be.eq( - getTxnStatusResp.executionStatus, - ); - } - expect(result.length).to.be.eq(4); - expect(result).to.be.eql(expectedMassagedTxns); - }); - - it('should get the transactions of testnet stored in snap state correctly', async function () { - const requestObject: GetTransactionsRequestParams = { - senderAddress: txn4.senderAddress, - pageSize: '10', - onlyFromState: true, - chainId: STARKNET_SEPOLIA_TESTNET_NETWORK.chainId, - }; - apiParams.requestParams = requestObject; - const result = await getTransactions(apiParams); - expect(walletStub.rpcStubs.snap_manageState).to.have.been.called; - expect(result.length).to.be.eq(2); - expect(result).to.be.eql([expectedMassagedTxn5, expectedMassagedTxn4]); - }); - - it('should get the transactions with deploy txn from Voyager of testnet correctly', async function () { - const requestObject: GetTransactionsRequestParams = { - senderAddress: txn4.senderAddress, - pageSize: '10', - withDeployTxn: true, - chainId: STARKNET_SEPOLIA_TESTNET_NETWORK.chainId, - }; - apiParams.requestParams = requestObject; - const result = await getTransactions(apiParams); - expect(walletStub.rpcStubs.snap_manageState).to.have.been.called; - expect(result.length).to.be.eq(4); - expect(result).to.be.eql(expectedMassagedTxns); - }); - - it('should throw error if upsertTransactions failed', async function () { - sandbox.stub(snapUtils, 'upsertTransactions').throws(new Error()); - const requestObject: GetTransactionsRequestParams = { - senderAddress: txn4.senderAddress, - pageSize: '10', - withDeployTxn: true, - chainId: STARKNET_SEPOLIA_TESTNET_NETWORK.chainId, - }; - apiParams.requestParams = requestObject; - - let result; - try { - await getTransactions(apiParams); - } catch (err) { - result = err; - } finally { - expect(result).to.be.an('Error'); - } - }); - - it('should throw an error if the sender address is an invalid address', async function () { - const requestObject: GetTransactionsRequestParams = { - senderAddress: 'wrongAddress', - pageSize: '10', - withDeployTxn: true, - }; - apiParams.requestParams = requestObject; - let result; - try { - result = await getTransactions(apiParams); - } catch (err) { - result = err; - } finally { - expect(result).to.be.an('Error'); - } - }); - - it('should throw an error if the contract address is an invalid address', async function () { - const requestObject: GetTransactionsRequestParams = { - senderAddress: txn4.senderAddress, - pageSize: '10', - withDeployTxn: true, - contractAddress: 'wrongAddress', - }; - apiParams.requestParams = requestObject; - let result; - try { - result = await getTransactions(apiParams); - } catch (err) { - result = err; - } finally { - expect(result).to.be.an('Error'); - } - }); -}); - -describe('Test function: getTransactions.updateStatus', function () { - let getTransactionStatusStub: sinon.SinonStub; - let txns: Transaction[] = []; - beforeEach(function () { - txns = [{ ...unsettedTransactionInMassagedTxn }]; - getTransactionStatusStub = sandbox - .stub(utils, 'getTransactionStatus') - .callsFake(async () => { - return getTxnStatusAcceptL2Resp; - }); - }); - - afterEach(function () { - sandbox.restore(); - }); - - it('should update status correctly', async function () { - await updateStatus(txns[0], STARKNET_SEPOLIA_TESTNET_NETWORK); - expect(getTransactionStatusStub.callCount).to.be.eq(1); - expect(txns[0].finalityStatus).to.be.eq( - getTxnStatusAcceptL2Resp.finalityStatus, - ); - expect(txns[0].executionStatus).to.be.eq( - getTxnStatusAcceptL2Resp.executionStatus, - ); - expect(txns[0].status).to.be.eq(''); - }); - - describe('when getTransactionStatus throw error', function () { - beforeEach(function () { - sandbox.restore(); - getTransactionStatusStub = sandbox - .stub(utils, 'getTransactionStatus') - .throws(new Error()); - }); - it('should not throw error', async function () { - await updateStatus(txns[0], STARKNET_SEPOLIA_TESTNET_NETWORK); - expect(txns[0].finalityStatus).to.be.eq( - unsettedTransactionInMassagedTxn.finalityStatus, - ); - expect(txns[0].executionStatus).to.be.eq( - unsettedTransactionInMassagedTxn.executionStatus, - ); - expect(txns[0].status).to.be.eq(unsettedTransactionInMassagedTxn.status); - }); - }); -});