fix: prioritize error states in waitForTransaction evaluation

starknet-io · Dec 10, 2023 · ac54404 · ac54404
1 parent 6d8c291
commit ac54404
Show file tree

Hide file tree

Showing 3 changed files with 82 additions and 11 deletions.
diff --git a/__tests__/fixtures.ts b/__tests__/fixtures.ts
@@ -54,11 +54,8 @@ export const getTestProvider = (): ProviderInterface => {
  if (process.env.IS_LOCALHOST_DEVNET === 'true') {
  // accelerate the tests when running locally
  const originalWaitForTransaction = provider.waitForTransaction.bind(provider);
- provider.waitForTransaction = (
- txHash: string,
- { retryInterval }: waitForTransactionOptions = {}
- ) => {
- return originalWaitForTransaction(txHash, { retryInterval: retryInterval || 1000 });
+ provider.waitForTransaction = (txHash: string, options: waitForTransactionOptions = {}) => {
+ return originalWaitForTransaction(txHash, { retryInterval: 1000, ...options });
  };
  }
 

diff --git a/__tests__/rpcProvider.test.ts b/__tests__/rpcProvider.test.ts
@@ -1,8 +1,17 @@
 import { getStarkKey, utils } from '@scure/starknet';
 
-import { Account, Contract, GetBlockResponse, RpcProvider, stark } from '../src';
+import {
+ Account,
+ CallData,
+ Contract,
+ GetBlockResponse,
+ RPC,
+ RpcProvider,
+ TransactionExecutionStatus,
+ stark,
+ waitForTransactionOptions,
+} from '../src';
 import { StarknetChainId } from '../src/constants';
-import { CallData } from '../src/utils/calldata';
 import { felt, uint256 } from '../src/utils/calldata/cairo';
 import { toHexString } from '../src/utils/num';
 import {
@@ -102,6 +111,66 @@ describeIfRpc('RPCProvider', () => {
  });
  });
 
+ describe('waitForTransaction', () => {
+ const receipt = {};
+ const transactionStatusSpy = jest.spyOn(rpcProvider as any, 'getTransactionStatus');
+ const transactionReceiptSpy = jest.spyOn(rpcProvider as any, 'getTransactionReceipt');
+
+ const generateOptions = (o: waitForTransactionOptions) => ({ retryInterval: 10, ...o });
+ const generateTransactionStatus = (
+ finality_status: RPC.SPEC.TXN_STATUS,
+ execution_status?: RPC.SPEC.TXN_EXECUTION_STATUS
+ ): RPC.TransactionStatus => ({
+ finality_status,
+ execution_status,
+ });
+ const response = {
+ successful: generateTransactionStatus('ACCEPTED_ON_L1', 'SUCCEEDED'),
+ reverted: generateTransactionStatus('ACCEPTED_ON_L2', 'REVERTED'),
+ rejected: generateTransactionStatus('REJECTED'),
+ };
+
+ beforeAll(() => {
+ transactionStatusSpy.mockResolvedValue(null);
+ transactionReceiptSpy.mockResolvedValue(receipt);
+ });
+
+ afterAll(() => {
+ transactionStatusSpy.mockRestore();
+ transactionReceiptSpy.mockRestore();
+ });
+
+ test('successful - default', async () => {
+ transactionStatusSpy.mockResolvedValueOnce(response.successful);
+ await expect(rpcProvider.waitForTransaction(0)).resolves.toBe(receipt);
+ });
+
+ test('reverted - default', async () => {
+ transactionStatusSpy.mockResolvedValueOnce(response.reverted);
+ await expect(rpcProvider.waitForTransaction(0)).resolves.toBe(receipt);
+ });
+
+ test('rejected - default', async () => {
+ transactionStatusSpy.mockResolvedValueOnce(response.rejected);
+ await expect(rpcProvider.waitForTransaction(0)).rejects.toThrow(
+ `${undefined}: ${RPC.ETransactionStatus.REJECTED}`
+ );
+ });
+
+ test('reverted - as error state', async () => {
+ transactionStatusSpy.mockResolvedValueOnce(response.reverted);
+ const options = generateOptions({ errorStates: [TransactionExecutionStatus.REVERTED] });
+ await expect(rpcProvider.waitForTransaction(0, options)).rejects.toThrow(
+ `${RPC.ETransactionExecutionStatus.REVERTED}: ${RPC.ETransactionStatus.ACCEPTED_ON_L2}`
+ );
+ });
+
+ test('no error state; timed-out', async () => {
+ const options = generateOptions({ errorStates: [] });
+ await expect(rpcProvider.waitForTransaction(0, options)).rejects.toThrow(/timed-out/);
+ });
+ });
+
  describe('RPC methods', () => {
  let latestBlock: GetBlockResponse;
 

diff --git a/src/provider/rpc.ts b/src/provider/rpc.ts
@@ -321,7 +321,9 @@ export class RpcProvider implements ProviderInterface {
  const retryInterval = options?.retryInterval ?? 5000;
  const errorStates: any = options?.errorStates ?? [
  RPC.ETransactionStatus.REJECTED,
- RPC.ETransactionExecutionStatus.REVERTED,
+ // TODO: commented out to preserve the long-standing behavior of "reverted" not being treated as an error by default
+ // should decide which behavior to keep in the future
+ // RPC.ETransactionExecutionStatus.REVERTED,
  ];
  const successStates: any = options?.successStates ?? [
  RPC.ETransactionExecutionStatus.SUCCEEDED,
@@ -347,14 +349,17 @@ export class RpcProvider implements ProviderInterface {
  throw error;
  }
 
- if (successStates.includes(executionStatus) || successStates.includes(finalityStatus)) {
- onchain = true;
- } else if (errorStates.includes(executionStatus) || errorStates.includes(finalityStatus)) {
+ if (errorStates.includes(executionStatus) || errorStates.includes(finalityStatus)) {
  const message = `${executionStatus}: ${finalityStatus}`;
  const error = new Error(message) as Error & { response: RPC.TransactionStatus };
  error.response = txStatus;
  isErrorState = true;
  throw error;
+ } else if (
+ successStates.includes(executionStatus) ||
+ successStates.includes(finalityStatus)
+ ) {
+ onchain = true;
  }
  } catch (error) {
  if (error instanceof Error && isErrorState) {