Modern fee negotiation

electrum/lnpeer.py

@@ -51,6 +51,7 @@
 from .lnutil import ln_dummy_address
 from .json_db import StoredDict
 from .invoices import PR_PAID
+from .simple_config import FEE_LN_ETA_TARGET
 
 if TYPE_CHECKING:
     from .lnworker import LNGossip, LNWallet
@@ -1824,6 +1825,31 @@ async def send_shutdown(self, chan: Channel):
         # can fullfill or fail htlcs. cannot add htlcs, because state != OPEN
         chan.set_can_send_ctx_updates(True)
 
+    def get_shutdown_fee_range(self, chan, closing_tx, is_local):
+        """ return the closing fee and fee range we initially try to enforce """
+        config = self.network.config
+        if config.get('test_shutdown_fee'):
+            our_fee = config.get('test_shutdown_fee')
+        else:
+            fee_rate_per_kb = config.eta_target_to_fee(FEE_LN_ETA_TARGET)
+            if not fee_rate_per_kb:  # fallback
+                fee_rate_per_kb = self.network.config.fee_per_kb()
+            our_fee = fee_rate_per_kb * closing_tx.estimated_size() // 1000
+            # TODO: anchors: remove this, as commitment fee rate can be below chain head fee rate?
+            # BOLT2: The sending node MUST set fee less than or equal to the base fee of the final ctx
+            max_fee = chan.get_latest_fee(LOCAL if is_local else REMOTE)
+            our_fee = min(our_fee, max_fee)
+        # config modern_fee_negotiation can be set in tests
+        if config.get('test_shutdown_legacy'):
+            our_fee_range = None
+        elif config.get('test_shutdown_fee_range'):
+            our_fee_range = config.get('test_shutdown_fee_range')
+        else:
+            # we aim at a fee between next block inclusion and some lower value
+            our_fee_range = {'min_fee_satoshis': our_fee // 2, 'max_fee_satoshis': our_fee * 2}
+        self.logger.info(f"Our fee range: {our_fee_range} and fee: {our_fee}")
+        return our_fee, our_fee_range
+
     @log_exceptions
     async def _shutdown(self, chan: Channel, payload, *, is_local: bool):
         # wait until no HTLCs remain in either commitment transaction
@@ -1840,62 +1866,145 @@ async def _shutdown(self, chan: Channel, payload, *, is_local: bool):
         assert our_scriptpubkey
         # estimate fee of closing tx
         our_sig, closing_tx = chan.make_closing_tx(our_scriptpubkey, their_scriptpubkey, fee_sat=0)
-        fee_rate = self.network.config.fee_per_kb()
-        our_fee = fee_rate * closing_tx.estimated_size() // 1000
-        # BOLT2: The sending node MUST set fee less than or equal to the base fee of the final ctx
-        max_fee = chan.get_latest_fee(LOCAL if is_local else REMOTE)
-        our_fee = min(our_fee, max_fee)
-        drop_to_remote = False
-        def send_closing_signed():
-            our_sig, closing_tx = chan.make_closing_tx(our_scriptpubkey, their_scriptpubkey, fee_sat=our_fee, drop_remote=drop_to_remote)
-            self.send_message('closing_signed', channel_id=chan.channel_id, fee_satoshis=our_fee, signature=our_sig)
+
+        is_initiator = chan.constraints.is_initiator
+        our_fee, our_fee_range = self.get_shutdown_fee_range(chan, closing_tx, is_local)
+
+        def send_closing_signed(our_fee, our_fee_range, drop_remote):
+            if our_fee_range:
+                closing_signed_tlvs = {'fee_range': our_fee_range}
+            else:
+                closing_signed_tlvs = {}
+            our_sig, closing_tx = chan.make_closing_tx(our_scriptpubkey, their_scriptpubkey, fee_sat=our_fee, drop_remote=drop_remote)
+            self.logger.info(f"Sending fee range: {closing_signed_tlvs} and fee: {our_fee}")
+            self.send_message(
+                'closing_signed',
+                channel_id=chan.channel_id,
+                fee_satoshis=our_fee,
+                signature=our_sig,
+                closing_signed_tlvs=closing_signed_tlvs,
+            )
+
         def verify_signature(tx, sig):
             their_pubkey = chan.config[REMOTE].multisig_key.pubkey
             preimage_hex = tx.serialize_preimage(0)
             pre_hash = sha256d(bfh(preimage_hex))
             return ecc.verify_signature(their_pubkey, sig, pre_hash)
-        # the funder sends the first 'closing_signed' message
-        if chan.constraints.is_initiator:
-            send_closing_signed()
-        # negotiate fee
-        while True:
-            # FIXME: the remote SHOULD send closing_signed, but some don't.
-            cs_payload = await self.wait_for_message('closing_signed', chan.channel_id)
+
+        async def receive_closing_signed():
+            try:
+                cs_payload = await self.wait_for_message('closing_signed', chan.channel_id)
+            except asyncio.exceptions.TimeoutError:
+                self.lnworker.schedule_force_closing(chan.channel_id)
+                raise Exception("closing_signed not received, force closing.")
             their_fee = cs_payload['fee_satoshis']
-            if their_fee > max_fee:
-                raise Exception(f'the proposed fee exceeds the base fee of the latest commitment transaction {is_local, their_fee, max_fee}')
+            their_fee_range = cs_payload['closing_signed_tlvs'].get('fee_range')
             their_sig = cs_payload['signature']
-            # verify their sig: they might have dropped their output
+            # perform checks
             our_sig, closing_tx = chan.make_closing_tx(our_scriptpubkey, their_scriptpubkey, fee_sat=their_fee, drop_remote=False)
             if verify_signature(closing_tx, their_sig):
-                drop_to_remote = False
+                drop_remote = False
             else:
                 our_sig, closing_tx = chan.make_closing_tx(our_scriptpubkey, their_scriptpubkey, fee_sat=their_fee, drop_remote=True)
                 if verify_signature(closing_tx, their_sig):
-                    drop_to_remote = True
+                    drop_remote = True
                 else:
                     # this can happen if we consider our output too valuable to drop,
                     # but the remote drops it because it violates their dust limit
                     raise Exception('failed to verify their signature')
             # at this point we know how the closing tx looks like
             # check that their output is above their scriptpubkey's network dust limit
             to_remote_set = closing_tx.get_output_idxs_from_scriptpubkey(their_scriptpubkey.hex())
-            if not drop_to_remote and to_remote_set:
+            if not drop_remote and to_remote_set:
                 to_remote_idx = to_remote_set.pop()
                 to_remote_amount = closing_tx.outputs()[to_remote_idx].value
                 transaction.check_scriptpubkey_template_and_dust(their_scriptpubkey, to_remote_amount)
-
-            # Agree if difference is lower or equal to one (see below)
-            if abs(our_fee - their_fee) < 2:
+            return their_fee, their_fee_range, their_sig, drop_remote
+
+        def choose_new_fee(our_fee, our_fee_range, their_fee, their_fee_range, their_previous_fee):
+            assert our_fee != their_fee
+            fee_range_sent = our_fee_range and (is_initiator or (their_previous_fee is not None))
+
+            # The sending node, if it is not the funder:
+            if our_fee_range and their_fee_range and not is_initiator and not self.network.config.get('test_shutdown_fee_range'):
+                # SHOULD set max_fee_satoshis to at least the max_fee_satoshis received
+                our_fee_range['max_fee_satoshis'] = max(their_fee_range['max_fee_satoshis'], our_fee_range['max_fee_satoshis'])
+                # SHOULD set min_fee_satoshis to a fairly low value
+                our_fee_range['min_fee_satoshis'] = min(their_fee_range['min_fee_satoshis'], our_fee_range['min_fee_satoshis'])
+                # Note: the BOLT describes what the sending node SHOULD do.
+                # However, this assumes that we have decided to send 'funding_signed' in response to their fee_range.
+                # In practice, we might prefer to fail the channel in some cases (TODO)
+
+            # the receiving node, if fee_satoshis matches its previously sent fee_range,
+            if fee_range_sent and (our_fee_range['min_fee_satoshis'] <= their_fee <= our_fee_range['max_fee_satoshis']):
+                # SHOULD reply with a closing_signed with the same fee_satoshis value if it is different from its previously sent fee_satoshis
                 our_fee = their_fee
+
+            # the receiving node, if the message contains a fee_range
+            elif our_fee_range and their_fee_range:
+                overlap_min = max(our_fee_range['min_fee_satoshis'], their_fee_range['min_fee_satoshis'])
+                overlap_max = min(our_fee_range['max_fee_satoshis'], their_fee_range['max_fee_satoshis'])
+                # if there is no overlap between that and its own fee_range
+                if overlap_min > overlap_max:
+                    # TODO: the receiving node should first send a warning, and fail the channel
+                    # only if it doesn't receive a satisfying fee_range after a reasonable amount of time
+                    self.lnworker.schedule_force_closing(chan.channel_id)
+                    raise Exception("There is no overlap between between their and our fee range.")
+                # otherwise, if it is the funder
+                if is_initiator:
+                    # if fee_satoshis is not in the overlap between the sent and received fee_range:
+                    if not (overlap_min <= their_fee <= overlap_max):
+                        # MUST fail the channel
+                        self.lnworker.schedule_force_closing(chan.channel_id)
+                        raise Exception("Their fee is not in the overlap region, we force closed.")
+                    # otherwise, MUST reply with the same fee_satoshis.
+                    our_fee = their_fee
+                # otherwise (it is not the funder):
+                else:
+                    # if it has already sent a closing_signed:
+                    if fee_range_sent:
+                        # fee_satoshis is not the same as the value we sent, we MUST fail the channel
+                        self.lnworker.schedule_force_closing(chan.channel_id)
+                        raise Exception("Expected the same fee as ours, we force closed.")
+                    # otherwise:
+                    # MUST propose a fee_satoshis in the overlap between received and (about-to-be) sent fee_range.
+                    our_fee = (overlap_min + overlap_max) // 2
+            else:
+                # otherwise, if fee_satoshis is not strictly between its last-sent fee_satoshis
+                # and its previously-received fee_satoshis, UNLESS it has since reconnected:
+                if their_previous_fee and not (min(our_fee, their_previous_fee) < their_fee < max(our_fee, their_previous_fee)):
+                    # SHOULD fail the connection.
+                    raise Exception('Their fee is not between our last sent and their last sent fee.')
+                # accept their fee if they are very close
+                if abs(their_fee - our_fee) < 2:
+                    our_fee = their_fee
+                else:
+                    # this will be "strictly between" (as in BOLT2) previous values because of the above
+                    our_fee = (our_fee + their_fee) // 2
+
+            return our_fee, our_fee_range
+
+        # Fee negotiation: both parties exchange 'funding_signed' messages.
+        # The funder sends the first message, the non-funder sends the last message.
+        # In the 'modern' case, at most 3 messages are exchanged, because choose_new_fee of the funder either returns their_fee or fails
+        their_fee = None
+        drop_remote = False  # does the peer drop its to_local output or not?
+        if is_initiator:
+            send_closing_signed(our_fee, our_fee_range, drop_remote)
+        while True:
+            their_previous_fee = their_fee
+            their_fee, their_fee_range, their_sig, drop_remote = await receive_closing_signed()
+            if our_fee == their_fee:
                 break
-            # this will be "strictly between" (as in BOLT2) previous values because of the above
-            our_fee = (our_fee + their_fee) // 2
-            # another round
-            send_closing_signed()
-        # the non-funder replies
-        if not chan.constraints.is_initiator:
-            send_closing_signed()
+            our_fee, our_fee_range = choose_new_fee(our_fee, our_fee_range, their_fee, their_fee_range, their_previous_fee)
+            if not is_initiator and our_fee == their_fee:
+                break
+            send_closing_signed(our_fee, our_fee_range, drop_remote)
+            if is_initiator and our_fee == their_fee:
+                break
+        if not is_initiator:
+            send_closing_signed(our_fee, our_fee_range, drop_remote)
+
         # add signatures
         closing_tx.add_signature_to_txin(
             txin_idx=0,

electrum/lnwire/peer_wire.csv

@@ -94,6 +94,10 @@ msgtype,closing_signed,39
 msgdata,closing_signed,channel_id,channel_id,
 msgdata,closing_signed,fee_satoshis,u64,
 msgdata,closing_signed,signature,signature,
+msgdata,closing_signed,tlvs,closing_signed_tlvs,
+tlvtype,closing_signed_tlvs,fee_range,1
+tlvdata,closing_signed_tlvs,fee_range,min_fee_satoshis,u64,
+tlvdata,closing_signed_tlvs,fee_range,max_fee_satoshis,u64,
 msgtype,update_add_htlc,128
 msgdata,update_add_htlc,channel_id,channel_id,
 msgdata,update_add_htlc,id,u64,

electrum/tests/regtest.py

@@ -44,6 +44,9 @@ def test_unixsockets(self):
 class TestLightningAB(TestLightning):
     agents = ['alice', 'bob']
 
+    def test_collaborative_close(self):
+        self.run_shell(['collaborative_close'])
+
     def test_backup(self):
         self.run_shell(['backup'])
 

electrum/tests/regtest/regtest.sh

@@ -158,6 +158,18 @@ if [[ $1 == "backup" ]]; then
 fi
 
 
+if [[ $1 == "collaborative_close" ]]; then
+    wait_for_balance alice 1
+    echo "alice opens channel"
+    bob_node=$($bob nodeid)
+    channel=$($alice open_channel $bob_node 0.15)
+    new_blocks 3
+    wait_until_channel_open alice
+    echo "alice closes channel"
+    request=$($bob close_channel $channel)
+fi
+
+
 if [[ $1 == "extract_preimage" ]]; then
     # instead of settling bob will broadcast
     $bob enable_htlc_settle false

electrum/tests/test_lnpeer.py

@@ -1063,13 +1063,46 @@ async def f():
             run(f())
 
     @needs_test_with_all_chacha20_implementations
-    def test_close(self):
+    def test_legacy_shutdown_low(self):
+        self._test_shutdown(alice_fee=100, bob_fee=150)
+
+    @needs_test_with_all_chacha20_implementations
+    def test_legacy_shutdown_high(self):
+        self._test_shutdown(alice_fee=2000, bob_fee=100)
+
+    @needs_test_with_all_chacha20_implementations
+    def test_modern_shutdown_with_overlap(self):
+        self._test_shutdown(
+            alice_fee=1,
+            bob_fee=200,
+            alice_fee_range={'min_fee_satoshis': 1, 'max_fee_satoshis': 10},
+            bob_fee_range={'min_fee_satoshis': 10, 'max_fee_satoshis': 300})
+
+    ## This test works but it is too slow (LN_P2P_NETWORK_TIMEOUT)
+    ## because tests do not use a proper LNWorker object
+    #@needs_test_with_all_chacha20_implementations
+    #def test_modern_shutdown_no_overlap(self):
+    #    self.assertRaises(Exception, lambda: asyncio.run(
+    #        self._test_shutdown(
+    #            alice_fee=1,
+    #            bob_fee=200,
+    #            alice_fee_range={'min_fee_satoshis': 1, 'max_fee_satoshis': 10},
+    #            bob_fee_range={'min_fee_satoshis': 50, 'max_fee_satoshis': 300})
+    #    ))
+
+    def _test_shutdown(self, alice_fee, bob_fee, alice_fee_range=None, bob_fee_range=None):
         alice_channel, bob_channel = create_test_channels()
         p1, p2, w1, w2, _q1, _q2 = self.prepare_peers(alice_channel, bob_channel)
-        w1.network.config.set_key('dynamic_fees', False)
-        w2.network.config.set_key('dynamic_fees', False)
-        w1.network.config.set_key('fee_per_kb', 5000)
-        w2.network.config.set_key('fee_per_kb', 1000)
+        w1.network.config.set_key('test_shutdown_fee', alice_fee)
+        w2.network.config.set_key('test_shutdown_fee', bob_fee)
+        if alice_fee_range is not None:
+            w1.network.config.set_key('test_shutdown_fee_range', alice_fee_range)
+        else:
+            w1.network.config.set_key('test_shutdown_legacy', True)
+        if bob_fee_range is not None:
+            w2.network.config.set_key('test_shutdown_fee_range', bob_fee_range)
+        else:
+            w2.network.config.set_key('test_shutdown_legacy', True)
         w2.enable_htlc_settle = False
         lnaddr, pay_req = run(self.prepare_invoice(w2))
         async def pay():