From 82e2f32c34eedb72b530f87718d476e181a733e4 Mon Sep 17 00:00:00 2001 From: Toms Date: Tue, 11 Nov 2025 15:44:39 +0200 Subject: [PATCH] =?UTF-8?q?feat:=20=F0=9F=8E=B8=20create=20asProposal=20me?= =?UTF-8?q?thods?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/base/PolymeshTransaction.ts | 11 +- src/base/PolymeshTransactionBase.ts | 83 ++++-- src/base/PolymeshTransactionBatch.ts | 11 +- src/base/__tests__/PolymeshTransactionBase.ts | 274 ++++++++++++++++++ src/base/types.ts | 4 +- 5 files changed, 359 insertions(+), 24 deletions(-) diff --git a/src/base/PolymeshTransaction.ts b/src/base/PolymeshTransaction.ts index f11ff11516..6e8c6c6dc8 100644 --- a/src/base/PolymeshTransaction.ts +++ b/src/base/PolymeshTransaction.ts @@ -104,11 +104,16 @@ export class PolymeshTransaction< // eslint-disable-next-line require-jsdoc protected composeTx(): SubmittableExtrinsic<'promise', ISubmittableResult> { - const { transaction, args } = this; + const baseTx = this.getBaseTransaction(); + + return this.wrapProposalIfNeeded(baseTx); + } - const tx = transaction(...args); + // eslint-disable-next-line require-jsdoc + protected getBaseTransaction(): SubmittableExtrinsic<'promise', ISubmittableResult> { + const { transaction, args } = this; - return this.wrapProposalIfNeeded(tx); + return transaction(...args); } // eslint-disable-next-line require-jsdoc diff --git a/src/base/PolymeshTransactionBase.ts b/src/base/PolymeshTransactionBase.ts index bd77888756..ea5e5a8e32 100644 --- a/src/base/PolymeshTransactionBase.ts +++ b/src/base/PolymeshTransactionBase.ts @@ -557,24 +557,27 @@ export abstract class PolymeshTransactionBase< /** * Retrieve a breakdown of the fees required to run this transaction, as well as the Account responsible for paying them * + * @param asProposal - When `true` (default), treats the transaction as a MultiSig proposal if the signing account is a MultiSig signer. + * When `false`, treats the transaction as a direct transaction from the signing account, ignoring the MultiSig. + * * @note these values might be inaccurate if the transaction is run at a later time. This can be due to a governance vote or other * chain related factors (like modifications to a specific subsidizer relationship or a chain upgrade) */ - public async getTotalFees(): Promise { + public async getTotalFees(asProposal = true): Promise { const { signingAddress } = this; - const composedTx = this.composeTx(); + const composedTx = this.composeTxForFees(asProposal); const paymentInfoPromise = composedTx.paymentInfo(signingAddress); const protocol = await this.getProtocolFees(); - const [payingAccount, { partialFee }] = await Promise.all([ - this.getPayingAccount(), + const payingAccount = await this.getPayingAccount(asProposal); + + const [{ partialFee }, { free: balance }] = await Promise.all([ paymentInfoPromise, + payingAccount.account.getBalance(), ]); - - const { free: balance } = await payingAccount.account.getBalance(); const gas = balanceToBigNumber(partialFee); return { @@ -850,13 +853,18 @@ export abstract class PolymeshTransactionBase< /** * Returns a representation intended for offline signers. * + * @param metadata - Additional information attached to the payload, such as IDs or memos about the transaction + * @param asProposal - When `true` (default), treats the transaction as a MultiSig proposal if the signing account is a MultiSig signer. + * When `false`, treats the transaction as a direct transaction from the signing account, ignoring the MultiSig. + * * @note Usually `.run()` should be preferred due to is simplicity. * * @note When using this method, details like account nonces, and transaction mortality require extra consideration. Generating a payload for offline sign implies asynchronicity. If using this API, be sure each procedure is created with the correct nonce, accounting for in flight transactions, and the lifetime is sufficient. * */ public async toSignablePayload( - metadata: Record = {} + metadata: Record = {}, + asProposal = true ): Promise { const { mortality, @@ -864,7 +872,7 @@ export abstract class PolymeshTransactionBase< context, context: { polymeshApi }, } = this; - const tx = this.composeTx(); + const tx = this.composeTxForFees(asProposal); const [tipHash, latestBlockNumber] = await Promise.all([ polymeshApi.rpc.chain.getFinalizedHead(), @@ -911,7 +919,7 @@ export abstract class PolymeshTransactionBase< rawPayload: rawSignerPayload.toRaw(), method: tx.toHex(), metadata, - multiSig: this.multiSig?.address ?? null, + multiSig: asProposal ? this.multiSig?.address ?? null : null, }; } @@ -928,11 +936,14 @@ export abstract class PolymeshTransactionBase< * Retrieve the Account that would pay fees for the transaction if it was run at this moment, as well as the total amount that can be * charged to it (allowance) in case of a subsidy * + * @param asProposal - When `true` (default), uses MultiSig payer if the signing account is a MultiSig signer. + * When `false`, uses the signing account directly, ignoring the MultiSig. + * * @note the paying Account might change if, before running the transaction, the caller Account enters (or leaves) * a subsidizer relationship. A governance vote or chain upgrade could also cause the value to change between the time * this method is called and the time the transaction is run */ - private async getPayingAccount(): Promise { + private async getPayingAccount(asProposal: boolean): Promise { const { paidForBy, multiSig, context } = this; if (paidForBy) { @@ -960,16 +971,26 @@ export abstract class PolymeshTransactionBase< } // For MultiSig the fees come from the creator's primary key - if (multiSig) { + // Only use MultiSig payer when asProposal is true + if (multiSig && asProposal) { const multiId = await multiSig.getPayer(); if (multiId) { - const { account } = await multiId.getPrimaryAccount(); - - return { - account, - type: PayingAccountType.MultiSigCreator, - }; + try { + const { account } = await multiId.getPrimaryAccount(); + + return { + account, + type: PayingAccountType.MultiSigCreator, + }; + } catch { + // If we can't get the primary account (e.g., it doesn't have an identity), + // fall back to using the MultiSig account directly + return { + type: PayingAccountType.Caller, + account: multiSig, + }; + } } else { return { type: PayingAccountType.Caller, @@ -1012,4 +1033,32 @@ export abstract class PolymeshTransactionBase< return tx; } + + /** + * @hidden + * + * Compose a transaction for fee calculation or payload generation, conditionally wrapping as a proposal + * + * @param asProposal - When `true` (default), wraps the transaction as a proposal if the signing account is a MultiSig signer. + * When `false`, returns the unwrapped transaction. + */ + protected composeTxForFees( + asProposal: boolean + ): SubmittableExtrinsic<'promise', ISubmittableResult> { + // Get the base transaction without wrapping + const baseTx = this.getBaseTransaction(); + + if (asProposal) { + return this.wrapProposalIfNeeded(baseTx); + } + + return baseTx; + } + + /** + * @hidden + * + * Get the base transaction without any proposal wrapping + */ + protected abstract getBaseTransaction(): SubmittableExtrinsic<'promise', ISubmittableResult>; } diff --git a/src/base/PolymeshTransactionBatch.ts b/src/base/PolymeshTransactionBatch.ts index 808660a5e5..26a46701c2 100644 --- a/src/base/PolymeshTransactionBatch.ts +++ b/src/base/PolymeshTransactionBatch.ts @@ -86,6 +86,13 @@ export class PolymeshTransactionBatch< * @hidden */ protected composeTx(): SubmittableExtrinsic<'promise', ISubmittableResult> { + const baseTx = this.getBaseTransaction(); + + return this.wrapProposalIfNeeded(baseTx); + } + + // eslint-disable-next-line require-jsdoc + protected getBaseTransaction(): SubmittableExtrinsic<'promise', ISubmittableResult> { const { context: { polymeshApi: { @@ -94,11 +101,9 @@ export class PolymeshTransactionBatch< }, } = this; - const tx = utility.batchAll( + return utility.batchAll( this.transactionData.map(({ transaction, args }) => transaction(...args)) ); - - return this.wrapProposalIfNeeded(tx); } /** diff --git a/src/base/__tests__/PolymeshTransactionBase.ts b/src/base/__tests__/PolymeshTransactionBase.ts index c3b819aeb4..fb1ff5c1e7 100644 --- a/src/base/__tests__/PolymeshTransactionBase.ts +++ b/src/base/__tests__/PolymeshTransactionBase.ts @@ -1554,6 +1554,172 @@ describe('Polymesh Transaction Base class', () => { expect(payingAccountData.account.address).toBe('0xdummy'); expect(payingAccountData.balance).toEqual(new BigNumber(100000)); }); + + it('should use MultiSig payer when asProposal is true and signing account is MultiSig signer', async () => { + const tx1 = dsMockUtils.createTxMock('asset', 'registerUniqueTicker', { + gas: rawGasFees[0]!, + }); + dsMockUtils.createTxMock('multiSig', 'createProposal'); + + const multiSig = entityMockUtils.getMultiSigInstance({ address: DUMMY_ACCOUNT_ID }); + const payerIdentity = entityMockUtils.getIdentityInstance({ did: 'payerDid' }); + const payerAccount = entityMockUtils.getAccountInstance({ address: 'payerAddress' }); + + multiSig.getPayer = jest.fn().mockResolvedValue(payerIdentity); + payerIdentity.getPrimaryAccount = jest.fn().mockResolvedValue({ account: payerAccount }); + payerAccount.getBalance = jest.fn().mockResolvedValue({ + free: new BigNumber(50000), + locked: new BigNumber(0), + total: new BigNumber(50000), + }); + + const args = tuple('TEST'); + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { fee, ...rest } = txSpec; + + const tx = new PolymeshTransaction( + { + ...rest, + transaction: tx1, + args, + resolver: undefined, + multiSig, + }, + context + ); + + // Test with explicit true to cover that path + const { payingAccountData } = await tx.getTotalFees(true); + + expect(payingAccountData.type).toBe(PayingAccountType.MultiSigCreator); + expect(payingAccountData.account.address).toBe('payerAddress'); + expect(multiSig.getPayer).toHaveBeenCalled(); + + // Test with default parameter (no argument) to cover default assignment in getPayingAccount + const { payingAccountData: payingAccountDataDefault } = await tx.getTotalFees(); + expect(payingAccountDataDefault.type).toBe(PayingAccountType.MultiSigCreator); + expect(payingAccountDataDefault.account.address).toBe('payerAddress'); + }); + + it('should use signing account when asProposal is false even if MultiSig is set', async () => { + const tx1 = dsMockUtils.createTxMock('asset', 'registerUniqueTicker', { + gas: rawGasFees[0]!, + }); + + const multiSig = entityMockUtils.getMultiSigInstance({ address: DUMMY_ACCOUNT_ID }); + + const args = tuple('TEST'); + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { fee, ...rest } = txSpec; + + const tx = new PolymeshTransaction( + { + ...rest, + transaction: tx1, + args, + resolver: undefined, + multiSig, + }, + context + ); + + // Mock createProposal to verify it's not called when asProposal is false + const createProposalMock = dsMockUtils.createTxMock('multiSig', 'createProposal'); + + const { payingAccountData } = await tx.getTotalFees(false); + + expect(payingAccountData.type).toBe(PayingAccountType.Caller); + expect(payingAccountData.account.address).toBe('0xdummy'); + // getPayer should not be called when asProposal is false + if (multiSig.getPayer) { + expect(multiSig.getPayer).not.toHaveBeenCalled(); + } + // createProposal should not be called when asProposal is false + expect(createProposalMock).not.toHaveBeenCalled(); + }); + + it('should fall back to MultiSig account when payer primary account has no identity', async () => { + const tx1 = dsMockUtils.createTxMock('asset', 'registerUniqueTicker', { + gas: rawGasFees[0]!, + }); + dsMockUtils.createTxMock('multiSig', 'createProposal'); + + const multiSig = entityMockUtils.getMultiSigInstance({ address: DUMMY_ACCOUNT_ID }); + const payerIdentity = entityMockUtils.getIdentityInstance({ did: 'payerDid' }); + + // Mock getPayer to return an identity + multiSig.getPayer = jest.fn().mockResolvedValue(payerIdentity); + + // Mock getPrimaryAccount to throw an error (simulating no identity on primary account) + payerIdentity.getPrimaryAccount = jest.fn().mockRejectedValue( + new PolymeshError({ + code: ErrorCode.DataUnavailable, + message: 'There is no Identity associated with this Account', + }) + ); + + const args = tuple('TEST'); + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { fee, ...rest } = txSpec; + + const tx = new PolymeshTransaction( + { + ...rest, + transaction: tx1, + args, + resolver: undefined, + multiSig, + }, + context + ); + + const { payingAccountData } = await tx.getTotalFees(true); + + // Should fall back to using MultiSig account directly + expect(payingAccountData.type).toBe(PayingAccountType.Caller); + expect(payingAccountData.account.address).toBe(DUMMY_ACCOUNT_ID); + expect(multiSig.getPayer).toHaveBeenCalled(); + expect(payerIdentity.getPrimaryAccount).toHaveBeenCalled(); + }); + + it('should use MultiSig account when payer is null', async () => { + const tx1 = dsMockUtils.createTxMock('asset', 'registerUniqueTicker', { + gas: rawGasFees[0]!, + }); + dsMockUtils.createTxMock('multiSig', 'createProposal'); + + const multiSig = entityMockUtils.getMultiSigInstance({ address: DUMMY_ACCOUNT_ID }); + + // Mock getPayer to return null (no payer set) + multiSig.getPayer = jest.fn().mockResolvedValue(null); + multiSig.getBalance = jest.fn().mockResolvedValue({ + free: new BigNumber(100000), + locked: new BigNumber(0), + total: new BigNumber(100000), + }); + + const args = tuple('TEST'); + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { fee, ...rest } = txSpec; + + const tx = new PolymeshTransaction( + { + ...rest, + transaction: tx1, + args, + resolver: undefined, + multiSig, + }, + context + ); + + const { payingAccountData } = await tx.getTotalFees(true); + + // Should use MultiSig account directly when payer is null + expect(payingAccountData.type).toBe(PayingAccountType.Caller); + expect(payingAccountData.account.address).toBe(DUMMY_ACCOUNT_ID); + expect(multiSig.getPayer).toHaveBeenCalled(); + }); }); describe('method: onProcessedByMiddleware', () => { @@ -1911,5 +2077,113 @@ describe('Polymesh Transaction Base class', () => { expect(mockNextIndex).toHaveBeenCalled(); }); + + it('should set multiSig to null when asProposal is false even if MultiSig is set', async () => { + const mockBlockNumber = dsMockUtils.createMockU32(new BigNumber(1)); + + dsMockUtils.configureMocks({ + contextOptions: { + nonce: new BigNumber(3), + }, + }); + + dsMockUtils.createRpcMock('chain', 'getFinalizedHead', { + returnValue: dsMockUtils.createMockSignedBlock({ + block: { + header: { + parentHash: 'hash', + number: dsMockUtils.createMockCompact(mockBlockNumber), + extrinsicsRoot: 'hash', + stateRoot: 'hash', + }, + extrinsics: undefined, + }, + }), + }); + + const genesisHash = '0x12341234123412341234123412341234'; + jest.spyOn(context.polymeshApi.genesisHash, 'toString').mockReturnValue(genesisHash); + + dsMockUtils.createMockExtrinsicsEra(); + const mockSignerPayload = createMockSigningPayload(); + + when(context.createType) + .calledWith('SignerPayload', expect.objectContaining({ genesisHash })) + .mockReturnValue(mockSignerPayload); + + const transaction = dsMockUtils.createTxMock('asset', 'registerUniqueTicker'); + const args = tuple('FOO'); + + const tx = new PolymeshTransaction( + { + ...txSpec, + transaction, + args, + resolver: undefined, + mortality: { immortal: true }, + multiSig: entityMockUtils.getMultiSigInstance({ address: DUMMY_ACCOUNT_ID }), + }, + context + ); + + const result = await tx.toSignablePayload({}, false); + + expect(result.multiSig).toBeNull(); + }); + + it('should set multiSig address when asProposal is true and MultiSig is set', async () => { + const mockBlockNumber = dsMockUtils.createMockU32(new BigNumber(1)); + + dsMockUtils.configureMocks({ + contextOptions: { + nonce: new BigNumber(3), + }, + }); + + dsMockUtils.createRpcMock('chain', 'getFinalizedHead', { + returnValue: dsMockUtils.createMockSignedBlock({ + block: { + header: { + parentHash: 'hash', + number: dsMockUtils.createMockCompact(mockBlockNumber), + extrinsicsRoot: 'hash', + stateRoot: 'hash', + }, + extrinsics: undefined, + }, + }), + }); + + const genesisHash = '0x12341234123412341234123412341234'; + jest.spyOn(context.polymeshApi.genesisHash, 'toString').mockReturnValue(genesisHash); + + dsMockUtils.createMockExtrinsicsEra(); + const mockSignerPayload = createMockSigningPayload(); + + when(context.createType) + .calledWith('SignerPayload', expect.objectContaining({ genesisHash })) + .mockReturnValue(mockSignerPayload); + + dsMockUtils.createTxMock('multiSig', 'createProposal'); + + const transaction = dsMockUtils.createTxMock('asset', 'registerUniqueTicker'); + const args = tuple('FOO'); + + const tx = new PolymeshTransaction( + { + ...txSpec, + transaction, + args, + resolver: undefined, + mortality: { immortal: true }, + multiSig: entityMockUtils.getMultiSigInstance({ address: DUMMY_ACCOUNT_ID }), + }, + context + ); + + const result = await tx.toSignablePayload({}, true); + + expect(result.multiSig).toEqual(DUMMY_ACCOUNT_ID); + }); }); }); diff --git a/src/base/types.ts b/src/base/types.ts index 263dd70520..e36e310cdd 100644 --- a/src/base/types.ts +++ b/src/base/types.ts @@ -212,7 +212,9 @@ export interface TransactionPayload { /** * The address of the MultiSig if the transaction is a proposal. * - * Will be set only if the signing account is a MultiSig signer and the transaction is not approving or rejecting an existing proposal + * Will be set only if the signing account is a MultiSig signer, the transaction is not approving or rejecting an existing proposal, + * and `asProposal: true` was used when calling `toSignablePayload()`. When `asProposal: false`, this will be `null` even if the + * signing account is a MultiSig signer. */ readonly multiSig: string | null; }