Skip to content

Commit

Permalink
fix: signatory order (#3109)
Browse files Browse the repository at this point in the history
* fix: correct sorting of signatories

* fix: correct sorting of signatories

* fix: correct sorting of signatories

* fix: correct sorting of signatories
  • Loading branch information
johnthecat authored Feb 4, 2025
1 parent c8a873c commit 0e9790c
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 29 deletions.
1 change: 1 addition & 0 deletions src/renderer/entities/multisig/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export * from './lib';
export * from './api';
export { multisigsModel } from './model/multisigs-model';
export { multisigUtils } from './lib/mulitisigs-utils';
28 changes: 28 additions & 0 deletions src/renderer/entities/multisig/lib/mulitisigs-utils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {
AccountType,
type Address,
type Chain,
ChainOptions,
CryptoType,
Expand All @@ -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;
}
Expand Down
11 changes: 7 additions & 4 deletions src/renderer/entities/transaction/lib/extrinsicService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -651,9 +652,11 @@ export const wrapAsMulti = <T extends Transaction = Transaction>({
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,
Expand Down
16 changes: 3 additions & 13 deletions src/renderer/pages/Operations/components/modals/ApproveTx.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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<Address[]>((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 {
Expand All @@ -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,
Expand Down
18 changes: 6 additions & 12 deletions src/renderer/pages/Operations/model/reject-model.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -82,13 +82,7 @@ sample({
},
filter: ({ account }) => nonNullable(account),
fn: ({ account, wrappedTx }, { signerAccountId, chain, tx }) => {
const otherSignatories = account!.signatories.reduce<Address[]>((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({
Expand All @@ -97,7 +91,7 @@ sample({
threshold: account!.threshold,
accountId: account!.accountId,
transaction: wrappedTx.wrappedTx,
otherSignatories: sortBy(otherSignatories),
otherSignatories,
tx,
});
}
Expand All @@ -106,7 +100,7 @@ sample({
chain,
signerAccountId,
threshold: account!.threshold,
otherSignatories: sortBy(otherSignatories),
otherSignatories,
tx,
});
},
Expand Down

0 comments on commit 0e9790c

Please sign in to comment.