fix: improve Fee Token Selection UX with Disclaimer and Error Messagi…

…ng (#433) * chore: add unit test for input controller * chore: remove unused mock * chore: update test * chore: update mock test * chore: update type * fix: improve readability on fee token selection and fallback mechanism * chore: handle pr comment review * fix: rollback request state in case of error in input-event-controller * chore: fix comments --------- Co-authored-by: stanleyyuen <[email protected]>
Consensys · Nov 25, 2024 · d7d2151 · d7d2151
1 parent 08640d3
commit d7d2151
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 16 deletions.
diff --git a/packages/starknet-snap/src/__tests__/helper.ts b/packages/starknet-snap/src/__tests__/helper.ts
@@ -22,6 +22,7 @@ import { v4 as uuidv4 } from 'uuid';
 import { FeeToken } from '../types/snapApi';
 import type {
   AccContract,
+  Erc20Token,
   Transaction,
   TransactionRequest,
 } from '../types/snapState';
@@ -298,11 +299,13 @@ export function generateTransactions({
 export function generateTransactionRequests({
   chainId,
   address,
+  selectedFeeToken,
   contractAddresses = PRELOADED_TOKENS.map((token) => token.address),
   cnt = 1,
 }: {
   chainId: constants.StarknetChainId | string;
   address: string;
+  selectedFeeToken?: Erc20Token;
   contractAddresses?: string[];
   cnt?: number;
 }): TransactionRequest[] {
@@ -330,6 +333,7 @@ export function generateTransactionRequests({
       addressIndex: 0,
       maxFee: '100',
       selectedFeeToken:
+        selectedFeeToken?.symbol ??
         feeTokens[Math.floor(generateRandomValue() * feeTokens.length)].symbol,
       calls: [
         {

diff --git a/packages/starknet-snap/src/ui/controllers/user-input-event-controller.test.ts b/packages/starknet-snap/src/ui/controllers/user-input-event-controller.test.ts
@@ -317,6 +317,7 @@ describe('UserInputEventController', () => {
   describe('handleFeeTokenChange', () => {
     const prepareHandleFeeTokenChange = async (
       feeToken: FeeToken = FeeToken.STRK,
+      selectedFeeToken?: Erc20Token,
     ) => {
       const network = STARKNET_TESTNET_NETWORK;
       const { chainId } = network;
@@ -325,6 +326,7 @@ describe('UserInputEventController', () => {
       const [transactionRequest] = generateTransactionRequests({
         chainId,
         address: account.address,
+        selectedFeeToken,
       });
 
       const event = generateInputEvent({
@@ -353,6 +355,10 @@ describe('UserInputEventController', () => {
       'updates the transaction request with the updated estimated fee: feeToken - %symbol',
       async (token: Erc20Token) => {
         const feeToken = FeeToken[token.symbol];
+        const selectedFeeToken =
+          token.symbol === FeeToken.ETH
+            ? STRK_SEPOLIA_TESTNET
+            : ETHER_SEPOLIA_TESTNET;
         const {
           event,
           account,
@@ -363,7 +369,7 @@ describe('UserInputEventController', () => {
           mockGetEstimatedFeesResponse,
           upsertTransactionRequestSpy,
           transactionRequest,
-        } = await prepareHandleFeeTokenChange(feeToken);
+        } = await prepareHandleFeeTokenChange(feeToken, selectedFeeToken);
         const feeTokenAddress = token.address;
         const { signer, calls } = transactionRequest;
         const { publicKey, privateKey, address } = account;
@@ -404,9 +410,10 @@ describe('UserInputEventController', () => {
           controller.eventId,
           transactionRequest,
         );
-        expect(upsertTransactionRequestSpy).toHaveBeenCalledWith(
-          transactionRequest,
-        );
+        expect(upsertTransactionRequestSpy).toHaveBeenCalledWith({
+          ...transactionRequest,
+          selectedFeeToken: feeToken,
+        });
       },
     );
 
@@ -430,33 +437,37 @@ describe('UserInputEventController', () => {
         transactionRequest,
         {
           errors: {
-            fees: `Not enough ${feeToken} to pay for fee`,
+            fees: `Not enough ${feeToken} to pay for fee, switching back to ${transactionRequest.selectedFeeToken}`,
           },
         },
       );
     });
 
-    it('updates the transaction request with an general error message if other error was thrown.', async () => {
+    it('rollback the transaction request and show a general error message if other error was thrown.', async () => {
       const {
         event,
-        hasSufficientFundsForFeeSpy,
         transactionRequest,
         updateExecuteTxnFlowSpy,
         upsertTransactionRequestSpy,
       } = await prepareHandleFeeTokenChange();
       // Simulate an error thrown to test the error handling
-      hasSufficientFundsForFeeSpy.mockRejectedValue(false);
+      upsertTransactionRequestSpy.mockRejectedValue(new Error('Failed!'));
+      const rollbackSnapshot = {
+        maxFee: transactionRequest.maxFee,
+        selectedFeeToken: transactionRequest.selectedFeeToken,
+        includeDeploy: transactionRequest.includeDeploy,
+        resourceBounds: [...transactionRequest.resourceBounds],
+      };
 
       const controller = createMockController(event);
       await controller.handleFeeTokenChange();
 
-      expect(upsertTransactionRequestSpy).not.toHaveBeenCalled();
       expect(updateExecuteTxnFlowSpy).toHaveBeenCalledWith(
         controller.eventId,
-        transactionRequest,
+        { ...transactionRequest, ...rollbackSnapshot },
         {
           errors: {
-            fees: `Fail to calculate the fees`,
+            fees: `Failed to calculate the fees, switching back to ${transactionRequest.selectedFeeToken}`,
           },
         },
       );

diff --git a/packages/starknet-snap/src/ui/controllers/user-input-event-controller.ts b/packages/starknet-snap/src/ui/controllers/user-input-event-controller.ts
@@ -130,8 +130,20 @@ export class UserInputEventController {
     return token.address;
   }
 
+  protected createRollbackSnapshot(
+    request: TransactionRequest,
+  ): Partial<TransactionRequest> {
+    return {
+      maxFee: request.maxFee,
+      selectedFeeToken: request.selectedFeeToken,
+      includeDeploy: request.includeDeploy,
+      resourceBounds: [...request.resourceBounds],
+    };
+  }
+
   protected async handleFeeTokenChange() {
     const request = this.context?.request as TransactionRequest;
+    const rollbackSnapshot = this.createRollbackSnapshot(request);
     const { addressIndex, calls, signer, chainId } = request;
     const feeToken = (this.event as InputChangeEvent)
       .value as unknown as FeeToken;
@@ -192,13 +204,17 @@ export class UserInputEventController {
     } catch (error) {
       const errorMessage =
         error instanceof InsufficientFundsError
-          ? `Not enough ${feeToken} to pay for fee`
-          : 'Fail to calculate the fees';
+          ? `Not enough ${feeToken} to pay for fee, switching back to ${request.selectedFeeToken}`
+          : `Failed to calculate the fees, switching back to ${request.selectedFeeToken}`;
 
       // On failure, display ExecuteTxnUI with an error message
-      await updateExecuteTxnFlow(this.eventId, request, {
-        errors: { fees: errorMessage },
-      });
+      await updateExecuteTxnFlow(
+        this.eventId,
+        { ...request, ...rollbackSnapshot },
+        {
+          errors: { fees: errorMessage },
+        },
+      );
     }
   }
 }
diff --git a/packages/starknet-snap/src/ui/fragments/FeeTokenSelector.tsx b/packages/starknet-snap/src/ui/fragments/FeeTokenSelector.tsx
@@ -3,6 +3,7 @@ import {
   Field,
   Form,
   Selector,
+  Text,
   SelectorOption,
   type SnapComponent,
 } from '@metamask/snaps-sdk/jsx';
@@ -47,6 +48,10 @@ export const FeeTokenSelector: SnapComponent<FeeTokenSelectorProps> = ({
           </SelectorOption>
         </Selector>
       </Field>
+      <Text color="muted">
+        If the chosen token has no funds, the system will automatically switch
+        to a funded token.
+      </Text>
     </Form>
   );
 };