maxFee in Aggregate transactions (#539)

* Fixed #533
- Added setMaxFeeForAggregate
- Changed setMaxFee for stand alone tx only
- Addes getMaxCosignatures
- Added getNetworkMaxCosignaturesPerAggregate

* Fixed typo

* Fixe typo

* Fixed PR review comments

* Added tests

Author: rg911

package-lock.json

@@ -394,12 +394,31 @@ export class AggregateTransaction extends Transaction {
      * @internal
      * @returns {AggregateTransaction}
      */
-    resolveAliases(statement: Statement): AggregateTransaction {
+    public resolveAliases(statement: Statement): AggregateTransaction {
         const transactionInfo = this.checkTransactionHeightAndIndex();
         return DtoMapping.assign(this, {
             innerTransactions: this.innerTransactions
                 .map((tx) => tx.resolveAliases(statement, transactionInfo.index))
                 .sort((a, b) => a.transactionInfo!.index - b.transactionInfo!.index),
         });
     }
+
+    /**
+     * Set transaction maxFee using fee multiplier for **ONLY AGGREGATE TRANSACTIONS**
+     * @param feeMultiplier The fee multiplier
+     * @param requiredCosignatures Required number of cosignatures
+     * @returns {AggregateTransaction}
+     */
+    public setMaxFeeForAggregate(feeMultiplier: number, requiredCosignatures: number): AggregateTransaction {
+        if (this.type !== TransactionType.AGGREGATE_BONDED && this.type !== TransactionType.AGGREGATE_COMPLETE) {
+            throw new Error('setMaxFeeForAggregate can only be used for aggregate transactions.');
+        }
+        // Check if current cosignature count is greater than requiredCosignatures.
+        const calculatedCosignatures = requiredCosignatures > this.cosignatures.length ? requiredCosignatures : this.cosignatures.length;
+        // Remove current cosignature length and use the calculated one.
+        const calculatedSize = this.size - this.cosignatures.length * 96 + calculatedCosignatures * 96;
+        return DtoMapping.assign(this, {
+            maxFee: UInt64.fromUint(calculatedSize * feeMultiplier),
+        });
+    }
 }

src/model/transaction/AggregateTransaction.ts

@@ -191,11 +191,14 @@ export abstract class Transaction {
     abstract resolveAliases(statement?: Statement, aggregateTransactionIndex?: number): Transaction;
 
     /**
-     * Set transaction maxFee using fee multiplier
+     * Set transaction maxFee using fee multiplier for **ONLY NONE AGGREGATE TRANSACTIONS**
      * @param feeMultiplier The fee multiplier
      * @returns {TransferTransaction}
      */
     public setMaxFee(feeMultiplier: number): Transaction {
+        if (this.type === TransactionType.AGGREGATE_BONDED || this.type === TransactionType.AGGREGATE_COMPLETE) {
+            throw new Error('setMaxFee can only be used for none aggregate transactions.');
+        }
         return DtoMapping.assign(this, {
             maxFee: UInt64.fromUint(this.size * feeMultiplier),
         });

src/model/transaction/Transaction.ts

@@ -24,16 +24,24 @@ import { InnerTransaction } from '../model/transaction/InnerTransaction';
 import { MultisigAccountModificationTransaction } from '../model/transaction/MultisigAccountModificationTransaction';
 import { SignedTransaction } from '../model/transaction/SignedTransaction';
 import { TransactionType } from '../model/transaction/TransactionType';
+import { Address } from '../model/account/Address';
+import { RepositoryFactory } from '../infrastructure/RepositoryFactory';
+import { NetworkRepository } from '../infrastructure/NetworkRepository';
 
 /**
  * Aggregated Transaction service
  */
 export class AggregateTransactionService {
+    private readonly multisigRepository: MultisigRepository;
+    private readonly networkRepository: NetworkRepository;
     /**
      * Constructor
      * @param multisigRepository
      */
-    constructor(private readonly multisigRepository: MultisigRepository) {}
+    constructor(repositoryFactory: RepositoryFactory) {
+        this.multisigRepository = repositoryFactory.createMultisigRepository();
+        this.networkRepository = repositoryFactory.createNetworkRepository();
+    }
 
     /**
      * Check if an aggregate complete transaction has all cosignatories attached
@@ -74,6 +82,40 @@ export class AggregateTransactionService {
             );
     }
 
+    /**
+     * Get total multisig account cosigner count
+     * @param address multisig account address
+     * @returns {Observable<number>}
+     */
+    public getMaxCosignatures(address: Address): Observable<number> {
+        return this.multisigRepository.getMultisigAccountGraphInfo(address).pipe(
+            map((graph) => {
+                let count = 0;
+                graph.multisigAccounts.forEach((multisigInfo) => {
+                    multisigInfo.map((multisig) => {
+                        count += multisig.cosignatories.length;
+                    });
+                });
+                return count;
+            }),
+        );
+    }
+
+    /**
+     * Get max cosignatures allowed per aggregate from network properties
+     * @returns {Observable<number>}
+     */
+    public getNetworkMaxCosignaturesPerAggregate(): Observable<number> {
+        return this.networkRepository.getNetworkProperties().pipe(
+            map((properties) => {
+                if (!properties.plugins.aggregate?.maxCosignaturesPerAggregate) {
+                    throw new Error('Cannot get maxCosignaturesPerAggregate from network properties.');
+                }
+                return parseInt(properties.plugins.aggregate?.maxCosignaturesPerAggregate.replace(`'`, ''));
+            }),
+        );
+    }
+
     /**
      * Validate cosignatories against multisig Account(s)
      * @param graphInfo - multisig account graph info

src/service/AggregateTransactionService.ts

@@ -607,13 +607,32 @@ describe('AggregateTransaction', () => {
             [transferTransaction.toAggregate(account.publicAccount)],
             NetworkType.MIJIN_TEST,
             [],
-        ).setMaxFee(2);
-        expect(aggregateTransaction.maxFee.compact()).to.be.equal(560);
+        ).setMaxFeeForAggregate(2, 10);
+        expect(aggregateTransaction.maxFee.compact()).to.be.equal(560 + 960 * 2);
 
         const signedTransaction = aggregateTransaction.signWith(account, generationHash);
         expect(signedTransaction.hash).not.to.be.undefined;
     });
 
+    it('Test set maxFee using multiplier', () => {
+        const transferTransaction = TransferTransaction.create(
+            Deadline.create(1, ChronoUnit.HOURS),
+            unresolvedAddress,
+            [new Mosaic(unresolvedMosaicId, UInt64.fromUint(1))],
+            PlainMessage.create('test-message'),
+            NetworkType.MIJIN_TEST,
+        );
+
+        expect(() => {
+            AggregateTransaction.createComplete(
+                Deadline.create(),
+                [transferTransaction.toAggregate(account.publicAccount)],
+                NetworkType.MIJIN_TEST,
+                [],
+            ).setMaxFee(2);
+        }).to.throw();
+    });
+
     it('Test resolveAlias can resolve', () => {
         const transferTransaction = new TransferTransaction(
             NetworkType.MIJIN_TEST,

test/model/transaction/AggregateTransaction.spec.ts

@@ -35,6 +35,7 @@ import { Deadline } from '../../../src/model/transaction/Deadline';
 import { TransferTransaction } from '../../../src/model/transaction/TransferTransaction';
 import { UInt64 } from '../../../src/model/UInt64';
 import { TestingAccount } from '../../conf/conf.spec';
+import { AggregateTransaction } from '../../../src/model/transaction/AggregateTransaction';
 
 describe('TransferTransaction', () => {
     let account: Account;
@@ -382,6 +383,25 @@ describe('TransferTransaction', () => {
         expect(signedTransaction.hash).not.to.be.undefined;
     });
 
+    it('Test set maxFee using multiplier to throw', () => {
+        const transferTransaction = TransferTransaction.create(
+            Deadline.create(),
+            Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'),
+            [NetworkCurrencyLocal.createAbsolute(1)],
+            PlainMessage.create('test-message'),
+            NetworkType.MIJIN_TEST,
+        );
+
+        expect(() => {
+            AggregateTransaction.createComplete(
+                Deadline.create(),
+                [transferTransaction.toAggregate(account.publicAccount)],
+                NetworkType.MIJIN_TEST,
+                [],
+            ).setMaxFee(2);
+        }).to.throw();
+    });
+
     it('Test resolveAlias can resolve', () => {
         const transferTransaction = new TransferTransaction(
             NetworkType.MIJIN_TEST,

test/model/transaction/TransferTransaction.spec.ts

@@ -31,6 +31,9 @@ import { Deadline } from '../../src/model/transaction/Deadline';
 import { MultisigAccountModificationTransaction } from '../../src/model/transaction/MultisigAccountModificationTransaction';
 import { TransferTransaction } from '../../src/model/transaction/TransferTransaction';
 import { AggregateTransactionService } from '../../src/service/AggregateTransactionService';
+import { RepositoryFactory } from '../../src/infrastructure/RepositoryFactory';
+import { NetworkRepository } from '../../src/infrastructure/NetworkRepository';
+import { NetworkConfigurationDTO, PluginsPropertiesDTO, AggregateNetworkPropertiesDTO } from 'symbol-openapi-typescript-node-client';
 
 /**
  * For multi level multisig scenario visit: https://github.com/nemtech/symbol-docs/issues/10
@@ -131,8 +134,20 @@ describe('AggregateTransactionService', () => {
         return new MultisigAccountGraphInfo(map);
     }
 
+    function getNetworkProperties(input: string): NetworkConfigurationDTO {
+        const body = new NetworkConfigurationDTO();
+        const plugin = new PluginsPropertiesDTO();
+        plugin.aggregate = new AggregateNetworkPropertiesDTO();
+        plugin.aggregate.maxCosignaturesPerAggregate = input;
+        body.plugins = plugin;
+        return body;
+    }
+
+    let mockNetworkRepository: NetworkRepository;
     before(() => {
-        const mockedAccountRepository: MultisigRepository = mock();
+        mockNetworkRepository = mock<NetworkRepository>();
+        const mockRepoFactory = mock<RepositoryFactory>();
+        const mockedAccountRepository: MultisigRepository = mock<MultisigRepository>();
 
         when(mockedAccountRepository.getMultisigAccountInfo(deepEqual(account1.address))).thenReturn(observableOf(givenAccount1Info()));
         when(mockedAccountRepository.getMultisigAccountInfo(deepEqual(account4.address))).thenReturn(observableOf(givenAccount4Info()));
@@ -152,7 +167,11 @@ describe('AggregateTransactionService', () => {
         when(mockedAccountRepository.getMultisigAccountInfo(deepEqual(account3.address))).thenReturn(observableOf(givenAccount3Info()));
 
         const accountRepository = instance(mockedAccountRepository);
-        aggregateTransactionService = new AggregateTransactionService(accountRepository);
+        const networkRespository = instance(mockNetworkRepository);
+        const repoFactory = instance(mockRepoFactory);
+        when(mockRepoFactory.createMultisigRepository()).thenReturn(accountRepository);
+        when(mockRepoFactory.createNetworkRepository()).thenReturn(networkRespository);
+        aggregateTransactionService = new AggregateTransactionService(repoFactory);
     });
 
     it('should return isComplete: true for aggregated complete transaction - 2 levels Multisig', () => {
@@ -580,4 +599,31 @@ describe('AggregateTransactionService', () => {
                 expect(isComplete).to.be.false;
             });
     });
+
+    it('should call getNetworkMaxCosignaturesPerAggregate and returns', async () => {
+        when(mockNetworkRepository.getNetworkProperties()).thenReturn(observableOf(getNetworkProperties('15')));
+        const max = await aggregateTransactionService.getNetworkMaxCosignaturesPerAggregate().toPromise();
+        expect(max).to.be.equal(15);
+    });
+
+    it('should call getNetworkMaxCosignaturesPerAggregate and returns with single quote', async () => {
+        when(mockNetworkRepository.getNetworkProperties()).thenReturn(observableOf(getNetworkProperties(`1'000`)));
+        const max = await aggregateTransactionService.getNetworkMaxCosignaturesPerAggregate().toPromise();
+        expect(max).to.be.equal(1000);
+    });
+
+    it('should call getNetworkMaxCosignaturesPerAggregate and throw', () => {
+        when(mockNetworkRepository.getNetworkProperties()).thenReturn(observableOf(getNetworkProperties('')));
+        aggregateTransactionService
+            .getNetworkMaxCosignaturesPerAggregate()
+            .toPromise()
+            .catch((error) => {
+                expect(error).not.to.be.undefined;
+            });
+    });
+
+    it('should call getMaxCosignatures and returns', async () => {
+        const max = await aggregateTransactionService.getMaxCosignatures(multisig2.address).toPromise();
+        expect(max).to.be.equal(4);
+    });
 });