Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: signatory order #3109

Merged
merged 4 commits into from
Feb 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading