fix(core-transaction-pool): disallow multiple registrations for same delegate

packages/core-transaction-pool/__tests__/guard.test.ts

@@ -394,6 +394,33 @@ describe("Transaction Guard", () => {
             expect(result.errors).toBeNull();
         });
 
+        it("should not validate when multiple wallets register the same username in the same transaction payload", async () => {
+            const delegateRegistrations = [
+                generateDelegateRegistration("unitnet", wallets[14].passphrase, 1, false, "test_delegate")[0],
+                generateDelegateRegistration("unitnet", wallets[15].passphrase, 1, false, "test_delegate")[0],
+            ];
+
+            const result = await guard.validate(delegateRegistrations);
+            expect(result.invalid).toEqual(delegateRegistrations.map(transaction => transaction.id));
+
+            delegateRegistrations.forEach(tx => {
+                expect(guard.errors[tx.id]).toEqual([
+                    {
+                        type: "ERR_CONFLICT",
+                        message: `Multiple delegate registrations for "${
+                            tx.asset.delegate.username
+                        }" in transaction payload`,
+                    },
+                ]);
+            });
+
+            const wallet1 = transactionPool.walletManager.findByPublicKey(wallets[14].keys.publicKey);
+            const wallet2 = transactionPool.walletManager.findByPublicKey(wallets[15].keys.publicKey);
+
+            expect(wallet1.username).toBe(null);
+            expect(wallet2.username).toBe(null);
+        });
+
         describe("Sign a transaction then change some fields shouldn't pass validation", () => {
             const secondSignatureError = (id, address) => [
                 id,
@@ -891,6 +918,33 @@ describe("Transaction Guard", () => {
             });
         });
 
+        it("should not validate a delegate registration if an existing registration for the same username from a different wallet exists in the pool", async () => {
+            const delegateRegistrations = [
+                generateDelegateRegistration("unitnet", wallets[16].passphrase, 1, false, "test_delegate")[0],
+                generateDelegateRegistration("unitnet", wallets[17].passphrase, 1, false, "test_delegate")[0],
+            ];
+
+            expect(guard.__validateTransaction(delegateRegistrations[0])).toBeTrue();
+            guard.accept.set(delegateRegistrations[0].id, delegateRegistrations[0]);
+            guard.__addTransactionsToPool();
+            expect(guard.errors).toEqual({});
+            expect(guard.__validateTransaction(delegateRegistrations[1])).toBeFalse();
+            expect(guard.errors[delegateRegistrations[1].id]).toEqual([
+                {
+                    type: "ERR_PENDING",
+                    message: `Delegate registration for "${
+                        delegateRegistrations[1].asset.delegate.username
+                    }" already in the pool`,
+                },
+            ]);
+
+            const wallet1 = transactionPool.walletManager.findByPublicKey(wallets[16].keys.publicKey);
+            const wallet2 = transactionPool.walletManager.findByPublicKey(wallets[17].keys.publicKey);
+
+            expect(wallet1.username).toBe("test_delegate");
+            expect(wallet2.username).toBe(null);
+        });
+
         it("should not validate when sender has same type transactions in the pool (only for 2nd sig, delegate registration, vote)", async () => {
             jest.spyOn(guard.pool.walletManager, "canApply").mockImplementation(() => true);
             const votes = [

packages/core-transaction-pool/src/connection.ts

@@ -77,6 +77,17 @@ export class TransactionPool implements transactionPool.ITransactionPool {
         this.storage.close();
     }
 
+    /**
+     * Get all transactions of a given type from the pool.
+     * @param {Number} type of transaction
+     * @return {Set of MemPoolTransaction} all transactions of the given type, could be empty Set
+     */
+    public getTransactionsByType(type) {
+        this.__purgeExpired();
+
+        return this.mem.getByType(type);
+    }
+
     /**
      * Get the number of transactions in the pool.
      * @return {Number}

packages/core-transaction-pool/src/guard.ts

@@ -4,6 +4,7 @@ import { configManager, constants, models, slots } from "@arkecosystem/crypto";
 import pluralize from "pluralize";
 import { TransactionPool } from "./connection";
 import { dynamicFeeMatcher } from "./dynamic-fee";
+import { MemPoolTransaction } from "./mem-pool-transaction";
 import { isRecipientOnActiveNetwork } from "./utils/is-on-active-network";
 
 const { TransactionTypes } = constants;
@@ -174,6 +175,36 @@ export class TransactionGuard implements transactionPool.ITransactionGuard {
         }
 
         const errors = [];
+
+        // This check must come before canApply otherwise a wallet may be incorrectly assigned a username when multiple
+        // conflicting delegate registrations for the same username exist in the same transaction payload
+        if (transaction.type === TransactionTypes.DelegateRegistration) {
+            const username = transaction.asset.delegate.username;
+            const delegateRegistrationsInPayload = this.transactions.filter(
+                tx => tx.type === TransactionTypes.DelegateRegistration && tx.asset.delegate.username === username,
+            );
+            if (delegateRegistrationsInPayload.length > 1) {
+                this.__pushError(
+                    transaction,
+                    "ERR_CONFLICT",
+                    `Multiple delegate registrations for "${username}" in transaction payload`,
+                );
+                return false;
+            }
+
+            const delegateRegistrationsInPool: MemPoolTransaction[] = Array.from(
+                this.pool.getTransactionsByType(TransactionTypes.DelegateRegistration),
+            );
+            if (delegateRegistrationsInPool.some(memTx => memTx.transaction.asset.delegate.username === username)) {
+                this.__pushError(
+                    transaction,
+                    "ERR_PENDING",
+                    `Delegate registration for "${username}" already in the pool`,
+                );
+                return false;
+            }
+        }
+
         if (!this.pool.walletManager.canApply(transaction, errors)) {
             this.__pushError(transaction, "ERR_APPLY", JSON.stringify(errors));
             return false;

packages/core-transaction-pool/src/mem.ts

@@ -8,6 +8,7 @@ export class Mem {
     public allIsSorted: boolean;
     public byId: { [key: string]: MemPoolTransaction };
     public bySender: { [key: string]: Set<MemPoolTransaction> };
+    public byType: { [key: number]: Set<MemPoolTransaction> };
     public byExpiration: MemPoolTransaction[];
     public byExpirationIsSorted: boolean;
     public dirty: { added: Set<string>; removed: Set<string> };
@@ -58,6 +59,13 @@ export class Mem {
          */
         this.bySender = {};
 
+        /**
+         * A map of (key=transaction type, value=Set of MemPoolTransaction).
+         * Used to:
+         * - get all transactions of a given type
+         */
+        this.byType = {};
+
         /**
          * An array of MemPoolTransaction, sorted by expiration (earliest date
          * comes first). This array may not contain all transactions that are
@@ -111,6 +119,8 @@ export class Mem {
         this.byId[transaction.id] = memPoolTransaction;
 
         const sender = transaction.senderPublicKey;
+        const type = transaction.type;
+
         if (this.bySender[sender] === undefined) {
             // First transaction from this sender, create a new Set.
             this.bySender[sender] = new Set([memPoolTransaction]);
@@ -119,6 +129,14 @@ export class Mem {
             this.bySender[sender].add(memPoolTransaction);
         }
 
+        if (this.byType[type] === undefined) {
+            // First transaction of this type, create a new Set.
+            this.byType[type] = new Set([memPoolTransaction]);
+        } else {
+            // Append to existing transaction ids for this type.
+            this.byType[type].add(memPoolTransaction);
+        }
+
         if (memPoolTransaction.expireAt(maxTransactionAge) !== null) {
             this.byExpiration.push(memPoolTransaction);
             this.byExpirationIsSorted = false;
@@ -152,6 +170,7 @@ export class Mem {
         }
 
         const memPoolTransaction = this.byId[id];
+        const type = this.byId[id].transaction.type;
 
         // XXX worst case: O(n)
         let i = this.byExpiration.findIndex(e => e.transaction.id === id);
@@ -164,6 +183,11 @@ export class Mem {
             delete this.bySender[senderPublicKey];
         }
 
+        this.byType[type].delete(memPoolTransaction);
+        if (this.byType[type].size === 0) {
+            delete this.byType[type];
+        }
+
         delete this.byId[id];
 
         i = this.all.findIndex(e => e.transaction.id === id);
@@ -202,6 +226,19 @@ export class Mem {
         return new Set();
     }
 
+    /**
+     * Get all transactions of a given type.
+     * @param {Number} type of transaction
+     * @return {Set of MemPoolTransaction} all transactions of the given type, could be empty Set
+     */
+    public getByType(type) {
+        const memPoolTransactions = this.byType[type];
+        if (memPoolTransactions !== undefined) {
+            return memPoolTransactions;
+        }
+        return new Set();
+    }
+
     /**
      * Get a transaction, given its id.
      * @param {String} id transaction id