multisignature attribute is set twice on multisignature wallet

[dated]

The multisignature attribute is currently set twice on the multisignature wallet, in the applyToSender as well as in the applyToRecipient method. Also the name recipientWallet is a bit confusing in this context.

core/packages/core-transactions/src/handlers/multi-signature.ts

Lines 106 to 118 in 6dae232

	public async applyToSender(
	transaction: Interfaces.ITransaction,
	walletManager: State.IWalletManager,
	): Promise<void> {
	await super.applyToSender(transaction, walletManager);
	// Create the multi sig wallet
	if (transaction.data.version >= 2) {
	walletManager
	.findByAddress(Identities.Address.fromMultiSignatureAsset(transaction.data.asset.multiSignature))
	.setAttribute("multiSignature", transaction.data.asset.multiSignature);
	}
	}

core/packages/core-transactions/src/handlers/multi-signature.ts

Lines 129 to 141 in 6dae232

	public async applyToRecipient(
	transaction: Interfaces.ITransaction,
	walletManager: State.IWalletManager,
	): Promise<void> {
	const { data }: Interfaces.ITransaction = transaction;
	if (data.version >= 2) {
	const recipientWallet: State.IWallet = walletManager.findByAddress(
	Identities.Address.fromMultiSignatureAsset(data.asset.multiSignature),
	);
	recipientWallet.setAttribute("multiSignature", transaction.data.asset.multiSignature);
	}
	}

[ghost]

Thanks for opening this issue! A maintainer will review this in the next few days and explicitly select labels so you know what's going on.

If no reviewer appears after a week, a reminder will be sent out.

[air1one]

Looks like it was introduced by @vasild here 090bc2d

Before only applyToRecipient was implemented, did you do this on purpose @vasild ?

[vasild]

No, I was probably confused by the recipientWallet naming and did not realize that the recipient and the sender are the same in this case (correct me if I am wrong, please).

Is the fact that we set the attribute twice (to the same value) causing any problems?

[dated]

Is the fact that we set the attribute twice (to the same value) causing any problems?

No, afaik. But If there is no reason it should probably be avoided.

[dated]

Fyi i tried removing it from applyToSender but that caused some problems, i didn't investigate further.

For reference #3263

[spkjp]

Afaik it resolves some problems for pool wallets, because applyToRecipient/applyToSender are called under different conditions. Without applying it in both cases, the pool wallet may end up without an asset.

[dated]

Sounds like it is necessary then 👍

[ghost]

This issue has been closed. If you wish to re-open it please provide additional information.