lnchannel: partly fix available_to_spend

we were looking at inconsistent ctns
and we were looking at the wrong subject's ctx

all the FIXMEs and TODOs here will still warrant some attention.

(note that test_DesyncHTLCs was passing incorrectly:
the "assertRaises" was catching a different exception)

electrum/commands.py

@@ -1004,8 +1004,8 @@ class Commands:
                 'remote_balance': chan.balance(REMOTE)//1000,
                 'local_reserve': chan.config[LOCAL].reserve_sat,
                 'remote_reserve': chan.config[REMOTE].reserve_sat,
-                'local_unsettled_sent': chan.unsettled_sent_balance(LOCAL)//1000,
+                'local_unsettled_sent': chan.balance_tied_up_in_htlcs_by_direction(LOCAL, direction=SENT) // 1000,
-                'remote_unsettled_sent': chan.unsettled_sent_balance(REMOTE)//1000,
+                'remote_unsettled_sent': chan.balance_tied_up_in_htlcs_by_direction(REMOTE, direction=SENT) // 1000,
             } for channel_id, chan in l
         ]
 

electrum/lnchannel.py

@@ -664,20 +664,28 @@ class Channel(Logger):
                                         ctn=ctn,
                                         initial_balance_msat=initial)
 
-    def balance_minus_outgoing_htlcs(self, whose: HTLCOwner, *, ctx_owner: HTLCOwner = HTLCOwner.LOCAL):
+    def balance_minus_outgoing_htlcs(self, whose: HTLCOwner, *, ctx_owner: HTLCOwner = HTLCOwner.LOCAL,
+                                     ctn: int = None):
         """
         This balance in mSAT, which includes the value of
         pending outgoing HTLCs, is used in the UI.
         """
         assert type(whose) is HTLCOwner
+        if ctn is None:
             ctn = self.get_next_ctn(ctx_owner)
-        return self.balance(whose, ctx_owner=ctx_owner, ctn=ctn) - self.unsettled_sent_balance(ctx_owner)
+        committed_balance = self.balance(whose, ctx_owner=ctx_owner, ctn=ctn)
-
+        direction = RECEIVED if whose != ctx_owner else SENT
-    def unsettled_sent_balance(self, subject: HTLCOwner = LOCAL):
+        balance_in_htlcs = self.balance_tied_up_in_htlcs_by_direction(ctx_owner, ctn=ctn, direction=direction)
-        ctn = self.get_next_ctn(subject)
+        return committed_balance - balance_in_htlcs
-        return htlcsum(self.hm.htlcs_by_direction(subject, SENT, ctn).values())
+
+    def balance_tied_up_in_htlcs_by_direction(self, ctx_owner: HTLCOwner = LOCAL, *, ctn: int = None,
+                                              direction: Direction):
+        # in msat
+        if ctn is None:
+            ctn = self.get_next_ctn(ctx_owner)
+        return htlcsum(self.hm.htlcs_by_direction(ctx_owner, direction, ctn).values())
 
-    def available_to_spend(self, subject):
+    def available_to_spend(self, subject: HTLCOwner) -> int:
         """
         This balance in mSAT, while technically correct, can
         not be used in the UI cause it fluctuates (commit fee)
@@ -685,14 +693,17 @@ class Channel(Logger):
         # FIXME whose balance? whose ctx?
         # FIXME confusing/mixing ctns (should probably use latest_ctn + 1; not oldest_unrevoked + 1)
         assert type(subject) is HTLCOwner
-        return self.balance_minus_outgoing_htlcs(subject, ctx_owner=subject)\
+        ctx_owner = subject.inverted()
-                - self.config[-subject].reserve_sat * 1000\
+        ctn = self.get_next_ctn(ctx_owner)
-                - calc_onchain_fees(
+        balance = self.balance_minus_outgoing_htlcs(whose=subject, ctx_owner=ctx_owner, ctn=ctn)
+        reserve = self.config[-subject].reserve_sat * 1000
         # TODO should we include a potential new htlc, when we are called from receive_htlc?
-                      len(self.included_htlcs(subject, SENT) + self.included_htlcs(subject, RECEIVED)),
+        fees = calc_onchain_fees(
-                      self.get_latest_feerate(subject),
+            num_htlcs=len(self.included_htlcs(ctx_owner, SENT, ctn=ctn) + self.included_htlcs(ctx_owner, RECEIVED, ctn=ctn)),
-                      self.constraints.is_initiator,
+            feerate=self.get_feerate(ctx_owner, ctn=ctn),
+            is_local_initiator=self.constraints.is_initiator,
         )[subject]
+        return balance - reserve - fees
 
     def included_htlcs(self, subject, direction, ctn=None):
         """
@@ -877,10 +888,12 @@ class Channel(Logger):
                     local_htlc_pubkey=this_htlc_pubkey,
                     payment_hash=htlc.payment_hash,
                     cltv_expiry=htlc.cltv_expiry), htlc))
+        # note: maybe flip initiator here for fee purposes, we want LOCAL and REMOTE
+        #       in the resulting dict to correspond to the to_local and to_remote *outputs* of the ctx
         onchain_fees = calc_onchain_fees(
-            len(htlcs),
+            num_htlcs=len(htlcs),
-            feerate,
+            feerate=feerate,
-            self.constraints.is_initiator == (subject == LOCAL),
+            is_local_initiator=self.constraints.is_initiator == (subject == LOCAL),
         )
         if self.is_static_remotekey_enabled():
             payment_pubkey = other_config.payment_basepoint.pubkey

electrum/lnutil.py

@@ -556,11 +556,17 @@ def make_commitment_outputs(*, fees_per_participant: Mapping[HTLCOwner, int], lo
     c_outputs_filtered = list(filter(lambda x: x.value >= dust_limit_sat, non_htlc_outputs + htlc_outputs))
     return htlc_outputs, c_outputs_filtered
 
-def calc_onchain_fees(num_htlcs, feerate, we_pay_fee):
+
+def calc_onchain_fees(*, num_htlcs: int, feerate: int, is_local_initiator: bool) -> Dict['HTLCOwner', int]:
+    # feerate is in sat/kw
+    # returns fees in msats
     overall_weight = 500 + 172 * num_htlcs + 224
     fee = feerate * overall_weight
     fee = fee // 1000 * 1000
-    return {LOCAL: fee if we_pay_fee else 0, REMOTE: fee if not we_pay_fee else 0}
+    return {
+        LOCAL: fee if is_local_initiator else 0,
+        REMOTE: fee if not is_local_initiator else 0,
+    }
 
 def make_commitment(ctn, local_funding_pubkey, remote_funding_pubkey,
                     remote_payment_pubkey, funder_payment_basepoint,

electrum/tests/test_lnchannel.py

@@ -605,21 +605,28 @@ class TestChannel(ElectrumTestCase):
 class TestAvailableToSpend(ElectrumTestCase):
     def test_DesyncHTLCs(self):
         alice_channel, bob_channel = create_test_channels()
+        self.assertEqual(499995656000, alice_channel.available_to_spend(LOCAL))
+        self.assertEqual(500000000000, bob_channel.available_to_spend(LOCAL))
 
         paymentPreimage = b"\x01" * 32
         paymentHash = bitcoin.sha256(paymentPreimage)
         htlc_dict = {
             'payment_hash' : paymentHash,
-            'amount_msat' :  int(4.1 * one_bitcoin_in_msat),
+            'amount_msat' :  one_bitcoin_in_msat * 41 // 10,
             'cltv_expiry' :  5,
             'timestamp'   :  0,
         }
 
         alice_idx = alice_channel.add_htlc(htlc_dict).htlc_id
         bob_idx = bob_channel.receive_htlc(htlc_dict).htlc_id
+        self.assertEqual(89994624000, alice_channel.available_to_spend(LOCAL))
+        self.assertEqual(500000000000, bob_channel.available_to_spend(LOCAL))
+
         force_state_transition(alice_channel, bob_channel)
         bob_channel.fail_htlc(bob_idx)
         alice_channel.receive_fail_htlc(alice_idx, error_bytes=None)
+        self.assertEqual(89994624000, alice_channel.available_to_spend(LOCAL))
+        self.assertEqual(500000000000, bob_channel.available_to_spend(LOCAL))
         # Alice now has gotten all her original balance (5 BTC) back, however,
         # adding a new HTLC at this point SHOULD fail, since if she adds the
         # HTLC and signs the next state, Bob cannot assume she received the
@@ -638,6 +645,8 @@ class TestAvailableToSpend(ElectrumTestCase):
         # Now do a state transition, which will ACK the FailHTLC, making Alice
         # able to add the new HTLC.
         force_state_transition(alice_channel, bob_channel)
+        self.assertEqual(499995656000, alice_channel.available_to_spend(LOCAL))
+        self.assertEqual(500000000000, bob_channel.available_to_spend(LOCAL))
         alice_channel.add_htlc(htlc_dict)
 
 class TestChanReserve(ElectrumTestCase):

electrum/tests/test_lnutil.py

@@ -516,7 +516,7 @@ class TestLNUtil(ElectrumTestCase):
             local_revocation_pubkey, local_delayedpubkey, local_delay,
             funding_tx_id, funding_output_index, funding_amount_satoshi,
             to_local_msat, to_remote_msat, local_dust_limit_satoshi,
-            calc_onchain_fees(len(htlcs), local_feerate_per_kw, True), htlcs=htlcs)
+            calc_onchain_fees(num_htlcs=len(htlcs), feerate=local_feerate_per_kw, is_local_initiator=True), htlcs=htlcs)
         self.sign_and_insert_remote_sig(our_commit_tx, remote_funding_pubkey, remote_signature, local_funding_pubkey, local_funding_privkey)
         self.assertEqual(str(our_commit_tx), output_commit_tx)
 
@@ -593,7 +593,7 @@ class TestLNUtil(ElectrumTestCase):
             local_revocation_pubkey, local_delayedpubkey, local_delay,
             funding_tx_id, funding_output_index, funding_amount_satoshi,
             to_local_msat, to_remote_msat, local_dust_limit_satoshi,
-            calc_onchain_fees(0, local_feerate_per_kw, True), htlcs=[])
+            calc_onchain_fees(num_htlcs=0, feerate=local_feerate_per_kw, is_local_initiator=True), htlcs=[])
         self.sign_and_insert_remote_sig(our_commit_tx, remote_funding_pubkey, remote_signature, local_funding_pubkey, local_funding_privkey)
 
         self.assertEqual(str(our_commit_tx), output_commit_tx)
@@ -612,7 +612,7 @@ class TestLNUtil(ElectrumTestCase):
             local_revocation_pubkey, local_delayedpubkey, local_delay,
             funding_tx_id, funding_output_index, funding_amount_satoshi,
             to_local_msat, to_remote_msat, local_dust_limit_satoshi,
-            calc_onchain_fees(0, local_feerate_per_kw, True), htlcs=[])
+            calc_onchain_fees(num_htlcs=0, feerate=local_feerate_per_kw, is_local_initiator=True), htlcs=[])
         self.sign_and_insert_remote_sig(our_commit_tx, remote_funding_pubkey, remote_signature, local_funding_pubkey, local_funding_privkey)
 
         self.assertEqual(str(our_commit_tx), output_commit_tx)
@@ -670,7 +670,7 @@ class TestLNUtil(ElectrumTestCase):
             local_revocation_pubkey, local_delayedpubkey, local_delay,
             funding_tx_id, funding_output_index, funding_amount_satoshi,
             to_local_msat, to_remote_msat, local_dust_limit_satoshi,
-            calc_onchain_fees(0, local_feerate_per_kw, True), htlcs=[])
+            calc_onchain_fees(num_htlcs=0, feerate=local_feerate_per_kw, is_local_initiator=True), htlcs=[])
         self.sign_and_insert_remote_sig(our_commit_tx, remote_funding_pubkey, remote_signature, local_funding_pubkey, local_funding_privkey)
         ref_commit_tx_str = '02000000000101bef67e4e2fb9ddeeb3461973cd4c62abb35050b1add772995b820b584a488489000000000038b02b8002c0c62d0000000000160014ccf1af2f2aabee14bb40fa3851ab2301de84311054a56a00000000002200204adb4e2f00643db396dd120d4e7dc17625f5f2c11a40d857accc862d6b7dd80e0400473044022051b75c73198c6deee1a875871c3961832909acd297c6b908d59e3319e5185a46022055c419379c5051a78d00dbbce11b5b664a0c22815fbcc6fcef6b1937c383693901483045022100f51d2e566a70ba740fc5d8c0f07b9b93d2ed741c3c0860c613173de7d39e7968022041376d520e9c0e1ad52248ddf4b22e12be8763007df977253ef45a4ca3bdb7c001475221023da092f6980e58d2c037173180e9a465476026ee50f96695963e8efe436f54eb21030e9f7b623d2ccc7c9bd44d66d5ce21ce504c0acf6385a132cec6d3c39fa711c152ae3e195220'
         self.assertEqual(str(our_commit_tx), ref_commit_tx_str)