Merge pull request #652 from thehubbleproject/bug/token-pool-zero-fee

Fix getHighestValueToken to work with zero fee txs

test/fast/txPool.test.ts

@@ -13,7 +13,8 @@ import {
     PoolEmptyError,
     PoolFullError,
     TokenNotConfiguredError,
-    TokenPoolEmpty
+    TokenPoolEmpty,
+    TokenPoolHighestFeeError
 } from "../../ts/exceptions";
 import { State } from "../../ts/state";
 import * as mcl from "../../ts/mcl";
@@ -130,6 +131,36 @@ describe("MultiTokenPool", () => {
                 PoolEmptyError
             );
         });
+
+        it("fails if unable to determine highest value token", async function() {
+            // This simulates a path which this func will fail
+            // @ts-ignore
+            pool.txCount = 1;
+
+            await assert.isRejected(
+                pool.getHighestValueToken(),
+                TokenPoolHighestFeeError
+            );
+        });
+
+        it("suceeds when summed tx fees are 0", async function() {
+            const user1Token1 = 0;
+            const s = State.new(-1, tokenID1BN, -1, -1);
+            await state.create(user1Token1, s);
+            await state.commit();
+
+            const token1Tx1 = txFactory(user1Token1, 0);
+            await pool.push(token1Tx1);
+
+            const highToken1 = highTokenToNum(
+                await pool.getHighestValueToken()
+            );
+            assert.deepEqual(highToken1, {
+                tokenID: tokenID1,
+                feeReceiverID: feeReceiver1,
+                sumFees: 0
+            });
+        });
     });
 
     describe("lifecycle", () => {

ts/client/txPool.ts

@@ -3,7 +3,8 @@ import {
     PoolEmptyError,
     PoolFullError,
     TokenNotConfiguredError,
-    TokenPoolEmpty
+    TokenPoolEmpty,
+    TokenPoolHighestFeeError
 } from "../exceptions";
 import { sum } from "../utils";
 import { FeeReceivers } from "./config";
@@ -133,7 +134,7 @@ export class MultiTokenPool<Item extends OffchainTx> {
             throw new PoolEmptyError();
         }
 
-        let highValue = BigNumber.from(0);
+        let highValue = BigNumber.from(-1);
         let tokenID = BigNumber.from(-1);
         for (const tokenIDStr of Object.keys(this.tokenIDStrToQueue)) {
             const value = this.getQueueValue(tokenIDStr);
@@ -143,6 +144,11 @@ export class MultiTokenPool<Item extends OffchainTx> {
             }
         }
 
+        // This case should never be hit, but best to be explicit if it does
+        if (highValue.lt(0) || tokenID.lt(0)) {
+            throw new TokenPoolHighestFeeError();
+        }
+
         const feeReceiverID = this.tokenIDStrToFeeRecieverID[
             tokenID.toString()
         ];
@@ -154,6 +160,11 @@ export class MultiTokenPool<Item extends OffchainTx> {
     }
 
     private getQueueValue(tokenIDStr: string): BigNumber {
-        return sum(this.tokenIDStrToQueue[tokenIDStr].map(tx => tx.fee));
+        const tokenQueue = this.tokenIDStrToQueue[tokenIDStr];
+        if (!tokenQueue.length) {
+            return BigNumber.from(-1);
+        }
+
+        return sum(tokenQueue.map(tx => tx.fee));
     }
 }

ts/exceptions.ts

@@ -255,3 +255,10 @@ export class TokenNotConfiguredError extends Error {
         this.name = "TokenNotConfiguredError";
     }
 }
+
+export class TokenPoolHighestFeeError extends Error {
+    constructor() {
+        super("unable to determine highest fee token for pool");
+        this.name = "TokenPoolHighestFeeError";
+    }
+}