From 486d378dc226572dec6d5156744551a2e693f0b4 Mon Sep 17 00:00:00 2001 From: Steven Liu Date: Mon, 18 Mar 2019 10:05:21 +0000 Subject: [PATCH 01/31] Added #4 check if an aggregate complete transaction has all cosignatories --- src/model/transaction/AggregateTransaction.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/model/transaction/AggregateTransaction.ts b/src/model/transaction/AggregateTransaction.ts index 1557012fa6..1e8f0efe50 100644 --- a/src/model/transaction/AggregateTransaction.ts +++ b/src/model/transaction/AggregateTransaction.ts @@ -147,4 +147,17 @@ export class AggregateTransaction extends Transaction { || (this.signer !== undefined && this.signer.equals(publicAccount)); } + /** + * Check if an aggregate complete transaction has all cosignatories + * @returns {boolean} + */ + public isComplete(): boolean { + let isCompleted = false; + this.innerTransactions.forEach((innerTransaction) => { + if (!isCompleted) { + isCompleted = this.cosignatures.find((cosignature) => cosignature.signer.equals(innerTransaction.signer)) !== undefined; + } + }); + return isCompleted; + } } From 7d1d2fbb525e572c072e755da1c133712ffc9131 Mon Sep 17 00:00:00 2001 From: Steven Liu Date: Mon, 18 Mar 2019 10:05:21 +0000 Subject: [PATCH 02/31] Added #4 check if an aggregate complete transaction has all cosignatories --- src/model/transaction/AggregateTransaction.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/model/transaction/AggregateTransaction.ts b/src/model/transaction/AggregateTransaction.ts index 1557012fa6..1e8f0efe50 100644 --- a/src/model/transaction/AggregateTransaction.ts +++ b/src/model/transaction/AggregateTransaction.ts @@ -147,4 +147,17 @@ export class AggregateTransaction extends Transaction { || (this.signer !== undefined && this.signer.equals(publicAccount)); } + /** + * Check if an aggregate complete transaction has all cosignatories + * @returns {boolean} + */ + public isComplete(): boolean { + let isCompleted = false; + this.innerTransactions.forEach((innerTransaction) => { + if (!isCompleted) { + isCompleted = this.cosignatures.find((cosignature) => cosignature.signer.equals(innerTransaction.signer)) !== undefined; + } + }); + return isCompleted; + } } From 41b0b45a4f85441e49e8eaffbf879b0587c26f7b Mon Sep 17 00:00:00 2001 From: Steven Liu Date: Mon, 1 Apr 2019 20:39:11 +0100 Subject: [PATCH 03/31] #4 1. Addes service for Aggregated Transaction - isComplete 2. Fixed couple of bugs in TransactionMappings --- .../CreateTransactionFromPayload.ts | 8 +- src/model/transaction/AggregateTransaction.ts | 14 - src/service/AggregatedTransactionService.ts | 127 +++++++++ src/service/service.ts | 1 + .../AggregatedTransactionService.spec.ts | 242 ++++++++++++++++++ 5 files changed, 374 insertions(+), 18 deletions(-) create mode 100644 src/service/AggregatedTransactionService.ts create mode 100644 test/service/AggregatedTransactionService.spec.ts diff --git a/src/infrastructure/transaction/CreateTransactionFromPayload.ts b/src/infrastructure/transaction/CreateTransactionFromPayload.ts index e8b66453ff..9b124a33bf 100644 --- a/src/infrastructure/transaction/CreateTransactionFromPayload.ts +++ b/src/infrastructure/transaction/CreateTransactionFromPayload.ts @@ -377,7 +377,7 @@ const CreateTransaction = (type: number, transactionData: string, networkType: N networkType, consignatureArray ? consignatureArray.map((cosignature) => new AggregateTransactionCosignature( cosignature.substring(0, 64), - PublicAccount.createFromPublicKey(cosignature.substring(64, 192), networkType), + PublicAccount.createFromPublicKey(cosignature.substring(0, 64), networkType), )) : [], ); case TransactionType.AGGREGATE_BONDED: @@ -401,7 +401,7 @@ const CreateTransaction = (type: number, transactionData: string, networkType: N networkType, bondedConsignatureArray ? bondedConsignatureArray.map((cosignature) => new AggregateTransactionCosignature( cosignature.substring(0, 64), - PublicAccount.createFromPublicKey(cosignature.substring(64, 192), networkType), + PublicAccount.createFromPublicKey(cosignature.substring(0, 64), networkType), )) : [], ); default: @@ -457,9 +457,9 @@ const parseInnerTransactionFromBinary = (innerTransactionBinary: string): string while (innerBinary.length) { const payloadSize = parseInt(convert.uint8ToHex(convert.hexToUint8(innerTransactionBinary.substring(0, 8)).reverse()), 16) * 2; - const innerTransaction = innerTransactionBinary.substring(8, 8 + payloadSize); + const innerTransaction = innerTransactionBinary.substring(8, payloadSize); embeddedTransaction.push(innerTransaction); - innerBinary = innerTransactionBinary.substring(8 + payloadSize); + innerBinary = innerBinary.substring(payloadSize); } return embeddedTransaction; }; diff --git a/src/model/transaction/AggregateTransaction.ts b/src/model/transaction/AggregateTransaction.ts index 1e8f0efe50..47312c1e4f 100644 --- a/src/model/transaction/AggregateTransaction.ts +++ b/src/model/transaction/AggregateTransaction.ts @@ -146,18 +146,4 @@ export class AggregateTransaction extends Transaction { return this.cosignatures.find((cosignature) => cosignature.signer.equals(publicAccount)) !== undefined || (this.signer !== undefined && this.signer.equals(publicAccount)); } - - /** - * Check if an aggregate complete transaction has all cosignatories - * @returns {boolean} - */ - public isComplete(): boolean { - let isCompleted = false; - this.innerTransactions.forEach((innerTransaction) => { - if (!isCompleted) { - isCompleted = this.cosignatures.find((cosignature) => cosignature.signer.equals(innerTransaction.signer)) !== undefined; - } - }); - return isCompleted; - } } diff --git a/src/service/AggregatedTransactionService.ts b/src/service/AggregatedTransactionService.ts new file mode 100644 index 0000000000..ccdd72cd3b --- /dev/null +++ b/src/service/AggregatedTransactionService.ts @@ -0,0 +1,127 @@ +/* + * Copyright 2019 NEM + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import {from as observableFrom , Observable, of as observableOf} from 'rxjs'; +import { map, mergeMap} from 'rxjs/operators'; +import { TransactionMapping } from '../core/utils/TransactionMapping'; +import { AccountHttp } from '../infrastructure/AccountHttp'; +import { MultisigAccountGraphInfo } from '../model/account/MultisigAccountGraphInfo'; +import { AggregateTransaction } from '../model/transaction/AggregateTransaction'; +import { SignedTransaction } from '../model/transaction/SignedTransaction'; + +/** + * Aggregated Transaction service + */ +export class AggregatedTransactionService { + + /** + * Constructor + * @param accountHttp + */ + constructor(private readonly accountHttp: AccountHttp) { + } + + /** + * Check if an aggregate complete transaction has all cosignatories attached + * @param signedTransaction - The signed aggregate transaction (complete) to be verified + * @returns {Observable} + */ + public isComplete(signedTransaction: SignedTransaction): Observable { + const aggregateTransaction = TransactionMapping.createFromPayload(signedTransaction.payload) as AggregateTransaction; + /** + * Include both initiator & cosigners + */ + const signers = (aggregateTransaction.cosignatures.map((cosigner) => cosigner.signer.publicKey)); + if (signedTransaction.signer) { + signers.push(signedTransaction.signer); + } + + return observableFrom(aggregateTransaction.innerTransactions).pipe( + mergeMap((innerTransaction) => this.accountHttp.getMultisigAccountInfo(innerTransaction.signer.address) + .pipe( + /** + * For multisig account, we need to get the graph info in case it has multiple levels + */ + mergeMap((_) => _.minApproval !== 0 && _.minRemoval !== 0 ? + this.accountHttp.getMultisigAccountGraphInfo(_.account.address) + .pipe( + map((graphInfo) => this.validateCosignatories(graphInfo, signers)), + ) : observableOf(true), + ), + ), + ), + ); + } + + /** + * Validate cosignatories against multisig Account(s) + * @param graphInfo - multisig account graph info + * @param cosignatories - array of cosignatories extracted from aggregated transaction + * @returns {boolean} + */ + private validateCosignatories(graphInfo: MultisigAccountGraphInfo, cosignatories: string[]): boolean { + /** + * Validate cosignatories from bottom level to top + */ + const sortedKeys = Array.from(graphInfo.multisigAccounts.keys()).sort((a, b) => b - a); + const cosignatoriesReceived = cosignatories; + let validationResult = true; + + sortedKeys.forEach((key) => { + const multisigInfo = graphInfo.multisigAccounts.get(key); + if (multisigInfo && validationResult) { + multisigInfo.forEach((multisig) => { + if (multisig.minApproval >= 1) { + const matchedCosignatories = this.compareArrays(cosignatoriesReceived, + multisig.cosignatories.map((cosig) => cosig.publicKey)); + + /** + * if minimal signature requirement met at current level, push the multisig account + * into the received signatories array for next level validation. + * Otherwise return validation failed. + */ + if (matchedCosignatories.length >= multisig.minApproval) { + if (cosignatoriesReceived.indexOf(multisig.account.publicKey) === -1) { + cosignatoriesReceived.push(multisig.account.publicKey); + } + } else { + validationResult = false; + } + } + }); + } + }); + + return validationResult; + } + + /** + * Compare two string arrays + * @param array1 - base array + * @param array2 - array to be matched + * @returns {string[]} - array of matched elements + */ + private compareArrays(array1: string[], array2: string[]): string[] { + const results: string[] = []; + array1.forEach((a1) => array2.forEach((a2) => { + if (a1 === a2) { + results.push(a1); + } + })); + + return results; + } +} diff --git a/src/service/service.ts b/src/service/service.ts index 6a9945c1cd..fcf4c84680 100644 --- a/src/service/service.ts +++ b/src/service/service.ts @@ -16,3 +16,4 @@ export * from './NamespaceService'; export * from './MosaicService'; +export * from './AggregatedTransactionService'; diff --git a/test/service/AggregatedTransactionService.spec.ts b/test/service/AggregatedTransactionService.spec.ts new file mode 100644 index 0000000000..0aff3d8f47 --- /dev/null +++ b/test/service/AggregatedTransactionService.spec.ts @@ -0,0 +1,242 @@ +/* + * Copyright 2018 NEM + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { expect } from 'chai'; +import { ChronoUnit } from 'js-joda'; +import {of as observableOf} from 'rxjs'; +import {deepEqual, instance, mock, when} from 'ts-mockito'; +import { AccountHttp } from '../../src/infrastructure/AccountHttp'; +import { Account } from '../../src/model/account/Account'; +import { Address } from '../../src/model/account/Address'; +import { MultisigAccountGraphInfo } from '../../src/model/account/MultisigAccountGraphInfo'; +import { MultisigAccountInfo } from '../../src/model/account/MultisigAccountInfo'; +import {NetworkType} from '../../src/model/blockchain/NetworkType'; +import { AggregateTransaction } from '../../src/model/transaction/AggregateTransaction'; +import { Deadline } from '../../src/model/transaction/Deadline'; +import { PlainMessage } from '../../src/model/transaction/PlainMessage'; +import { TransferTransaction } from '../../src/model/transaction/TransferTransaction'; +import { AggregatedTransactionService } from '../../src/service/AggregatedTransactionService'; + +/** + * For multi level multisig scenario visit: https://github.com/nemtech/nem2-docs/issues/10 + */ +describe('AggregatedTransactionService', () => { + let aggregatedTransactionService: AggregatedTransactionService; + + /** + * Multisig2 Account: SBROWP-7YMG2M-K45RO6-Q7ZPK7-G7GXWQ-JK5VNQ-OSUX + * Public Key: 5E628EA59818D97AA4118780D9A88C5512FCE7A21C195E1574727EFCE5DF7C0D + * Private Key: 22A1D67F8519D1A45BD7116600BB6E857786E816FE0B45E4C5B9FFF3D64BC177 + * + * + * Multisig1 Account: SAK32M-5JQ43R-WYHWEH-WRBCW4-RXERT2-DLASGL-EANS + * Public Key: BFDF2610C5666A626434FE12FB4A9D896D2B9B033F5F84CCEABE82E043A6307E + * Private Key: 8B0622C2CCFC5CCC5A74B500163E3C68F3AD3643DB12932FC931143EAC67280D + */ + + /** + * Test accounts: + * Multisig1 (1/1): Account2, Account3 + * Multisig2 (2/1): Account1, Multisig1 + * Stranger Account: Account4 + */ + + const account1 = Account.createFromPrivateKey('82DB2528834C9926F0FCCE042466B24A266F5B685CB66D2869AF6648C043E950', + NetworkType.MIJIN_TEST); + const multisig1 = Account.createFromPrivateKey('8B0622C2CCFC5CCC5A74B500163E3C68F3AD3643DB12932FC931143EAC67280D', + NetworkType.MIJIN_TEST); + const multisig2 = Account.createFromPrivateKey('22A1D67F8519D1A45BD7116600BB6E857786E816FE0B45E4C5B9FFF3D64BC177', + NetworkType.MIJIN_TEST); + + const account2 = Account.createFromPrivateKey('A4D410270E01CECDCDEADCDE32EC79C8D9CDEA4DCD426CB1EB666EFEF148FBCE', + NetworkType.MIJIN_TEST); + const account3 = Account.createFromPrivateKey('336AB45EE65A6AFFC0E7ADC5342F91E34BACA0B901A1D9C876FA25A1E590077E', + NetworkType.MIJIN_TEST); + + const account4 = Account.createFromPrivateKey('4D8B3756592532753344E11E2B7541317BCCFBBCF4444274CDBF359D2C4AE0F1', + NetworkType.MIJIN_TEST); + before(() => { + const mockedAccountHttp = mock(AccountHttp); + + when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(account1.address))) + .thenReturn(observableOf(givenAccount1Info())); + when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(account4.address))) + .thenReturn(observableOf(givenAccount4Info())); + when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(multisig2.address))) + .thenReturn(observableOf(givenMultisig2AccountInfo())); + when(mockedAccountHttp.getMultisigAccountGraphInfo(deepEqual(multisig2.address))) + .thenReturn(observableOf(givenMultisig2AccountGraphInfo())); + + const accountHttp = instance(mockedAccountHttp); + aggregatedTransactionService = new AggregatedTransactionService(accountHttp); + }); + + it('should return isComplete: true for aggregated complete transaction - 2 levels Multisig', () => { + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(multisig2.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account2]); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.true; + }); + }); + + it('should return isComplete: false for aggregated complete transaction - 2 levels Multisig', () => { + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(multisig2.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, []); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.false; + }); + }); + + it('should return isComplete: false for aggregated complete transaction - 2 levels Multisig', () => { + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(multisig2.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account4]); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.false; + }); + }); + + it('should return correct isComplete status for aggregated complete transaction - 2 levels Multisig, multi inner transaction', () => { + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const transferTransaction2 = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(multisig2.publicAccount), + transferTransaction.toAggregate(account4.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account2]); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.true; + }); + }); + + it('should return correct isComplete status for aggregated complete transaction - none multisig', () => { + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(account4.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signWith(account1); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.true; + }); + }); + + function givenMultisig2AccountInfo(): MultisigAccountInfo { + return new MultisigAccountInfo(multisig2.publicAccount, + 2, 1, + [multisig1.publicAccount, + account1.publicAccount], + [], + ); + } + + function givenAccount1Info(): MultisigAccountInfo { + return new MultisigAccountInfo(account1.publicAccount, + 0, 0, + [], + [multisig2.publicAccount], + ); + } + function givenAccount4Info(): MultisigAccountInfo { + return new MultisigAccountInfo(account4.publicAccount, + 0, 0, + [], + [], + ); + } + + function givenMultisig2AccountGraphInfo(): MultisigAccountGraphInfo { + const map = new Map(); + map.set(0, [new MultisigAccountInfo(multisig2.publicAccount, + 2, 1, + [multisig1.publicAccount, + account1.publicAccount], + [], + )]) + .set(1, [new MultisigAccountInfo(multisig1.publicAccount, + 1, 1, + [account2.publicAccount, account3.publicAccount], + [multisig2.publicAccount], + )]); + + return new MultisigAccountGraphInfo(map); + } + +}); From 319ba1de13cdb590938536bb7a0dce53a71b40a6 Mon Sep 17 00:00:00 2001 From: Steven Liu Date: Mon, 18 Mar 2019 10:05:21 +0000 Subject: [PATCH 04/31] Added #4 check if an aggregate complete transaction has all cosignatories --- src/model/transaction/AggregateTransaction.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/model/transaction/AggregateTransaction.ts b/src/model/transaction/AggregateTransaction.ts index 1557012fa6..1e8f0efe50 100644 --- a/src/model/transaction/AggregateTransaction.ts +++ b/src/model/transaction/AggregateTransaction.ts @@ -147,4 +147,17 @@ export class AggregateTransaction extends Transaction { || (this.signer !== undefined && this.signer.equals(publicAccount)); } + /** + * Check if an aggregate complete transaction has all cosignatories + * @returns {boolean} + */ + public isComplete(): boolean { + let isCompleted = false; + this.innerTransactions.forEach((innerTransaction) => { + if (!isCompleted) { + isCompleted = this.cosignatures.find((cosignature) => cosignature.signer.equals(innerTransaction.signer)) !== undefined; + } + }); + return isCompleted; + } } From b740dee31de4f4bf40fb97fb97960e002ba87686 Mon Sep 17 00:00:00 2001 From: Steven Liu Date: Mon, 1 Apr 2019 20:39:11 +0100 Subject: [PATCH 05/31] #4 1. Addes service for Aggregated Transaction - isComplete 2. Fixed couple of bugs in TransactionMappings --- .../CreateTransactionFromPayload.ts | 8 +- src/model/transaction/AggregateTransaction.ts | 14 - src/service/AggregatedTransactionService.ts | 127 +++++++++ src/service/service.ts | 1 + .../AggregatedTransactionService.spec.ts | 242 ++++++++++++++++++ 5 files changed, 374 insertions(+), 18 deletions(-) create mode 100644 src/service/AggregatedTransactionService.ts create mode 100644 test/service/AggregatedTransactionService.spec.ts diff --git a/src/infrastructure/transaction/CreateTransactionFromPayload.ts b/src/infrastructure/transaction/CreateTransactionFromPayload.ts index e8b66453ff..9b124a33bf 100644 --- a/src/infrastructure/transaction/CreateTransactionFromPayload.ts +++ b/src/infrastructure/transaction/CreateTransactionFromPayload.ts @@ -377,7 +377,7 @@ const CreateTransaction = (type: number, transactionData: string, networkType: N networkType, consignatureArray ? consignatureArray.map((cosignature) => new AggregateTransactionCosignature( cosignature.substring(0, 64), - PublicAccount.createFromPublicKey(cosignature.substring(64, 192), networkType), + PublicAccount.createFromPublicKey(cosignature.substring(0, 64), networkType), )) : [], ); case TransactionType.AGGREGATE_BONDED: @@ -401,7 +401,7 @@ const CreateTransaction = (type: number, transactionData: string, networkType: N networkType, bondedConsignatureArray ? bondedConsignatureArray.map((cosignature) => new AggregateTransactionCosignature( cosignature.substring(0, 64), - PublicAccount.createFromPublicKey(cosignature.substring(64, 192), networkType), + PublicAccount.createFromPublicKey(cosignature.substring(0, 64), networkType), )) : [], ); default: @@ -457,9 +457,9 @@ const parseInnerTransactionFromBinary = (innerTransactionBinary: string): string while (innerBinary.length) { const payloadSize = parseInt(convert.uint8ToHex(convert.hexToUint8(innerTransactionBinary.substring(0, 8)).reverse()), 16) * 2; - const innerTransaction = innerTransactionBinary.substring(8, 8 + payloadSize); + const innerTransaction = innerTransactionBinary.substring(8, payloadSize); embeddedTransaction.push(innerTransaction); - innerBinary = innerTransactionBinary.substring(8 + payloadSize); + innerBinary = innerBinary.substring(payloadSize); } return embeddedTransaction; }; diff --git a/src/model/transaction/AggregateTransaction.ts b/src/model/transaction/AggregateTransaction.ts index 1e8f0efe50..47312c1e4f 100644 --- a/src/model/transaction/AggregateTransaction.ts +++ b/src/model/transaction/AggregateTransaction.ts @@ -146,18 +146,4 @@ export class AggregateTransaction extends Transaction { return this.cosignatures.find((cosignature) => cosignature.signer.equals(publicAccount)) !== undefined || (this.signer !== undefined && this.signer.equals(publicAccount)); } - - /** - * Check if an aggregate complete transaction has all cosignatories - * @returns {boolean} - */ - public isComplete(): boolean { - let isCompleted = false; - this.innerTransactions.forEach((innerTransaction) => { - if (!isCompleted) { - isCompleted = this.cosignatures.find((cosignature) => cosignature.signer.equals(innerTransaction.signer)) !== undefined; - } - }); - return isCompleted; - } } diff --git a/src/service/AggregatedTransactionService.ts b/src/service/AggregatedTransactionService.ts new file mode 100644 index 0000000000..ccdd72cd3b --- /dev/null +++ b/src/service/AggregatedTransactionService.ts @@ -0,0 +1,127 @@ +/* + * Copyright 2019 NEM + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import {from as observableFrom , Observable, of as observableOf} from 'rxjs'; +import { map, mergeMap} from 'rxjs/operators'; +import { TransactionMapping } from '../core/utils/TransactionMapping'; +import { AccountHttp } from '../infrastructure/AccountHttp'; +import { MultisigAccountGraphInfo } from '../model/account/MultisigAccountGraphInfo'; +import { AggregateTransaction } from '../model/transaction/AggregateTransaction'; +import { SignedTransaction } from '../model/transaction/SignedTransaction'; + +/** + * Aggregated Transaction service + */ +export class AggregatedTransactionService { + + /** + * Constructor + * @param accountHttp + */ + constructor(private readonly accountHttp: AccountHttp) { + } + + /** + * Check if an aggregate complete transaction has all cosignatories attached + * @param signedTransaction - The signed aggregate transaction (complete) to be verified + * @returns {Observable} + */ + public isComplete(signedTransaction: SignedTransaction): Observable { + const aggregateTransaction = TransactionMapping.createFromPayload(signedTransaction.payload) as AggregateTransaction; + /** + * Include both initiator & cosigners + */ + const signers = (aggregateTransaction.cosignatures.map((cosigner) => cosigner.signer.publicKey)); + if (signedTransaction.signer) { + signers.push(signedTransaction.signer); + } + + return observableFrom(aggregateTransaction.innerTransactions).pipe( + mergeMap((innerTransaction) => this.accountHttp.getMultisigAccountInfo(innerTransaction.signer.address) + .pipe( + /** + * For multisig account, we need to get the graph info in case it has multiple levels + */ + mergeMap((_) => _.minApproval !== 0 && _.minRemoval !== 0 ? + this.accountHttp.getMultisigAccountGraphInfo(_.account.address) + .pipe( + map((graphInfo) => this.validateCosignatories(graphInfo, signers)), + ) : observableOf(true), + ), + ), + ), + ); + } + + /** + * Validate cosignatories against multisig Account(s) + * @param graphInfo - multisig account graph info + * @param cosignatories - array of cosignatories extracted from aggregated transaction + * @returns {boolean} + */ + private validateCosignatories(graphInfo: MultisigAccountGraphInfo, cosignatories: string[]): boolean { + /** + * Validate cosignatories from bottom level to top + */ + const sortedKeys = Array.from(graphInfo.multisigAccounts.keys()).sort((a, b) => b - a); + const cosignatoriesReceived = cosignatories; + let validationResult = true; + + sortedKeys.forEach((key) => { + const multisigInfo = graphInfo.multisigAccounts.get(key); + if (multisigInfo && validationResult) { + multisigInfo.forEach((multisig) => { + if (multisig.minApproval >= 1) { + const matchedCosignatories = this.compareArrays(cosignatoriesReceived, + multisig.cosignatories.map((cosig) => cosig.publicKey)); + + /** + * if minimal signature requirement met at current level, push the multisig account + * into the received signatories array for next level validation. + * Otherwise return validation failed. + */ + if (matchedCosignatories.length >= multisig.minApproval) { + if (cosignatoriesReceived.indexOf(multisig.account.publicKey) === -1) { + cosignatoriesReceived.push(multisig.account.publicKey); + } + } else { + validationResult = false; + } + } + }); + } + }); + + return validationResult; + } + + /** + * Compare two string arrays + * @param array1 - base array + * @param array2 - array to be matched + * @returns {string[]} - array of matched elements + */ + private compareArrays(array1: string[], array2: string[]): string[] { + const results: string[] = []; + array1.forEach((a1) => array2.forEach((a2) => { + if (a1 === a2) { + results.push(a1); + } + })); + + return results; + } +} diff --git a/src/service/service.ts b/src/service/service.ts index 6a9945c1cd..fcf4c84680 100644 --- a/src/service/service.ts +++ b/src/service/service.ts @@ -16,3 +16,4 @@ export * from './NamespaceService'; export * from './MosaicService'; +export * from './AggregatedTransactionService'; diff --git a/test/service/AggregatedTransactionService.spec.ts b/test/service/AggregatedTransactionService.spec.ts new file mode 100644 index 0000000000..0aff3d8f47 --- /dev/null +++ b/test/service/AggregatedTransactionService.spec.ts @@ -0,0 +1,242 @@ +/* + * Copyright 2018 NEM + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { expect } from 'chai'; +import { ChronoUnit } from 'js-joda'; +import {of as observableOf} from 'rxjs'; +import {deepEqual, instance, mock, when} from 'ts-mockito'; +import { AccountHttp } from '../../src/infrastructure/AccountHttp'; +import { Account } from '../../src/model/account/Account'; +import { Address } from '../../src/model/account/Address'; +import { MultisigAccountGraphInfo } from '../../src/model/account/MultisigAccountGraphInfo'; +import { MultisigAccountInfo } from '../../src/model/account/MultisigAccountInfo'; +import {NetworkType} from '../../src/model/blockchain/NetworkType'; +import { AggregateTransaction } from '../../src/model/transaction/AggregateTransaction'; +import { Deadline } from '../../src/model/transaction/Deadline'; +import { PlainMessage } from '../../src/model/transaction/PlainMessage'; +import { TransferTransaction } from '../../src/model/transaction/TransferTransaction'; +import { AggregatedTransactionService } from '../../src/service/AggregatedTransactionService'; + +/** + * For multi level multisig scenario visit: https://github.com/nemtech/nem2-docs/issues/10 + */ +describe('AggregatedTransactionService', () => { + let aggregatedTransactionService: AggregatedTransactionService; + + /** + * Multisig2 Account: SBROWP-7YMG2M-K45RO6-Q7ZPK7-G7GXWQ-JK5VNQ-OSUX + * Public Key: 5E628EA59818D97AA4118780D9A88C5512FCE7A21C195E1574727EFCE5DF7C0D + * Private Key: 22A1D67F8519D1A45BD7116600BB6E857786E816FE0B45E4C5B9FFF3D64BC177 + * + * + * Multisig1 Account: SAK32M-5JQ43R-WYHWEH-WRBCW4-RXERT2-DLASGL-EANS + * Public Key: BFDF2610C5666A626434FE12FB4A9D896D2B9B033F5F84CCEABE82E043A6307E + * Private Key: 8B0622C2CCFC5CCC5A74B500163E3C68F3AD3643DB12932FC931143EAC67280D + */ + + /** + * Test accounts: + * Multisig1 (1/1): Account2, Account3 + * Multisig2 (2/1): Account1, Multisig1 + * Stranger Account: Account4 + */ + + const account1 = Account.createFromPrivateKey('82DB2528834C9926F0FCCE042466B24A266F5B685CB66D2869AF6648C043E950', + NetworkType.MIJIN_TEST); + const multisig1 = Account.createFromPrivateKey('8B0622C2CCFC5CCC5A74B500163E3C68F3AD3643DB12932FC931143EAC67280D', + NetworkType.MIJIN_TEST); + const multisig2 = Account.createFromPrivateKey('22A1D67F8519D1A45BD7116600BB6E857786E816FE0B45E4C5B9FFF3D64BC177', + NetworkType.MIJIN_TEST); + + const account2 = Account.createFromPrivateKey('A4D410270E01CECDCDEADCDE32EC79C8D9CDEA4DCD426CB1EB666EFEF148FBCE', + NetworkType.MIJIN_TEST); + const account3 = Account.createFromPrivateKey('336AB45EE65A6AFFC0E7ADC5342F91E34BACA0B901A1D9C876FA25A1E590077E', + NetworkType.MIJIN_TEST); + + const account4 = Account.createFromPrivateKey('4D8B3756592532753344E11E2B7541317BCCFBBCF4444274CDBF359D2C4AE0F1', + NetworkType.MIJIN_TEST); + before(() => { + const mockedAccountHttp = mock(AccountHttp); + + when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(account1.address))) + .thenReturn(observableOf(givenAccount1Info())); + when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(account4.address))) + .thenReturn(observableOf(givenAccount4Info())); + when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(multisig2.address))) + .thenReturn(observableOf(givenMultisig2AccountInfo())); + when(mockedAccountHttp.getMultisigAccountGraphInfo(deepEqual(multisig2.address))) + .thenReturn(observableOf(givenMultisig2AccountGraphInfo())); + + const accountHttp = instance(mockedAccountHttp); + aggregatedTransactionService = new AggregatedTransactionService(accountHttp); + }); + + it('should return isComplete: true for aggregated complete transaction - 2 levels Multisig', () => { + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(multisig2.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account2]); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.true; + }); + }); + + it('should return isComplete: false for aggregated complete transaction - 2 levels Multisig', () => { + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(multisig2.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, []); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.false; + }); + }); + + it('should return isComplete: false for aggregated complete transaction - 2 levels Multisig', () => { + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(multisig2.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account4]); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.false; + }); + }); + + it('should return correct isComplete status for aggregated complete transaction - 2 levels Multisig, multi inner transaction', () => { + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const transferTransaction2 = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(multisig2.publicAccount), + transferTransaction.toAggregate(account4.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account2]); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.true; + }); + }); + + it('should return correct isComplete status for aggregated complete transaction - none multisig', () => { + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(account4.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signWith(account1); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.true; + }); + }); + + function givenMultisig2AccountInfo(): MultisigAccountInfo { + return new MultisigAccountInfo(multisig2.publicAccount, + 2, 1, + [multisig1.publicAccount, + account1.publicAccount], + [], + ); + } + + function givenAccount1Info(): MultisigAccountInfo { + return new MultisigAccountInfo(account1.publicAccount, + 0, 0, + [], + [multisig2.publicAccount], + ); + } + function givenAccount4Info(): MultisigAccountInfo { + return new MultisigAccountInfo(account4.publicAccount, + 0, 0, + [], + [], + ); + } + + function givenMultisig2AccountGraphInfo(): MultisigAccountGraphInfo { + const map = new Map(); + map.set(0, [new MultisigAccountInfo(multisig2.publicAccount, + 2, 1, + [multisig1.publicAccount, + account1.publicAccount], + [], + )]) + .set(1, [new MultisigAccountInfo(multisig1.publicAccount, + 1, 1, + [account2.publicAccount, account3.publicAccount], + [multisig2.publicAccount], + )]); + + return new MultisigAccountGraphInfo(map); + } + +}); From 08c8ce3a71729242aa14609c850582df4aec3c05 Mon Sep 17 00:00:00 2001 From: Steven Liu Date: Mon, 1 Apr 2019 21:25:50 +0100 Subject: [PATCH 06/31] Updated license year --- test/service/AggregatedTransactionService.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/service/AggregatedTransactionService.spec.ts b/test/service/AggregatedTransactionService.spec.ts index 0aff3d8f47..bc0c11f535 100644 --- a/test/service/AggregatedTransactionService.spec.ts +++ b/test/service/AggregatedTransactionService.spec.ts @@ -1,5 +1,5 @@ /* - * Copyright 2018 NEM + * Copyright 2019 NEM * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. From f32982dce21f4cb789ac7d600399720d633c132a Mon Sep 17 00:00:00 2001 From: Steven Liu Date: Mon, 18 Mar 2019 10:05:21 +0000 Subject: [PATCH 07/31] Added #4 check if an aggregate complete transaction has all cosignatories --- src/model/transaction/AggregateTransaction.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/model/transaction/AggregateTransaction.ts b/src/model/transaction/AggregateTransaction.ts index 1557012fa6..1e8f0efe50 100644 --- a/src/model/transaction/AggregateTransaction.ts +++ b/src/model/transaction/AggregateTransaction.ts @@ -147,4 +147,17 @@ export class AggregateTransaction extends Transaction { || (this.signer !== undefined && this.signer.equals(publicAccount)); } + /** + * Check if an aggregate complete transaction has all cosignatories + * @returns {boolean} + */ + public isComplete(): boolean { + let isCompleted = false; + this.innerTransactions.forEach((innerTransaction) => { + if (!isCompleted) { + isCompleted = this.cosignatures.find((cosignature) => cosignature.signer.equals(innerTransaction.signer)) !== undefined; + } + }); + return isCompleted; + } } From 41eb60e5c9b20d7f3f4cd06fbf487f9d299ff415 Mon Sep 17 00:00:00 2001 From: Steven Liu Date: Mon, 1 Apr 2019 20:39:11 +0100 Subject: [PATCH 08/31] #4 1. Addes service for Aggregated Transaction - isComplete 2. Fixed couple of bugs in TransactionMappings --- .../CreateTransactionFromPayload.ts | 8 +- src/model/transaction/AggregateTransaction.ts | 14 - src/service/AggregatedTransactionService.ts | 127 +++++++++ src/service/service.ts | 1 + .../AggregatedTransactionService.spec.ts | 242 ++++++++++++++++++ 5 files changed, 374 insertions(+), 18 deletions(-) create mode 100644 src/service/AggregatedTransactionService.ts create mode 100644 test/service/AggregatedTransactionService.spec.ts diff --git a/src/infrastructure/transaction/CreateTransactionFromPayload.ts b/src/infrastructure/transaction/CreateTransactionFromPayload.ts index e8b66453ff..9b124a33bf 100644 --- a/src/infrastructure/transaction/CreateTransactionFromPayload.ts +++ b/src/infrastructure/transaction/CreateTransactionFromPayload.ts @@ -377,7 +377,7 @@ const CreateTransaction = (type: number, transactionData: string, networkType: N networkType, consignatureArray ? consignatureArray.map((cosignature) => new AggregateTransactionCosignature( cosignature.substring(0, 64), - PublicAccount.createFromPublicKey(cosignature.substring(64, 192), networkType), + PublicAccount.createFromPublicKey(cosignature.substring(0, 64), networkType), )) : [], ); case TransactionType.AGGREGATE_BONDED: @@ -401,7 +401,7 @@ const CreateTransaction = (type: number, transactionData: string, networkType: N networkType, bondedConsignatureArray ? bondedConsignatureArray.map((cosignature) => new AggregateTransactionCosignature( cosignature.substring(0, 64), - PublicAccount.createFromPublicKey(cosignature.substring(64, 192), networkType), + PublicAccount.createFromPublicKey(cosignature.substring(0, 64), networkType), )) : [], ); default: @@ -457,9 +457,9 @@ const parseInnerTransactionFromBinary = (innerTransactionBinary: string): string while (innerBinary.length) { const payloadSize = parseInt(convert.uint8ToHex(convert.hexToUint8(innerTransactionBinary.substring(0, 8)).reverse()), 16) * 2; - const innerTransaction = innerTransactionBinary.substring(8, 8 + payloadSize); + const innerTransaction = innerTransactionBinary.substring(8, payloadSize); embeddedTransaction.push(innerTransaction); - innerBinary = innerTransactionBinary.substring(8 + payloadSize); + innerBinary = innerBinary.substring(payloadSize); } return embeddedTransaction; }; diff --git a/src/model/transaction/AggregateTransaction.ts b/src/model/transaction/AggregateTransaction.ts index 1e8f0efe50..47312c1e4f 100644 --- a/src/model/transaction/AggregateTransaction.ts +++ b/src/model/transaction/AggregateTransaction.ts @@ -146,18 +146,4 @@ export class AggregateTransaction extends Transaction { return this.cosignatures.find((cosignature) => cosignature.signer.equals(publicAccount)) !== undefined || (this.signer !== undefined && this.signer.equals(publicAccount)); } - - /** - * Check if an aggregate complete transaction has all cosignatories - * @returns {boolean} - */ - public isComplete(): boolean { - let isCompleted = false; - this.innerTransactions.forEach((innerTransaction) => { - if (!isCompleted) { - isCompleted = this.cosignatures.find((cosignature) => cosignature.signer.equals(innerTransaction.signer)) !== undefined; - } - }); - return isCompleted; - } } diff --git a/src/service/AggregatedTransactionService.ts b/src/service/AggregatedTransactionService.ts new file mode 100644 index 0000000000..ccdd72cd3b --- /dev/null +++ b/src/service/AggregatedTransactionService.ts @@ -0,0 +1,127 @@ +/* + * Copyright 2019 NEM + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import {from as observableFrom , Observable, of as observableOf} from 'rxjs'; +import { map, mergeMap} from 'rxjs/operators'; +import { TransactionMapping } from '../core/utils/TransactionMapping'; +import { AccountHttp } from '../infrastructure/AccountHttp'; +import { MultisigAccountGraphInfo } from '../model/account/MultisigAccountGraphInfo'; +import { AggregateTransaction } from '../model/transaction/AggregateTransaction'; +import { SignedTransaction } from '../model/transaction/SignedTransaction'; + +/** + * Aggregated Transaction service + */ +export class AggregatedTransactionService { + + /** + * Constructor + * @param accountHttp + */ + constructor(private readonly accountHttp: AccountHttp) { + } + + /** + * Check if an aggregate complete transaction has all cosignatories attached + * @param signedTransaction - The signed aggregate transaction (complete) to be verified + * @returns {Observable} + */ + public isComplete(signedTransaction: SignedTransaction): Observable { + const aggregateTransaction = TransactionMapping.createFromPayload(signedTransaction.payload) as AggregateTransaction; + /** + * Include both initiator & cosigners + */ + const signers = (aggregateTransaction.cosignatures.map((cosigner) => cosigner.signer.publicKey)); + if (signedTransaction.signer) { + signers.push(signedTransaction.signer); + } + + return observableFrom(aggregateTransaction.innerTransactions).pipe( + mergeMap((innerTransaction) => this.accountHttp.getMultisigAccountInfo(innerTransaction.signer.address) + .pipe( + /** + * For multisig account, we need to get the graph info in case it has multiple levels + */ + mergeMap((_) => _.minApproval !== 0 && _.minRemoval !== 0 ? + this.accountHttp.getMultisigAccountGraphInfo(_.account.address) + .pipe( + map((graphInfo) => this.validateCosignatories(graphInfo, signers)), + ) : observableOf(true), + ), + ), + ), + ); + } + + /** + * Validate cosignatories against multisig Account(s) + * @param graphInfo - multisig account graph info + * @param cosignatories - array of cosignatories extracted from aggregated transaction + * @returns {boolean} + */ + private validateCosignatories(graphInfo: MultisigAccountGraphInfo, cosignatories: string[]): boolean { + /** + * Validate cosignatories from bottom level to top + */ + const sortedKeys = Array.from(graphInfo.multisigAccounts.keys()).sort((a, b) => b - a); + const cosignatoriesReceived = cosignatories; + let validationResult = true; + + sortedKeys.forEach((key) => { + const multisigInfo = graphInfo.multisigAccounts.get(key); + if (multisigInfo && validationResult) { + multisigInfo.forEach((multisig) => { + if (multisig.minApproval >= 1) { + const matchedCosignatories = this.compareArrays(cosignatoriesReceived, + multisig.cosignatories.map((cosig) => cosig.publicKey)); + + /** + * if minimal signature requirement met at current level, push the multisig account + * into the received signatories array for next level validation. + * Otherwise return validation failed. + */ + if (matchedCosignatories.length >= multisig.minApproval) { + if (cosignatoriesReceived.indexOf(multisig.account.publicKey) === -1) { + cosignatoriesReceived.push(multisig.account.publicKey); + } + } else { + validationResult = false; + } + } + }); + } + }); + + return validationResult; + } + + /** + * Compare two string arrays + * @param array1 - base array + * @param array2 - array to be matched + * @returns {string[]} - array of matched elements + */ + private compareArrays(array1: string[], array2: string[]): string[] { + const results: string[] = []; + array1.forEach((a1) => array2.forEach((a2) => { + if (a1 === a2) { + results.push(a1); + } + })); + + return results; + } +} diff --git a/src/service/service.ts b/src/service/service.ts index 6a9945c1cd..fcf4c84680 100644 --- a/src/service/service.ts +++ b/src/service/service.ts @@ -16,3 +16,4 @@ export * from './NamespaceService'; export * from './MosaicService'; +export * from './AggregatedTransactionService'; diff --git a/test/service/AggregatedTransactionService.spec.ts b/test/service/AggregatedTransactionService.spec.ts new file mode 100644 index 0000000000..0aff3d8f47 --- /dev/null +++ b/test/service/AggregatedTransactionService.spec.ts @@ -0,0 +1,242 @@ +/* + * Copyright 2018 NEM + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { expect } from 'chai'; +import { ChronoUnit } from 'js-joda'; +import {of as observableOf} from 'rxjs'; +import {deepEqual, instance, mock, when} from 'ts-mockito'; +import { AccountHttp } from '../../src/infrastructure/AccountHttp'; +import { Account } from '../../src/model/account/Account'; +import { Address } from '../../src/model/account/Address'; +import { MultisigAccountGraphInfo } from '../../src/model/account/MultisigAccountGraphInfo'; +import { MultisigAccountInfo } from '../../src/model/account/MultisigAccountInfo'; +import {NetworkType} from '../../src/model/blockchain/NetworkType'; +import { AggregateTransaction } from '../../src/model/transaction/AggregateTransaction'; +import { Deadline } from '../../src/model/transaction/Deadline'; +import { PlainMessage } from '../../src/model/transaction/PlainMessage'; +import { TransferTransaction } from '../../src/model/transaction/TransferTransaction'; +import { AggregatedTransactionService } from '../../src/service/AggregatedTransactionService'; + +/** + * For multi level multisig scenario visit: https://github.com/nemtech/nem2-docs/issues/10 + */ +describe('AggregatedTransactionService', () => { + let aggregatedTransactionService: AggregatedTransactionService; + + /** + * Multisig2 Account: SBROWP-7YMG2M-K45RO6-Q7ZPK7-G7GXWQ-JK5VNQ-OSUX + * Public Key: 5E628EA59818D97AA4118780D9A88C5512FCE7A21C195E1574727EFCE5DF7C0D + * Private Key: 22A1D67F8519D1A45BD7116600BB6E857786E816FE0B45E4C5B9FFF3D64BC177 + * + * + * Multisig1 Account: SAK32M-5JQ43R-WYHWEH-WRBCW4-RXERT2-DLASGL-EANS + * Public Key: BFDF2610C5666A626434FE12FB4A9D896D2B9B033F5F84CCEABE82E043A6307E + * Private Key: 8B0622C2CCFC5CCC5A74B500163E3C68F3AD3643DB12932FC931143EAC67280D + */ + + /** + * Test accounts: + * Multisig1 (1/1): Account2, Account3 + * Multisig2 (2/1): Account1, Multisig1 + * Stranger Account: Account4 + */ + + const account1 = Account.createFromPrivateKey('82DB2528834C9926F0FCCE042466B24A266F5B685CB66D2869AF6648C043E950', + NetworkType.MIJIN_TEST); + const multisig1 = Account.createFromPrivateKey('8B0622C2CCFC5CCC5A74B500163E3C68F3AD3643DB12932FC931143EAC67280D', + NetworkType.MIJIN_TEST); + const multisig2 = Account.createFromPrivateKey('22A1D67F8519D1A45BD7116600BB6E857786E816FE0B45E4C5B9FFF3D64BC177', + NetworkType.MIJIN_TEST); + + const account2 = Account.createFromPrivateKey('A4D410270E01CECDCDEADCDE32EC79C8D9CDEA4DCD426CB1EB666EFEF148FBCE', + NetworkType.MIJIN_TEST); + const account3 = Account.createFromPrivateKey('336AB45EE65A6AFFC0E7ADC5342F91E34BACA0B901A1D9C876FA25A1E590077E', + NetworkType.MIJIN_TEST); + + const account4 = Account.createFromPrivateKey('4D8B3756592532753344E11E2B7541317BCCFBBCF4444274CDBF359D2C4AE0F1', + NetworkType.MIJIN_TEST); + before(() => { + const mockedAccountHttp = mock(AccountHttp); + + when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(account1.address))) + .thenReturn(observableOf(givenAccount1Info())); + when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(account4.address))) + .thenReturn(observableOf(givenAccount4Info())); + when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(multisig2.address))) + .thenReturn(observableOf(givenMultisig2AccountInfo())); + when(mockedAccountHttp.getMultisigAccountGraphInfo(deepEqual(multisig2.address))) + .thenReturn(observableOf(givenMultisig2AccountGraphInfo())); + + const accountHttp = instance(mockedAccountHttp); + aggregatedTransactionService = new AggregatedTransactionService(accountHttp); + }); + + it('should return isComplete: true for aggregated complete transaction - 2 levels Multisig', () => { + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(multisig2.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account2]); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.true; + }); + }); + + it('should return isComplete: false for aggregated complete transaction - 2 levels Multisig', () => { + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(multisig2.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, []); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.false; + }); + }); + + it('should return isComplete: false for aggregated complete transaction - 2 levels Multisig', () => { + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(multisig2.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account4]); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.false; + }); + }); + + it('should return correct isComplete status for aggregated complete transaction - 2 levels Multisig, multi inner transaction', () => { + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const transferTransaction2 = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(multisig2.publicAccount), + transferTransaction.toAggregate(account4.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account2]); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.true; + }); + }); + + it('should return correct isComplete status for aggregated complete transaction - none multisig', () => { + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(account4.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signWith(account1); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.true; + }); + }); + + function givenMultisig2AccountInfo(): MultisigAccountInfo { + return new MultisigAccountInfo(multisig2.publicAccount, + 2, 1, + [multisig1.publicAccount, + account1.publicAccount], + [], + ); + } + + function givenAccount1Info(): MultisigAccountInfo { + return new MultisigAccountInfo(account1.publicAccount, + 0, 0, + [], + [multisig2.publicAccount], + ); + } + function givenAccount4Info(): MultisigAccountInfo { + return new MultisigAccountInfo(account4.publicAccount, + 0, 0, + [], + [], + ); + } + + function givenMultisig2AccountGraphInfo(): MultisigAccountGraphInfo { + const map = new Map(); + map.set(0, [new MultisigAccountInfo(multisig2.publicAccount, + 2, 1, + [multisig1.publicAccount, + account1.publicAccount], + [], + )]) + .set(1, [new MultisigAccountInfo(multisig1.publicAccount, + 1, 1, + [account2.publicAccount, account3.publicAccount], + [multisig2.publicAccount], + )]); + + return new MultisigAccountGraphInfo(map); + } + +}); From 478c8a4e4f2020415a12240c31732c36057279c3 Mon Sep 17 00:00:00 2001 From: Steven Liu Date: Mon, 1 Apr 2019 21:25:50 +0100 Subject: [PATCH 09/31] Updated license year --- test/service/AggregatedTransactionService.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/service/AggregatedTransactionService.spec.ts b/test/service/AggregatedTransactionService.spec.ts index 0aff3d8f47..bc0c11f535 100644 --- a/test/service/AggregatedTransactionService.spec.ts +++ b/test/service/AggregatedTransactionService.spec.ts @@ -1,5 +1,5 @@ /* - * Copyright 2018 NEM + * Copyright 2019 NEM * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. From 3d1d1f6d4aeeee632d12785fe0f4fdcc2ca62d5b Mon Sep 17 00:00:00 2001 From: Steven Liu Date: Wed, 3 Apr 2019 16:12:08 +0100 Subject: [PATCH 10/31] Added check in validate cosignatories for modify multisig account (removal) --- src/service/AggregatedTransactionService.ts | 29 ++++++++++++++++--- .../AggregatedTransactionService.spec.ts | 27 +++++++++++++++++ 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/src/service/AggregatedTransactionService.ts b/src/service/AggregatedTransactionService.ts index ccdd72cd3b..7803b48bae 100644 --- a/src/service/AggregatedTransactionService.ts +++ b/src/service/AggregatedTransactionService.ts @@ -20,7 +20,11 @@ import { TransactionMapping } from '../core/utils/TransactionMapping'; import { AccountHttp } from '../infrastructure/AccountHttp'; import { MultisigAccountGraphInfo } from '../model/account/MultisigAccountGraphInfo'; import { AggregateTransaction } from '../model/transaction/AggregateTransaction'; +import { InnerTransaction } from '../model/transaction/InnerTransaction'; +import { ModifyMultisigAccountTransaction } from '../model/transaction/ModifyMultisigAccountTransaction'; +import { MultisigCosignatoryModificationType } from '../model/transaction/MultisigCosignatoryModificationType'; import { SignedTransaction } from '../model/transaction/SignedTransaction'; +import { TransactionType } from '../model/transaction/TransactionType'; /** * Aggregated Transaction service @@ -58,7 +62,7 @@ export class AggregatedTransactionService { mergeMap((_) => _.minApproval !== 0 && _.minRemoval !== 0 ? this.accountHttp.getMultisigAccountGraphInfo(_.account.address) .pipe( - map((graphInfo) => this.validateCosignatories(graphInfo, signers)), + map((graphInfo) => this.validateCosignatories(graphInfo, signers, innerTransaction)), ) : observableOf(true), ), ), @@ -70,9 +74,12 @@ export class AggregatedTransactionService { * Validate cosignatories against multisig Account(s) * @param graphInfo - multisig account graph info * @param cosignatories - array of cosignatories extracted from aggregated transaction + * @param innerTransaction - the inner transaction of the aggregated transaction * @returns {boolean} */ - private validateCosignatories(graphInfo: MultisigAccountGraphInfo, cosignatories: string[]): boolean { + private validateCosignatories(graphInfo: MultisigAccountGraphInfo, + cosignatories: string[], + innerTransaction: InnerTransaction): boolean { /** * Validate cosignatories from bottom level to top */ @@ -80,11 +87,24 @@ export class AggregatedTransactionService { const cosignatoriesReceived = cosignatories; let validationResult = true; + let isMultisigRemoval = false; + + /** + * Check inner transaction. If remove cosigner from multisig account, + * use minRemoval instead of minApproval for cosignatories validation. + */ + if (innerTransaction.type === TransactionType.MODIFY_MULTISIG_ACCOUNT) { + if ((innerTransaction as ModifyMultisigAccountTransaction).modifications + .find((modification) => modification.type === MultisigCosignatoryModificationType.Remove) !== undefined) { + isMultisigRemoval = true; + } + } + sortedKeys.forEach((key) => { const multisigInfo = graphInfo.multisigAccounts.get(key); if (multisigInfo && validationResult) { multisigInfo.forEach((multisig) => { - if (multisig.minApproval >= 1) { + if (multisig.minApproval >= 1 && multisig.minRemoval) { // To make sure it is multisig account const matchedCosignatories = this.compareArrays(cosignatoriesReceived, multisig.cosignatories.map((cosig) => cosig.publicKey)); @@ -93,7 +113,8 @@ export class AggregatedTransactionService { * into the received signatories array for next level validation. * Otherwise return validation failed. */ - if (matchedCosignatories.length >= multisig.minApproval) { + if ((matchedCosignatories.length >= multisig.minApproval && !isMultisigRemoval) || + (matchedCosignatories.length >= multisig.minRemoval && isMultisigRemoval)) { if (cosignatoriesReceived.indexOf(multisig.account.publicKey) === -1) { cosignatoriesReceived.push(multisig.account.publicKey); } diff --git a/test/service/AggregatedTransactionService.spec.ts b/test/service/AggregatedTransactionService.spec.ts index bc0c11f535..8ea1432ca6 100644 --- a/test/service/AggregatedTransactionService.spec.ts +++ b/test/service/AggregatedTransactionService.spec.ts @@ -26,6 +26,9 @@ import { MultisigAccountInfo } from '../../src/model/account/MultisigAccountInfo import {NetworkType} from '../../src/model/blockchain/NetworkType'; import { AggregateTransaction } from '../../src/model/transaction/AggregateTransaction'; import { Deadline } from '../../src/model/transaction/Deadline'; +import { ModifyMultisigAccountTransaction } from '../../src/model/transaction/ModifyMultisigAccountTransaction'; +import { MultisigCosignatoryModification } from '../../src/model/transaction/MultisigCosignatoryModification'; +import { MultisigCosignatoryModificationType } from '../../src/model/transaction/MultisigCosignatoryModificationType'; import { PlainMessage } from '../../src/model/transaction/PlainMessage'; import { TransferTransaction } from '../../src/model/transaction/TransferTransaction'; import { AggregatedTransactionService } from '../../src/service/AggregatedTransactionService'; @@ -177,6 +180,30 @@ describe('AggregatedTransactionService', () => { }); }); + it('should use minRemoval for multisig account validation if inner transaction is modify multisig remove', () => { + const modifyMultisigTransaction = ModifyMultisigAccountTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + 1, + 1, + [new MultisigCosignatoryModification( + MultisigCosignatoryModificationType.Remove, + account1.publicAccount, + )], + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [modifyMultisigTransaction.toAggregate(multisig2.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signWith(account2); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.true; + }); + }); + it('should return correct isComplete status for aggregated complete transaction - none multisig', () => { const transferTransaction = TransferTransaction.create( Deadline.create(1, ChronoUnit.HOURS), From 7a420e7cafa33c6d7935fa18b9c53387cc59c548 Mon Sep 17 00:00:00 2001 From: Steven Liu Date: Thu, 4 Apr 2019 15:03:56 +0100 Subject: [PATCH 11/31] Fixed bug in create transaction from payload --- .../transaction/CreateTransactionFromPayload.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/infrastructure/transaction/CreateTransactionFromPayload.ts b/src/infrastructure/transaction/CreateTransactionFromPayload.ts index 9b124a33bf..7b73d6f6d0 100644 --- a/src/infrastructure/transaction/CreateTransactionFromPayload.ts +++ b/src/infrastructure/transaction/CreateTransactionFromPayload.ts @@ -376,7 +376,7 @@ const CreateTransaction = (type: number, transactionData: string, networkType: N }), networkType, consignatureArray ? consignatureArray.map((cosignature) => new AggregateTransactionCosignature( - cosignature.substring(0, 64), + cosignature.substring(64, 192), PublicAccount.createFromPublicKey(cosignature.substring(0, 64), networkType), )) : [], ); @@ -400,7 +400,7 @@ const CreateTransaction = (type: number, transactionData: string, networkType: N }), networkType, bondedConsignatureArray ? bondedConsignatureArray.map((cosignature) => new AggregateTransactionCosignature( - cosignature.substring(0, 64), + cosignature.substring(64, 192), PublicAccount.createFromPublicKey(cosignature.substring(0, 64), networkType), )) : [], ); From c37da2d89f59673d88007c1845985af86d36cb5a Mon Sep 17 00:00:00 2001 From: Steven Liu Date: Fri, 5 Apr 2019 19:01:52 +0100 Subject: [PATCH 12/31] Added more test cases and fixed a few bugs --- .../CreateTransactionFromPayload.ts | 4 +- src/service/AggregatedTransactionService.ts | 12 +- .../AggregatedTransactionService.spec.ts | 305 +++++++++++++++++- 3 files changed, 309 insertions(+), 12 deletions(-) diff --git a/src/infrastructure/transaction/CreateTransactionFromPayload.ts b/src/infrastructure/transaction/CreateTransactionFromPayload.ts index 7b73d6f6d0..bf36d24a20 100644 --- a/src/infrastructure/transaction/CreateTransactionFromPayload.ts +++ b/src/infrastructure/transaction/CreateTransactionFromPayload.ts @@ -456,8 +456,8 @@ const parseInnerTransactionFromBinary = (innerTransactionBinary: string): string let innerBinary = innerTransactionBinary; while (innerBinary.length) { - const payloadSize = parseInt(convert.uint8ToHex(convert.hexToUint8(innerTransactionBinary.substring(0, 8)).reverse()), 16) * 2; - const innerTransaction = innerTransactionBinary.substring(8, payloadSize); + const payloadSize = parseInt(convert.uint8ToHex(convert.hexToUint8(innerBinary.substring(0, 8)).reverse()), 16) * 2; + const innerTransaction = innerBinary.substring(8, payloadSize); embeddedTransaction.push(innerTransaction); innerBinary = innerBinary.substring(payloadSize); } diff --git a/src/service/AggregatedTransactionService.ts b/src/service/AggregatedTransactionService.ts index 7803b48bae..75a806bf96 100644 --- a/src/service/AggregatedTransactionService.ts +++ b/src/service/AggregatedTransactionService.ts @@ -15,7 +15,7 @@ */ import {from as observableFrom , Observable, of as observableOf} from 'rxjs'; -import { map, mergeMap} from 'rxjs/operators'; +import { flatMap, map, mergeMap, toArray} from 'rxjs/operators'; import { TransactionMapping } from '../core/utils/TransactionMapping'; import { AccountHttp } from '../infrastructure/AccountHttp'; import { MultisigAccountGraphInfo } from '../model/account/MultisigAccountGraphInfo'; @@ -52,7 +52,6 @@ export class AggregatedTransactionService { if (signedTransaction.signer) { signers.push(signedTransaction.signer); } - return observableFrom(aggregateTransaction.innerTransactions).pipe( mergeMap((innerTransaction) => this.accountHttp.getMultisigAccountInfo(innerTransaction.signer.address) .pipe( @@ -63,11 +62,16 @@ export class AggregatedTransactionService { this.accountHttp.getMultisigAccountGraphInfo(_.account.address) .pipe( map((graphInfo) => this.validateCosignatories(graphInfo, signers, innerTransaction)), - ) : observableOf(true), + ) : observableOf(signers.find((s) => s === _.account.publicKey ) !== undefined), ), ), ), - ); + toArray(), + ).pipe( + flatMap((results) => { + return observableOf(results.every((isComplete) => isComplete)); + }), + ); } /** diff --git a/test/service/AggregatedTransactionService.spec.ts b/test/service/AggregatedTransactionService.spec.ts index 8ea1432ca6..23470cb2a1 100644 --- a/test/service/AggregatedTransactionService.spec.ts +++ b/test/service/AggregatedTransactionService.spec.ts @@ -54,6 +54,7 @@ describe('AggregatedTransactionService', () => { * Test accounts: * Multisig1 (1/1): Account2, Account3 * Multisig2 (2/1): Account1, Multisig1 + * Multisig3 (2/2): Account2, Account3 * Stranger Account: Account4 */ @@ -64,6 +65,8 @@ describe('AggregatedTransactionService', () => { const multisig2 = Account.createFromPrivateKey('22A1D67F8519D1A45BD7116600BB6E857786E816FE0B45E4C5B9FFF3D64BC177', NetworkType.MIJIN_TEST); + const multisig3 = Account.createFromPrivateKey('5E7812AB0E709ABC45466034E1A209099F6A12C4698748A63CDCAA9B0DDE1DBD', + NetworkType.MIJIN_TEST); const account2 = Account.createFromPrivateKey('A4D410270E01CECDCDEADCDE32EC79C8D9CDEA4DCD426CB1EB666EFEF148FBCE', NetworkType.MIJIN_TEST); const account3 = Account.createFromPrivateKey('336AB45EE65A6AFFC0E7ADC5342F91E34BACA0B901A1D9C876FA25A1E590077E', @@ -80,14 +83,30 @@ describe('AggregatedTransactionService', () => { .thenReturn(observableOf(givenAccount4Info())); when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(multisig2.address))) .thenReturn(observableOf(givenMultisig2AccountInfo())); + when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(multisig3.address))) + .thenReturn(observableOf(givenMultisig3AccountInfo())); when(mockedAccountHttp.getMultisigAccountGraphInfo(deepEqual(multisig2.address))) .thenReturn(observableOf(givenMultisig2AccountGraphInfo())); + when(mockedAccountHttp.getMultisigAccountGraphInfo(deepEqual(multisig3.address))) + .thenReturn(observableOf(givenMultisig3AccountGraphInfo())); + when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(account2.address))) + .thenReturn(observableOf(givenAccount2Info())); + when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(account3.address))) + .thenReturn(observableOf(givenAccount3Info())); const accountHttp = instance(mockedAccountHttp); aggregatedTransactionService = new AggregatedTransactionService(accountHttp); }); it('should return isComplete: true for aggregated complete transaction - 2 levels Multisig', () => { + /** + * MLMA + * Alice (account1): normal account + * Bob (multisig2) - Multisig 2-1 (account1 && multisig1) + * Charles (multisig1) - Multisig 1-1 (account2 || account3) + * Given signatories: Account1 && Account4 + * Expecting complete as Bob needs 2 signatures (account1 && (account2 || account3)) + */ const transferTransaction = TransferTransaction.create( Deadline.create(1, ChronoUnit.HOURS), Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), @@ -109,6 +128,14 @@ describe('AggregatedTransactionService', () => { }); it('should return isComplete: false for aggregated complete transaction - 2 levels Multisig', () => { + /** + * MLMA + * Alice (account1): normal account + * Bob (multisig2) - Multisig 2-1 (account1 && multisig1) + * Charles (multisig1) - Multisig 1-1 (account2 || account3) + * Given signatories: Account1 && Account4 + * Expecting incomplete as Bob needs 2 signatures (account1 && (account2 || account3)) but only got account1 + */ const transferTransaction = TransferTransaction.create( Deadline.create(1, ChronoUnit.HOURS), Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), @@ -130,6 +157,14 @@ describe('AggregatedTransactionService', () => { }); it('should return isComplete: false for aggregated complete transaction - 2 levels Multisig', () => { + /** + * MLMA + * Alice (account1): normal account + * Bob (multisig2) - Multisig 2-1 (account1 && multisig1) + * Charles (multisig1) - Multisig 1-1 (account2 || account3) + * Given signatories: Account1 && Account4 + * Expecting incomplete as Bob needs 2 signatures (account1 && (account2 || account3)) but got account4 + */ const transferTransaction = TransferTransaction.create( Deadline.create(1, ChronoUnit.HOURS), Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), @@ -151,9 +186,18 @@ describe('AggregatedTransactionService', () => { }); it('should return correct isComplete status for aggregated complete transaction - 2 levels Multisig, multi inner transaction', () => { + /** + * MLMA - with multiple transaction + * Alice (account1): normal account + * Bob (multisig2) - Multisig 2-1 (account1 && multisig1) + * Charles (multisig1) - Multisig 1-1 (account2 || account3) + * An extra inner transaction to account4 (just to increase the complexity) + * Given signatories: Account1 && Account4 + * Expecting incomplete as Bob needs 2 signatures (account1 && (account2 || account3)) + */ const transferTransaction = TransferTransaction.create( Deadline.create(1, ChronoUnit.HOURS), - Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + account2.address, [], PlainMessage.create('test-message'), NetworkType.MIJIN_TEST, @@ -161,7 +205,7 @@ describe('AggregatedTransactionService', () => { const transferTransaction2 = TransferTransaction.create( Deadline.create(1, ChronoUnit.HOURS), - Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + account2.address, [], PlainMessage.create('test-message'), NetworkType.MIJIN_TEST, @@ -170,17 +214,60 @@ describe('AggregatedTransactionService', () => { const aggregateTransaction = AggregateTransaction.createComplete( Deadline.create(), [transferTransaction.toAggregate(multisig2.publicAccount), - transferTransaction.toAggregate(account4.publicAccount)], + transferTransaction2.toAggregate(account4.publicAccount)], NetworkType.MIJIN_TEST, []); + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account4]); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.false; + }); + }); - const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account2]); + it('should return correct isComplete status for aggregated complete transaction - 2 levels Multisig, multi inner transaction', () => { + /** + * MLMA - with multiple transaction + * Alice (account1): normal account + * Bob (multisig2) - Multisig 2-1 (account1 && multisig1) + * Charles (multisig1) - Multisig 1-1 (account2 || account3) + * An extra inner transaction to account4 (just to increase the complexity) + * Given signatories: Account1 && Account4 && Account2 + * Expecting complete + */ + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + account2.address, + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const transferTransaction2 = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + account2.address, + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(multisig2.publicAccount), + transferTransaction2.toAggregate(account4.publicAccount)], + NetworkType.MIJIN_TEST, + []); + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account4, account2]); aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.true; }); }); it('should use minRemoval for multisig account validation if inner transaction is modify multisig remove', () => { + /** + * If the inner transaction is issued to a multisig account + * and the inner transaction itself is a ModifyMultiSigAccountTransaction - Removal + * The validator should use minRemoval value rather than minApproval value + * to determine if the act is complete or not + */ const modifyMultisigTransaction = ModifyMultisigAccountTransaction.create( Deadline.create(1, ChronoUnit.HOURS), 1, @@ -197,14 +284,19 @@ describe('AggregatedTransactionService', () => { [modifyMultisigTransaction.toAggregate(multisig2.publicAccount)], NetworkType.MIJIN_TEST, []); - const signedTransaction = aggregateTransaction.signWith(account2); aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.true; }); }); - it('should return correct isComplete status for aggregated complete transaction - none multisig', () => { + it('should return correct isComplete status (false) for aggregated complete transaction - none multisig', () => { + /** + * If the inner transaction is issued to a multisig account + * and the inner transaction itself is a ModifyMultiSigAccountTransaction - Removal + * The validator should use minRemoval value rather than minApproval value + * to determine if the act is complete or not + */ const transferTransaction = TransferTransaction.create( Deadline.create(1, ChronoUnit.HOURS), Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), @@ -220,11 +312,176 @@ describe('AggregatedTransactionService', () => { []); const signedTransaction = aggregateTransaction.signWith(account1); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.false; + }); + }); + + it('should return correct isComplete status (true) for aggregated complete transaction - none multisig', () => { + /** + * ACT + * Alice (account1): normal account + * Bob (account4) - normal account + * Alice initiate the transaction + * Bob sign + */ + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(account4.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signWith(account4); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.true; + }); + }); + + it('should return correct isComplete status TRUE - multiple normal account', () => { + /** + * ACT + * Alice: account1 + * Bog: account4 + * An escrow contract is signed by all the participants (normal accounts) + * Given Alice defined the following escrow contract: + * | sender | recipient | type | data | + * | Alice | Bob | send-an-asset | 1 concert.ticket | + * | Bob | Alice | send-an-asset | 20 euros | + * And Bob signs the contract + * And Alice signs the contract + * Then the contract should appear as complete + */ + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + account1.address, + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const transferTransaction2 = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + account4.address, + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(account4.publicAccount), + transferTransaction2.toAggregate(account1.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account4]); aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.true; }); }); + it('should return correct isComplete status FALSE - multiple normal account', () => { + /** + * ACT + * Alice: account1 + * Bog: account4 + * An escrow contract is signed by all the participants (normal accounts) + * Given Alice defined the following escrow contract: + * | sender | recipient | type | data | + * | Alice | Bob | send-an-asset | 1 concert.ticket | + * | Bob | Alice | send-an-asset | 20 euros | + * And Alice signs the contract + * Then the contract should appear as incomplete + */ + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + account1.address, + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const transferTransaction2 = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + account4.address, + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(account4.publicAccount), + transferTransaction2.toAggregate(account1.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, []); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.false; + }); + }); + + it('should return correct isComplete status TRUE - multisig Single Level', () => { + /** + * ACT - Multisig single level + * Alice (account1): initiate an transfer to Bob + * Bob (multisig3): is a 2/2 multisig account (account2 && account3) + */ + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + account4.address, + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(multisig3.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account2, [account3]); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.true; + }); + }); + + it('should return correct isComplete status FALSE - multisig Single Level', () => { + /** + * ACT - Multisig single level + * Alice (account1): initiate an transfer to Bob + * Bob (multisig3): is a 2/2 multisig account (account2 && account3) + */ + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + account4.address, + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(multisig3.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account2, []); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.false; + }); + }); + function givenMultisig2AccountInfo(): MultisigAccountInfo { return new MultisigAccountInfo(multisig2.publicAccount, 2, 1, @@ -233,6 +490,14 @@ describe('AggregatedTransactionService', () => { [], ); } + function givenMultisig3AccountInfo(): MultisigAccountInfo { + return new MultisigAccountInfo(multisig3.publicAccount, + 2, 2, + [account2.publicAccount, + account3.publicAccount], + [], + ); + } function givenAccount1Info(): MultisigAccountInfo { return new MultisigAccountInfo(account1.publicAccount, @@ -241,6 +506,22 @@ describe('AggregatedTransactionService', () => { [multisig2.publicAccount], ); } + function givenAccount2Info(): MultisigAccountInfo { + return new MultisigAccountInfo(account2.publicAccount, + 0, 0, + [], + [multisig2.publicAccount, + multisig3.publicAccount], + ); + } + function givenAccount3Info(): MultisigAccountInfo { + return new MultisigAccountInfo(account3.publicAccount, + 0, 0, + [], + [multisig2.publicAccount, + multisig3.publicAccount], + ); + } function givenAccount4Info(): MultisigAccountInfo { return new MultisigAccountInfo(account4.publicAccount, 0, 0, @@ -266,4 +547,16 @@ describe('AggregatedTransactionService', () => { return new MultisigAccountGraphInfo(map); } + function givenMultisig3AccountGraphInfo(): MultisigAccountGraphInfo { + const map = new Map(); + map.set(0, [new MultisigAccountInfo(multisig3.publicAccount, + 2, 2, + [account2.publicAccount, + account3.publicAccount], + [], + )]); + + return new MultisigAccountGraphInfo(map); + } + }); From eb2e12f0ffa62c1f683f197198ceb0eeb00ca01b Mon Sep 17 00:00:00 2001 From: Steven Liu Date: Wed, 10 Apr 2019 15:13:01 +0100 Subject: [PATCH 13/31] renamed AggregatedTransactionService --- ...vice.ts => AggregateTransactionService.ts} | 2 +- src/service/service.ts | 2 +- ...ts => AggregateTransactionService.spec.ts} | 32 +++++++++---------- 3 files changed, 18 insertions(+), 18 deletions(-) rename src/service/{AggregatedTransactionService.ts => AggregateTransactionService.ts} (99%) rename test/service/{AggregatedTransactionService.spec.ts => AggregateTransactionService.spec.ts} (93%) diff --git a/src/service/AggregatedTransactionService.ts b/src/service/AggregateTransactionService.ts similarity index 99% rename from src/service/AggregatedTransactionService.ts rename to src/service/AggregateTransactionService.ts index 75a806bf96..800d1a1b89 100644 --- a/src/service/AggregatedTransactionService.ts +++ b/src/service/AggregateTransactionService.ts @@ -29,7 +29,7 @@ import { TransactionType } from '../model/transaction/TransactionType'; /** * Aggregated Transaction service */ -export class AggregatedTransactionService { +export class AggregateTransactionService { /** * Constructor diff --git a/src/service/service.ts b/src/service/service.ts index fcf4c84680..c988e13ffb 100644 --- a/src/service/service.ts +++ b/src/service/service.ts @@ -16,4 +16,4 @@ export * from './NamespaceService'; export * from './MosaicService'; -export * from './AggregatedTransactionService'; +export * from './AggregateTransactionService'; diff --git a/test/service/AggregatedTransactionService.spec.ts b/test/service/AggregateTransactionService.spec.ts similarity index 93% rename from test/service/AggregatedTransactionService.spec.ts rename to test/service/AggregateTransactionService.spec.ts index 23470cb2a1..ae2deab99b 100644 --- a/test/service/AggregatedTransactionService.spec.ts +++ b/test/service/AggregateTransactionService.spec.ts @@ -31,13 +31,13 @@ import { MultisigCosignatoryModification } from '../../src/model/transaction/Mul import { MultisigCosignatoryModificationType } from '../../src/model/transaction/MultisigCosignatoryModificationType'; import { PlainMessage } from '../../src/model/transaction/PlainMessage'; import { TransferTransaction } from '../../src/model/transaction/TransferTransaction'; -import { AggregatedTransactionService } from '../../src/service/AggregatedTransactionService'; +import { AggregateTransactionService } from '../../src/service/AggregateTransactionService'; /** * For multi level multisig scenario visit: https://github.com/nemtech/nem2-docs/issues/10 */ -describe('AggregatedTransactionService', () => { - let aggregatedTransactionService: AggregatedTransactionService; +describe('AggregateTransactionService', () => { + let aggregateTransactionService: AggregateTransactionService; /** * Multisig2 Account: SBROWP-7YMG2M-K45RO6-Q7ZPK7-G7GXWQ-JK5VNQ-OSUX @@ -95,7 +95,7 @@ describe('AggregatedTransactionService', () => { .thenReturn(observableOf(givenAccount3Info())); const accountHttp = instance(mockedAccountHttp); - aggregatedTransactionService = new AggregatedTransactionService(accountHttp); + aggregateTransactionService = new AggregateTransactionService(accountHttp); }); it('should return isComplete: true for aggregated complete transaction - 2 levels Multisig', () => { @@ -122,7 +122,7 @@ describe('AggregatedTransactionService', () => { []); const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account2]); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + aggregateTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.true; }); }); @@ -151,7 +151,7 @@ describe('AggregatedTransactionService', () => { []); const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, []); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + aggregateTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.false; }); }); @@ -180,7 +180,7 @@ describe('AggregatedTransactionService', () => { []); const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account4]); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + aggregateTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.false; }); }); @@ -218,7 +218,7 @@ describe('AggregatedTransactionService', () => { NetworkType.MIJIN_TEST, []); const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account4]); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + aggregateTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.false; }); }); @@ -256,7 +256,7 @@ describe('AggregatedTransactionService', () => { NetworkType.MIJIN_TEST, []); const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account4, account2]); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + aggregateTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.true; }); }); @@ -285,7 +285,7 @@ describe('AggregatedTransactionService', () => { NetworkType.MIJIN_TEST, []); const signedTransaction = aggregateTransaction.signWith(account2); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + aggregateTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.true; }); }); @@ -312,7 +312,7 @@ describe('AggregatedTransactionService', () => { []); const signedTransaction = aggregateTransaction.signWith(account1); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + aggregateTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.false; }); }); @@ -340,7 +340,7 @@ describe('AggregatedTransactionService', () => { []); const signedTransaction = aggregateTransaction.signWith(account4); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + aggregateTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.true; }); }); @@ -383,7 +383,7 @@ describe('AggregatedTransactionService', () => { []); const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account4]); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + aggregateTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.true; }); }); @@ -425,7 +425,7 @@ describe('AggregatedTransactionService', () => { []); const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, []); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + aggregateTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.false; }); }); @@ -451,7 +451,7 @@ describe('AggregatedTransactionService', () => { []); const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account2, [account3]); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + aggregateTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.true; }); }); @@ -477,7 +477,7 @@ describe('AggregatedTransactionService', () => { []); const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account2, []); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + aggregateTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.false; }); }); From 5776201f72278a200cd7cafa2ef3c00abc80c88a Mon Sep 17 00:00:00 2001 From: Steven Liu Date: Mon, 1 Apr 2019 20:39:11 +0100 Subject: [PATCH 14/31] #4 1. Addes service for Aggregated Transaction - isComplete 2. Fixed couple of bugs in TransactionMappings --- .../CreateTransactionFromPayload.ts | 8 +- src/service/AggregatedTransactionService.ts | 127 +++++++++ src/service/service.ts | 1 + .../AggregatedTransactionService.spec.ts | 242 ++++++++++++++++++ 4 files changed, 374 insertions(+), 4 deletions(-) create mode 100644 src/service/AggregatedTransactionService.ts create mode 100644 test/service/AggregatedTransactionService.spec.ts diff --git a/src/infrastructure/transaction/CreateTransactionFromPayload.ts b/src/infrastructure/transaction/CreateTransactionFromPayload.ts index e8b66453ff..9b124a33bf 100644 --- a/src/infrastructure/transaction/CreateTransactionFromPayload.ts +++ b/src/infrastructure/transaction/CreateTransactionFromPayload.ts @@ -377,7 +377,7 @@ const CreateTransaction = (type: number, transactionData: string, networkType: N networkType, consignatureArray ? consignatureArray.map((cosignature) => new AggregateTransactionCosignature( cosignature.substring(0, 64), - PublicAccount.createFromPublicKey(cosignature.substring(64, 192), networkType), + PublicAccount.createFromPublicKey(cosignature.substring(0, 64), networkType), )) : [], ); case TransactionType.AGGREGATE_BONDED: @@ -401,7 +401,7 @@ const CreateTransaction = (type: number, transactionData: string, networkType: N networkType, bondedConsignatureArray ? bondedConsignatureArray.map((cosignature) => new AggregateTransactionCosignature( cosignature.substring(0, 64), - PublicAccount.createFromPublicKey(cosignature.substring(64, 192), networkType), + PublicAccount.createFromPublicKey(cosignature.substring(0, 64), networkType), )) : [], ); default: @@ -457,9 +457,9 @@ const parseInnerTransactionFromBinary = (innerTransactionBinary: string): string while (innerBinary.length) { const payloadSize = parseInt(convert.uint8ToHex(convert.hexToUint8(innerTransactionBinary.substring(0, 8)).reverse()), 16) * 2; - const innerTransaction = innerTransactionBinary.substring(8, 8 + payloadSize); + const innerTransaction = innerTransactionBinary.substring(8, payloadSize); embeddedTransaction.push(innerTransaction); - innerBinary = innerTransactionBinary.substring(8 + payloadSize); + innerBinary = innerBinary.substring(payloadSize); } return embeddedTransaction; }; diff --git a/src/service/AggregatedTransactionService.ts b/src/service/AggregatedTransactionService.ts new file mode 100644 index 0000000000..ccdd72cd3b --- /dev/null +++ b/src/service/AggregatedTransactionService.ts @@ -0,0 +1,127 @@ +/* + * Copyright 2019 NEM + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import {from as observableFrom , Observable, of as observableOf} from 'rxjs'; +import { map, mergeMap} from 'rxjs/operators'; +import { TransactionMapping } from '../core/utils/TransactionMapping'; +import { AccountHttp } from '../infrastructure/AccountHttp'; +import { MultisigAccountGraphInfo } from '../model/account/MultisigAccountGraphInfo'; +import { AggregateTransaction } from '../model/transaction/AggregateTransaction'; +import { SignedTransaction } from '../model/transaction/SignedTransaction'; + +/** + * Aggregated Transaction service + */ +export class AggregatedTransactionService { + + /** + * Constructor + * @param accountHttp + */ + constructor(private readonly accountHttp: AccountHttp) { + } + + /** + * Check if an aggregate complete transaction has all cosignatories attached + * @param signedTransaction - The signed aggregate transaction (complete) to be verified + * @returns {Observable} + */ + public isComplete(signedTransaction: SignedTransaction): Observable { + const aggregateTransaction = TransactionMapping.createFromPayload(signedTransaction.payload) as AggregateTransaction; + /** + * Include both initiator & cosigners + */ + const signers = (aggregateTransaction.cosignatures.map((cosigner) => cosigner.signer.publicKey)); + if (signedTransaction.signer) { + signers.push(signedTransaction.signer); + } + + return observableFrom(aggregateTransaction.innerTransactions).pipe( + mergeMap((innerTransaction) => this.accountHttp.getMultisigAccountInfo(innerTransaction.signer.address) + .pipe( + /** + * For multisig account, we need to get the graph info in case it has multiple levels + */ + mergeMap((_) => _.minApproval !== 0 && _.minRemoval !== 0 ? + this.accountHttp.getMultisigAccountGraphInfo(_.account.address) + .pipe( + map((graphInfo) => this.validateCosignatories(graphInfo, signers)), + ) : observableOf(true), + ), + ), + ), + ); + } + + /** + * Validate cosignatories against multisig Account(s) + * @param graphInfo - multisig account graph info + * @param cosignatories - array of cosignatories extracted from aggregated transaction + * @returns {boolean} + */ + private validateCosignatories(graphInfo: MultisigAccountGraphInfo, cosignatories: string[]): boolean { + /** + * Validate cosignatories from bottom level to top + */ + const sortedKeys = Array.from(graphInfo.multisigAccounts.keys()).sort((a, b) => b - a); + const cosignatoriesReceived = cosignatories; + let validationResult = true; + + sortedKeys.forEach((key) => { + const multisigInfo = graphInfo.multisigAccounts.get(key); + if (multisigInfo && validationResult) { + multisigInfo.forEach((multisig) => { + if (multisig.minApproval >= 1) { + const matchedCosignatories = this.compareArrays(cosignatoriesReceived, + multisig.cosignatories.map((cosig) => cosig.publicKey)); + + /** + * if minimal signature requirement met at current level, push the multisig account + * into the received signatories array for next level validation. + * Otherwise return validation failed. + */ + if (matchedCosignatories.length >= multisig.minApproval) { + if (cosignatoriesReceived.indexOf(multisig.account.publicKey) === -1) { + cosignatoriesReceived.push(multisig.account.publicKey); + } + } else { + validationResult = false; + } + } + }); + } + }); + + return validationResult; + } + + /** + * Compare two string arrays + * @param array1 - base array + * @param array2 - array to be matched + * @returns {string[]} - array of matched elements + */ + private compareArrays(array1: string[], array2: string[]): string[] { + const results: string[] = []; + array1.forEach((a1) => array2.forEach((a2) => { + if (a1 === a2) { + results.push(a1); + } + })); + + return results; + } +} diff --git a/src/service/service.ts b/src/service/service.ts index 6a9945c1cd..fcf4c84680 100644 --- a/src/service/service.ts +++ b/src/service/service.ts @@ -16,3 +16,4 @@ export * from './NamespaceService'; export * from './MosaicService'; +export * from './AggregatedTransactionService'; diff --git a/test/service/AggregatedTransactionService.spec.ts b/test/service/AggregatedTransactionService.spec.ts new file mode 100644 index 0000000000..0aff3d8f47 --- /dev/null +++ b/test/service/AggregatedTransactionService.spec.ts @@ -0,0 +1,242 @@ +/* + * Copyright 2018 NEM + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { expect } from 'chai'; +import { ChronoUnit } from 'js-joda'; +import {of as observableOf} from 'rxjs'; +import {deepEqual, instance, mock, when} from 'ts-mockito'; +import { AccountHttp } from '../../src/infrastructure/AccountHttp'; +import { Account } from '../../src/model/account/Account'; +import { Address } from '../../src/model/account/Address'; +import { MultisigAccountGraphInfo } from '../../src/model/account/MultisigAccountGraphInfo'; +import { MultisigAccountInfo } from '../../src/model/account/MultisigAccountInfo'; +import {NetworkType} from '../../src/model/blockchain/NetworkType'; +import { AggregateTransaction } from '../../src/model/transaction/AggregateTransaction'; +import { Deadline } from '../../src/model/transaction/Deadline'; +import { PlainMessage } from '../../src/model/transaction/PlainMessage'; +import { TransferTransaction } from '../../src/model/transaction/TransferTransaction'; +import { AggregatedTransactionService } from '../../src/service/AggregatedTransactionService'; + +/** + * For multi level multisig scenario visit: https://github.com/nemtech/nem2-docs/issues/10 + */ +describe('AggregatedTransactionService', () => { + let aggregatedTransactionService: AggregatedTransactionService; + + /** + * Multisig2 Account: SBROWP-7YMG2M-K45RO6-Q7ZPK7-G7GXWQ-JK5VNQ-OSUX + * Public Key: 5E628EA59818D97AA4118780D9A88C5512FCE7A21C195E1574727EFCE5DF7C0D + * Private Key: 22A1D67F8519D1A45BD7116600BB6E857786E816FE0B45E4C5B9FFF3D64BC177 + * + * + * Multisig1 Account: SAK32M-5JQ43R-WYHWEH-WRBCW4-RXERT2-DLASGL-EANS + * Public Key: BFDF2610C5666A626434FE12FB4A9D896D2B9B033F5F84CCEABE82E043A6307E + * Private Key: 8B0622C2CCFC5CCC5A74B500163E3C68F3AD3643DB12932FC931143EAC67280D + */ + + /** + * Test accounts: + * Multisig1 (1/1): Account2, Account3 + * Multisig2 (2/1): Account1, Multisig1 + * Stranger Account: Account4 + */ + + const account1 = Account.createFromPrivateKey('82DB2528834C9926F0FCCE042466B24A266F5B685CB66D2869AF6648C043E950', + NetworkType.MIJIN_TEST); + const multisig1 = Account.createFromPrivateKey('8B0622C2CCFC5CCC5A74B500163E3C68F3AD3643DB12932FC931143EAC67280D', + NetworkType.MIJIN_TEST); + const multisig2 = Account.createFromPrivateKey('22A1D67F8519D1A45BD7116600BB6E857786E816FE0B45E4C5B9FFF3D64BC177', + NetworkType.MIJIN_TEST); + + const account2 = Account.createFromPrivateKey('A4D410270E01CECDCDEADCDE32EC79C8D9CDEA4DCD426CB1EB666EFEF148FBCE', + NetworkType.MIJIN_TEST); + const account3 = Account.createFromPrivateKey('336AB45EE65A6AFFC0E7ADC5342F91E34BACA0B901A1D9C876FA25A1E590077E', + NetworkType.MIJIN_TEST); + + const account4 = Account.createFromPrivateKey('4D8B3756592532753344E11E2B7541317BCCFBBCF4444274CDBF359D2C4AE0F1', + NetworkType.MIJIN_TEST); + before(() => { + const mockedAccountHttp = mock(AccountHttp); + + when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(account1.address))) + .thenReturn(observableOf(givenAccount1Info())); + when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(account4.address))) + .thenReturn(observableOf(givenAccount4Info())); + when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(multisig2.address))) + .thenReturn(observableOf(givenMultisig2AccountInfo())); + when(mockedAccountHttp.getMultisigAccountGraphInfo(deepEqual(multisig2.address))) + .thenReturn(observableOf(givenMultisig2AccountGraphInfo())); + + const accountHttp = instance(mockedAccountHttp); + aggregatedTransactionService = new AggregatedTransactionService(accountHttp); + }); + + it('should return isComplete: true for aggregated complete transaction - 2 levels Multisig', () => { + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(multisig2.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account2]); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.true; + }); + }); + + it('should return isComplete: false for aggregated complete transaction - 2 levels Multisig', () => { + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(multisig2.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, []); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.false; + }); + }); + + it('should return isComplete: false for aggregated complete transaction - 2 levels Multisig', () => { + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(multisig2.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account4]); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.false; + }); + }); + + it('should return correct isComplete status for aggregated complete transaction - 2 levels Multisig, multi inner transaction', () => { + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const transferTransaction2 = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(multisig2.publicAccount), + transferTransaction.toAggregate(account4.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account2]); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.true; + }); + }); + + it('should return correct isComplete status for aggregated complete transaction - none multisig', () => { + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(account4.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signWith(account1); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.true; + }); + }); + + function givenMultisig2AccountInfo(): MultisigAccountInfo { + return new MultisigAccountInfo(multisig2.publicAccount, + 2, 1, + [multisig1.publicAccount, + account1.publicAccount], + [], + ); + } + + function givenAccount1Info(): MultisigAccountInfo { + return new MultisigAccountInfo(account1.publicAccount, + 0, 0, + [], + [multisig2.publicAccount], + ); + } + function givenAccount4Info(): MultisigAccountInfo { + return new MultisigAccountInfo(account4.publicAccount, + 0, 0, + [], + [], + ); + } + + function givenMultisig2AccountGraphInfo(): MultisigAccountGraphInfo { + const map = new Map(); + map.set(0, [new MultisigAccountInfo(multisig2.publicAccount, + 2, 1, + [multisig1.publicAccount, + account1.publicAccount], + [], + )]) + .set(1, [new MultisigAccountInfo(multisig1.publicAccount, + 1, 1, + [account2.publicAccount, account3.publicAccount], + [multisig2.publicAccount], + )]); + + return new MultisigAccountGraphInfo(map); + } + +}); From 90e805f3ad802bd0b4137894525eb4fb15164f1f Mon Sep 17 00:00:00 2001 From: Steven Liu Date: Mon, 1 Apr 2019 21:25:50 +0100 Subject: [PATCH 15/31] Updated license year --- test/service/AggregatedTransactionService.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/service/AggregatedTransactionService.spec.ts b/test/service/AggregatedTransactionService.spec.ts index 0aff3d8f47..bc0c11f535 100644 --- a/test/service/AggregatedTransactionService.spec.ts +++ b/test/service/AggregatedTransactionService.spec.ts @@ -1,5 +1,5 @@ /* - * Copyright 2018 NEM + * Copyright 2019 NEM * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. From c33a4c36974ba8689eb342b9c6bf802e5a433c7c Mon Sep 17 00:00:00 2001 From: Steven Liu Date: Wed, 3 Apr 2019 16:12:08 +0100 Subject: [PATCH 16/31] Added check in validate cosignatories for modify multisig account (removal) --- src/service/AggregatedTransactionService.ts | 29 ++++++++++++++++--- .../AggregatedTransactionService.spec.ts | 27 +++++++++++++++++ 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/src/service/AggregatedTransactionService.ts b/src/service/AggregatedTransactionService.ts index ccdd72cd3b..7803b48bae 100644 --- a/src/service/AggregatedTransactionService.ts +++ b/src/service/AggregatedTransactionService.ts @@ -20,7 +20,11 @@ import { TransactionMapping } from '../core/utils/TransactionMapping'; import { AccountHttp } from '../infrastructure/AccountHttp'; import { MultisigAccountGraphInfo } from '../model/account/MultisigAccountGraphInfo'; import { AggregateTransaction } from '../model/transaction/AggregateTransaction'; +import { InnerTransaction } from '../model/transaction/InnerTransaction'; +import { ModifyMultisigAccountTransaction } from '../model/transaction/ModifyMultisigAccountTransaction'; +import { MultisigCosignatoryModificationType } from '../model/transaction/MultisigCosignatoryModificationType'; import { SignedTransaction } from '../model/transaction/SignedTransaction'; +import { TransactionType } from '../model/transaction/TransactionType'; /** * Aggregated Transaction service @@ -58,7 +62,7 @@ export class AggregatedTransactionService { mergeMap((_) => _.minApproval !== 0 && _.minRemoval !== 0 ? this.accountHttp.getMultisigAccountGraphInfo(_.account.address) .pipe( - map((graphInfo) => this.validateCosignatories(graphInfo, signers)), + map((graphInfo) => this.validateCosignatories(graphInfo, signers, innerTransaction)), ) : observableOf(true), ), ), @@ -70,9 +74,12 @@ export class AggregatedTransactionService { * Validate cosignatories against multisig Account(s) * @param graphInfo - multisig account graph info * @param cosignatories - array of cosignatories extracted from aggregated transaction + * @param innerTransaction - the inner transaction of the aggregated transaction * @returns {boolean} */ - private validateCosignatories(graphInfo: MultisigAccountGraphInfo, cosignatories: string[]): boolean { + private validateCosignatories(graphInfo: MultisigAccountGraphInfo, + cosignatories: string[], + innerTransaction: InnerTransaction): boolean { /** * Validate cosignatories from bottom level to top */ @@ -80,11 +87,24 @@ export class AggregatedTransactionService { const cosignatoriesReceived = cosignatories; let validationResult = true; + let isMultisigRemoval = false; + + /** + * Check inner transaction. If remove cosigner from multisig account, + * use minRemoval instead of minApproval for cosignatories validation. + */ + if (innerTransaction.type === TransactionType.MODIFY_MULTISIG_ACCOUNT) { + if ((innerTransaction as ModifyMultisigAccountTransaction).modifications + .find((modification) => modification.type === MultisigCosignatoryModificationType.Remove) !== undefined) { + isMultisigRemoval = true; + } + } + sortedKeys.forEach((key) => { const multisigInfo = graphInfo.multisigAccounts.get(key); if (multisigInfo && validationResult) { multisigInfo.forEach((multisig) => { - if (multisig.minApproval >= 1) { + if (multisig.minApproval >= 1 && multisig.minRemoval) { // To make sure it is multisig account const matchedCosignatories = this.compareArrays(cosignatoriesReceived, multisig.cosignatories.map((cosig) => cosig.publicKey)); @@ -93,7 +113,8 @@ export class AggregatedTransactionService { * into the received signatories array for next level validation. * Otherwise return validation failed. */ - if (matchedCosignatories.length >= multisig.minApproval) { + if ((matchedCosignatories.length >= multisig.minApproval && !isMultisigRemoval) || + (matchedCosignatories.length >= multisig.minRemoval && isMultisigRemoval)) { if (cosignatoriesReceived.indexOf(multisig.account.publicKey) === -1) { cosignatoriesReceived.push(multisig.account.publicKey); } diff --git a/test/service/AggregatedTransactionService.spec.ts b/test/service/AggregatedTransactionService.spec.ts index bc0c11f535..8ea1432ca6 100644 --- a/test/service/AggregatedTransactionService.spec.ts +++ b/test/service/AggregatedTransactionService.spec.ts @@ -26,6 +26,9 @@ import { MultisigAccountInfo } from '../../src/model/account/MultisigAccountInfo import {NetworkType} from '../../src/model/blockchain/NetworkType'; import { AggregateTransaction } from '../../src/model/transaction/AggregateTransaction'; import { Deadline } from '../../src/model/transaction/Deadline'; +import { ModifyMultisigAccountTransaction } from '../../src/model/transaction/ModifyMultisigAccountTransaction'; +import { MultisigCosignatoryModification } from '../../src/model/transaction/MultisigCosignatoryModification'; +import { MultisigCosignatoryModificationType } from '../../src/model/transaction/MultisigCosignatoryModificationType'; import { PlainMessage } from '../../src/model/transaction/PlainMessage'; import { TransferTransaction } from '../../src/model/transaction/TransferTransaction'; import { AggregatedTransactionService } from '../../src/service/AggregatedTransactionService'; @@ -177,6 +180,30 @@ describe('AggregatedTransactionService', () => { }); }); + it('should use minRemoval for multisig account validation if inner transaction is modify multisig remove', () => { + const modifyMultisigTransaction = ModifyMultisigAccountTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + 1, + 1, + [new MultisigCosignatoryModification( + MultisigCosignatoryModificationType.Remove, + account1.publicAccount, + )], + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [modifyMultisigTransaction.toAggregate(multisig2.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signWith(account2); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.true; + }); + }); + it('should return correct isComplete status for aggregated complete transaction - none multisig', () => { const transferTransaction = TransferTransaction.create( Deadline.create(1, ChronoUnit.HOURS), From 9c4ae75f55c1835af8e01c83fbae96cbc6771ea5 Mon Sep 17 00:00:00 2001 From: Steven Liu Date: Thu, 4 Apr 2019 15:03:56 +0100 Subject: [PATCH 17/31] Fixed bug in create transaction from payload --- .../transaction/CreateTransactionFromPayload.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/infrastructure/transaction/CreateTransactionFromPayload.ts b/src/infrastructure/transaction/CreateTransactionFromPayload.ts index 9b124a33bf..7b73d6f6d0 100644 --- a/src/infrastructure/transaction/CreateTransactionFromPayload.ts +++ b/src/infrastructure/transaction/CreateTransactionFromPayload.ts @@ -376,7 +376,7 @@ const CreateTransaction = (type: number, transactionData: string, networkType: N }), networkType, consignatureArray ? consignatureArray.map((cosignature) => new AggregateTransactionCosignature( - cosignature.substring(0, 64), + cosignature.substring(64, 192), PublicAccount.createFromPublicKey(cosignature.substring(0, 64), networkType), )) : [], ); @@ -400,7 +400,7 @@ const CreateTransaction = (type: number, transactionData: string, networkType: N }), networkType, bondedConsignatureArray ? bondedConsignatureArray.map((cosignature) => new AggregateTransactionCosignature( - cosignature.substring(0, 64), + cosignature.substring(64, 192), PublicAccount.createFromPublicKey(cosignature.substring(0, 64), networkType), )) : [], ); From ffed0a7e9a6819b9372f6edf7d3caa78acde8b75 Mon Sep 17 00:00:00 2001 From: Steven Liu Date: Fri, 5 Apr 2019 19:01:52 +0100 Subject: [PATCH 18/31] Added more test cases and fixed a few bugs --- .../CreateTransactionFromPayload.ts | 4 +- src/service/AggregatedTransactionService.ts | 12 +- .../AggregatedTransactionService.spec.ts | 305 +++++++++++++++++- 3 files changed, 309 insertions(+), 12 deletions(-) diff --git a/src/infrastructure/transaction/CreateTransactionFromPayload.ts b/src/infrastructure/transaction/CreateTransactionFromPayload.ts index 7b73d6f6d0..bf36d24a20 100644 --- a/src/infrastructure/transaction/CreateTransactionFromPayload.ts +++ b/src/infrastructure/transaction/CreateTransactionFromPayload.ts @@ -456,8 +456,8 @@ const parseInnerTransactionFromBinary = (innerTransactionBinary: string): string let innerBinary = innerTransactionBinary; while (innerBinary.length) { - const payloadSize = parseInt(convert.uint8ToHex(convert.hexToUint8(innerTransactionBinary.substring(0, 8)).reverse()), 16) * 2; - const innerTransaction = innerTransactionBinary.substring(8, payloadSize); + const payloadSize = parseInt(convert.uint8ToHex(convert.hexToUint8(innerBinary.substring(0, 8)).reverse()), 16) * 2; + const innerTransaction = innerBinary.substring(8, payloadSize); embeddedTransaction.push(innerTransaction); innerBinary = innerBinary.substring(payloadSize); } diff --git a/src/service/AggregatedTransactionService.ts b/src/service/AggregatedTransactionService.ts index 7803b48bae..75a806bf96 100644 --- a/src/service/AggregatedTransactionService.ts +++ b/src/service/AggregatedTransactionService.ts @@ -15,7 +15,7 @@ */ import {from as observableFrom , Observable, of as observableOf} from 'rxjs'; -import { map, mergeMap} from 'rxjs/operators'; +import { flatMap, map, mergeMap, toArray} from 'rxjs/operators'; import { TransactionMapping } from '../core/utils/TransactionMapping'; import { AccountHttp } from '../infrastructure/AccountHttp'; import { MultisigAccountGraphInfo } from '../model/account/MultisigAccountGraphInfo'; @@ -52,7 +52,6 @@ export class AggregatedTransactionService { if (signedTransaction.signer) { signers.push(signedTransaction.signer); } - return observableFrom(aggregateTransaction.innerTransactions).pipe( mergeMap((innerTransaction) => this.accountHttp.getMultisigAccountInfo(innerTransaction.signer.address) .pipe( @@ -63,11 +62,16 @@ export class AggregatedTransactionService { this.accountHttp.getMultisigAccountGraphInfo(_.account.address) .pipe( map((graphInfo) => this.validateCosignatories(graphInfo, signers, innerTransaction)), - ) : observableOf(true), + ) : observableOf(signers.find((s) => s === _.account.publicKey ) !== undefined), ), ), ), - ); + toArray(), + ).pipe( + flatMap((results) => { + return observableOf(results.every((isComplete) => isComplete)); + }), + ); } /** diff --git a/test/service/AggregatedTransactionService.spec.ts b/test/service/AggregatedTransactionService.spec.ts index 8ea1432ca6..23470cb2a1 100644 --- a/test/service/AggregatedTransactionService.spec.ts +++ b/test/service/AggregatedTransactionService.spec.ts @@ -54,6 +54,7 @@ describe('AggregatedTransactionService', () => { * Test accounts: * Multisig1 (1/1): Account2, Account3 * Multisig2 (2/1): Account1, Multisig1 + * Multisig3 (2/2): Account2, Account3 * Stranger Account: Account4 */ @@ -64,6 +65,8 @@ describe('AggregatedTransactionService', () => { const multisig2 = Account.createFromPrivateKey('22A1D67F8519D1A45BD7116600BB6E857786E816FE0B45E4C5B9FFF3D64BC177', NetworkType.MIJIN_TEST); + const multisig3 = Account.createFromPrivateKey('5E7812AB0E709ABC45466034E1A209099F6A12C4698748A63CDCAA9B0DDE1DBD', + NetworkType.MIJIN_TEST); const account2 = Account.createFromPrivateKey('A4D410270E01CECDCDEADCDE32EC79C8D9CDEA4DCD426CB1EB666EFEF148FBCE', NetworkType.MIJIN_TEST); const account3 = Account.createFromPrivateKey('336AB45EE65A6AFFC0E7ADC5342F91E34BACA0B901A1D9C876FA25A1E590077E', @@ -80,14 +83,30 @@ describe('AggregatedTransactionService', () => { .thenReturn(observableOf(givenAccount4Info())); when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(multisig2.address))) .thenReturn(observableOf(givenMultisig2AccountInfo())); + when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(multisig3.address))) + .thenReturn(observableOf(givenMultisig3AccountInfo())); when(mockedAccountHttp.getMultisigAccountGraphInfo(deepEqual(multisig2.address))) .thenReturn(observableOf(givenMultisig2AccountGraphInfo())); + when(mockedAccountHttp.getMultisigAccountGraphInfo(deepEqual(multisig3.address))) + .thenReturn(observableOf(givenMultisig3AccountGraphInfo())); + when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(account2.address))) + .thenReturn(observableOf(givenAccount2Info())); + when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(account3.address))) + .thenReturn(observableOf(givenAccount3Info())); const accountHttp = instance(mockedAccountHttp); aggregatedTransactionService = new AggregatedTransactionService(accountHttp); }); it('should return isComplete: true for aggregated complete transaction - 2 levels Multisig', () => { + /** + * MLMA + * Alice (account1): normal account + * Bob (multisig2) - Multisig 2-1 (account1 && multisig1) + * Charles (multisig1) - Multisig 1-1 (account2 || account3) + * Given signatories: Account1 && Account4 + * Expecting complete as Bob needs 2 signatures (account1 && (account2 || account3)) + */ const transferTransaction = TransferTransaction.create( Deadline.create(1, ChronoUnit.HOURS), Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), @@ -109,6 +128,14 @@ describe('AggregatedTransactionService', () => { }); it('should return isComplete: false for aggregated complete transaction - 2 levels Multisig', () => { + /** + * MLMA + * Alice (account1): normal account + * Bob (multisig2) - Multisig 2-1 (account1 && multisig1) + * Charles (multisig1) - Multisig 1-1 (account2 || account3) + * Given signatories: Account1 && Account4 + * Expecting incomplete as Bob needs 2 signatures (account1 && (account2 || account3)) but only got account1 + */ const transferTransaction = TransferTransaction.create( Deadline.create(1, ChronoUnit.HOURS), Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), @@ -130,6 +157,14 @@ describe('AggregatedTransactionService', () => { }); it('should return isComplete: false for aggregated complete transaction - 2 levels Multisig', () => { + /** + * MLMA + * Alice (account1): normal account + * Bob (multisig2) - Multisig 2-1 (account1 && multisig1) + * Charles (multisig1) - Multisig 1-1 (account2 || account3) + * Given signatories: Account1 && Account4 + * Expecting incomplete as Bob needs 2 signatures (account1 && (account2 || account3)) but got account4 + */ const transferTransaction = TransferTransaction.create( Deadline.create(1, ChronoUnit.HOURS), Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), @@ -151,9 +186,18 @@ describe('AggregatedTransactionService', () => { }); it('should return correct isComplete status for aggregated complete transaction - 2 levels Multisig, multi inner transaction', () => { + /** + * MLMA - with multiple transaction + * Alice (account1): normal account + * Bob (multisig2) - Multisig 2-1 (account1 && multisig1) + * Charles (multisig1) - Multisig 1-1 (account2 || account3) + * An extra inner transaction to account4 (just to increase the complexity) + * Given signatories: Account1 && Account4 + * Expecting incomplete as Bob needs 2 signatures (account1 && (account2 || account3)) + */ const transferTransaction = TransferTransaction.create( Deadline.create(1, ChronoUnit.HOURS), - Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + account2.address, [], PlainMessage.create('test-message'), NetworkType.MIJIN_TEST, @@ -161,7 +205,7 @@ describe('AggregatedTransactionService', () => { const transferTransaction2 = TransferTransaction.create( Deadline.create(1, ChronoUnit.HOURS), - Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + account2.address, [], PlainMessage.create('test-message'), NetworkType.MIJIN_TEST, @@ -170,17 +214,60 @@ describe('AggregatedTransactionService', () => { const aggregateTransaction = AggregateTransaction.createComplete( Deadline.create(), [transferTransaction.toAggregate(multisig2.publicAccount), - transferTransaction.toAggregate(account4.publicAccount)], + transferTransaction2.toAggregate(account4.publicAccount)], NetworkType.MIJIN_TEST, []); + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account4]); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.false; + }); + }); - const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account2]); + it('should return correct isComplete status for aggregated complete transaction - 2 levels Multisig, multi inner transaction', () => { + /** + * MLMA - with multiple transaction + * Alice (account1): normal account + * Bob (multisig2) - Multisig 2-1 (account1 && multisig1) + * Charles (multisig1) - Multisig 1-1 (account2 || account3) + * An extra inner transaction to account4 (just to increase the complexity) + * Given signatories: Account1 && Account4 && Account2 + * Expecting complete + */ + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + account2.address, + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const transferTransaction2 = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + account2.address, + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(multisig2.publicAccount), + transferTransaction2.toAggregate(account4.publicAccount)], + NetworkType.MIJIN_TEST, + []); + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account4, account2]); aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.true; }); }); it('should use minRemoval for multisig account validation if inner transaction is modify multisig remove', () => { + /** + * If the inner transaction is issued to a multisig account + * and the inner transaction itself is a ModifyMultiSigAccountTransaction - Removal + * The validator should use minRemoval value rather than minApproval value + * to determine if the act is complete or not + */ const modifyMultisigTransaction = ModifyMultisigAccountTransaction.create( Deadline.create(1, ChronoUnit.HOURS), 1, @@ -197,14 +284,19 @@ describe('AggregatedTransactionService', () => { [modifyMultisigTransaction.toAggregate(multisig2.publicAccount)], NetworkType.MIJIN_TEST, []); - const signedTransaction = aggregateTransaction.signWith(account2); aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.true; }); }); - it('should return correct isComplete status for aggregated complete transaction - none multisig', () => { + it('should return correct isComplete status (false) for aggregated complete transaction - none multisig', () => { + /** + * If the inner transaction is issued to a multisig account + * and the inner transaction itself is a ModifyMultiSigAccountTransaction - Removal + * The validator should use minRemoval value rather than minApproval value + * to determine if the act is complete or not + */ const transferTransaction = TransferTransaction.create( Deadline.create(1, ChronoUnit.HOURS), Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), @@ -220,11 +312,176 @@ describe('AggregatedTransactionService', () => { []); const signedTransaction = aggregateTransaction.signWith(account1); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.false; + }); + }); + + it('should return correct isComplete status (true) for aggregated complete transaction - none multisig', () => { + /** + * ACT + * Alice (account1): normal account + * Bob (account4) - normal account + * Alice initiate the transaction + * Bob sign + */ + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(account4.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signWith(account4); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.true; + }); + }); + + it('should return correct isComplete status TRUE - multiple normal account', () => { + /** + * ACT + * Alice: account1 + * Bog: account4 + * An escrow contract is signed by all the participants (normal accounts) + * Given Alice defined the following escrow contract: + * | sender | recipient | type | data | + * | Alice | Bob | send-an-asset | 1 concert.ticket | + * | Bob | Alice | send-an-asset | 20 euros | + * And Bob signs the contract + * And Alice signs the contract + * Then the contract should appear as complete + */ + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + account1.address, + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const transferTransaction2 = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + account4.address, + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(account4.publicAccount), + transferTransaction2.toAggregate(account1.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account4]); aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.true; }); }); + it('should return correct isComplete status FALSE - multiple normal account', () => { + /** + * ACT + * Alice: account1 + * Bog: account4 + * An escrow contract is signed by all the participants (normal accounts) + * Given Alice defined the following escrow contract: + * | sender | recipient | type | data | + * | Alice | Bob | send-an-asset | 1 concert.ticket | + * | Bob | Alice | send-an-asset | 20 euros | + * And Alice signs the contract + * Then the contract should appear as incomplete + */ + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + account1.address, + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const transferTransaction2 = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + account4.address, + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(account4.publicAccount), + transferTransaction2.toAggregate(account1.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, []); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.false; + }); + }); + + it('should return correct isComplete status TRUE - multisig Single Level', () => { + /** + * ACT - Multisig single level + * Alice (account1): initiate an transfer to Bob + * Bob (multisig3): is a 2/2 multisig account (account2 && account3) + */ + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + account4.address, + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(multisig3.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account2, [account3]); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.true; + }); + }); + + it('should return correct isComplete status FALSE - multisig Single Level', () => { + /** + * ACT - Multisig single level + * Alice (account1): initiate an transfer to Bob + * Bob (multisig3): is a 2/2 multisig account (account2 && account3) + */ + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + account4.address, + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(multisig3.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account2, []); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.false; + }); + }); + function givenMultisig2AccountInfo(): MultisigAccountInfo { return new MultisigAccountInfo(multisig2.publicAccount, 2, 1, @@ -233,6 +490,14 @@ describe('AggregatedTransactionService', () => { [], ); } + function givenMultisig3AccountInfo(): MultisigAccountInfo { + return new MultisigAccountInfo(multisig3.publicAccount, + 2, 2, + [account2.publicAccount, + account3.publicAccount], + [], + ); + } function givenAccount1Info(): MultisigAccountInfo { return new MultisigAccountInfo(account1.publicAccount, @@ -241,6 +506,22 @@ describe('AggregatedTransactionService', () => { [multisig2.publicAccount], ); } + function givenAccount2Info(): MultisigAccountInfo { + return new MultisigAccountInfo(account2.publicAccount, + 0, 0, + [], + [multisig2.publicAccount, + multisig3.publicAccount], + ); + } + function givenAccount3Info(): MultisigAccountInfo { + return new MultisigAccountInfo(account3.publicAccount, + 0, 0, + [], + [multisig2.publicAccount, + multisig3.publicAccount], + ); + } function givenAccount4Info(): MultisigAccountInfo { return new MultisigAccountInfo(account4.publicAccount, 0, 0, @@ -266,4 +547,16 @@ describe('AggregatedTransactionService', () => { return new MultisigAccountGraphInfo(map); } + function givenMultisig3AccountGraphInfo(): MultisigAccountGraphInfo { + const map = new Map(); + map.set(0, [new MultisigAccountInfo(multisig3.publicAccount, + 2, 2, + [account2.publicAccount, + account3.publicAccount], + [], + )]); + + return new MultisigAccountGraphInfo(map); + } + }); From 461f34b0f792393eff761daceeded468593ac15d Mon Sep 17 00:00:00 2001 From: Steven Liu Date: Wed, 10 Apr 2019 15:13:01 +0100 Subject: [PATCH 19/31] renamed AggregatedTransactionService --- ...vice.ts => AggregateTransactionService.ts} | 2 +- src/service/service.ts | 2 +- ...ts => AggregateTransactionService.spec.ts} | 32 +++++++++---------- 3 files changed, 18 insertions(+), 18 deletions(-) rename src/service/{AggregatedTransactionService.ts => AggregateTransactionService.ts} (99%) rename test/service/{AggregatedTransactionService.spec.ts => AggregateTransactionService.spec.ts} (93%) diff --git a/src/service/AggregatedTransactionService.ts b/src/service/AggregateTransactionService.ts similarity index 99% rename from src/service/AggregatedTransactionService.ts rename to src/service/AggregateTransactionService.ts index 75a806bf96..800d1a1b89 100644 --- a/src/service/AggregatedTransactionService.ts +++ b/src/service/AggregateTransactionService.ts @@ -29,7 +29,7 @@ import { TransactionType } from '../model/transaction/TransactionType'; /** * Aggregated Transaction service */ -export class AggregatedTransactionService { +export class AggregateTransactionService { /** * Constructor diff --git a/src/service/service.ts b/src/service/service.ts index fcf4c84680..c988e13ffb 100644 --- a/src/service/service.ts +++ b/src/service/service.ts @@ -16,4 +16,4 @@ export * from './NamespaceService'; export * from './MosaicService'; -export * from './AggregatedTransactionService'; +export * from './AggregateTransactionService'; diff --git a/test/service/AggregatedTransactionService.spec.ts b/test/service/AggregateTransactionService.spec.ts similarity index 93% rename from test/service/AggregatedTransactionService.spec.ts rename to test/service/AggregateTransactionService.spec.ts index 23470cb2a1..ae2deab99b 100644 --- a/test/service/AggregatedTransactionService.spec.ts +++ b/test/service/AggregateTransactionService.spec.ts @@ -31,13 +31,13 @@ import { MultisigCosignatoryModification } from '../../src/model/transaction/Mul import { MultisigCosignatoryModificationType } from '../../src/model/transaction/MultisigCosignatoryModificationType'; import { PlainMessage } from '../../src/model/transaction/PlainMessage'; import { TransferTransaction } from '../../src/model/transaction/TransferTransaction'; -import { AggregatedTransactionService } from '../../src/service/AggregatedTransactionService'; +import { AggregateTransactionService } from '../../src/service/AggregateTransactionService'; /** * For multi level multisig scenario visit: https://github.com/nemtech/nem2-docs/issues/10 */ -describe('AggregatedTransactionService', () => { - let aggregatedTransactionService: AggregatedTransactionService; +describe('AggregateTransactionService', () => { + let aggregateTransactionService: AggregateTransactionService; /** * Multisig2 Account: SBROWP-7YMG2M-K45RO6-Q7ZPK7-G7GXWQ-JK5VNQ-OSUX @@ -95,7 +95,7 @@ describe('AggregatedTransactionService', () => { .thenReturn(observableOf(givenAccount3Info())); const accountHttp = instance(mockedAccountHttp); - aggregatedTransactionService = new AggregatedTransactionService(accountHttp); + aggregateTransactionService = new AggregateTransactionService(accountHttp); }); it('should return isComplete: true for aggregated complete transaction - 2 levels Multisig', () => { @@ -122,7 +122,7 @@ describe('AggregatedTransactionService', () => { []); const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account2]); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + aggregateTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.true; }); }); @@ -151,7 +151,7 @@ describe('AggregatedTransactionService', () => { []); const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, []); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + aggregateTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.false; }); }); @@ -180,7 +180,7 @@ describe('AggregatedTransactionService', () => { []); const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account4]); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + aggregateTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.false; }); }); @@ -218,7 +218,7 @@ describe('AggregatedTransactionService', () => { NetworkType.MIJIN_TEST, []); const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account4]); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + aggregateTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.false; }); }); @@ -256,7 +256,7 @@ describe('AggregatedTransactionService', () => { NetworkType.MIJIN_TEST, []); const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account4, account2]); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + aggregateTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.true; }); }); @@ -285,7 +285,7 @@ describe('AggregatedTransactionService', () => { NetworkType.MIJIN_TEST, []); const signedTransaction = aggregateTransaction.signWith(account2); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + aggregateTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.true; }); }); @@ -312,7 +312,7 @@ describe('AggregatedTransactionService', () => { []); const signedTransaction = aggregateTransaction.signWith(account1); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + aggregateTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.false; }); }); @@ -340,7 +340,7 @@ describe('AggregatedTransactionService', () => { []); const signedTransaction = aggregateTransaction.signWith(account4); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + aggregateTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.true; }); }); @@ -383,7 +383,7 @@ describe('AggregatedTransactionService', () => { []); const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account4]); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + aggregateTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.true; }); }); @@ -425,7 +425,7 @@ describe('AggregatedTransactionService', () => { []); const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, []); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + aggregateTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.false; }); }); @@ -451,7 +451,7 @@ describe('AggregatedTransactionService', () => { []); const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account2, [account3]); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + aggregateTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.true; }); }); @@ -477,7 +477,7 @@ describe('AggregatedTransactionService', () => { []); const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account2, []); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + aggregateTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.false; }); }); From f944d3beba246b063946e8104eb60288b4abd11e Mon Sep 17 00:00:00 2001 From: Steven Liu Date: Mon, 1 Apr 2019 20:39:11 +0100 Subject: [PATCH 20/31] #4 1. Addes service for Aggregated Transaction - isComplete 2. Fixed couple of bugs in TransactionMappings --- .../CreateTransactionFromPayload.ts | 8 +- src/service/AggregatedTransactionService.ts | 127 +++++++++ src/service/service.ts | 1 + .../AggregatedTransactionService.spec.ts | 242 ++++++++++++++++++ 4 files changed, 374 insertions(+), 4 deletions(-) create mode 100644 src/service/AggregatedTransactionService.ts create mode 100644 test/service/AggregatedTransactionService.spec.ts diff --git a/src/infrastructure/transaction/CreateTransactionFromPayload.ts b/src/infrastructure/transaction/CreateTransactionFromPayload.ts index af8bd12125..494d4bc10e 100644 --- a/src/infrastructure/transaction/CreateTransactionFromPayload.ts +++ b/src/infrastructure/transaction/CreateTransactionFromPayload.ts @@ -377,7 +377,7 @@ const CreateTransaction = (type: number, transactionData: string, networkType: N networkType, consignatureArray ? consignatureArray.map((cosignature) => new AggregateTransactionCosignature( cosignature.substring(0, 64), - PublicAccount.createFromPublicKey(cosignature.substring(64, 192), networkType), + PublicAccount.createFromPublicKey(cosignature.substring(0, 64), networkType), )) : [], ); case TransactionType.AGGREGATE_BONDED: @@ -401,7 +401,7 @@ const CreateTransaction = (type: number, transactionData: string, networkType: N networkType, bondedConsignatureArray ? bondedConsignatureArray.map((cosignature) => new AggregateTransactionCosignature( cosignature.substring(0, 64), - PublicAccount.createFromPublicKey(cosignature.substring(64, 192), networkType), + PublicAccount.createFromPublicKey(cosignature.substring(0, 64), networkType), )) : [], ); default: @@ -457,9 +457,9 @@ const parseInnerTransactionFromBinary = (innerTransactionBinary: string): string while (innerBinary.length) { const payloadSize = parseInt(convert.uint8ToHex(convert.hexToUint8(innerTransactionBinary.substring(0, 8)).reverse()), 16) * 2; - const innerTransaction = innerTransactionBinary.substring(8, 8 + payloadSize); + const innerTransaction = innerTransactionBinary.substring(8, payloadSize); embeddedTransaction.push(innerTransaction); - innerBinary = innerTransactionBinary.substring(8 + payloadSize); + innerBinary = innerBinary.substring(payloadSize); } return embeddedTransaction; }; diff --git a/src/service/AggregatedTransactionService.ts b/src/service/AggregatedTransactionService.ts new file mode 100644 index 0000000000..ccdd72cd3b --- /dev/null +++ b/src/service/AggregatedTransactionService.ts @@ -0,0 +1,127 @@ +/* + * Copyright 2019 NEM + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import {from as observableFrom , Observable, of as observableOf} from 'rxjs'; +import { map, mergeMap} from 'rxjs/operators'; +import { TransactionMapping } from '../core/utils/TransactionMapping'; +import { AccountHttp } from '../infrastructure/AccountHttp'; +import { MultisigAccountGraphInfo } from '../model/account/MultisigAccountGraphInfo'; +import { AggregateTransaction } from '../model/transaction/AggregateTransaction'; +import { SignedTransaction } from '../model/transaction/SignedTransaction'; + +/** + * Aggregated Transaction service + */ +export class AggregatedTransactionService { + + /** + * Constructor + * @param accountHttp + */ + constructor(private readonly accountHttp: AccountHttp) { + } + + /** + * Check if an aggregate complete transaction has all cosignatories attached + * @param signedTransaction - The signed aggregate transaction (complete) to be verified + * @returns {Observable} + */ + public isComplete(signedTransaction: SignedTransaction): Observable { + const aggregateTransaction = TransactionMapping.createFromPayload(signedTransaction.payload) as AggregateTransaction; + /** + * Include both initiator & cosigners + */ + const signers = (aggregateTransaction.cosignatures.map((cosigner) => cosigner.signer.publicKey)); + if (signedTransaction.signer) { + signers.push(signedTransaction.signer); + } + + return observableFrom(aggregateTransaction.innerTransactions).pipe( + mergeMap((innerTransaction) => this.accountHttp.getMultisigAccountInfo(innerTransaction.signer.address) + .pipe( + /** + * For multisig account, we need to get the graph info in case it has multiple levels + */ + mergeMap((_) => _.minApproval !== 0 && _.minRemoval !== 0 ? + this.accountHttp.getMultisigAccountGraphInfo(_.account.address) + .pipe( + map((graphInfo) => this.validateCosignatories(graphInfo, signers)), + ) : observableOf(true), + ), + ), + ), + ); + } + + /** + * Validate cosignatories against multisig Account(s) + * @param graphInfo - multisig account graph info + * @param cosignatories - array of cosignatories extracted from aggregated transaction + * @returns {boolean} + */ + private validateCosignatories(graphInfo: MultisigAccountGraphInfo, cosignatories: string[]): boolean { + /** + * Validate cosignatories from bottom level to top + */ + const sortedKeys = Array.from(graphInfo.multisigAccounts.keys()).sort((a, b) => b - a); + const cosignatoriesReceived = cosignatories; + let validationResult = true; + + sortedKeys.forEach((key) => { + const multisigInfo = graphInfo.multisigAccounts.get(key); + if (multisigInfo && validationResult) { + multisigInfo.forEach((multisig) => { + if (multisig.minApproval >= 1) { + const matchedCosignatories = this.compareArrays(cosignatoriesReceived, + multisig.cosignatories.map((cosig) => cosig.publicKey)); + + /** + * if minimal signature requirement met at current level, push the multisig account + * into the received signatories array for next level validation. + * Otherwise return validation failed. + */ + if (matchedCosignatories.length >= multisig.minApproval) { + if (cosignatoriesReceived.indexOf(multisig.account.publicKey) === -1) { + cosignatoriesReceived.push(multisig.account.publicKey); + } + } else { + validationResult = false; + } + } + }); + } + }); + + return validationResult; + } + + /** + * Compare two string arrays + * @param array1 - base array + * @param array2 - array to be matched + * @returns {string[]} - array of matched elements + */ + private compareArrays(array1: string[], array2: string[]): string[] { + const results: string[] = []; + array1.forEach((a1) => array2.forEach((a2) => { + if (a1 === a2) { + results.push(a1); + } + })); + + return results; + } +} diff --git a/src/service/service.ts b/src/service/service.ts index 6a9945c1cd..fcf4c84680 100644 --- a/src/service/service.ts +++ b/src/service/service.ts @@ -16,3 +16,4 @@ export * from './NamespaceService'; export * from './MosaicService'; +export * from './AggregatedTransactionService'; diff --git a/test/service/AggregatedTransactionService.spec.ts b/test/service/AggregatedTransactionService.spec.ts new file mode 100644 index 0000000000..0aff3d8f47 --- /dev/null +++ b/test/service/AggregatedTransactionService.spec.ts @@ -0,0 +1,242 @@ +/* + * Copyright 2018 NEM + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { expect } from 'chai'; +import { ChronoUnit } from 'js-joda'; +import {of as observableOf} from 'rxjs'; +import {deepEqual, instance, mock, when} from 'ts-mockito'; +import { AccountHttp } from '../../src/infrastructure/AccountHttp'; +import { Account } from '../../src/model/account/Account'; +import { Address } from '../../src/model/account/Address'; +import { MultisigAccountGraphInfo } from '../../src/model/account/MultisigAccountGraphInfo'; +import { MultisigAccountInfo } from '../../src/model/account/MultisigAccountInfo'; +import {NetworkType} from '../../src/model/blockchain/NetworkType'; +import { AggregateTransaction } from '../../src/model/transaction/AggregateTransaction'; +import { Deadline } from '../../src/model/transaction/Deadline'; +import { PlainMessage } from '../../src/model/transaction/PlainMessage'; +import { TransferTransaction } from '../../src/model/transaction/TransferTransaction'; +import { AggregatedTransactionService } from '../../src/service/AggregatedTransactionService'; + +/** + * For multi level multisig scenario visit: https://github.com/nemtech/nem2-docs/issues/10 + */ +describe('AggregatedTransactionService', () => { + let aggregatedTransactionService: AggregatedTransactionService; + + /** + * Multisig2 Account: SBROWP-7YMG2M-K45RO6-Q7ZPK7-G7GXWQ-JK5VNQ-OSUX + * Public Key: 5E628EA59818D97AA4118780D9A88C5512FCE7A21C195E1574727EFCE5DF7C0D + * Private Key: 22A1D67F8519D1A45BD7116600BB6E857786E816FE0B45E4C5B9FFF3D64BC177 + * + * + * Multisig1 Account: SAK32M-5JQ43R-WYHWEH-WRBCW4-RXERT2-DLASGL-EANS + * Public Key: BFDF2610C5666A626434FE12FB4A9D896D2B9B033F5F84CCEABE82E043A6307E + * Private Key: 8B0622C2CCFC5CCC5A74B500163E3C68F3AD3643DB12932FC931143EAC67280D + */ + + /** + * Test accounts: + * Multisig1 (1/1): Account2, Account3 + * Multisig2 (2/1): Account1, Multisig1 + * Stranger Account: Account4 + */ + + const account1 = Account.createFromPrivateKey('82DB2528834C9926F0FCCE042466B24A266F5B685CB66D2869AF6648C043E950', + NetworkType.MIJIN_TEST); + const multisig1 = Account.createFromPrivateKey('8B0622C2CCFC5CCC5A74B500163E3C68F3AD3643DB12932FC931143EAC67280D', + NetworkType.MIJIN_TEST); + const multisig2 = Account.createFromPrivateKey('22A1D67F8519D1A45BD7116600BB6E857786E816FE0B45E4C5B9FFF3D64BC177', + NetworkType.MIJIN_TEST); + + const account2 = Account.createFromPrivateKey('A4D410270E01CECDCDEADCDE32EC79C8D9CDEA4DCD426CB1EB666EFEF148FBCE', + NetworkType.MIJIN_TEST); + const account3 = Account.createFromPrivateKey('336AB45EE65A6AFFC0E7ADC5342F91E34BACA0B901A1D9C876FA25A1E590077E', + NetworkType.MIJIN_TEST); + + const account4 = Account.createFromPrivateKey('4D8B3756592532753344E11E2B7541317BCCFBBCF4444274CDBF359D2C4AE0F1', + NetworkType.MIJIN_TEST); + before(() => { + const mockedAccountHttp = mock(AccountHttp); + + when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(account1.address))) + .thenReturn(observableOf(givenAccount1Info())); + when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(account4.address))) + .thenReturn(observableOf(givenAccount4Info())); + when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(multisig2.address))) + .thenReturn(observableOf(givenMultisig2AccountInfo())); + when(mockedAccountHttp.getMultisigAccountGraphInfo(deepEqual(multisig2.address))) + .thenReturn(observableOf(givenMultisig2AccountGraphInfo())); + + const accountHttp = instance(mockedAccountHttp); + aggregatedTransactionService = new AggregatedTransactionService(accountHttp); + }); + + it('should return isComplete: true for aggregated complete transaction - 2 levels Multisig', () => { + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(multisig2.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account2]); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.true; + }); + }); + + it('should return isComplete: false for aggregated complete transaction - 2 levels Multisig', () => { + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(multisig2.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, []); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.false; + }); + }); + + it('should return isComplete: false for aggregated complete transaction - 2 levels Multisig', () => { + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(multisig2.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account4]); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.false; + }); + }); + + it('should return correct isComplete status for aggregated complete transaction - 2 levels Multisig, multi inner transaction', () => { + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const transferTransaction2 = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(multisig2.publicAccount), + transferTransaction.toAggregate(account4.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account2]); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.true; + }); + }); + + it('should return correct isComplete status for aggregated complete transaction - none multisig', () => { + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(account4.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signWith(account1); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.true; + }); + }); + + function givenMultisig2AccountInfo(): MultisigAccountInfo { + return new MultisigAccountInfo(multisig2.publicAccount, + 2, 1, + [multisig1.publicAccount, + account1.publicAccount], + [], + ); + } + + function givenAccount1Info(): MultisigAccountInfo { + return new MultisigAccountInfo(account1.publicAccount, + 0, 0, + [], + [multisig2.publicAccount], + ); + } + function givenAccount4Info(): MultisigAccountInfo { + return new MultisigAccountInfo(account4.publicAccount, + 0, 0, + [], + [], + ); + } + + function givenMultisig2AccountGraphInfo(): MultisigAccountGraphInfo { + const map = new Map(); + map.set(0, [new MultisigAccountInfo(multisig2.publicAccount, + 2, 1, + [multisig1.publicAccount, + account1.publicAccount], + [], + )]) + .set(1, [new MultisigAccountInfo(multisig1.publicAccount, + 1, 1, + [account2.publicAccount, account3.publicAccount], + [multisig2.publicAccount], + )]); + + return new MultisigAccountGraphInfo(map); + } + +}); From 516b5e64604a8403060cf04ac7dde3c7628f3f81 Mon Sep 17 00:00:00 2001 From: Steven Liu Date: Mon, 1 Apr 2019 21:25:50 +0100 Subject: [PATCH 21/31] Updated license year --- test/service/AggregatedTransactionService.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/service/AggregatedTransactionService.spec.ts b/test/service/AggregatedTransactionService.spec.ts index 0aff3d8f47..bc0c11f535 100644 --- a/test/service/AggregatedTransactionService.spec.ts +++ b/test/service/AggregatedTransactionService.spec.ts @@ -1,5 +1,5 @@ /* - * Copyright 2018 NEM + * Copyright 2019 NEM * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. From fb471643fec486e8313c6c722a6728b624348a6d Mon Sep 17 00:00:00 2001 From: Steven Liu Date: Wed, 3 Apr 2019 16:12:08 +0100 Subject: [PATCH 22/31] Added check in validate cosignatories for modify multisig account (removal) --- src/service/AggregatedTransactionService.ts | 29 ++++++++++++++++--- .../AggregatedTransactionService.spec.ts | 27 +++++++++++++++++ 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/src/service/AggregatedTransactionService.ts b/src/service/AggregatedTransactionService.ts index ccdd72cd3b..7803b48bae 100644 --- a/src/service/AggregatedTransactionService.ts +++ b/src/service/AggregatedTransactionService.ts @@ -20,7 +20,11 @@ import { TransactionMapping } from '../core/utils/TransactionMapping'; import { AccountHttp } from '../infrastructure/AccountHttp'; import { MultisigAccountGraphInfo } from '../model/account/MultisigAccountGraphInfo'; import { AggregateTransaction } from '../model/transaction/AggregateTransaction'; +import { InnerTransaction } from '../model/transaction/InnerTransaction'; +import { ModifyMultisigAccountTransaction } from '../model/transaction/ModifyMultisigAccountTransaction'; +import { MultisigCosignatoryModificationType } from '../model/transaction/MultisigCosignatoryModificationType'; import { SignedTransaction } from '../model/transaction/SignedTransaction'; +import { TransactionType } from '../model/transaction/TransactionType'; /** * Aggregated Transaction service @@ -58,7 +62,7 @@ export class AggregatedTransactionService { mergeMap((_) => _.minApproval !== 0 && _.minRemoval !== 0 ? this.accountHttp.getMultisigAccountGraphInfo(_.account.address) .pipe( - map((graphInfo) => this.validateCosignatories(graphInfo, signers)), + map((graphInfo) => this.validateCosignatories(graphInfo, signers, innerTransaction)), ) : observableOf(true), ), ), @@ -70,9 +74,12 @@ export class AggregatedTransactionService { * Validate cosignatories against multisig Account(s) * @param graphInfo - multisig account graph info * @param cosignatories - array of cosignatories extracted from aggregated transaction + * @param innerTransaction - the inner transaction of the aggregated transaction * @returns {boolean} */ - private validateCosignatories(graphInfo: MultisigAccountGraphInfo, cosignatories: string[]): boolean { + private validateCosignatories(graphInfo: MultisigAccountGraphInfo, + cosignatories: string[], + innerTransaction: InnerTransaction): boolean { /** * Validate cosignatories from bottom level to top */ @@ -80,11 +87,24 @@ export class AggregatedTransactionService { const cosignatoriesReceived = cosignatories; let validationResult = true; + let isMultisigRemoval = false; + + /** + * Check inner transaction. If remove cosigner from multisig account, + * use minRemoval instead of minApproval for cosignatories validation. + */ + if (innerTransaction.type === TransactionType.MODIFY_MULTISIG_ACCOUNT) { + if ((innerTransaction as ModifyMultisigAccountTransaction).modifications + .find((modification) => modification.type === MultisigCosignatoryModificationType.Remove) !== undefined) { + isMultisigRemoval = true; + } + } + sortedKeys.forEach((key) => { const multisigInfo = graphInfo.multisigAccounts.get(key); if (multisigInfo && validationResult) { multisigInfo.forEach((multisig) => { - if (multisig.minApproval >= 1) { + if (multisig.minApproval >= 1 && multisig.minRemoval) { // To make sure it is multisig account const matchedCosignatories = this.compareArrays(cosignatoriesReceived, multisig.cosignatories.map((cosig) => cosig.publicKey)); @@ -93,7 +113,8 @@ export class AggregatedTransactionService { * into the received signatories array for next level validation. * Otherwise return validation failed. */ - if (matchedCosignatories.length >= multisig.minApproval) { + if ((matchedCosignatories.length >= multisig.minApproval && !isMultisigRemoval) || + (matchedCosignatories.length >= multisig.minRemoval && isMultisigRemoval)) { if (cosignatoriesReceived.indexOf(multisig.account.publicKey) === -1) { cosignatoriesReceived.push(multisig.account.publicKey); } diff --git a/test/service/AggregatedTransactionService.spec.ts b/test/service/AggregatedTransactionService.spec.ts index bc0c11f535..8ea1432ca6 100644 --- a/test/service/AggregatedTransactionService.spec.ts +++ b/test/service/AggregatedTransactionService.spec.ts @@ -26,6 +26,9 @@ import { MultisigAccountInfo } from '../../src/model/account/MultisigAccountInfo import {NetworkType} from '../../src/model/blockchain/NetworkType'; import { AggregateTransaction } from '../../src/model/transaction/AggregateTransaction'; import { Deadline } from '../../src/model/transaction/Deadline'; +import { ModifyMultisigAccountTransaction } from '../../src/model/transaction/ModifyMultisigAccountTransaction'; +import { MultisigCosignatoryModification } from '../../src/model/transaction/MultisigCosignatoryModification'; +import { MultisigCosignatoryModificationType } from '../../src/model/transaction/MultisigCosignatoryModificationType'; import { PlainMessage } from '../../src/model/transaction/PlainMessage'; import { TransferTransaction } from '../../src/model/transaction/TransferTransaction'; import { AggregatedTransactionService } from '../../src/service/AggregatedTransactionService'; @@ -177,6 +180,30 @@ describe('AggregatedTransactionService', () => { }); }); + it('should use minRemoval for multisig account validation if inner transaction is modify multisig remove', () => { + const modifyMultisigTransaction = ModifyMultisigAccountTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + 1, + 1, + [new MultisigCosignatoryModification( + MultisigCosignatoryModificationType.Remove, + account1.publicAccount, + )], + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [modifyMultisigTransaction.toAggregate(multisig2.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signWith(account2); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.true; + }); + }); + it('should return correct isComplete status for aggregated complete transaction - none multisig', () => { const transferTransaction = TransferTransaction.create( Deadline.create(1, ChronoUnit.HOURS), From daeb1cb7ca972745084f626e32f55b9e5f6c4988 Mon Sep 17 00:00:00 2001 From: Steven Liu Date: Thu, 4 Apr 2019 15:03:56 +0100 Subject: [PATCH 23/31] Fixed bug in create transaction from payload --- .../transaction/CreateTransactionFromPayload.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/infrastructure/transaction/CreateTransactionFromPayload.ts b/src/infrastructure/transaction/CreateTransactionFromPayload.ts index 494d4bc10e..ce68c0f974 100644 --- a/src/infrastructure/transaction/CreateTransactionFromPayload.ts +++ b/src/infrastructure/transaction/CreateTransactionFromPayload.ts @@ -376,7 +376,7 @@ const CreateTransaction = (type: number, transactionData: string, networkType: N }), networkType, consignatureArray ? consignatureArray.map((cosignature) => new AggregateTransactionCosignature( - cosignature.substring(0, 64), + cosignature.substring(64, 192), PublicAccount.createFromPublicKey(cosignature.substring(0, 64), networkType), )) : [], ); @@ -400,7 +400,7 @@ const CreateTransaction = (type: number, transactionData: string, networkType: N }), networkType, bondedConsignatureArray ? bondedConsignatureArray.map((cosignature) => new AggregateTransactionCosignature( - cosignature.substring(0, 64), + cosignature.substring(64, 192), PublicAccount.createFromPublicKey(cosignature.substring(0, 64), networkType), )) : [], ); From 3da7cf7aa8ee3825e5d3bf773195f2ea48f6186f Mon Sep 17 00:00:00 2001 From: Steven Liu Date: Fri, 5 Apr 2019 19:01:52 +0100 Subject: [PATCH 24/31] Added more test cases and fixed a few bugs --- .../CreateTransactionFromPayload.ts | 4 +- src/service/AggregatedTransactionService.ts | 12 +- .../AggregatedTransactionService.spec.ts | 305 +++++++++++++++++- 3 files changed, 309 insertions(+), 12 deletions(-) diff --git a/src/infrastructure/transaction/CreateTransactionFromPayload.ts b/src/infrastructure/transaction/CreateTransactionFromPayload.ts index ce68c0f974..f32089fb31 100644 --- a/src/infrastructure/transaction/CreateTransactionFromPayload.ts +++ b/src/infrastructure/transaction/CreateTransactionFromPayload.ts @@ -456,8 +456,8 @@ const parseInnerTransactionFromBinary = (innerTransactionBinary: string): string let innerBinary = innerTransactionBinary; while (innerBinary.length) { - const payloadSize = parseInt(convert.uint8ToHex(convert.hexToUint8(innerTransactionBinary.substring(0, 8)).reverse()), 16) * 2; - const innerTransaction = innerTransactionBinary.substring(8, payloadSize); + const payloadSize = parseInt(convert.uint8ToHex(convert.hexToUint8(innerBinary.substring(0, 8)).reverse()), 16) * 2; + const innerTransaction = innerBinary.substring(8, payloadSize); embeddedTransaction.push(innerTransaction); innerBinary = innerBinary.substring(payloadSize); } diff --git a/src/service/AggregatedTransactionService.ts b/src/service/AggregatedTransactionService.ts index 7803b48bae..75a806bf96 100644 --- a/src/service/AggregatedTransactionService.ts +++ b/src/service/AggregatedTransactionService.ts @@ -15,7 +15,7 @@ */ import {from as observableFrom , Observable, of as observableOf} from 'rxjs'; -import { map, mergeMap} from 'rxjs/operators'; +import { flatMap, map, mergeMap, toArray} from 'rxjs/operators'; import { TransactionMapping } from '../core/utils/TransactionMapping'; import { AccountHttp } from '../infrastructure/AccountHttp'; import { MultisigAccountGraphInfo } from '../model/account/MultisigAccountGraphInfo'; @@ -52,7 +52,6 @@ export class AggregatedTransactionService { if (signedTransaction.signer) { signers.push(signedTransaction.signer); } - return observableFrom(aggregateTransaction.innerTransactions).pipe( mergeMap((innerTransaction) => this.accountHttp.getMultisigAccountInfo(innerTransaction.signer.address) .pipe( @@ -63,11 +62,16 @@ export class AggregatedTransactionService { this.accountHttp.getMultisigAccountGraphInfo(_.account.address) .pipe( map((graphInfo) => this.validateCosignatories(graphInfo, signers, innerTransaction)), - ) : observableOf(true), + ) : observableOf(signers.find((s) => s === _.account.publicKey ) !== undefined), ), ), ), - ); + toArray(), + ).pipe( + flatMap((results) => { + return observableOf(results.every((isComplete) => isComplete)); + }), + ); } /** diff --git a/test/service/AggregatedTransactionService.spec.ts b/test/service/AggregatedTransactionService.spec.ts index 8ea1432ca6..23470cb2a1 100644 --- a/test/service/AggregatedTransactionService.spec.ts +++ b/test/service/AggregatedTransactionService.spec.ts @@ -54,6 +54,7 @@ describe('AggregatedTransactionService', () => { * Test accounts: * Multisig1 (1/1): Account2, Account3 * Multisig2 (2/1): Account1, Multisig1 + * Multisig3 (2/2): Account2, Account3 * Stranger Account: Account4 */ @@ -64,6 +65,8 @@ describe('AggregatedTransactionService', () => { const multisig2 = Account.createFromPrivateKey('22A1D67F8519D1A45BD7116600BB6E857786E816FE0B45E4C5B9FFF3D64BC177', NetworkType.MIJIN_TEST); + const multisig3 = Account.createFromPrivateKey('5E7812AB0E709ABC45466034E1A209099F6A12C4698748A63CDCAA9B0DDE1DBD', + NetworkType.MIJIN_TEST); const account2 = Account.createFromPrivateKey('A4D410270E01CECDCDEADCDE32EC79C8D9CDEA4DCD426CB1EB666EFEF148FBCE', NetworkType.MIJIN_TEST); const account3 = Account.createFromPrivateKey('336AB45EE65A6AFFC0E7ADC5342F91E34BACA0B901A1D9C876FA25A1E590077E', @@ -80,14 +83,30 @@ describe('AggregatedTransactionService', () => { .thenReturn(observableOf(givenAccount4Info())); when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(multisig2.address))) .thenReturn(observableOf(givenMultisig2AccountInfo())); + when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(multisig3.address))) + .thenReturn(observableOf(givenMultisig3AccountInfo())); when(mockedAccountHttp.getMultisigAccountGraphInfo(deepEqual(multisig2.address))) .thenReturn(observableOf(givenMultisig2AccountGraphInfo())); + when(mockedAccountHttp.getMultisigAccountGraphInfo(deepEqual(multisig3.address))) + .thenReturn(observableOf(givenMultisig3AccountGraphInfo())); + when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(account2.address))) + .thenReturn(observableOf(givenAccount2Info())); + when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(account3.address))) + .thenReturn(observableOf(givenAccount3Info())); const accountHttp = instance(mockedAccountHttp); aggregatedTransactionService = new AggregatedTransactionService(accountHttp); }); it('should return isComplete: true for aggregated complete transaction - 2 levels Multisig', () => { + /** + * MLMA + * Alice (account1): normal account + * Bob (multisig2) - Multisig 2-1 (account1 && multisig1) + * Charles (multisig1) - Multisig 1-1 (account2 || account3) + * Given signatories: Account1 && Account4 + * Expecting complete as Bob needs 2 signatures (account1 && (account2 || account3)) + */ const transferTransaction = TransferTransaction.create( Deadline.create(1, ChronoUnit.HOURS), Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), @@ -109,6 +128,14 @@ describe('AggregatedTransactionService', () => { }); it('should return isComplete: false for aggregated complete transaction - 2 levels Multisig', () => { + /** + * MLMA + * Alice (account1): normal account + * Bob (multisig2) - Multisig 2-1 (account1 && multisig1) + * Charles (multisig1) - Multisig 1-1 (account2 || account3) + * Given signatories: Account1 && Account4 + * Expecting incomplete as Bob needs 2 signatures (account1 && (account2 || account3)) but only got account1 + */ const transferTransaction = TransferTransaction.create( Deadline.create(1, ChronoUnit.HOURS), Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), @@ -130,6 +157,14 @@ describe('AggregatedTransactionService', () => { }); it('should return isComplete: false for aggregated complete transaction - 2 levels Multisig', () => { + /** + * MLMA + * Alice (account1): normal account + * Bob (multisig2) - Multisig 2-1 (account1 && multisig1) + * Charles (multisig1) - Multisig 1-1 (account2 || account3) + * Given signatories: Account1 && Account4 + * Expecting incomplete as Bob needs 2 signatures (account1 && (account2 || account3)) but got account4 + */ const transferTransaction = TransferTransaction.create( Deadline.create(1, ChronoUnit.HOURS), Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), @@ -151,9 +186,18 @@ describe('AggregatedTransactionService', () => { }); it('should return correct isComplete status for aggregated complete transaction - 2 levels Multisig, multi inner transaction', () => { + /** + * MLMA - with multiple transaction + * Alice (account1): normal account + * Bob (multisig2) - Multisig 2-1 (account1 && multisig1) + * Charles (multisig1) - Multisig 1-1 (account2 || account3) + * An extra inner transaction to account4 (just to increase the complexity) + * Given signatories: Account1 && Account4 + * Expecting incomplete as Bob needs 2 signatures (account1 && (account2 || account3)) + */ const transferTransaction = TransferTransaction.create( Deadline.create(1, ChronoUnit.HOURS), - Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + account2.address, [], PlainMessage.create('test-message'), NetworkType.MIJIN_TEST, @@ -161,7 +205,7 @@ describe('AggregatedTransactionService', () => { const transferTransaction2 = TransferTransaction.create( Deadline.create(1, ChronoUnit.HOURS), - Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + account2.address, [], PlainMessage.create('test-message'), NetworkType.MIJIN_TEST, @@ -170,17 +214,60 @@ describe('AggregatedTransactionService', () => { const aggregateTransaction = AggregateTransaction.createComplete( Deadline.create(), [transferTransaction.toAggregate(multisig2.publicAccount), - transferTransaction.toAggregate(account4.publicAccount)], + transferTransaction2.toAggregate(account4.publicAccount)], NetworkType.MIJIN_TEST, []); + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account4]); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.false; + }); + }); - const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account2]); + it('should return correct isComplete status for aggregated complete transaction - 2 levels Multisig, multi inner transaction', () => { + /** + * MLMA - with multiple transaction + * Alice (account1): normal account + * Bob (multisig2) - Multisig 2-1 (account1 && multisig1) + * Charles (multisig1) - Multisig 1-1 (account2 || account3) + * An extra inner transaction to account4 (just to increase the complexity) + * Given signatories: Account1 && Account4 && Account2 + * Expecting complete + */ + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + account2.address, + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const transferTransaction2 = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + account2.address, + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(multisig2.publicAccount), + transferTransaction2.toAggregate(account4.publicAccount)], + NetworkType.MIJIN_TEST, + []); + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account4, account2]); aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.true; }); }); it('should use minRemoval for multisig account validation if inner transaction is modify multisig remove', () => { + /** + * If the inner transaction is issued to a multisig account + * and the inner transaction itself is a ModifyMultiSigAccountTransaction - Removal + * The validator should use minRemoval value rather than minApproval value + * to determine if the act is complete or not + */ const modifyMultisigTransaction = ModifyMultisigAccountTransaction.create( Deadline.create(1, ChronoUnit.HOURS), 1, @@ -197,14 +284,19 @@ describe('AggregatedTransactionService', () => { [modifyMultisigTransaction.toAggregate(multisig2.publicAccount)], NetworkType.MIJIN_TEST, []); - const signedTransaction = aggregateTransaction.signWith(account2); aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.true; }); }); - it('should return correct isComplete status for aggregated complete transaction - none multisig', () => { + it('should return correct isComplete status (false) for aggregated complete transaction - none multisig', () => { + /** + * If the inner transaction is issued to a multisig account + * and the inner transaction itself is a ModifyMultiSigAccountTransaction - Removal + * The validator should use minRemoval value rather than minApproval value + * to determine if the act is complete or not + */ const transferTransaction = TransferTransaction.create( Deadline.create(1, ChronoUnit.HOURS), Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), @@ -220,11 +312,176 @@ describe('AggregatedTransactionService', () => { []); const signedTransaction = aggregateTransaction.signWith(account1); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.false; + }); + }); + + it('should return correct isComplete status (true) for aggregated complete transaction - none multisig', () => { + /** + * ACT + * Alice (account1): normal account + * Bob (account4) - normal account + * Alice initiate the transaction + * Bob sign + */ + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(account4.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signWith(account4); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.true; + }); + }); + + it('should return correct isComplete status TRUE - multiple normal account', () => { + /** + * ACT + * Alice: account1 + * Bog: account4 + * An escrow contract is signed by all the participants (normal accounts) + * Given Alice defined the following escrow contract: + * | sender | recipient | type | data | + * | Alice | Bob | send-an-asset | 1 concert.ticket | + * | Bob | Alice | send-an-asset | 20 euros | + * And Bob signs the contract + * And Alice signs the contract + * Then the contract should appear as complete + */ + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + account1.address, + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const transferTransaction2 = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + account4.address, + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(account4.publicAccount), + transferTransaction2.toAggregate(account1.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account4]); aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.true; }); }); + it('should return correct isComplete status FALSE - multiple normal account', () => { + /** + * ACT + * Alice: account1 + * Bog: account4 + * An escrow contract is signed by all the participants (normal accounts) + * Given Alice defined the following escrow contract: + * | sender | recipient | type | data | + * | Alice | Bob | send-an-asset | 1 concert.ticket | + * | Bob | Alice | send-an-asset | 20 euros | + * And Alice signs the contract + * Then the contract should appear as incomplete + */ + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + account1.address, + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const transferTransaction2 = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + account4.address, + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(account4.publicAccount), + transferTransaction2.toAggregate(account1.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, []); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.false; + }); + }); + + it('should return correct isComplete status TRUE - multisig Single Level', () => { + /** + * ACT - Multisig single level + * Alice (account1): initiate an transfer to Bob + * Bob (multisig3): is a 2/2 multisig account (account2 && account3) + */ + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + account4.address, + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(multisig3.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account2, [account3]); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.true; + }); + }); + + it('should return correct isComplete status FALSE - multisig Single Level', () => { + /** + * ACT - Multisig single level + * Alice (account1): initiate an transfer to Bob + * Bob (multisig3): is a 2/2 multisig account (account2 && account3) + */ + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + account4.address, + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(multisig3.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account2, []); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.false; + }); + }); + function givenMultisig2AccountInfo(): MultisigAccountInfo { return new MultisigAccountInfo(multisig2.publicAccount, 2, 1, @@ -233,6 +490,14 @@ describe('AggregatedTransactionService', () => { [], ); } + function givenMultisig3AccountInfo(): MultisigAccountInfo { + return new MultisigAccountInfo(multisig3.publicAccount, + 2, 2, + [account2.publicAccount, + account3.publicAccount], + [], + ); + } function givenAccount1Info(): MultisigAccountInfo { return new MultisigAccountInfo(account1.publicAccount, @@ -241,6 +506,22 @@ describe('AggregatedTransactionService', () => { [multisig2.publicAccount], ); } + function givenAccount2Info(): MultisigAccountInfo { + return new MultisigAccountInfo(account2.publicAccount, + 0, 0, + [], + [multisig2.publicAccount, + multisig3.publicAccount], + ); + } + function givenAccount3Info(): MultisigAccountInfo { + return new MultisigAccountInfo(account3.publicAccount, + 0, 0, + [], + [multisig2.publicAccount, + multisig3.publicAccount], + ); + } function givenAccount4Info(): MultisigAccountInfo { return new MultisigAccountInfo(account4.publicAccount, 0, 0, @@ -266,4 +547,16 @@ describe('AggregatedTransactionService', () => { return new MultisigAccountGraphInfo(map); } + function givenMultisig3AccountGraphInfo(): MultisigAccountGraphInfo { + const map = new Map(); + map.set(0, [new MultisigAccountInfo(multisig3.publicAccount, + 2, 2, + [account2.publicAccount, + account3.publicAccount], + [], + )]); + + return new MultisigAccountGraphInfo(map); + } + }); From 1cef7f82ebe3afc76be4389cf4dd66f59adf425f Mon Sep 17 00:00:00 2001 From: Steven Liu Date: Wed, 10 Apr 2019 15:13:01 +0100 Subject: [PATCH 25/31] renamed AggregatedTransactionService --- ...vice.ts => AggregateTransactionService.ts} | 2 +- src/service/service.ts | 2 +- ...ts => AggregateTransactionService.spec.ts} | 32 +++++++++---------- 3 files changed, 18 insertions(+), 18 deletions(-) rename src/service/{AggregatedTransactionService.ts => AggregateTransactionService.ts} (99%) rename test/service/{AggregatedTransactionService.spec.ts => AggregateTransactionService.spec.ts} (93%) diff --git a/src/service/AggregatedTransactionService.ts b/src/service/AggregateTransactionService.ts similarity index 99% rename from src/service/AggregatedTransactionService.ts rename to src/service/AggregateTransactionService.ts index 75a806bf96..800d1a1b89 100644 --- a/src/service/AggregatedTransactionService.ts +++ b/src/service/AggregateTransactionService.ts @@ -29,7 +29,7 @@ import { TransactionType } from '../model/transaction/TransactionType'; /** * Aggregated Transaction service */ -export class AggregatedTransactionService { +export class AggregateTransactionService { /** * Constructor diff --git a/src/service/service.ts b/src/service/service.ts index fcf4c84680..c988e13ffb 100644 --- a/src/service/service.ts +++ b/src/service/service.ts @@ -16,4 +16,4 @@ export * from './NamespaceService'; export * from './MosaicService'; -export * from './AggregatedTransactionService'; +export * from './AggregateTransactionService'; diff --git a/test/service/AggregatedTransactionService.spec.ts b/test/service/AggregateTransactionService.spec.ts similarity index 93% rename from test/service/AggregatedTransactionService.spec.ts rename to test/service/AggregateTransactionService.spec.ts index 23470cb2a1..ae2deab99b 100644 --- a/test/service/AggregatedTransactionService.spec.ts +++ b/test/service/AggregateTransactionService.spec.ts @@ -31,13 +31,13 @@ import { MultisigCosignatoryModification } from '../../src/model/transaction/Mul import { MultisigCosignatoryModificationType } from '../../src/model/transaction/MultisigCosignatoryModificationType'; import { PlainMessage } from '../../src/model/transaction/PlainMessage'; import { TransferTransaction } from '../../src/model/transaction/TransferTransaction'; -import { AggregatedTransactionService } from '../../src/service/AggregatedTransactionService'; +import { AggregateTransactionService } from '../../src/service/AggregateTransactionService'; /** * For multi level multisig scenario visit: https://github.com/nemtech/nem2-docs/issues/10 */ -describe('AggregatedTransactionService', () => { - let aggregatedTransactionService: AggregatedTransactionService; +describe('AggregateTransactionService', () => { + let aggregateTransactionService: AggregateTransactionService; /** * Multisig2 Account: SBROWP-7YMG2M-K45RO6-Q7ZPK7-G7GXWQ-JK5VNQ-OSUX @@ -95,7 +95,7 @@ describe('AggregatedTransactionService', () => { .thenReturn(observableOf(givenAccount3Info())); const accountHttp = instance(mockedAccountHttp); - aggregatedTransactionService = new AggregatedTransactionService(accountHttp); + aggregateTransactionService = new AggregateTransactionService(accountHttp); }); it('should return isComplete: true for aggregated complete transaction - 2 levels Multisig', () => { @@ -122,7 +122,7 @@ describe('AggregatedTransactionService', () => { []); const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account2]); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + aggregateTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.true; }); }); @@ -151,7 +151,7 @@ describe('AggregatedTransactionService', () => { []); const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, []); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + aggregateTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.false; }); }); @@ -180,7 +180,7 @@ describe('AggregatedTransactionService', () => { []); const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account4]); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + aggregateTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.false; }); }); @@ -218,7 +218,7 @@ describe('AggregatedTransactionService', () => { NetworkType.MIJIN_TEST, []); const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account4]); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + aggregateTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.false; }); }); @@ -256,7 +256,7 @@ describe('AggregatedTransactionService', () => { NetworkType.MIJIN_TEST, []); const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account4, account2]); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + aggregateTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.true; }); }); @@ -285,7 +285,7 @@ describe('AggregatedTransactionService', () => { NetworkType.MIJIN_TEST, []); const signedTransaction = aggregateTransaction.signWith(account2); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + aggregateTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.true; }); }); @@ -312,7 +312,7 @@ describe('AggregatedTransactionService', () => { []); const signedTransaction = aggregateTransaction.signWith(account1); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + aggregateTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.false; }); }); @@ -340,7 +340,7 @@ describe('AggregatedTransactionService', () => { []); const signedTransaction = aggregateTransaction.signWith(account4); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + aggregateTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.true; }); }); @@ -383,7 +383,7 @@ describe('AggregatedTransactionService', () => { []); const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account4]); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + aggregateTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.true; }); }); @@ -425,7 +425,7 @@ describe('AggregatedTransactionService', () => { []); const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, []); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + aggregateTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.false; }); }); @@ -451,7 +451,7 @@ describe('AggregatedTransactionService', () => { []); const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account2, [account3]); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + aggregateTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.true; }); }); @@ -477,7 +477,7 @@ describe('AggregatedTransactionService', () => { []); const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account2, []); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + aggregateTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.false; }); }); From 18a5f2a5c1b41587417a6383181d9a11b4ca8993 Mon Sep 17 00:00:00 2001 From: Steven Liu Date: Mon, 1 Apr 2019 20:39:11 +0100 Subject: [PATCH 26/31] #4 1. Addes service for Aggregated Transaction - isComplete 2. Fixed couple of bugs in TransactionMappings --- src/service/AggregatedTransactionService.ts | 127 +++++++++ .../AggregatedTransactionService.spec.ts | 242 ++++++++++++++++++ 2 files changed, 369 insertions(+) create mode 100644 src/service/AggregatedTransactionService.ts create mode 100644 test/service/AggregatedTransactionService.spec.ts diff --git a/src/service/AggregatedTransactionService.ts b/src/service/AggregatedTransactionService.ts new file mode 100644 index 0000000000..ccdd72cd3b --- /dev/null +++ b/src/service/AggregatedTransactionService.ts @@ -0,0 +1,127 @@ +/* + * Copyright 2019 NEM + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import {from as observableFrom , Observable, of as observableOf} from 'rxjs'; +import { map, mergeMap} from 'rxjs/operators'; +import { TransactionMapping } from '../core/utils/TransactionMapping'; +import { AccountHttp } from '../infrastructure/AccountHttp'; +import { MultisigAccountGraphInfo } from '../model/account/MultisigAccountGraphInfo'; +import { AggregateTransaction } from '../model/transaction/AggregateTransaction'; +import { SignedTransaction } from '../model/transaction/SignedTransaction'; + +/** + * Aggregated Transaction service + */ +export class AggregatedTransactionService { + + /** + * Constructor + * @param accountHttp + */ + constructor(private readonly accountHttp: AccountHttp) { + } + + /** + * Check if an aggregate complete transaction has all cosignatories attached + * @param signedTransaction - The signed aggregate transaction (complete) to be verified + * @returns {Observable} + */ + public isComplete(signedTransaction: SignedTransaction): Observable { + const aggregateTransaction = TransactionMapping.createFromPayload(signedTransaction.payload) as AggregateTransaction; + /** + * Include both initiator & cosigners + */ + const signers = (aggregateTransaction.cosignatures.map((cosigner) => cosigner.signer.publicKey)); + if (signedTransaction.signer) { + signers.push(signedTransaction.signer); + } + + return observableFrom(aggregateTransaction.innerTransactions).pipe( + mergeMap((innerTransaction) => this.accountHttp.getMultisigAccountInfo(innerTransaction.signer.address) + .pipe( + /** + * For multisig account, we need to get the graph info in case it has multiple levels + */ + mergeMap((_) => _.minApproval !== 0 && _.minRemoval !== 0 ? + this.accountHttp.getMultisigAccountGraphInfo(_.account.address) + .pipe( + map((graphInfo) => this.validateCosignatories(graphInfo, signers)), + ) : observableOf(true), + ), + ), + ), + ); + } + + /** + * Validate cosignatories against multisig Account(s) + * @param graphInfo - multisig account graph info + * @param cosignatories - array of cosignatories extracted from aggregated transaction + * @returns {boolean} + */ + private validateCosignatories(graphInfo: MultisigAccountGraphInfo, cosignatories: string[]): boolean { + /** + * Validate cosignatories from bottom level to top + */ + const sortedKeys = Array.from(graphInfo.multisigAccounts.keys()).sort((a, b) => b - a); + const cosignatoriesReceived = cosignatories; + let validationResult = true; + + sortedKeys.forEach((key) => { + const multisigInfo = graphInfo.multisigAccounts.get(key); + if (multisigInfo && validationResult) { + multisigInfo.forEach((multisig) => { + if (multisig.minApproval >= 1) { + const matchedCosignatories = this.compareArrays(cosignatoriesReceived, + multisig.cosignatories.map((cosig) => cosig.publicKey)); + + /** + * if minimal signature requirement met at current level, push the multisig account + * into the received signatories array for next level validation. + * Otherwise return validation failed. + */ + if (matchedCosignatories.length >= multisig.minApproval) { + if (cosignatoriesReceived.indexOf(multisig.account.publicKey) === -1) { + cosignatoriesReceived.push(multisig.account.publicKey); + } + } else { + validationResult = false; + } + } + }); + } + }); + + return validationResult; + } + + /** + * Compare two string arrays + * @param array1 - base array + * @param array2 - array to be matched + * @returns {string[]} - array of matched elements + */ + private compareArrays(array1: string[], array2: string[]): string[] { + const results: string[] = []; + array1.forEach((a1) => array2.forEach((a2) => { + if (a1 === a2) { + results.push(a1); + } + })); + + return results; + } +} diff --git a/test/service/AggregatedTransactionService.spec.ts b/test/service/AggregatedTransactionService.spec.ts new file mode 100644 index 0000000000..0aff3d8f47 --- /dev/null +++ b/test/service/AggregatedTransactionService.spec.ts @@ -0,0 +1,242 @@ +/* + * Copyright 2018 NEM + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { expect } from 'chai'; +import { ChronoUnit } from 'js-joda'; +import {of as observableOf} from 'rxjs'; +import {deepEqual, instance, mock, when} from 'ts-mockito'; +import { AccountHttp } from '../../src/infrastructure/AccountHttp'; +import { Account } from '../../src/model/account/Account'; +import { Address } from '../../src/model/account/Address'; +import { MultisigAccountGraphInfo } from '../../src/model/account/MultisigAccountGraphInfo'; +import { MultisigAccountInfo } from '../../src/model/account/MultisigAccountInfo'; +import {NetworkType} from '../../src/model/blockchain/NetworkType'; +import { AggregateTransaction } from '../../src/model/transaction/AggregateTransaction'; +import { Deadline } from '../../src/model/transaction/Deadline'; +import { PlainMessage } from '../../src/model/transaction/PlainMessage'; +import { TransferTransaction } from '../../src/model/transaction/TransferTransaction'; +import { AggregatedTransactionService } from '../../src/service/AggregatedTransactionService'; + +/** + * For multi level multisig scenario visit: https://github.com/nemtech/nem2-docs/issues/10 + */ +describe('AggregatedTransactionService', () => { + let aggregatedTransactionService: AggregatedTransactionService; + + /** + * Multisig2 Account: SBROWP-7YMG2M-K45RO6-Q7ZPK7-G7GXWQ-JK5VNQ-OSUX + * Public Key: 5E628EA59818D97AA4118780D9A88C5512FCE7A21C195E1574727EFCE5DF7C0D + * Private Key: 22A1D67F8519D1A45BD7116600BB6E857786E816FE0B45E4C5B9FFF3D64BC177 + * + * + * Multisig1 Account: SAK32M-5JQ43R-WYHWEH-WRBCW4-RXERT2-DLASGL-EANS + * Public Key: BFDF2610C5666A626434FE12FB4A9D896D2B9B033F5F84CCEABE82E043A6307E + * Private Key: 8B0622C2CCFC5CCC5A74B500163E3C68F3AD3643DB12932FC931143EAC67280D + */ + + /** + * Test accounts: + * Multisig1 (1/1): Account2, Account3 + * Multisig2 (2/1): Account1, Multisig1 + * Stranger Account: Account4 + */ + + const account1 = Account.createFromPrivateKey('82DB2528834C9926F0FCCE042466B24A266F5B685CB66D2869AF6648C043E950', + NetworkType.MIJIN_TEST); + const multisig1 = Account.createFromPrivateKey('8B0622C2CCFC5CCC5A74B500163E3C68F3AD3643DB12932FC931143EAC67280D', + NetworkType.MIJIN_TEST); + const multisig2 = Account.createFromPrivateKey('22A1D67F8519D1A45BD7116600BB6E857786E816FE0B45E4C5B9FFF3D64BC177', + NetworkType.MIJIN_TEST); + + const account2 = Account.createFromPrivateKey('A4D410270E01CECDCDEADCDE32EC79C8D9CDEA4DCD426CB1EB666EFEF148FBCE', + NetworkType.MIJIN_TEST); + const account3 = Account.createFromPrivateKey('336AB45EE65A6AFFC0E7ADC5342F91E34BACA0B901A1D9C876FA25A1E590077E', + NetworkType.MIJIN_TEST); + + const account4 = Account.createFromPrivateKey('4D8B3756592532753344E11E2B7541317BCCFBBCF4444274CDBF359D2C4AE0F1', + NetworkType.MIJIN_TEST); + before(() => { + const mockedAccountHttp = mock(AccountHttp); + + when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(account1.address))) + .thenReturn(observableOf(givenAccount1Info())); + when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(account4.address))) + .thenReturn(observableOf(givenAccount4Info())); + when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(multisig2.address))) + .thenReturn(observableOf(givenMultisig2AccountInfo())); + when(mockedAccountHttp.getMultisigAccountGraphInfo(deepEqual(multisig2.address))) + .thenReturn(observableOf(givenMultisig2AccountGraphInfo())); + + const accountHttp = instance(mockedAccountHttp); + aggregatedTransactionService = new AggregatedTransactionService(accountHttp); + }); + + it('should return isComplete: true for aggregated complete transaction - 2 levels Multisig', () => { + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(multisig2.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account2]); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.true; + }); + }); + + it('should return isComplete: false for aggregated complete transaction - 2 levels Multisig', () => { + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(multisig2.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, []); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.false; + }); + }); + + it('should return isComplete: false for aggregated complete transaction - 2 levels Multisig', () => { + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(multisig2.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account4]); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.false; + }); + }); + + it('should return correct isComplete status for aggregated complete transaction - 2 levels Multisig, multi inner transaction', () => { + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const transferTransaction2 = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(multisig2.publicAccount), + transferTransaction.toAggregate(account4.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account2]); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.true; + }); + }); + + it('should return correct isComplete status for aggregated complete transaction - none multisig', () => { + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(account4.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signWith(account1); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.true; + }); + }); + + function givenMultisig2AccountInfo(): MultisigAccountInfo { + return new MultisigAccountInfo(multisig2.publicAccount, + 2, 1, + [multisig1.publicAccount, + account1.publicAccount], + [], + ); + } + + function givenAccount1Info(): MultisigAccountInfo { + return new MultisigAccountInfo(account1.publicAccount, + 0, 0, + [], + [multisig2.publicAccount], + ); + } + function givenAccount4Info(): MultisigAccountInfo { + return new MultisigAccountInfo(account4.publicAccount, + 0, 0, + [], + [], + ); + } + + function givenMultisig2AccountGraphInfo(): MultisigAccountGraphInfo { + const map = new Map(); + map.set(0, [new MultisigAccountInfo(multisig2.publicAccount, + 2, 1, + [multisig1.publicAccount, + account1.publicAccount], + [], + )]) + .set(1, [new MultisigAccountInfo(multisig1.publicAccount, + 1, 1, + [account2.publicAccount, account3.publicAccount], + [multisig2.publicAccount], + )]); + + return new MultisigAccountGraphInfo(map); + } + +}); From df799f6d04c9be6b78162157ea712b106daca054 Mon Sep 17 00:00:00 2001 From: Steven Liu Date: Mon, 1 Apr 2019 21:25:50 +0100 Subject: [PATCH 27/31] Updated license year --- test/service/AggregatedTransactionService.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/service/AggregatedTransactionService.spec.ts b/test/service/AggregatedTransactionService.spec.ts index 0aff3d8f47..bc0c11f535 100644 --- a/test/service/AggregatedTransactionService.spec.ts +++ b/test/service/AggregatedTransactionService.spec.ts @@ -1,5 +1,5 @@ /* - * Copyright 2018 NEM + * Copyright 2019 NEM * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. From 6a01621614bea4a597f2ff969a38178d0b8351a4 Mon Sep 17 00:00:00 2001 From: Steven Liu Date: Wed, 3 Apr 2019 16:12:08 +0100 Subject: [PATCH 28/31] Added check in validate cosignatories for modify multisig account (removal) --- src/service/AggregatedTransactionService.ts | 29 ++++++++++++++++--- .../AggregatedTransactionService.spec.ts | 27 +++++++++++++++++ 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/src/service/AggregatedTransactionService.ts b/src/service/AggregatedTransactionService.ts index ccdd72cd3b..7803b48bae 100644 --- a/src/service/AggregatedTransactionService.ts +++ b/src/service/AggregatedTransactionService.ts @@ -20,7 +20,11 @@ import { TransactionMapping } from '../core/utils/TransactionMapping'; import { AccountHttp } from '../infrastructure/AccountHttp'; import { MultisigAccountGraphInfo } from '../model/account/MultisigAccountGraphInfo'; import { AggregateTransaction } from '../model/transaction/AggregateTransaction'; +import { InnerTransaction } from '../model/transaction/InnerTransaction'; +import { ModifyMultisigAccountTransaction } from '../model/transaction/ModifyMultisigAccountTransaction'; +import { MultisigCosignatoryModificationType } from '../model/transaction/MultisigCosignatoryModificationType'; import { SignedTransaction } from '../model/transaction/SignedTransaction'; +import { TransactionType } from '../model/transaction/TransactionType'; /** * Aggregated Transaction service @@ -58,7 +62,7 @@ export class AggregatedTransactionService { mergeMap((_) => _.minApproval !== 0 && _.minRemoval !== 0 ? this.accountHttp.getMultisigAccountGraphInfo(_.account.address) .pipe( - map((graphInfo) => this.validateCosignatories(graphInfo, signers)), + map((graphInfo) => this.validateCosignatories(graphInfo, signers, innerTransaction)), ) : observableOf(true), ), ), @@ -70,9 +74,12 @@ export class AggregatedTransactionService { * Validate cosignatories against multisig Account(s) * @param graphInfo - multisig account graph info * @param cosignatories - array of cosignatories extracted from aggregated transaction + * @param innerTransaction - the inner transaction of the aggregated transaction * @returns {boolean} */ - private validateCosignatories(graphInfo: MultisigAccountGraphInfo, cosignatories: string[]): boolean { + private validateCosignatories(graphInfo: MultisigAccountGraphInfo, + cosignatories: string[], + innerTransaction: InnerTransaction): boolean { /** * Validate cosignatories from bottom level to top */ @@ -80,11 +87,24 @@ export class AggregatedTransactionService { const cosignatoriesReceived = cosignatories; let validationResult = true; + let isMultisigRemoval = false; + + /** + * Check inner transaction. If remove cosigner from multisig account, + * use minRemoval instead of minApproval for cosignatories validation. + */ + if (innerTransaction.type === TransactionType.MODIFY_MULTISIG_ACCOUNT) { + if ((innerTransaction as ModifyMultisigAccountTransaction).modifications + .find((modification) => modification.type === MultisigCosignatoryModificationType.Remove) !== undefined) { + isMultisigRemoval = true; + } + } + sortedKeys.forEach((key) => { const multisigInfo = graphInfo.multisigAccounts.get(key); if (multisigInfo && validationResult) { multisigInfo.forEach((multisig) => { - if (multisig.minApproval >= 1) { + if (multisig.minApproval >= 1 && multisig.minRemoval) { // To make sure it is multisig account const matchedCosignatories = this.compareArrays(cosignatoriesReceived, multisig.cosignatories.map((cosig) => cosig.publicKey)); @@ -93,7 +113,8 @@ export class AggregatedTransactionService { * into the received signatories array for next level validation. * Otherwise return validation failed. */ - if (matchedCosignatories.length >= multisig.minApproval) { + if ((matchedCosignatories.length >= multisig.minApproval && !isMultisigRemoval) || + (matchedCosignatories.length >= multisig.minRemoval && isMultisigRemoval)) { if (cosignatoriesReceived.indexOf(multisig.account.publicKey) === -1) { cosignatoriesReceived.push(multisig.account.publicKey); } diff --git a/test/service/AggregatedTransactionService.spec.ts b/test/service/AggregatedTransactionService.spec.ts index bc0c11f535..8ea1432ca6 100644 --- a/test/service/AggregatedTransactionService.spec.ts +++ b/test/service/AggregatedTransactionService.spec.ts @@ -26,6 +26,9 @@ import { MultisigAccountInfo } from '../../src/model/account/MultisigAccountInfo import {NetworkType} from '../../src/model/blockchain/NetworkType'; import { AggregateTransaction } from '../../src/model/transaction/AggregateTransaction'; import { Deadline } from '../../src/model/transaction/Deadline'; +import { ModifyMultisigAccountTransaction } from '../../src/model/transaction/ModifyMultisigAccountTransaction'; +import { MultisigCosignatoryModification } from '../../src/model/transaction/MultisigCosignatoryModification'; +import { MultisigCosignatoryModificationType } from '../../src/model/transaction/MultisigCosignatoryModificationType'; import { PlainMessage } from '../../src/model/transaction/PlainMessage'; import { TransferTransaction } from '../../src/model/transaction/TransferTransaction'; import { AggregatedTransactionService } from '../../src/service/AggregatedTransactionService'; @@ -177,6 +180,30 @@ describe('AggregatedTransactionService', () => { }); }); + it('should use minRemoval for multisig account validation if inner transaction is modify multisig remove', () => { + const modifyMultisigTransaction = ModifyMultisigAccountTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + 1, + 1, + [new MultisigCosignatoryModification( + MultisigCosignatoryModificationType.Remove, + account1.publicAccount, + )], + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [modifyMultisigTransaction.toAggregate(multisig2.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signWith(account2); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.true; + }); + }); + it('should return correct isComplete status for aggregated complete transaction - none multisig', () => { const transferTransaction = TransferTransaction.create( Deadline.create(1, ChronoUnit.HOURS), From e6136e08f8462d417d872426c377e921330a0abb Mon Sep 17 00:00:00 2001 From: Steven Liu Date: Fri, 5 Apr 2019 19:01:52 +0100 Subject: [PATCH 29/31] Added more test cases and fixed a few bugs --- src/service/AggregatedTransactionService.ts | 12 +- .../AggregatedTransactionService.spec.ts | 305 +++++++++++++++++- 2 files changed, 307 insertions(+), 10 deletions(-) diff --git a/src/service/AggregatedTransactionService.ts b/src/service/AggregatedTransactionService.ts index 7803b48bae..75a806bf96 100644 --- a/src/service/AggregatedTransactionService.ts +++ b/src/service/AggregatedTransactionService.ts @@ -15,7 +15,7 @@ */ import {from as observableFrom , Observable, of as observableOf} from 'rxjs'; -import { map, mergeMap} from 'rxjs/operators'; +import { flatMap, map, mergeMap, toArray} from 'rxjs/operators'; import { TransactionMapping } from '../core/utils/TransactionMapping'; import { AccountHttp } from '../infrastructure/AccountHttp'; import { MultisigAccountGraphInfo } from '../model/account/MultisigAccountGraphInfo'; @@ -52,7 +52,6 @@ export class AggregatedTransactionService { if (signedTransaction.signer) { signers.push(signedTransaction.signer); } - return observableFrom(aggregateTransaction.innerTransactions).pipe( mergeMap((innerTransaction) => this.accountHttp.getMultisigAccountInfo(innerTransaction.signer.address) .pipe( @@ -63,11 +62,16 @@ export class AggregatedTransactionService { this.accountHttp.getMultisigAccountGraphInfo(_.account.address) .pipe( map((graphInfo) => this.validateCosignatories(graphInfo, signers, innerTransaction)), - ) : observableOf(true), + ) : observableOf(signers.find((s) => s === _.account.publicKey ) !== undefined), ), ), ), - ); + toArray(), + ).pipe( + flatMap((results) => { + return observableOf(results.every((isComplete) => isComplete)); + }), + ); } /** diff --git a/test/service/AggregatedTransactionService.spec.ts b/test/service/AggregatedTransactionService.spec.ts index 8ea1432ca6..23470cb2a1 100644 --- a/test/service/AggregatedTransactionService.spec.ts +++ b/test/service/AggregatedTransactionService.spec.ts @@ -54,6 +54,7 @@ describe('AggregatedTransactionService', () => { * Test accounts: * Multisig1 (1/1): Account2, Account3 * Multisig2 (2/1): Account1, Multisig1 + * Multisig3 (2/2): Account2, Account3 * Stranger Account: Account4 */ @@ -64,6 +65,8 @@ describe('AggregatedTransactionService', () => { const multisig2 = Account.createFromPrivateKey('22A1D67F8519D1A45BD7116600BB6E857786E816FE0B45E4C5B9FFF3D64BC177', NetworkType.MIJIN_TEST); + const multisig3 = Account.createFromPrivateKey('5E7812AB0E709ABC45466034E1A209099F6A12C4698748A63CDCAA9B0DDE1DBD', + NetworkType.MIJIN_TEST); const account2 = Account.createFromPrivateKey('A4D410270E01CECDCDEADCDE32EC79C8D9CDEA4DCD426CB1EB666EFEF148FBCE', NetworkType.MIJIN_TEST); const account3 = Account.createFromPrivateKey('336AB45EE65A6AFFC0E7ADC5342F91E34BACA0B901A1D9C876FA25A1E590077E', @@ -80,14 +83,30 @@ describe('AggregatedTransactionService', () => { .thenReturn(observableOf(givenAccount4Info())); when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(multisig2.address))) .thenReturn(observableOf(givenMultisig2AccountInfo())); + when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(multisig3.address))) + .thenReturn(observableOf(givenMultisig3AccountInfo())); when(mockedAccountHttp.getMultisigAccountGraphInfo(deepEqual(multisig2.address))) .thenReturn(observableOf(givenMultisig2AccountGraphInfo())); + when(mockedAccountHttp.getMultisigAccountGraphInfo(deepEqual(multisig3.address))) + .thenReturn(observableOf(givenMultisig3AccountGraphInfo())); + when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(account2.address))) + .thenReturn(observableOf(givenAccount2Info())); + when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(account3.address))) + .thenReturn(observableOf(givenAccount3Info())); const accountHttp = instance(mockedAccountHttp); aggregatedTransactionService = new AggregatedTransactionService(accountHttp); }); it('should return isComplete: true for aggregated complete transaction - 2 levels Multisig', () => { + /** + * MLMA + * Alice (account1): normal account + * Bob (multisig2) - Multisig 2-1 (account1 && multisig1) + * Charles (multisig1) - Multisig 1-1 (account2 || account3) + * Given signatories: Account1 && Account4 + * Expecting complete as Bob needs 2 signatures (account1 && (account2 || account3)) + */ const transferTransaction = TransferTransaction.create( Deadline.create(1, ChronoUnit.HOURS), Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), @@ -109,6 +128,14 @@ describe('AggregatedTransactionService', () => { }); it('should return isComplete: false for aggregated complete transaction - 2 levels Multisig', () => { + /** + * MLMA + * Alice (account1): normal account + * Bob (multisig2) - Multisig 2-1 (account1 && multisig1) + * Charles (multisig1) - Multisig 1-1 (account2 || account3) + * Given signatories: Account1 && Account4 + * Expecting incomplete as Bob needs 2 signatures (account1 && (account2 || account3)) but only got account1 + */ const transferTransaction = TransferTransaction.create( Deadline.create(1, ChronoUnit.HOURS), Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), @@ -130,6 +157,14 @@ describe('AggregatedTransactionService', () => { }); it('should return isComplete: false for aggregated complete transaction - 2 levels Multisig', () => { + /** + * MLMA + * Alice (account1): normal account + * Bob (multisig2) - Multisig 2-1 (account1 && multisig1) + * Charles (multisig1) - Multisig 1-1 (account2 || account3) + * Given signatories: Account1 && Account4 + * Expecting incomplete as Bob needs 2 signatures (account1 && (account2 || account3)) but got account4 + */ const transferTransaction = TransferTransaction.create( Deadline.create(1, ChronoUnit.HOURS), Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), @@ -151,9 +186,18 @@ describe('AggregatedTransactionService', () => { }); it('should return correct isComplete status for aggregated complete transaction - 2 levels Multisig, multi inner transaction', () => { + /** + * MLMA - with multiple transaction + * Alice (account1): normal account + * Bob (multisig2) - Multisig 2-1 (account1 && multisig1) + * Charles (multisig1) - Multisig 1-1 (account2 || account3) + * An extra inner transaction to account4 (just to increase the complexity) + * Given signatories: Account1 && Account4 + * Expecting incomplete as Bob needs 2 signatures (account1 && (account2 || account3)) + */ const transferTransaction = TransferTransaction.create( Deadline.create(1, ChronoUnit.HOURS), - Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + account2.address, [], PlainMessage.create('test-message'), NetworkType.MIJIN_TEST, @@ -161,7 +205,7 @@ describe('AggregatedTransactionService', () => { const transferTransaction2 = TransferTransaction.create( Deadline.create(1, ChronoUnit.HOURS), - Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + account2.address, [], PlainMessage.create('test-message'), NetworkType.MIJIN_TEST, @@ -170,17 +214,60 @@ describe('AggregatedTransactionService', () => { const aggregateTransaction = AggregateTransaction.createComplete( Deadline.create(), [transferTransaction.toAggregate(multisig2.publicAccount), - transferTransaction.toAggregate(account4.publicAccount)], + transferTransaction2.toAggregate(account4.publicAccount)], NetworkType.MIJIN_TEST, []); + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account4]); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.false; + }); + }); - const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account2]); + it('should return correct isComplete status for aggregated complete transaction - 2 levels Multisig, multi inner transaction', () => { + /** + * MLMA - with multiple transaction + * Alice (account1): normal account + * Bob (multisig2) - Multisig 2-1 (account1 && multisig1) + * Charles (multisig1) - Multisig 1-1 (account2 || account3) + * An extra inner transaction to account4 (just to increase the complexity) + * Given signatories: Account1 && Account4 && Account2 + * Expecting complete + */ + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + account2.address, + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const transferTransaction2 = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + account2.address, + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(multisig2.publicAccount), + transferTransaction2.toAggregate(account4.publicAccount)], + NetworkType.MIJIN_TEST, + []); + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account4, account2]); aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.true; }); }); it('should use minRemoval for multisig account validation if inner transaction is modify multisig remove', () => { + /** + * If the inner transaction is issued to a multisig account + * and the inner transaction itself is a ModifyMultiSigAccountTransaction - Removal + * The validator should use minRemoval value rather than minApproval value + * to determine if the act is complete or not + */ const modifyMultisigTransaction = ModifyMultisigAccountTransaction.create( Deadline.create(1, ChronoUnit.HOURS), 1, @@ -197,14 +284,19 @@ describe('AggregatedTransactionService', () => { [modifyMultisigTransaction.toAggregate(multisig2.publicAccount)], NetworkType.MIJIN_TEST, []); - const signedTransaction = aggregateTransaction.signWith(account2); aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.true; }); }); - it('should return correct isComplete status for aggregated complete transaction - none multisig', () => { + it('should return correct isComplete status (false) for aggregated complete transaction - none multisig', () => { + /** + * If the inner transaction is issued to a multisig account + * and the inner transaction itself is a ModifyMultiSigAccountTransaction - Removal + * The validator should use minRemoval value rather than minApproval value + * to determine if the act is complete or not + */ const transferTransaction = TransferTransaction.create( Deadline.create(1, ChronoUnit.HOURS), Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), @@ -220,11 +312,176 @@ describe('AggregatedTransactionService', () => { []); const signedTransaction = aggregateTransaction.signWith(account1); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.false; + }); + }); + + it('should return correct isComplete status (true) for aggregated complete transaction - none multisig', () => { + /** + * ACT + * Alice (account1): normal account + * Bob (account4) - normal account + * Alice initiate the transaction + * Bob sign + */ + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(account4.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signWith(account4); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.true; + }); + }); + + it('should return correct isComplete status TRUE - multiple normal account', () => { + /** + * ACT + * Alice: account1 + * Bog: account4 + * An escrow contract is signed by all the participants (normal accounts) + * Given Alice defined the following escrow contract: + * | sender | recipient | type | data | + * | Alice | Bob | send-an-asset | 1 concert.ticket | + * | Bob | Alice | send-an-asset | 20 euros | + * And Bob signs the contract + * And Alice signs the contract + * Then the contract should appear as complete + */ + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + account1.address, + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const transferTransaction2 = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + account4.address, + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(account4.publicAccount), + transferTransaction2.toAggregate(account1.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account4]); aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { expect(isComplete).to.be.true; }); }); + it('should return correct isComplete status FALSE - multiple normal account', () => { + /** + * ACT + * Alice: account1 + * Bog: account4 + * An escrow contract is signed by all the participants (normal accounts) + * Given Alice defined the following escrow contract: + * | sender | recipient | type | data | + * | Alice | Bob | send-an-asset | 1 concert.ticket | + * | Bob | Alice | send-an-asset | 20 euros | + * And Alice signs the contract + * Then the contract should appear as incomplete + */ + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + account1.address, + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const transferTransaction2 = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + account4.address, + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(account4.publicAccount), + transferTransaction2.toAggregate(account1.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, []); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.false; + }); + }); + + it('should return correct isComplete status TRUE - multisig Single Level', () => { + /** + * ACT - Multisig single level + * Alice (account1): initiate an transfer to Bob + * Bob (multisig3): is a 2/2 multisig account (account2 && account3) + */ + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + account4.address, + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(multisig3.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account2, [account3]); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.true; + }); + }); + + it('should return correct isComplete status FALSE - multisig Single Level', () => { + /** + * ACT - Multisig single level + * Alice (account1): initiate an transfer to Bob + * Bob (multisig3): is a 2/2 multisig account (account2 && account3) + */ + const transferTransaction = TransferTransaction.create( + Deadline.create(1, ChronoUnit.HOURS), + account4.address, + [], + PlainMessage.create('test-message'), + NetworkType.MIJIN_TEST, + ); + + const aggregateTransaction = AggregateTransaction.createComplete( + Deadline.create(), + [transferTransaction.toAggregate(multisig3.publicAccount)], + NetworkType.MIJIN_TEST, + []); + + const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account2, []); + aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { + expect(isComplete).to.be.false; + }); + }); + function givenMultisig2AccountInfo(): MultisigAccountInfo { return new MultisigAccountInfo(multisig2.publicAccount, 2, 1, @@ -233,6 +490,14 @@ describe('AggregatedTransactionService', () => { [], ); } + function givenMultisig3AccountInfo(): MultisigAccountInfo { + return new MultisigAccountInfo(multisig3.publicAccount, + 2, 2, + [account2.publicAccount, + account3.publicAccount], + [], + ); + } function givenAccount1Info(): MultisigAccountInfo { return new MultisigAccountInfo(account1.publicAccount, @@ -241,6 +506,22 @@ describe('AggregatedTransactionService', () => { [multisig2.publicAccount], ); } + function givenAccount2Info(): MultisigAccountInfo { + return new MultisigAccountInfo(account2.publicAccount, + 0, 0, + [], + [multisig2.publicAccount, + multisig3.publicAccount], + ); + } + function givenAccount3Info(): MultisigAccountInfo { + return new MultisigAccountInfo(account3.publicAccount, + 0, 0, + [], + [multisig2.publicAccount, + multisig3.publicAccount], + ); + } function givenAccount4Info(): MultisigAccountInfo { return new MultisigAccountInfo(account4.publicAccount, 0, 0, @@ -266,4 +547,16 @@ describe('AggregatedTransactionService', () => { return new MultisigAccountGraphInfo(map); } + function givenMultisig3AccountGraphInfo(): MultisigAccountGraphInfo { + const map = new Map(); + map.set(0, [new MultisigAccountInfo(multisig3.publicAccount, + 2, 2, + [account2.publicAccount, + account3.publicAccount], + [], + )]); + + return new MultisigAccountGraphInfo(map); + } + }); From 9754295b2694308262f4365476c3d81faad3376e Mon Sep 17 00:00:00 2001 From: Steven Liu Date: Wed, 10 Apr 2019 15:13:01 +0100 Subject: [PATCH 30/31] renamed AggregatedTransactionService --- src/service/AggregatedTransactionService.ts | 152 ----- .../AggregatedTransactionService.spec.ts | 562 ------------------ 2 files changed, 714 deletions(-) delete mode 100644 src/service/AggregatedTransactionService.ts delete mode 100644 test/service/AggregatedTransactionService.spec.ts diff --git a/src/service/AggregatedTransactionService.ts b/src/service/AggregatedTransactionService.ts deleted file mode 100644 index 75a806bf96..0000000000 --- a/src/service/AggregatedTransactionService.ts +++ /dev/null @@ -1,152 +0,0 @@ -/* - * Copyright 2019 NEM - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import {from as observableFrom , Observable, of as observableOf} from 'rxjs'; -import { flatMap, map, mergeMap, toArray} from 'rxjs/operators'; -import { TransactionMapping } from '../core/utils/TransactionMapping'; -import { AccountHttp } from '../infrastructure/AccountHttp'; -import { MultisigAccountGraphInfo } from '../model/account/MultisigAccountGraphInfo'; -import { AggregateTransaction } from '../model/transaction/AggregateTransaction'; -import { InnerTransaction } from '../model/transaction/InnerTransaction'; -import { ModifyMultisigAccountTransaction } from '../model/transaction/ModifyMultisigAccountTransaction'; -import { MultisigCosignatoryModificationType } from '../model/transaction/MultisigCosignatoryModificationType'; -import { SignedTransaction } from '../model/transaction/SignedTransaction'; -import { TransactionType } from '../model/transaction/TransactionType'; - -/** - * Aggregated Transaction service - */ -export class AggregatedTransactionService { - - /** - * Constructor - * @param accountHttp - */ - constructor(private readonly accountHttp: AccountHttp) { - } - - /** - * Check if an aggregate complete transaction has all cosignatories attached - * @param signedTransaction - The signed aggregate transaction (complete) to be verified - * @returns {Observable} - */ - public isComplete(signedTransaction: SignedTransaction): Observable { - const aggregateTransaction = TransactionMapping.createFromPayload(signedTransaction.payload) as AggregateTransaction; - /** - * Include both initiator & cosigners - */ - const signers = (aggregateTransaction.cosignatures.map((cosigner) => cosigner.signer.publicKey)); - if (signedTransaction.signer) { - signers.push(signedTransaction.signer); - } - return observableFrom(aggregateTransaction.innerTransactions).pipe( - mergeMap((innerTransaction) => this.accountHttp.getMultisigAccountInfo(innerTransaction.signer.address) - .pipe( - /** - * For multisig account, we need to get the graph info in case it has multiple levels - */ - mergeMap((_) => _.minApproval !== 0 && _.minRemoval !== 0 ? - this.accountHttp.getMultisigAccountGraphInfo(_.account.address) - .pipe( - map((graphInfo) => this.validateCosignatories(graphInfo, signers, innerTransaction)), - ) : observableOf(signers.find((s) => s === _.account.publicKey ) !== undefined), - ), - ), - ), - toArray(), - ).pipe( - flatMap((results) => { - return observableOf(results.every((isComplete) => isComplete)); - }), - ); - } - - /** - * Validate cosignatories against multisig Account(s) - * @param graphInfo - multisig account graph info - * @param cosignatories - array of cosignatories extracted from aggregated transaction - * @param innerTransaction - the inner transaction of the aggregated transaction - * @returns {boolean} - */ - private validateCosignatories(graphInfo: MultisigAccountGraphInfo, - cosignatories: string[], - innerTransaction: InnerTransaction): boolean { - /** - * Validate cosignatories from bottom level to top - */ - const sortedKeys = Array.from(graphInfo.multisigAccounts.keys()).sort((a, b) => b - a); - const cosignatoriesReceived = cosignatories; - let validationResult = true; - - let isMultisigRemoval = false; - - /** - * Check inner transaction. If remove cosigner from multisig account, - * use minRemoval instead of minApproval for cosignatories validation. - */ - if (innerTransaction.type === TransactionType.MODIFY_MULTISIG_ACCOUNT) { - if ((innerTransaction as ModifyMultisigAccountTransaction).modifications - .find((modification) => modification.type === MultisigCosignatoryModificationType.Remove) !== undefined) { - isMultisigRemoval = true; - } - } - - sortedKeys.forEach((key) => { - const multisigInfo = graphInfo.multisigAccounts.get(key); - if (multisigInfo && validationResult) { - multisigInfo.forEach((multisig) => { - if (multisig.minApproval >= 1 && multisig.minRemoval) { // To make sure it is multisig account - const matchedCosignatories = this.compareArrays(cosignatoriesReceived, - multisig.cosignatories.map((cosig) => cosig.publicKey)); - - /** - * if minimal signature requirement met at current level, push the multisig account - * into the received signatories array for next level validation. - * Otherwise return validation failed. - */ - if ((matchedCosignatories.length >= multisig.minApproval && !isMultisigRemoval) || - (matchedCosignatories.length >= multisig.minRemoval && isMultisigRemoval)) { - if (cosignatoriesReceived.indexOf(multisig.account.publicKey) === -1) { - cosignatoriesReceived.push(multisig.account.publicKey); - } - } else { - validationResult = false; - } - } - }); - } - }); - - return validationResult; - } - - /** - * Compare two string arrays - * @param array1 - base array - * @param array2 - array to be matched - * @returns {string[]} - array of matched elements - */ - private compareArrays(array1: string[], array2: string[]): string[] { - const results: string[] = []; - array1.forEach((a1) => array2.forEach((a2) => { - if (a1 === a2) { - results.push(a1); - } - })); - - return results; - } -} diff --git a/test/service/AggregatedTransactionService.spec.ts b/test/service/AggregatedTransactionService.spec.ts deleted file mode 100644 index 23470cb2a1..0000000000 --- a/test/service/AggregatedTransactionService.spec.ts +++ /dev/null @@ -1,562 +0,0 @@ -/* - * Copyright 2019 NEM - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { expect } from 'chai'; -import { ChronoUnit } from 'js-joda'; -import {of as observableOf} from 'rxjs'; -import {deepEqual, instance, mock, when} from 'ts-mockito'; -import { AccountHttp } from '../../src/infrastructure/AccountHttp'; -import { Account } from '../../src/model/account/Account'; -import { Address } from '../../src/model/account/Address'; -import { MultisigAccountGraphInfo } from '../../src/model/account/MultisigAccountGraphInfo'; -import { MultisigAccountInfo } from '../../src/model/account/MultisigAccountInfo'; -import {NetworkType} from '../../src/model/blockchain/NetworkType'; -import { AggregateTransaction } from '../../src/model/transaction/AggregateTransaction'; -import { Deadline } from '../../src/model/transaction/Deadline'; -import { ModifyMultisigAccountTransaction } from '../../src/model/transaction/ModifyMultisigAccountTransaction'; -import { MultisigCosignatoryModification } from '../../src/model/transaction/MultisigCosignatoryModification'; -import { MultisigCosignatoryModificationType } from '../../src/model/transaction/MultisigCosignatoryModificationType'; -import { PlainMessage } from '../../src/model/transaction/PlainMessage'; -import { TransferTransaction } from '../../src/model/transaction/TransferTransaction'; -import { AggregatedTransactionService } from '../../src/service/AggregatedTransactionService'; - -/** - * For multi level multisig scenario visit: https://github.com/nemtech/nem2-docs/issues/10 - */ -describe('AggregatedTransactionService', () => { - let aggregatedTransactionService: AggregatedTransactionService; - - /** - * Multisig2 Account: SBROWP-7YMG2M-K45RO6-Q7ZPK7-G7GXWQ-JK5VNQ-OSUX - * Public Key: 5E628EA59818D97AA4118780D9A88C5512FCE7A21C195E1574727EFCE5DF7C0D - * Private Key: 22A1D67F8519D1A45BD7116600BB6E857786E816FE0B45E4C5B9FFF3D64BC177 - * - * - * Multisig1 Account: SAK32M-5JQ43R-WYHWEH-WRBCW4-RXERT2-DLASGL-EANS - * Public Key: BFDF2610C5666A626434FE12FB4A9D896D2B9B033F5F84CCEABE82E043A6307E - * Private Key: 8B0622C2CCFC5CCC5A74B500163E3C68F3AD3643DB12932FC931143EAC67280D - */ - - /** - * Test accounts: - * Multisig1 (1/1): Account2, Account3 - * Multisig2 (2/1): Account1, Multisig1 - * Multisig3 (2/2): Account2, Account3 - * Stranger Account: Account4 - */ - - const account1 = Account.createFromPrivateKey('82DB2528834C9926F0FCCE042466B24A266F5B685CB66D2869AF6648C043E950', - NetworkType.MIJIN_TEST); - const multisig1 = Account.createFromPrivateKey('8B0622C2CCFC5CCC5A74B500163E3C68F3AD3643DB12932FC931143EAC67280D', - NetworkType.MIJIN_TEST); - const multisig2 = Account.createFromPrivateKey('22A1D67F8519D1A45BD7116600BB6E857786E816FE0B45E4C5B9FFF3D64BC177', - NetworkType.MIJIN_TEST); - - const multisig3 = Account.createFromPrivateKey('5E7812AB0E709ABC45466034E1A209099F6A12C4698748A63CDCAA9B0DDE1DBD', - NetworkType.MIJIN_TEST); - const account2 = Account.createFromPrivateKey('A4D410270E01CECDCDEADCDE32EC79C8D9CDEA4DCD426CB1EB666EFEF148FBCE', - NetworkType.MIJIN_TEST); - const account3 = Account.createFromPrivateKey('336AB45EE65A6AFFC0E7ADC5342F91E34BACA0B901A1D9C876FA25A1E590077E', - NetworkType.MIJIN_TEST); - - const account4 = Account.createFromPrivateKey('4D8B3756592532753344E11E2B7541317BCCFBBCF4444274CDBF359D2C4AE0F1', - NetworkType.MIJIN_TEST); - before(() => { - const mockedAccountHttp = mock(AccountHttp); - - when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(account1.address))) - .thenReturn(observableOf(givenAccount1Info())); - when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(account4.address))) - .thenReturn(observableOf(givenAccount4Info())); - when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(multisig2.address))) - .thenReturn(observableOf(givenMultisig2AccountInfo())); - when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(multisig3.address))) - .thenReturn(observableOf(givenMultisig3AccountInfo())); - when(mockedAccountHttp.getMultisigAccountGraphInfo(deepEqual(multisig2.address))) - .thenReturn(observableOf(givenMultisig2AccountGraphInfo())); - when(mockedAccountHttp.getMultisigAccountGraphInfo(deepEqual(multisig3.address))) - .thenReturn(observableOf(givenMultisig3AccountGraphInfo())); - when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(account2.address))) - .thenReturn(observableOf(givenAccount2Info())); - when(mockedAccountHttp.getMultisigAccountInfo(deepEqual(account3.address))) - .thenReturn(observableOf(givenAccount3Info())); - - const accountHttp = instance(mockedAccountHttp); - aggregatedTransactionService = new AggregatedTransactionService(accountHttp); - }); - - it('should return isComplete: true for aggregated complete transaction - 2 levels Multisig', () => { - /** - * MLMA - * Alice (account1): normal account - * Bob (multisig2) - Multisig 2-1 (account1 && multisig1) - * Charles (multisig1) - Multisig 1-1 (account2 || account3) - * Given signatories: Account1 && Account4 - * Expecting complete as Bob needs 2 signatures (account1 && (account2 || account3)) - */ - const transferTransaction = TransferTransaction.create( - Deadline.create(1, ChronoUnit.HOURS), - Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), - [], - PlainMessage.create('test-message'), - NetworkType.MIJIN_TEST, - ); - - const aggregateTransaction = AggregateTransaction.createComplete( - Deadline.create(), - [transferTransaction.toAggregate(multisig2.publicAccount)], - NetworkType.MIJIN_TEST, - []); - - const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account2]); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { - expect(isComplete).to.be.true; - }); - }); - - it('should return isComplete: false for aggregated complete transaction - 2 levels Multisig', () => { - /** - * MLMA - * Alice (account1): normal account - * Bob (multisig2) - Multisig 2-1 (account1 && multisig1) - * Charles (multisig1) - Multisig 1-1 (account2 || account3) - * Given signatories: Account1 && Account4 - * Expecting incomplete as Bob needs 2 signatures (account1 && (account2 || account3)) but only got account1 - */ - const transferTransaction = TransferTransaction.create( - Deadline.create(1, ChronoUnit.HOURS), - Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), - [], - PlainMessage.create('test-message'), - NetworkType.MIJIN_TEST, - ); - - const aggregateTransaction = AggregateTransaction.createComplete( - Deadline.create(), - [transferTransaction.toAggregate(multisig2.publicAccount)], - NetworkType.MIJIN_TEST, - []); - - const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, []); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { - expect(isComplete).to.be.false; - }); - }); - - it('should return isComplete: false for aggregated complete transaction - 2 levels Multisig', () => { - /** - * MLMA - * Alice (account1): normal account - * Bob (multisig2) - Multisig 2-1 (account1 && multisig1) - * Charles (multisig1) - Multisig 1-1 (account2 || account3) - * Given signatories: Account1 && Account4 - * Expecting incomplete as Bob needs 2 signatures (account1 && (account2 || account3)) but got account4 - */ - const transferTransaction = TransferTransaction.create( - Deadline.create(1, ChronoUnit.HOURS), - Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), - [], - PlainMessage.create('test-message'), - NetworkType.MIJIN_TEST, - ); - - const aggregateTransaction = AggregateTransaction.createComplete( - Deadline.create(), - [transferTransaction.toAggregate(multisig2.publicAccount)], - NetworkType.MIJIN_TEST, - []); - - const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account4]); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { - expect(isComplete).to.be.false; - }); - }); - - it('should return correct isComplete status for aggregated complete transaction - 2 levels Multisig, multi inner transaction', () => { - /** - * MLMA - with multiple transaction - * Alice (account1): normal account - * Bob (multisig2) - Multisig 2-1 (account1 && multisig1) - * Charles (multisig1) - Multisig 1-1 (account2 || account3) - * An extra inner transaction to account4 (just to increase the complexity) - * Given signatories: Account1 && Account4 - * Expecting incomplete as Bob needs 2 signatures (account1 && (account2 || account3)) - */ - const transferTransaction = TransferTransaction.create( - Deadline.create(1, ChronoUnit.HOURS), - account2.address, - [], - PlainMessage.create('test-message'), - NetworkType.MIJIN_TEST, - ); - - const transferTransaction2 = TransferTransaction.create( - Deadline.create(1, ChronoUnit.HOURS), - account2.address, - [], - PlainMessage.create('test-message'), - NetworkType.MIJIN_TEST, - ); - - const aggregateTransaction = AggregateTransaction.createComplete( - Deadline.create(), - [transferTransaction.toAggregate(multisig2.publicAccount), - transferTransaction2.toAggregate(account4.publicAccount)], - NetworkType.MIJIN_TEST, - []); - const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account4]); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { - expect(isComplete).to.be.false; - }); - }); - - it('should return correct isComplete status for aggregated complete transaction - 2 levels Multisig, multi inner transaction', () => { - /** - * MLMA - with multiple transaction - * Alice (account1): normal account - * Bob (multisig2) - Multisig 2-1 (account1 && multisig1) - * Charles (multisig1) - Multisig 1-1 (account2 || account3) - * An extra inner transaction to account4 (just to increase the complexity) - * Given signatories: Account1 && Account4 && Account2 - * Expecting complete - */ - const transferTransaction = TransferTransaction.create( - Deadline.create(1, ChronoUnit.HOURS), - account2.address, - [], - PlainMessage.create('test-message'), - NetworkType.MIJIN_TEST, - ); - - const transferTransaction2 = TransferTransaction.create( - Deadline.create(1, ChronoUnit.HOURS), - account2.address, - [], - PlainMessage.create('test-message'), - NetworkType.MIJIN_TEST, - ); - - const aggregateTransaction = AggregateTransaction.createComplete( - Deadline.create(), - [transferTransaction.toAggregate(multisig2.publicAccount), - transferTransaction2.toAggregate(account4.publicAccount)], - NetworkType.MIJIN_TEST, - []); - const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account4, account2]); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { - expect(isComplete).to.be.true; - }); - }); - - it('should use minRemoval for multisig account validation if inner transaction is modify multisig remove', () => { - /** - * If the inner transaction is issued to a multisig account - * and the inner transaction itself is a ModifyMultiSigAccountTransaction - Removal - * The validator should use minRemoval value rather than minApproval value - * to determine if the act is complete or not - */ - const modifyMultisigTransaction = ModifyMultisigAccountTransaction.create( - Deadline.create(1, ChronoUnit.HOURS), - 1, - 1, - [new MultisigCosignatoryModification( - MultisigCosignatoryModificationType.Remove, - account1.publicAccount, - )], - NetworkType.MIJIN_TEST, - ); - - const aggregateTransaction = AggregateTransaction.createComplete( - Deadline.create(), - [modifyMultisigTransaction.toAggregate(multisig2.publicAccount)], - NetworkType.MIJIN_TEST, - []); - const signedTransaction = aggregateTransaction.signWith(account2); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { - expect(isComplete).to.be.true; - }); - }); - - it('should return correct isComplete status (false) for aggregated complete transaction - none multisig', () => { - /** - * If the inner transaction is issued to a multisig account - * and the inner transaction itself is a ModifyMultiSigAccountTransaction - Removal - * The validator should use minRemoval value rather than minApproval value - * to determine if the act is complete or not - */ - const transferTransaction = TransferTransaction.create( - Deadline.create(1, ChronoUnit.HOURS), - Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), - [], - PlainMessage.create('test-message'), - NetworkType.MIJIN_TEST, - ); - - const aggregateTransaction = AggregateTransaction.createComplete( - Deadline.create(), - [transferTransaction.toAggregate(account4.publicAccount)], - NetworkType.MIJIN_TEST, - []); - - const signedTransaction = aggregateTransaction.signWith(account1); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { - expect(isComplete).to.be.false; - }); - }); - - it('should return correct isComplete status (true) for aggregated complete transaction - none multisig', () => { - /** - * ACT - * Alice (account1): normal account - * Bob (account4) - normal account - * Alice initiate the transaction - * Bob sign - */ - const transferTransaction = TransferTransaction.create( - Deadline.create(1, ChronoUnit.HOURS), - Address.createFromRawAddress('SBILTA367K2LX2FEXG5TFWAS7GEFYAGY7QLFBYKC'), - [], - PlainMessage.create('test-message'), - NetworkType.MIJIN_TEST, - ); - - const aggregateTransaction = AggregateTransaction.createComplete( - Deadline.create(), - [transferTransaction.toAggregate(account4.publicAccount)], - NetworkType.MIJIN_TEST, - []); - - const signedTransaction = aggregateTransaction.signWith(account4); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { - expect(isComplete).to.be.true; - }); - }); - - it('should return correct isComplete status TRUE - multiple normal account', () => { - /** - * ACT - * Alice: account1 - * Bog: account4 - * An escrow contract is signed by all the participants (normal accounts) - * Given Alice defined the following escrow contract: - * | sender | recipient | type | data | - * | Alice | Bob | send-an-asset | 1 concert.ticket | - * | Bob | Alice | send-an-asset | 20 euros | - * And Bob signs the contract - * And Alice signs the contract - * Then the contract should appear as complete - */ - const transferTransaction = TransferTransaction.create( - Deadline.create(1, ChronoUnit.HOURS), - account1.address, - [], - PlainMessage.create('test-message'), - NetworkType.MIJIN_TEST, - ); - - const transferTransaction2 = TransferTransaction.create( - Deadline.create(1, ChronoUnit.HOURS), - account4.address, - [], - PlainMessage.create('test-message'), - NetworkType.MIJIN_TEST, - ); - - const aggregateTransaction = AggregateTransaction.createComplete( - Deadline.create(), - [transferTransaction.toAggregate(account4.publicAccount), - transferTransaction2.toAggregate(account1.publicAccount)], - NetworkType.MIJIN_TEST, - []); - - const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, [account4]); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { - expect(isComplete).to.be.true; - }); - }); - - it('should return correct isComplete status FALSE - multiple normal account', () => { - /** - * ACT - * Alice: account1 - * Bog: account4 - * An escrow contract is signed by all the participants (normal accounts) - * Given Alice defined the following escrow contract: - * | sender | recipient | type | data | - * | Alice | Bob | send-an-asset | 1 concert.ticket | - * | Bob | Alice | send-an-asset | 20 euros | - * And Alice signs the contract - * Then the contract should appear as incomplete - */ - const transferTransaction = TransferTransaction.create( - Deadline.create(1, ChronoUnit.HOURS), - account1.address, - [], - PlainMessage.create('test-message'), - NetworkType.MIJIN_TEST, - ); - - const transferTransaction2 = TransferTransaction.create( - Deadline.create(1, ChronoUnit.HOURS), - account4.address, - [], - PlainMessage.create('test-message'), - NetworkType.MIJIN_TEST, - ); - - const aggregateTransaction = AggregateTransaction.createComplete( - Deadline.create(), - [transferTransaction.toAggregate(account4.publicAccount), - transferTransaction2.toAggregate(account1.publicAccount)], - NetworkType.MIJIN_TEST, - []); - - const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account1, []); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { - expect(isComplete).to.be.false; - }); - }); - - it('should return correct isComplete status TRUE - multisig Single Level', () => { - /** - * ACT - Multisig single level - * Alice (account1): initiate an transfer to Bob - * Bob (multisig3): is a 2/2 multisig account (account2 && account3) - */ - const transferTransaction = TransferTransaction.create( - Deadline.create(1, ChronoUnit.HOURS), - account4.address, - [], - PlainMessage.create('test-message'), - NetworkType.MIJIN_TEST, - ); - - const aggregateTransaction = AggregateTransaction.createComplete( - Deadline.create(), - [transferTransaction.toAggregate(multisig3.publicAccount)], - NetworkType.MIJIN_TEST, - []); - - const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account2, [account3]); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { - expect(isComplete).to.be.true; - }); - }); - - it('should return correct isComplete status FALSE - multisig Single Level', () => { - /** - * ACT - Multisig single level - * Alice (account1): initiate an transfer to Bob - * Bob (multisig3): is a 2/2 multisig account (account2 && account3) - */ - const transferTransaction = TransferTransaction.create( - Deadline.create(1, ChronoUnit.HOURS), - account4.address, - [], - PlainMessage.create('test-message'), - NetworkType.MIJIN_TEST, - ); - - const aggregateTransaction = AggregateTransaction.createComplete( - Deadline.create(), - [transferTransaction.toAggregate(multisig3.publicAccount)], - NetworkType.MIJIN_TEST, - []); - - const signedTransaction = aggregateTransaction.signTransactionWithCosignatories(account2, []); - aggregatedTransactionService.isComplete(signedTransaction).toPromise().then((isComplete) => { - expect(isComplete).to.be.false; - }); - }); - - function givenMultisig2AccountInfo(): MultisigAccountInfo { - return new MultisigAccountInfo(multisig2.publicAccount, - 2, 1, - [multisig1.publicAccount, - account1.publicAccount], - [], - ); - } - function givenMultisig3AccountInfo(): MultisigAccountInfo { - return new MultisigAccountInfo(multisig3.publicAccount, - 2, 2, - [account2.publicAccount, - account3.publicAccount], - [], - ); - } - - function givenAccount1Info(): MultisigAccountInfo { - return new MultisigAccountInfo(account1.publicAccount, - 0, 0, - [], - [multisig2.publicAccount], - ); - } - function givenAccount2Info(): MultisigAccountInfo { - return new MultisigAccountInfo(account2.publicAccount, - 0, 0, - [], - [multisig2.publicAccount, - multisig3.publicAccount], - ); - } - function givenAccount3Info(): MultisigAccountInfo { - return new MultisigAccountInfo(account3.publicAccount, - 0, 0, - [], - [multisig2.publicAccount, - multisig3.publicAccount], - ); - } - function givenAccount4Info(): MultisigAccountInfo { - return new MultisigAccountInfo(account4.publicAccount, - 0, 0, - [], - [], - ); - } - - function givenMultisig2AccountGraphInfo(): MultisigAccountGraphInfo { - const map = new Map(); - map.set(0, [new MultisigAccountInfo(multisig2.publicAccount, - 2, 1, - [multisig1.publicAccount, - account1.publicAccount], - [], - )]) - .set(1, [new MultisigAccountInfo(multisig1.publicAccount, - 1, 1, - [account2.publicAccount, account3.publicAccount], - [multisig2.publicAccount], - )]); - - return new MultisigAccountGraphInfo(map); - } - - function givenMultisig3AccountGraphInfo(): MultisigAccountGraphInfo { - const map = new Map(); - map.set(0, [new MultisigAccountInfo(multisig3.publicAccount, - 2, 2, - [account2.publicAccount, - account3.publicAccount], - [], - )]); - - return new MultisigAccountGraphInfo(map); - } - -}); From 85439ba9142722814a04c7052bef93eaaf1a27d6 Mon Sep 17 00:00:00 2001 From: Steven Liu Date: Wed, 10 Apr 2019 15:35:36 +0100 Subject: [PATCH 31/31] Fixed issue on createTransactionFromJson Changed default validation result value to false; --- src/model/transaction/Transaction.ts | 2 +- src/service/AggregateTransactionService.ts | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/model/transaction/Transaction.ts b/src/model/transaction/Transaction.ts index e4cb7f2cfc..ed197082ff 100644 --- a/src/model/transaction/Transaction.ts +++ b/src/model/transaction/Transaction.ts @@ -211,7 +211,7 @@ export abstract class Transaction { const commonTransactionObject = { type: this.type, networkType: this.networkType, - version: this.version, + version: this.versionToDTO(), maxFee: this.maxFee.toDTO(), deadline: this.deadline.toDTO(), signature: this.signature ? this.signature : '', diff --git a/src/service/AggregateTransactionService.ts b/src/service/AggregateTransactionService.ts index 800d1a1b89..cd7bb44381 100644 --- a/src/service/AggregateTransactionService.ts +++ b/src/service/AggregateTransactionService.ts @@ -89,7 +89,7 @@ export class AggregateTransactionService { */ const sortedKeys = Array.from(graphInfo.multisigAccounts.keys()).sort((a, b) => b - a); const cosignatoriesReceived = cosignatories; - let validationResult = true; + let validationResult = false; let isMultisigRemoval = false; @@ -106,7 +106,7 @@ export class AggregateTransactionService { sortedKeys.forEach((key) => { const multisigInfo = graphInfo.multisigAccounts.get(key); - if (multisigInfo && validationResult) { + if (multisigInfo && !validationResult) { multisigInfo.forEach((multisig) => { if (multisig.minApproval >= 1 && multisig.minRemoval) { // To make sure it is multisig account const matchedCosignatories = this.compareArrays(cosignatoriesReceived, @@ -122,6 +122,7 @@ export class AggregateTransactionService { if (cosignatoriesReceived.indexOf(multisig.account.publicKey) === -1) { cosignatoriesReceived.push(multisig.account.publicKey); } + validationResult = true; } else { validationResult = false; }