refactor: removed weird parts of signatories form in multisig creation (

#2605)
novasamatech · Nov 8, 2024 · 5c798f2 · 5c798f2
1 parent 49bb049
commit 5c798f2
Show file tree

Hide file tree

Showing 11 changed files with 123 additions and 106 deletions.
diff --git a/src/renderer/widgets/CreateWallet/model/__tests__/flow-model.test.ts b/src/renderer/widgets/CreateWallet/model/__tests__/flow-model.test.ts
@@ -44,11 +44,11 @@ describe('widgets/CreateWallet/model/form-model', () => {
         .set(walletModel.$allWallets, [initiatorWallet, signerWallet]),
     });
 
-    await allSettled(signatoryModel.events.signatoriesChanged, {
+    await allSettled(signatoryModel.events.changeSignatory, {
       scope,
       params: { index: 0, name: signerWallet.name, address: toAddress(signerWallet.accounts[0].accountId) },
     });
-    await allSettled(signatoryModel.events.signatoriesChanged, {
+    await allSettled(signatoryModel.events.changeSignatory, {
       scope,
       params: { index: 1, name: 'Alice', address: '5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY' },
     });

diff --git a/src/renderer/widgets/CreateWallet/model/__tests__/form-model.test.ts b/src/renderer/widgets/CreateWallet/model/__tests__/form-model.test.ts
@@ -64,14 +64,14 @@ describe('widgets/CreateWallet/model/form-model', () => {
         .set(networkModel.$chains, { '0x00': testChain })
         .set(networkModel.$connectionStatuses, { '0x00': ConnectionStatus.CONNECTED })
         .set(walletModel.$allWallets, [initiatorWallet, signerWallet, multisigWallet])
-        .set(signatoryModel.$signatories, new Map([])),
+        .set(signatoryModel.$signatories, []),
     });
 
-    await allSettled(signatoryModel.events.signatoriesChanged, {
+    await allSettled(signatoryModel.events.changeSignatory, {
       scope,
       params: { index: 0, name: 'test', address: toAddress(signerWallet.accounts[0].accountId) },
     });
-    await allSettled(signatoryModel.events.signatoriesChanged, {
+    await allSettled(signatoryModel.events.changeSignatory, {
       scope,
       params: { index: 1, name: 'Alice', address: toAddress(signatoryWallet.accounts[0].accountId) },
     });
@@ -106,15 +106,15 @@ describe('widgets/CreateWallet/model/form-model', () => {
         .set(networkModel.$chains, { '0x00': testChain })
         .set(networkModel.$connectionStatuses, { '0x00': ConnectionStatus.CONNECTED })
         .set(walletModel.$allWallets, [initiatorWallet, signerWallet, multisigWallet])
-        .set(signatoryModel.$signatories, new Map([])),
+        .set(signatoryModel.$signatories, []),
     });
 
     await allSettled(formModel.$createMultisigForm.fields.chain.onChange, { scope, params: testChain });
-    await allSettled(signatoryModel.events.signatoriesChanged, {
+    await allSettled(signatoryModel.events.changeSignatory, {
       scope,
       params: { index: 0, name: 'test', address: toAddress(signerWallet.accounts[0].accountId) },
     });
-    await allSettled(signatoryModel.events.signatoriesChanged, {
+    await allSettled(signatoryModel.events.changeSignatory, {
       scope,
       params: { index: 1, name: 'Alice', address: toAddress(signatoryWallet.accounts[0].accountId) },
     });

diff --git a/src/renderer/widgets/CreateWallet/model/__tests__/mock.ts b/src/renderer/widgets/CreateWallet/model/__tests__/mock.ts
@@ -19,6 +19,7 @@ export const testApi = {
 export const testChain = {
   name: 'test-chain',
   chainId: '0x00',
+  assets: [{ assetId: 0 }],
   options: [ChainOptions.MULTISIG],
   type: ChainType.SUBSTRATE,
 } as unknown as Chain;

diff --git a/src/renderer/widgets/CreateWallet/model/__tests__/signatory-model.test.ts b/src/renderer/widgets/CreateWallet/model/__tests__/signatory-model.test.ts
@@ -13,59 +13,59 @@ describe('widgets/CreateWallet/model/signatory-model', () => {
 
   test('should correctly add signatories', async () => {
     const scope = fork({
-      values: new Map().set(signatoryModel.$signatories, new Map([])),
+      values: new Map().set(signatoryModel.$signatories, []),
     });
 
-    expect(scope.getState(signatoryModel.$signatories).size).toEqual(0);
+    expect(scope.getState(signatoryModel.$signatories).length).toEqual(0);
 
-    await allSettled(signatoryModel.events.signatoriesChanged, {
+    await allSettled(signatoryModel.events.addSignatory, {
       scope,
-      params: { index: 1, name: 'Alice', address: toAddress(signerWallet.accounts[0].accountId) },
+      params: { name: 'Alice', address: toAddress(signerWallet.accounts[0].accountId) },
     });
 
-    await allSettled(signatoryModel.events.signatoriesChanged, {
+    await allSettled(signatoryModel.events.addSignatory, {
       scope,
-      params: { index: 0, name: 'test', address: toAddress(signerWallet.accounts[0].accountId) },
+      params: { name: 'test', address: toAddress(signerWallet.accounts[0].accountId) },
     });
 
-    expect(scope.getState(signatoryModel.$signatories).size).toEqual(2);
+    expect(scope.getState(signatoryModel.$signatories).length).toEqual(2);
   });
 
   test('should correctly delete signatories', async () => {
     const scope = fork({
-      values: new Map().set(signatoryModel.$signatories, new Map([])),
+      values: new Map().set(signatoryModel.$signatories, []),
     });
 
-    expect(scope.getState(signatoryModel.$signatories).size).toEqual(0);
+    expect(scope.getState(signatoryModel.$signatories).length).toEqual(0);
 
-    await allSettled(signatoryModel.events.signatoriesChanged, {
+    await allSettled(signatoryModel.events.changeSignatory, {
       scope,
       params: { index: 0, name: 'test', address: toAddress(signerWallet.accounts[0].accountId) },
     });
 
-    expect(scope.getState(signatoryModel.$signatories).size).toEqual(1);
+    expect(scope.getState(signatoryModel.$signatories).length).toEqual(1);
 
-    await allSettled(signatoryModel.events.signatoryDeleted, {
+    await allSettled(signatoryModel.events.deleteSignatory, {
       scope,
       params: 0,
     });
 
-    expect(scope.getState(signatoryModel.$signatories).size).toEqual(0);
+    expect(scope.getState(signatoryModel.$signatories).length).toEqual(0);
   });
 
   test('should have correct value for $ownSignatoryWallets', async () => {
     const scope = fork({
       values: new Map().set(walletModel.$allWallets, [initiatorWallet, signerWallet]),
     });
 
-    await allSettled(signatoryModel.events.signatoriesChanged, {
+    await allSettled(signatoryModel.events.changeSignatory, {
       scope,
       params: { index: 1, name: 'Alice', address: toAddress(signatoryWallet.accounts[0].accountId) },
     });
 
     expect(scope.getState(signatoryModel.$ownedSignatoriesWallets)?.length).toEqual(0);
 
-    await allSettled(signatoryModel.events.signatoriesChanged, {
+    await allSettled(signatoryModel.events.changeSignatory, {
       scope,
       params: { index: 0, name: 'test', address: toAddress(signerWallet.accounts[0].accountId) },
     });

diff --git a/src/renderer/widgets/CreateWallet/model/flow-model.ts b/src/renderer/widgets/CreateWallet/model/flow-model.ts
@@ -126,7 +126,7 @@ const $transaction = combine(
   ({ apis, chain, remarkTx, signatories, signer, threshold, multisigAccountId }) => {
     if (!chain || !remarkTx || !signer) return undefined;
 
-    const signatoriesWrapped = Array.from(signatories.values()).map((s) => ({
+    const signatoriesWrapped = signatories.map((s) => ({
       accountId: toAccountId(s.address),
       adress: s.address,
     }));
@@ -412,9 +412,9 @@ sample({
     step: $step,
     hiddenMultisig: formModel.$hiddenMultisig,
   },
-  filter: ({ step }, results) => {
+  filter: ({ step, hiddenMultisig }, results) => {
     const isSubmitStep = isStep(step, Step.SUBMIT);
-    const isNonNullable = nonNullable(formModel.$hiddenMultisig);
+    const isNonNullable = nonNullable(hiddenMultisig);
     const isSuccessResult = results[0]?.result === ExtrinsicResult.SUCCESS;
 
     return isSubmitStep && isNonNullable && isSuccessResult;

diff --git a/src/renderer/widgets/CreateWallet/model/form-model.ts b/src/renderer/widgets/CreateWallet/model/form-model.ts
@@ -50,7 +50,7 @@ const $multisigAccountId = combine(
     const cryptoType = networkUtils.isEthereumBased(chain.options) ? CryptoType.ETHEREUM : CryptoType.SR25519;
 
     return accountUtils.getMultisigAccountId(
-      Array.from(signatories.values()).map((s) => toAccountId(s.address)),
+      signatories.map((s) => toAccountId(s.address)),
       threshold,
       cryptoType,
     );
@@ -113,7 +113,7 @@ const $availableAccounts = combine(
 );
 
 sample({
-  clock: signatoryModel.events.signatoryDeleted,
+  clock: signatoryModel.events.deleteSignatory,
   target: $createMultisigForm.fields.threshold.reset,
 });
 

diff --git a/src/renderer/widgets/CreateWallet/model/signatory-model.ts b/src/renderer/widgets/CreateWallet/model/signatory-model.ts
@@ -1,28 +1,46 @@
 import { combine, createEffect, createEvent, createStore, sample } from 'effector';
+import { produce } from 'immer';
 
-import { type Wallet } from '@/shared/core';
+import { type Address, type Wallet } from '@/shared/core';
 import { toAccountId } from '@/shared/lib/utils';
 import { walletModel, walletUtils } from '@/entities/wallet';
 import { balanceSubModel } from '@/features/balances';
 import { type SignatoryInfo } from '../lib/types';
 
-const signatoriesChanged = createEvent<SignatoryInfo>();
-const signatoryDeleted = createEvent<number>();
+const addSignatory = createEvent<Omit<SignatoryInfo, 'index'>>();
+const changeSignatory = createEvent<SignatoryInfo>();
+const deleteSignatory = createEvent<number>();
 const getSignatoriesBalance = createEvent<Wallet[]>();
 
-const $signatories = createStore<Map<number, Omit<SignatoryInfo, 'index'>>>(new Map([[0, { name: '', address: '' }]]));
+const $signatories = createStore<Omit<SignatoryInfo, 'index'>[]>([{ name: '', address: '' }]);
 const $hasDuplicateSignatories = combine($signatories, (signatories) => {
-  const signatoriesArray = Array.from(signatories.values()).map(({ address }) => toAccountId(address));
+  const existingKeys: Set<Address> = new Set();
 
-  return new Set(signatoriesArray).size !== signatoriesArray.length;
+  for (const signatory of signatories) {
+    if (signatory.address.length === 0) {
+      continue;
+    }
+
+    if (existingKeys.has(signatory.address)) {
+      return true;
+    }
+
+    existingKeys.add(signatory.address);
+  }
+
+  return false;
+});
+
+const $hasEmptySignatories = combine($signatories, (signatories) => {
+  return signatories.map(({ address }) => address).includes('');
 });
 
 const $ownedSignatoriesWallets = combine(
   { wallets: walletModel.$wallets, signatories: $signatories },
   ({ wallets, signatories }) =>
     walletUtils.getWalletsFilteredAccounts(wallets, {
       walletFn: (w) => !walletUtils.isWatchOnly(w) && !walletUtils.isMultisig(w),
-      accountFn: (a) => Array.from(signatories.values()).some((s) => toAccountId(s.address) === a.accountId),
+      accountFn: (a) => signatories.some((s) => toAccountId(s.address) === a.accountId),
     }) || [],
 );
 
@@ -38,28 +56,39 @@ sample({
 });
 
 sample({
-  clock: signatoriesChanged,
+  clock: addSignatory,
   source: $signatories,
-  fn: (signatories, { index, name, address }) => {
-    // we need to return new Map to trigger re-render
-    const newMap = new Map(signatories);
-    newMap.set(index, { name, address });
+  fn: (signatories, { name, address }) => {
+    return produce(signatories, (draft) => {
+      draft.push({ name, address });
+    });
+  },
+  target: $signatories,
+});
 
-    return newMap;
+sample({
+  clock: changeSignatory,
+  source: $signatories,
+  fn: (signatories, { index, name, address }) => {
+    return produce(signatories, (draft) => {
+      if (index >= draft.length) {
+        draft.push({ name, address });
+      } else {
+        draft[index] = { name, address };
+      }
+    });
   },
   target: $signatories,
 });
 
 sample({
-  clock: signatoryDeleted,
+  clock: deleteSignatory,
   source: $signatories,
-  filter: (signatories, index) => signatories.has(index),
+  filter: (signatories, index) => signatories.length > index,
   fn: (signatories, index) => {
-    // we need to return new Map to trigger re-render
-    const newMap = new Map(signatories);
-    newMap.delete(index);
-
-    return newMap;
+    return produce(signatories, (draft) => {
+      draft.splice(index, 1);
+    });
   },
   target: $signatories,
 });
@@ -68,9 +97,11 @@ export const signatoryModel = {
   $signatories,
   $ownedSignatoriesWallets,
   $hasDuplicateSignatories,
+  $hasEmptySignatories,
   events: {
-    signatoriesChanged,
-    signatoryDeleted,
+    addSignatory,
+    changeSignatory,
+    deleteSignatory,
     getSignatoriesBalance,
   },
 };
diff --git a/src/renderer/widgets/CreateWallet/ui/MultisigWallet/ConfirmationStep.tsx b/src/renderer/widgets/CreateWallet/ui/MultisigWallet/ConfirmationStep.tsx
@@ -18,8 +18,7 @@ import { WalletItem } from './components/WalletItem';
 
 export const ConfirmationStep = () => {
   const { t } = useI18n();
-  const signatoriesMap = useUnit(signatoryModel.$signatories);
-  const signatories = Array.from(signatoriesMap.values());
+  const signatories = useUnit(signatoryModel.$signatories);
   const signerWallet = useUnit(flowModel.$signerWallet);
   const signer = useUnit(flowModel.$signer);
   const {

diff --git a/src/renderer/widgets/CreateWallet/ui/MultisigWallet/SelectSignatoriesThreshold.tsx b/src/renderer/widgets/CreateWallet/ui/MultisigWallet/SelectSignatoriesThreshold.tsx
@@ -1,6 +1,6 @@
 import { useForm } from 'effector-forms';
 import { useUnit } from 'effector-react';
-import { type FormEvent, useState } from 'react';
+import { type FormEvent, useMemo, useState } from 'react';
 
 import { useI18n } from '@/shared/i18n';
 import { Alert, Button, InputHint, Select, SmallTitleText } from '@/shared/ui';
@@ -33,8 +33,7 @@ export const SelectSignatoriesThreshold = () => {
   const { t } = useI18n();
 
   const [hasClickedNext, setHasClickedNext] = useState(false);
-  const signatoriesMap = useUnit(signatoryModel.$signatories);
-  const signatories = Array.from(signatoriesMap.values());
+  const signatories = useUnit(signatoryModel.$signatories);
   const fakeTx = useUnit(flowModel.$fakeTx);
   const {
     fields: { threshold, chain },
@@ -44,17 +43,18 @@ export const SelectSignatoriesThreshold = () => {
   const hiddenMultisig = useUnit(formModel.$hiddenMultisig);
   const ownedSignatoriesWallets = useUnit(signatoryModel.$ownedSignatoriesWallets);
   const hasDuplicateSignatories = useUnit(signatoryModel.$hasDuplicateSignatories);
-  const thresholdOptions = getThresholdOptions(signatories.length - 1);
+  const hasEmptySignatories = useUnit(signatoryModel.$hasEmptySignatories);
+
+  const thresholdOptions = useMemo(() => getThresholdOptions(signatories.length - 1), [signatories.length]);
 
   const hasOwnedSignatory = !!ownedSignatoriesWallets && ownedSignatoriesWallets?.length > 0;
   const hasEnoughSignatories = signatories.length >= MIN_THRESHOLD;
-  const hasEmptySignatory = signatories.map(({ address }) => address).includes('');
   const isThresholdValid = threshold.value >= MIN_THRESHOLD && threshold.value <= signatories.length;
   const canSubmit =
     hasOwnedSignatory &&
     hasEnoughSignatories &&
     !multisigAlreadyExists &&
-    !hasEmptySignatory &&
+    !hasEmptySignatories &&
     isThresholdValid &&
     !hasDuplicateSignatories;
 
@@ -85,9 +85,9 @@ export const SelectSignatoriesThreshold = () => {
         {t('createMultisigAccount.multisigStep', { step: 2 })}{' '}
         {t('createMultisigAccount.signatoryThresholdDescription')}
       </SmallTitleText>
-      <div className="flex flex-col gap-y-4 px-5 py-4">
+      <div className="flex flex-col gap-4 px-5 py-4">
         <SelectSignatories />
-        <div className="flex items-end gap-x-4">
+        <div className="flex items-end gap-4">
           <Alert
             active={hasClickedNext && !hasOwnedSignatory && signatories.length > 0}
             title={t('createMultisigAccount.noOwnSignatoryTitle')}
@@ -105,7 +105,7 @@ export const SelectSignatoriesThreshold = () => {
           </Alert>
 
           <Alert
-            active={hasClickedNext && hasEmptySignatory}
+            active={hasClickedNext && hasEmptySignatories}
             title={t('createMultisigAccount.notEmptySignatoryTitle')}
             variant="error"
           >
@@ -120,6 +120,7 @@ export const SelectSignatoriesThreshold = () => {
             selectedId={threshold.value.toString()}
             options={thresholdOptions}
             invalid={threshold.hasError()}
+            disabled={thresholdOptions.length === 0}
             position={thresholdOptions.length > 2 ? 'up' : 'down'}
             onChange={({ value }) => threshold.onChange(value)}
           />