fix support for zero fees

alexbosworth · Jul 14, 2020 · 33a92f2 · 33a92f2
1 parent b2f4829
commit 33a92f2
Show file tree

Hide file tree

Showing 6 changed files with 128 additions and 80 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,9 @@
 # Versions
 
+## 49.3.2
+
+- `updateRoutingFees`: Allow specifying zero `base_fee_tokens`, `fee_rate`
+
 ## 49.3.1
 
 - `getFeeRates`: Add support for `base_fee_mtokens` to show precise base fees

diff --git a/lightning/update_routing_fees.js b/lightning/update_routing_fees.js
@@ -1,12 +1,16 @@
 const asyncAuto = require('async/auto');
 const {returnResult} = require('asyncjs-util');
 
+const {isLnd} = require('./../grpc');
+
 const defaultBaseFee = 1;
 const defaultCltvDelta = 40;
-const defaultFeeRate = 1;
+const defaultRate = 1;
 const feeRatio = 1e6;
 const {floor} = Math;
+const method = 'updateChannelPolicy';
 const tokensAsMtokens = tokens => (BigInt(tokens) * BigInt(1e3)).toString();
+const type = 'default';
 
 /** Update routing fees on a single channel or on all channels
 
@@ -40,7 +44,7 @@ module.exports = (args, cbk) => {
           return cbk([400, 'ExpectedEitherBaseFeeMtokensOrTokensNotBoth']);
         }
 
-        if (!args.lnd || !args.lnd.default) {
+        if (!isLnd({method, type, lnd: args.lnd})) {
           return cbk([400, 'ExpectedLndForRoutingFeesUpdate']);
         }
 
@@ -63,14 +67,17 @@ module.exports = (args, cbk) => {
           return cbk(null, args.base_fee_mtokens);
         }
 
-        const baseFeeTokens = args.base_fee_tokens || defaultBaseFee;
+        if (args.base_fee_tokens === undefined) {
+          return cbk(null, tokensAsMtokens(defaultBaseFee));
+        }
 
-        return cbk(null, tokensAsMtokens(baseFeeTokens));
+        return cbk(null, tokensAsMtokens(args.base_fee_tokens));
       }],
 
       // Set the routing fee policy
       updateFees: ['baseFeeMillitokens', ({baseFeeMillitokens}, cbk) => {
         const id = args.transaction_id || undefined;
+        const rate = args.fee_rate === undefined ? defaultRate : args.fee_rate;
         const vout = args.transaction_vout;
 
         const isGlobal = !args.transaction_id && vout === undefined;
@@ -80,10 +87,10 @@ module.exports = (args, cbk) => {
           output_index: vout === undefined ? undefined : vout,
         };
 
-        return args.lnd.default.updateChannelPolicy({
+        return args.lnd[type][method]({
           base_fee_msat: baseFeeMillitokens,
           chan_point: !isGlobal ? chan : undefined,
-          fee_rate: ((args.fee_rate || defaultFeeRate) / feeRatio),
+          fee_rate: rate / feeRatio,
           global: isGlobal || undefined,
           max_htlc_msat: args.max_htlc_mtokens || undefined,
           min_htlc_msat: args.min_htlc_mtokens || undefined,

diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -60,5 +60,5 @@
     "tower_server-integration-tests": "tap --no-coverage test/tower_serverrpc-integration/*.js",
     "wallet-integration-tests": "tap --no-coverage test/walletrpc-integration/*.js"
   },
-  "version": "49.3.1"
+  "version": "49.3.2"
 }
diff --git a/test/integration/test_update_routing_fees.js b/test/integration/test_update_routing_fees.js
@@ -11,7 +11,7 @@ const channelCapacityTokens = 1e6;
 const cltvDelta = 10;
 const confirmationCount = 6;
 const defaultFee = 1e3;
-const feeRate = 11;
+const feeRate = 0;
 const giftTokens = 1e5;
 const mtokPerTok = 1e3;
 const n = 2;

diff --git a/test/routing/test_update_routing_fees.js b/test/routing/test_update_routing_fees.js
@@ -2,114 +2,151 @@ const {test} = require('tap');
 
 const {updateRoutingFees} = require('./../../');
 
+const makeLnd = ({err, policy}) => {
+  return {
+    default: {
+      updateChannelPolicy: (args, cbk) => {
+        if (!!err) {
+          return cbk(err);
+        }
+
+        if (args.base_fee_msat !== policy.base_fee_msat) {
+          return cbk('UnexpectedBaseFeeMsat');
+        }
+
+        const gotPoint = args.chan_point || {};
+        const chanPoint = policy.chan_point || {};
+
+        if (gotPoint.funding_txid_str !== chanPoint.funding_txid_str) {
+          return cbk('UnexpectedChanTxId');
+        }
+
+        if (gotPoint.output_index !== chanPoint.output_index) {
+          return cbk('UnexpectedChanTxVout');
+        }
+
+        if (policy.fee_rate !== args.fee_rate) {
+          return cbk('UnexpectedFeeRate');
+        }
+
+        if (policy.global !== args.global) {
+          return cbk('UnexpectedGlobalFeeUpdate');
+        }
+
+        if (policy.max_htlc_msat !== args.max_htlc_msat) {
+          return cbk('UnexpectedMaxHtlcMsatUndefined');
+        }
+
+        if (policy.time_lock_delta !== args.time_lock_delta) {
+          return cbk('UnexpectedCltvDelta');
+        }
+
+        return cbk();
+      },
+    },
+  };
+};
+
+const makeArgs = overrides => {
+  const args = {
+    base_fee_tokens: '99',
+    cltv_delta: 1,
+    fee_rate: 99,
+    lnd: makeLnd({}),
+    max_htlc_mtokens: '1',
+    transaction_id: '1',
+    transaction_vout: 0,
+  };
+
+  Object.keys(overrides).forEach(key => args[key] = overrides[key]);
+
+  return args;
+};
+
 const tests = [
   {
-    args: {},
+    args: makeArgs({lnd: undefined}),
     description: 'Authenticated LND is required to update routing fees',
     error: [400, 'ExpectedLndForRoutingFeesUpdate'],
   },
   {
-    args: {lnd: {}},
-    description: 'Authenticated regular LND required to update fees',
-    error: [400, 'ExpectedLndForRoutingFeesUpdate'],
+    args: makeArgs({base_fee_mtokens: '1', base_fee_tokens: 1}),
+    description: 'A single unit format base fee is expected',
+    error: [400, 'ExpectedEitherBaseFeeMtokensOrTokensNotBoth'],
   },
   {
-    args: {lnd: {default: {}}, transaction_vout: 0},
+    args: makeArgs({transaction_id: undefined}),
     description: 'A full chanpoint with vout is required for an update',
     error: [400, 'UnexpectedTransactionIdForGlobalFeeUpdate'],
   },
   {
-    args: {lnd: {default: {}}, transaction_id: 'id'},
+    args: makeArgs({transaction_vout: undefined}),
     description: 'A full chanpoint is required for a routing fee update',
     error: [400, 'UnexpectedTxOutputIndexForGlobalFeeUpdate'],
   },
   {
-    args: {lnd: {default: {updateChannelPolicy: ({}, cbk) => cbk('err')}}},
+    args: {lnd: makeLnd({err: 'err'})},
     description: 'Errors are returned',
     error: [503, 'UnexpectedErrorUpdatingRoutingFees', {err: 'err'}],
   },
   {
-    args: {lnd: {default: {updateChannelPolicy: (args, cbk) => {
-      if (args.base_fee_msat !== '1000') {
-        return cbk('ExpectedBaseFeeMsat');
-      }
-
-      if (args.chan_point !== undefined) {
-        return cbk('ExpectedChanPointUndefined');
-      }
-
-      if (args.fee_rate !== 0.000001) {
-        return cbk('ExpectedMinimalFeeRate');
-      }
-
-      if (args.global !== true) {
-        return cbk('ExpectedGlobalRoutingFeeUpdate');
-      }
-
-      if (args.max_htlc_msat !== undefined) {
-        return cbk('ExpectedMaxHtlcMsatUndefined');
-      }
-
-      if (args.time_lock_delta !== 40) {
-        return cbk('ExpectedDefaultCltvDelta');
-      }
-
-      return cbk();
-    }}}},
-    description: 'Channel policy is updated',
+    args: {
+      lnd: makeLnd({
+        policy: {
+          base_fee_msat: '1000',
+          chan_point: undefined,
+          fee_rate: 0.000001,
+          global: true,
+          max_htlc_msat: undefined,
+          time_lock_delta: 40,
+        },
+      }),
+      description: 'The global channel policy is updated',
+    },
+  },
+  {
+    args: {
+      base_fee_mtokens: '1',
+      lnd: makeLnd({
+        policy: {
+          base_fee_msat: '1',
+          chan_point: undefined,
+          fee_rate: 0.000001,
+          global: true,
+          max_htlc_msat: undefined,
+          time_lock_delta: 40,
+        },
+      }),
+      description: 'The global channel policy base fee mtokens is updated',
+    },
   },
   {
     args: {
       base_fee_tokens: '99',
       cltv_delta: 1,
       fee_rate: 99,
-      lnd: {
-        default: {
-          updateChannelPolicy: (args, cbk) => {
-            if (args.base_fee_msat !== '99000') {
-              return cbk('ExpectedBaseFeeMsat');
-            }
-
-            if (args.chan_point.funding_txid_str !== '1') {
-              return cbk('ExpectedChanTxId');
-            }
-
-            if (args.chan_point.output_index !== 0) {
-              return cbk('ExpectedChanTxVout');
-            }
-
-            if (args.fee_rate !== 0.000099) {
-              return cbk('ExpectedFeeRate');
-            }
-
-            if (args.global !== undefined) {
-              return cbk('ExpectedLocalRoutingFeeUpdate');
-            }
-
-            if (args.max_htlc_msat !== '1') {
-              return cbk('ExpectedMaxHtlcMsatUndefined');
-            }
-
-            if (args.time_lock_delta !== 1) {
-              return cbk('ExpectedCltvDelta');
-            }
-
-            return cbk();
-          },
+      lnd: makeLnd({
+        policy: {
+          base_fee_msat: '99000',
+          chan_point: {funding_txid_str: '1', output_index: 0},
+          fee_rate: 0.000099,
+          global: undefined,
+          max_htlc_msat: '1',
+          time_lock_delta: 1,
         },
-      },
+      }),
       max_htlc_mtokens: '1',
       transaction_id: '1',
       transaction_vout: 0,
     },
-    description: 'Channel policy is updated',
+    description: 'A local channel policy is updated',
   },
 ];
 
 tests.forEach(({args, description, error, expected}) => {
   return test(description, async ({deepIs, end, rejects}) => {
     if (!!error) {
-      rejects(updateRoutingFees(args), error, 'Got expected error');
+      await rejects(updateRoutingFees(args), error, 'Got expected error');
     } else {
       await updateRoutingFees(args);
     }