From 0e9790c55ad0b070c5f3138a8cc1f8f4dffb9f23 Mon Sep 17 00:00:00 2001 From: Sergey Zhuravlev Date: Tue, 4 Feb 2025 22:46:54 +0100 Subject: [PATCH] fix: signatory order (#3109) * fix: correct sorting of signatories * fix: correct sorting of signatories * fix: correct sorting of signatories * fix: correct sorting of signatories --- src/renderer/entities/multisig/index.ts | 1 + .../entities/multisig/lib/mulitisigs-utils.ts | 28 +++++++++++++++++++ .../transaction/lib/extrinsicService.ts | 11 +++++--- .../components/modals/ApproveTx.tsx | 16 ++--------- .../pages/Operations/model/reject-model.ts | 18 ++++-------- 5 files changed, 45 insertions(+), 29 deletions(-) diff --git a/src/renderer/entities/multisig/index.ts b/src/renderer/entities/multisig/index.ts index 58cd9a6a34..ce4168e534 100644 --- a/src/renderer/entities/multisig/index.ts +++ b/src/renderer/entities/multisig/index.ts @@ -1,3 +1,4 @@ export * from './lib'; export * from './api'; export { multisigsModel } from './model/multisigs-model'; +export { multisigUtils } from './lib/mulitisigs-utils'; diff --git a/src/renderer/entities/multisig/lib/mulitisigs-utils.ts b/src/renderer/entities/multisig/lib/mulitisigs-utils.ts index daeb2fb936..3a22821112 100644 --- a/src/renderer/entities/multisig/lib/mulitisigs-utils.ts +++ b/src/renderer/entities/multisig/lib/mulitisigs-utils.ts @@ -1,5 +1,6 @@ import { AccountType, + type Address, type Chain, ChainOptions, CryptoType, @@ -17,8 +18,35 @@ export const multisigUtils = { isFlexibleMultisigSupported, buildMultisigAccount, buildFlexibleMultisigAccount, + getOtherSignatories, }; +function getOtherSignatories( + account: MultisigAccount | FlexibleMultisigAccount, + signer: AccountId | Address, + addressPrefix: number, +) { + const signerAddress = toAddress(signer, { prefix: addressPrefix }); + + return ( + Array.from(account.signatories) + /** + * Public keys of signers' wallets are compared byte-for-byte and sorted + * ascending before being used to generate the multisig address. For + * example, consider the scenario with three addresses, A, B, and C, + * starting with 5FUGT, 5HMfS, and 5GhKJ. If we build the ABC multisig + * with the accounts in that specific order (i.e. first A, then B, and C), + * the real order of the accounts in the multisig will be ACB. If, in the + * Extrinsic tab, we initiate a multisig call with C, the order of the + * other signatories will be first A, then B. If we put first B, then A, + * the transaction will fail. + */ + .sort((a, b) => a.accountId.localeCompare(b.accountId)) + .map((s) => toAddress(s.accountId, { prefix: addressPrefix })) + .filter((address) => address !== signerAddress) + ); +} + function isMultisigSupported(chain: Chain) { return chain.options?.includes(ChainOptions.MULTISIG) ?? false; } diff --git a/src/renderer/entities/transaction/lib/extrinsicService.ts b/src/renderer/entities/transaction/lib/extrinsicService.ts index 80c62ee6e1..d85420f21e 100644 --- a/src/renderer/entities/transaction/lib/extrinsicService.ts +++ b/src/renderer/entities/transaction/lib/extrinsicService.ts @@ -8,10 +8,11 @@ import { defineMethod, methods, } from '@substrate/txwrapper-polkadot'; -import { sortBy, zipWith } from 'lodash'; +import { zipWith } from 'lodash'; import { type MultisigTxWrapper, type ProxyTxWrapper, type Transaction, TransactionType } from '@/shared/core'; import { toAddress } from '@/shared/lib/utils'; +import { multisigUtils } from '@/entities/multisig'; import { DEFAULT_FEE_ASSET_ITEM } from './common/constants'; import { getMaxWeight, hasDestWeight, isControllerMissing, isOldMultisigPallet } from './common/utils'; @@ -651,9 +652,11 @@ export const wrapAsMulti = ({ console.log(`🟡 ${transaction.type} - not enough data to construct Extrinsic`); } - const otherSignatories = sortBy(txWrapper.multisigAccount.signatories, 'accountId') - .filter(({ accountId }) => accountId !== txWrapper.signer.accountId) - .map(({ accountId }) => toAddress(accountId, { prefix: addressPrefix })); + const otherSignatories = multisigUtils.getOtherSignatories( + txWrapper.multisigAccount, + txWrapper.signer.accountId, + addressPrefix, + ); return { chainId: transaction.chainId, diff --git a/src/renderer/pages/Operations/components/modals/ApproveTx.tsx b/src/renderer/pages/Operations/components/modals/ApproveTx.tsx index e4a5ba936b..b69ede3bb8 100644 --- a/src/renderer/pages/Operations/components/modals/ApproveTx.tsx +++ b/src/renderer/pages/Operations/components/modals/ApproveTx.tsx @@ -23,7 +23,7 @@ import { Button } from '@/shared/ui'; import { Modal } from '@/shared/ui-kit'; import { balanceModel, balanceUtils } from '@/entities/balance'; import { OperationTitle } from '@/entities/chain'; -import { useMultisigEvent } from '@/entities/multisig'; +import { multisigUtils, useMultisigEvent } from '@/entities/multisig'; import { networkModel } from '@/entities/network'; import { operationDetailsUtils } from '@/entities/operations'; import { priceProviderModel } from '@/entities/price'; @@ -154,17 +154,7 @@ const ApproveTxModal = ({ tx, account, api, chain, children }: Props) => { const getMultisigTx = (signer: Address): Transaction => { const signerAddress = toAddress(signer, { prefix: chain?.addressPrefix }); - - const otherSignatories = account.signatories.reduce((acc, s) => { - const signatoryAddress = toAddress(s.accountId, { prefix: chain?.addressPrefix }); - - if (signerAddress !== signatoryAddress) { - acc.push(signatoryAddress); - } - - return acc; - }, []); - + const otherSignatories = multisigUtils.getOtherSignatories(account, signer, chain.addressPrefix); const hasCallData = tx.callData && validateCallData(tx.callData, tx.callHash); return { @@ -173,7 +163,7 @@ const ApproveTxModal = ({ tx, account, api, chain, children }: Props) => { type: hasCallData ? TransactionType.MULTISIG_AS_MULTI : TransactionType.MULTISIG_APPROVE_AS_MULTI, args: { threshold: account.threshold, - otherSignatories: otherSignatories.sort(), + otherSignatories, maxWeight: txWeight, maybeTimepoint: { height: tx.blockCreated, diff --git a/src/renderer/pages/Operations/model/reject-model.ts b/src/renderer/pages/Operations/model/reject-model.ts index 672fb3baaf..63f338c510 100644 --- a/src/renderer/pages/Operations/model/reject-model.ts +++ b/src/renderer/pages/Operations/model/reject-model.ts @@ -1,13 +1,13 @@ import { combine, createEvent, createStore, sample } from 'effector'; import { createGate } from 'effector-react'; -import { sortBy } from 'lodash'; import { type FlexibleMultisigTransactionDS, type MultisigTransactionDS } from '@/shared/api/storage'; -import { type Address, type Chain, type Transaction } from '@/shared/core'; -import { nonNullable, nullable, toAddress } from '@/shared/lib/utils'; +import { type Chain, type Transaction } from '@/shared/core'; +import { nonNullable, nullable } from '@/shared/lib/utils'; import { type AccountId } from '@/shared/polkadotjs-schemas'; import { createTxStore } from '@/shared/transactions'; import { type AnyAccount } from '@/domains/network'; +import { multisigUtils } from '@/entities/multisig'; import { networkModel } from '@/entities/network'; import { transactionBuilder } from '@/entities/transaction'; import { accountUtils, walletModel } from '@/entities/wallet'; @@ -82,13 +82,7 @@ sample({ }, filter: ({ account }) => nonNullable(account), fn: ({ account, wrappedTx }, { signerAccountId, chain, tx }) => { - const otherSignatories = account!.signatories.reduce((acc, s) => { - if (signerAccountId !== s.accountId) { - acc.push(toAddress(s.accountId, { prefix: chain?.addressPrefix })); - } - - return acc; - }, []); + const otherSignatories = multisigUtils.getOtherSignatories(account!, signerAccountId, chain.addressPrefix); if (accountUtils.isFlexibleMultisigAccount(account!) && wrappedTx) { return transactionBuilder.buildRejectFlexibleMultisigTx({ @@ -97,7 +91,7 @@ sample({ threshold: account!.threshold, accountId: account!.accountId, transaction: wrappedTx.wrappedTx, - otherSignatories: sortBy(otherSignatories), + otherSignatories, tx, }); } @@ -106,7 +100,7 @@ sample({ chain, signerAccountId, threshold: account!.threshold, - otherSignatories: sortBy(otherSignatories), + otherSignatories, tx, }); },