ln: remove unneeded forwarding htlc features, check commitment sig using lnhtlc while receiving

lib/lnbase.py

@@ -490,17 +490,23 @@ def make_commitment_using_open_channel(chan, ctn, for_us, pcp, local_msat, remot
         remote_msat,
         chan.local_config.dust_limit_sat,
         chan.constraints.feerate,
-        for_us, htlcs=htlcs)
+        for_us,
-
+        chan.constraints.is_initiator,
-def make_commitment(ctn, local_funding_pubkey, remote_funding_pubkey, remote_payment_pubkey,
+        htlcs=htlcs)
-                    payment_basepoint, remote_payment_basepoint,
+
-                    revocation_pubkey, delayed_pubkey, to_self_delay,
+def make_commitment(ctn, local_funding_pubkey, remote_funding_pubkey,
-                    funding_txid, funding_pos, funding_sat,
+                    remote_payment_pubkey, payment_basepoint,
-                    local_amount, remote_amount,
+                    remote_payment_basepoint, revocation_pubkey,
-                    dust_limit_sat, local_feerate, for_us, htlcs):
+                    delayed_pubkey, to_self_delay, funding_txid,
+                    funding_pos, funding_sat, local_amount, remote_amount,
+                    dust_limit_sat, local_feerate, for_us, we_are_initiator,
+                    htlcs):
 
     pubkeys = sorted([bh2u(local_funding_pubkey), bh2u(remote_funding_pubkey)])
-    obs = get_obscured_ctn(ctn, payment_basepoint, remote_payment_basepoint)
+    payments = [payment_basepoint, remote_payment_basepoint]
+    if not we_are_initiator:
+        payments.reverse()
+    obs = get_obscured_ctn(ctn, *payments)
     locktime = (0x20 << 24) + (obs & 0xffffff)
     sequence = (0x80 << 24) + (obs >> 24)
     print_error('locktime', locktime, hex(locktime))
@@ -524,10 +530,11 @@ def make_commitment(ctn, local_funding_pubkey, remote_funding_pubkey, remote_pay
     # TODO trim htlc outputs here while also considering 2nd stage htlc transactions
     fee = local_feerate * overall_weight(len(htlcs)) # TODO incorrect if anything is trimmed
     assert type(fee) is int
-    to_local_amt = local_amount - (fee if for_us else 0)
+    we_pay_fee = for_us == we_are_initiator
+    to_local_amt = local_amount - (fee if we_pay_fee else 0)
     assert type(to_local_amt) is int
     to_local = (bitcoin.TYPE_ADDRESS, local_address, to_local_amt // 1000)
-    to_remote_amt = remote_amount - (fee if not for_us else 0)
+    to_remote_amt = remote_amount - (fee if not we_pay_fee else 0)
     assert type(to_remote_amt) is int
     to_remote = (bitcoin.TYPE_ADDRESS, remote_address, to_remote_amt // 1000)
     c_outputs = [to_local, to_remote]
@@ -878,7 +885,7 @@ class Peer(PrintError):
             local_config.payment_basepoint.pubkey, remote_config.payment_basepoint.pubkey,
             revocation_pubkey, remote_delayedpubkey, local_config.to_self_delay,
             funding_txid, funding_index, funding_sat,
-            remote_amount, local_amount, remote_config.dust_limit_sat, local_feerate, False, htlcs=[])
+            remote_amount, local_amount, remote_config.dust_limit_sat, local_feerate, False, we_are_initiator=True, htlcs=[])
         sig_64 = sign_and_get_sig_string(remote_ctx, local_config, remote_config)
         channel_id, funding_txid_bytes = channel_id_from_funding_tx(funding_txid, funding_index)
         self.send_message(gen_msg("funding_created",
@@ -896,7 +903,7 @@ class Peer(PrintError):
             local_config.payment_basepoint.pubkey, remote_config.payment_basepoint.pubkey,
             remote_revocation_pubkey, local_delayedpubkey, remote_config.to_self_delay,
             funding_txid, funding_index, funding_sat,
-            local_amount, remote_amount, local_config.dust_limit_sat, local_feerate, True, htlcs=[])
+            local_amount, remote_amount, local_config.dust_limit_sat, local_feerate, True, we_are_initiator=True, htlcs=[])
         pre_hash = bitcoin.Hash(bfh(local_ctx.serialize_preimage(0)))
         if not ecc.verify_signature(remote_config.multisig_key.pubkey, remote_sig, pre_hash):
             raise Exception('verifying remote signature failed.')
@@ -1045,7 +1052,7 @@ class Peer(PrintError):
         m.add_htlc(htlc)
 
         sig_64, htlc_sigs = m.sign_next_commitment()
-        htlc_sig = htlc_sigs[0][1]
+        htlc_sig = htlc_sigs[0]
 
         self.send_message(gen_msg("commitment_signed", channel_id=chan.channel_id, signature=sig_64, num_htlcs=1, htlc_signature=htlc_sig))
 
@@ -1117,37 +1124,23 @@ class Peer(PrintError):
 
         assert self.channel_state[channel_id] == "OPEN"
         their_revstore = chan.remote_state.revocation_store
+
         cltv_expiry = int.from_bytes(htlc["cltv_expiry"], 'big')
         # TODO verify sanity of their cltv expiry
         amount_msat = int.from_bytes(htlc["amount_msat"], 'big')
         assert amount_msat == expected_received_msat
         payment_hash = htlc["payment_hash"]
+
+        htlc = UpdateAddHtlc(amount_msat, payment_hash, cltv_expiry, 0)
+
+        m = HTLCStateMachine(chan)
+        m.receive_htlc(htlc)
+
+        data = commitment_signed_msg["htlc_signature"]
+        htlc_sigs = [data[i:i+64] for i in range(0, len(data), 64)]
+        m.receive_new_commitment(commitment_signed_msg["signature"], htlc_sigs)
+
         chan, last_secret, this_point, next_point = self.derive_and_incr(chan)
-        remote_htlc_pubkey = derive_pubkey(chan.remote_config.htlc_basepoint.pubkey, this_point)
-        local_htlc_pubkey = derive_pubkey(chan.local_config.htlc_basepoint.pubkey, this_point)
-        remote_revocation_pubkey = derive_blinded_pubkey(chan.remote_config.revocation_basepoint.pubkey, this_point)
-        htlcs_in_local = [
-            (
-                make_received_htlc(remote_revocation_pubkey, remote_htlc_pubkey, local_htlc_pubkey, payment_hash, cltv_expiry),
-                amount_msat
-            )
-        ]
-        new_commitment = make_commitment_using_open_channel(chan, chan.local_state.ctn, True, this_point,
-            chan.local_state.amount_msat,
-            chan.remote_state.amount_msat - expected_received_msat,
-            htlcs_in_local)
-        preimage_hex = new_commitment.serialize_preimage(0)
-        pre_hash = bitcoin.Hash(bfh(preimage_hex))
-        if not ecc.verify_signature(chan.remote_config.multisig_key.pubkey, commitment_signed_msg["signature"], pre_hash):
-            raise Exception('failed verifying signature of our updated commitment transaction')
-        htlc_sigs_len = len(commitment_signed_msg["htlc_signature"])
-        if htlc_sigs_len != 64:
-            raise Exception("unexpected number of htlc signatures: " + str(htlc_sigs_len))
-        htlc_tx = make_htlc_tx_with_open_channel(chan, this_point, True, True, amount_msat, cltv_expiry, payment_hash, new_commitment, 0)
-        pre_hash = bitcoin.Hash(bfh(htlc_tx.serialize_preimage(0)))
-        remote_htlc_pubkey = derive_pubkey(chan.remote_config.htlc_basepoint.pubkey, this_point)
-        if not ecc.verify_signature(remote_htlc_pubkey, commitment_signed_msg["htlc_signature"], pre_hash):
-            raise Exception("failed verifying signature an HTLC tx spending from one of our commit tx'es HTLC outputs")
 
         their_revstore.add_next_entry(last_secret)
         self.send_message(gen_msg("revoke_and_ack",

lib/lnhtlc.py

@@ -1,9 +1,11 @@
 # ported from lnd 42de4400bff5105352d0552155f73589166d162b
 from ecdsa.util import sigencode_string_canonize, sigdecode_der
 from .util import bfh, PrintError
+from .bitcoin import Hash, address_to_script
 from collections import namedtuple
 from ecdsa.curves import SECP256k1
 from .crypto import sha256
+from . import ecc
 
 SettleHtlc = namedtuple("SettleHtlc", ["htlc_id"])
 RevokeAndAck = namedtuple("RevokeAndAck", ["height", "per_commitment_secret", "next_per_commitment_point"])
@@ -53,8 +55,6 @@ class HTLCStateMachine(PrintError):
 
         self.name = name
 
-        self.l_current_height = 0
-        self.r_current_height = 0
         self.total_msat_sent = 0
         self.total_msat_received = 0
 
@@ -97,13 +97,9 @@ class HTLCStateMachine(PrintError):
         HTLC's on the commitment transaction.
         """
         from .lnbase import sign_and_get_sig_string, derive_privkey, make_htlc_tx_with_open_channel
-        self.l_current_height += 1
         for htlc in self.local_update_log:
             if not type(htlc) is UpdateAddHtlc: continue
-            if htlc.l_locked_in is None: htlc.l_locked_in = self.l_current_height
+            if htlc.l_locked_in is None: htlc.l_locked_in = self.state.local_state.ctn
-        for htlc in self.remote_update_log:
-            if not type(htlc) is UpdateAddHtlc: continue
-            if htlc.r_locked_in is None: htlc.r_locked_in = self.r_current_height
         self.print_error("sign_next_commitment")
 
         sig_64 = sign_and_get_sig_string(self.remote_commitment, self.state.local_config, self.state.remote_config)
@@ -126,7 +122,7 @@ class HTLCStateMachine(PrintError):
                 sig = bfh(htlc_tx.sign_txin(0, their_remote_htlc_privkey))
                 r, s = sigdecode_der(sig[:-1], SECP256k1.generator.order())
                 htlc_sig = sigencode_string_canonize(r, s, SECP256k1.generator.order())
-                htlcsigs.append((htlc_tx, htlc_sig))
+                htlcsigs.append(htlc_sig)
 
         return sig_64, htlcsigs
 
@@ -141,8 +137,35 @@ class HTLCStateMachine(PrintError):
         state, then this newly added commitment becomes our current accepted channel
         state.
         """
+        from .lnbase import make_htlc_tx_with_open_channel , derive_pubkey
+
         self.print_error("receive_new_commitment")
-        # TODO
+        for htlc in self.remote_update_log:
+            if not type(htlc) is UpdateAddHtlc: continue
+            if htlc.r_locked_in is None: htlc.r_locked_in = self.state.remote_state.ctn
+        assert len(htlc_sigs) == 0 or type(htlc_sigs[0]) is bytes
+
+        assert len(htlc_sigs) == len(self.local_commitment.outputs()) - 2, (len(htlc_sigs), len(self.local_commitment.outputs()) - 2, self.diagnostic_name())
+
+        preimage_hex = self.local_commitment.serialize_preimage(0)
+        pre_hash = Hash(bfh(preimage_hex))
+        if not ecc.verify_signature(self.state.remote_config.multisig_key.pubkey, sig, pre_hash):
+            raise Exception('failed verifying signature of our updated commitment transaction: ' + str(sig))
+
+        _, this_point, _ = self.points
+
+        if len(self.htlcs_in_remote) > 0:
+            print("CHECKING HTLC SIGS")
+            assert len(self.local_commitment.outputs()) == 3 # TODO
+            we_receive = True # TODO
+            payment_hash = self.htlcs_in_remote[0].payment_hash
+            amount_msat = self.htlcs_in_remote[0].amount_msat
+            cltv_expiry = self.htlcs_in_remote[0].cltv_expiry
+            htlc_tx = make_htlc_tx_with_open_channel(self.state, this_point, True, we_receive, amount_msat, cltv_expiry, payment_hash, self.local_commitment, 0)
+            pre_hash = Hash(bfh(htlc_tx.serialize_preimage(0)))
+            remote_htlc_pubkey = derive_pubkey(self.state.remote_config.htlc_basepoint.pubkey, this_point)
+            if not ecc.verify_signature(remote_htlc_pubkey, htlc_sigs[0], pre_hash):
+                raise Exception("failed verifying signature an HTLC tx spending from one of our commit tx'es HTLC outputs")
 
     def revoke_current_commitment(self):
         """
@@ -155,10 +178,21 @@ class HTLCStateMachine(PrintError):
         transaction. This return value allows callers to act once an HTLC has been
         locked into our commitment transaction.
         """
-        from .lnbase import get_per_commitment_secret_from_seed, secret_to_pubkey
-        self.r_current_height += 1
         self.print_error("revoke_current_commitment")
 
+        last_secret, this_point, next_point = self.points
+
+        self.state = self.state._replace(
+            local_state=self.state.local_state._replace(
+                ctn=self.state.local_state.ctn + 1
+            )
+        )
+
+        return RevokeAndAck(self.state.local_state.ctn - 1, last_secret, next_point), "current htlcs"
+
+    @property
+    def points(self):
+        from .lnbase import get_per_commitment_secret_from_seed, secret_to_pubkey
         chan = self.state
         last_small_num = chan.local_state.ctn
         next_small_num = last_small_num + 2
@@ -168,13 +202,7 @@ class HTLCStateMachine(PrintError):
         this_point = secret_to_pubkey(int.from_bytes(this_secret, 'big'))
         next_secret = get_per_commitment_secret_from_seed(chan.local_state.per_commitment_secret_seed, 2**48-next_small_num-1)
         next_point = secret_to_pubkey(int.from_bytes(next_secret, 'big'))
-        self.state = chan._replace(
+        return last_secret, this_point, next_point
-            local_state=chan.local_state._replace(
-                ctn=chan.local_state.ctn + 1
-            )
-        )
-
-        return RevokeAndAck(self.r_current_height, last_secret, next_point), "current htlcs"
 
     def receive_revocation(self, revocation):
         """
@@ -196,13 +224,6 @@ class HTLCStateMachine(PrintError):
                 continue
             settle_fails2.append(x)
 
-        if revocation.height is not None:
-            adds2 = list(x for x in self.htlcs_in_remote if x.r_locked_in == revocation.height)
-
-            class FwdPkg:
-                adds = adds2
-                settle_fails = settle_fails2
-
         for x in settle_fails2:
             self.total_msat_sent += self.lookup_htlc(self.local_update_log, x.htlc_id).amount_msat
 
@@ -219,6 +240,9 @@ class HTLCStateMachine(PrintError):
         for x in filter(lambda x: type(x) is SettleHtlc, self.remote_update_log):
             to_remove += [y for y in self.local_update_log if y.htlc_id == x.htlc_id]
 
+        if to_remove != []:
+            print("REMOVING")
+
         # assert that we should have compacted the log earlier
         assert len(to_remove) <= 1, to_remove
         if len(to_remove) == 1:
@@ -246,11 +270,6 @@ class HTLCStateMachine(PrintError):
             )
         )
 
-        if revocation.height is not None:
-            return FwdPkg
-        else:
-            return None
-
     @staticmethod
     def htlcsum(htlcs):
         return sum(x.amount_msat for x in htlcs), sum(x.total_fee for x in htlcs)
@@ -295,18 +314,14 @@ class HTLCStateMachine(PrintError):
         from .lnbase import make_commitment_using_open_channel, make_received_htlc, make_offered_htlc, derive_pubkey, derive_blinded_pubkey, get_per_commitment_secret_from_seed, secret_to_pubkey
         htlc_value_local, total_fee_local = self.htlcsum(self.htlcs_in_local)
         htlc_value_remote, total_fee_remote = self.htlcsum(self.htlcs_in_remote)
-        print("htlc_value_local, total_fee_local", htlc_value_local, total_fee_local)
         local_msat = self.state.local_state.amount_msat -\
           htlc_value_local
-        print("htlc_value_remote, total_fee_remote", htlc_value_remote, total_fee_remote)
         remote_msat = self.state.remote_state.amount_msat -\
           htlc_value_remote
         assert local_msat > 0
         assert remote_msat > 0
 
-        this_small_num = self.state.local_state.ctn + 1
+        _, this_point, _ = self.points
-        this_secret = get_per_commitment_secret_from_seed(self.state.local_state.per_commitment_secret_seed, 2**48-this_small_num-1)
-        this_point = secret_to_pubkey(int.from_bytes(this_secret, 'big'))
 
         remote_htlc_pubkey = derive_pubkey(self.state.remote_config.htlc_basepoint.pubkey, this_point)
         local_htlc_pubkey = derive_pubkey(self.state.local_config.htlc_basepoint.pubkey, this_point)
@@ -315,13 +330,11 @@ class HTLCStateMachine(PrintError):
 
         htlcs_in_local = []
         for htlc in self.htlcs_in_local:
-            print("adding local htlc", htlc)
             htlcs_in_local.append(
                 ( make_offered_htlc(local_revocation_pubkey, local_htlc_pubkey, remote_htlc_pubkey, htlc.payment_hash), htlc.amount_msat + total_fee_local))
 
         htlcs_in_remote = []
         for htlc in self.htlcs_in_remote:
-            print("adding remote htlc", htlc)
             htlcs_in_remote.append(
                 ( make_received_htlc(remote_revocation_pubkey, remote_htlc_pubkey, local_htlc_pubkey, htlc.payment_hash, htlc.cltv_expiry), htlc.amount_msat + total_fee_remote))
 
@@ -338,10 +351,11 @@ class HTLCStateMachine(PrintError):
         for htlc in update_log:
             if type(htlc) is not UpdateAddHtlc:
                 continue
-            height = (self.r_current_height if subject == "remote" else self.l_current_height)
+            height = (self.state.local_state.ctn if subject == "remote" else self.state.remote_state.ctn)
-            locked_in = (htlc.r_locked_in   if subject == "remote" else htlc.l_locked_in)
+            locked_in = (htlc.r_locked_in if subject == "remote" else htlc.l_locked_in)
 
-            if locked_in is None or locked_in < height:
+            if locked_in is None:
+                print("skipping", locked_in, height)
                 continue
             res.append(htlc)
         return res

lib/tests/test_lnbase.py

@@ -156,7 +156,7 @@ class Test_LNBase(unittest.TestCase):
             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,
-            local_feerate_per_kw, True, htlcs=htlcs)
+            local_feerate_per_kw, True, we_are_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)
 

lib/tests/test_lnhtlc.py

@@ -154,9 +154,7 @@ class TestLNBaseHTLCStateMachine(unittest.TestCase):
         # Alice then processes this revocation, sending her own revocation for
         # her prior commitment transaction. Alice shouldn't have any HTLCs to
         # forward since she's sending an outgoing HTLC.
-        fwdPkg = alice_channel.receive_revocation(bobRevocation)
+        alice_channel.receive_revocation(bobRevocation)
-        self.assertEqual(fwdPkg.adds, [], "alice forwards %s add htlcs, should forward none"% len(fwdPkg.adds))
-        self.assertEqual(fwdPkg.settle_fails, [], "alice forwards %s settle/fail htlcs, should forward none"% len(fwdPkg.settle_fails))
 
         # Alice then processes bob's signature, and since she just received
         # the revocation, she expect this signature to cover everything up to
@@ -170,9 +168,7 @@ class TestLNBaseHTLCStateMachine(unittest.TestCase):
         # is fully locked in within both commitment transactions. Bob should
         # also be able to forward an HTLC now that the HTLC has been locked
         # into both commitment transactions.
-        fwdPkg = bob_channel.receive_revocation(aliceRevocation)
+        bob_channel.receive_revocation(aliceRevocation)
-        self.assertEqual(len(fwdPkg.adds), 1, "bob forwards %s add htlcs, should only forward one"% len(fwdPkg.adds))
-        self.assertEqual(fwdPkg.settle_fails, [], "bob forwards %s settle/fail htlcs, should forward none"% len(fwdPkg.settle_fails))
 
         # At this point, both sides should have the proper number of satoshis
         # sent, and commitment height updated within their local channel
@@ -184,8 +180,8 @@ class TestLNBaseHTLCStateMachine(unittest.TestCase):
         self.assertEqual(alice_channel.total_msat_received, bobSent, "alice has incorrect milli-satoshis received %s vs %s"% (alice_channel.total_msat_received, bobSent))
         self.assertEqual(bob_channel.total_msat_sent, bobSent, "bob has incorrect milli-satoshis sent %s vs %s"% (bob_channel.total_msat_sent, bobSent))
         self.assertEqual(bob_channel.total_msat_received, aliceSent, "bob has incorrect milli-satoshis received %s vs %s"% (bob_channel.total_msat_received, aliceSent))
-        self.assertEqual(bob_channel.l_current_height, 1, "bob has incorrect commitment height, %s vs %s"% (bob_channel.l_current_height, 1))
+        self.assertEqual(bob_channel.state.local_state.ctn, 1, "bob has incorrect commitment height, %s vs %s"% (bob_channel.state.local_state.ctn, 1))
-        self.assertEqual(alice_channel.l_current_height, 1, "alice has incorrect commitment height, %s vs %s"% (alice_channel.l_current_height, 1))
+        self.assertEqual(alice_channel.state.local_state.ctn, 1, "alice has incorrect commitment height, %s vs %s"% (alice_channel.state.local_state.ctn, 1))
 
         # Both commitment transactions should have three outputs, and one of
         # them should be exactly the amount of the HTLC.
@@ -207,18 +203,12 @@ class TestLNBaseHTLCStateMachine(unittest.TestCase):
         aliceRevocation2, _ = alice_channel.revoke_current_commitment()
         aliceSig2, aliceHtlcSigs2 = alice_channel.sign_next_commitment()
 
-        fwdPkg = bob_channel.receive_revocation(aliceRevocation2)
+        bob_channel.receive_revocation(aliceRevocation2)
-        self.assertEqual(fwdPkg.adds, [], "bob forwards %s add htlcs, should forward none"% len(fwdPkg.adds))
-        self.assertEqual(fwdPkg.settle_fails, [], "bob forwards %s settle/fail htlcs, should forward none"% len(fwdPkg.settle_fails))
 
         bob_channel.receive_new_commitment(aliceSig2, aliceHtlcSigs2)
 
         bobRevocation2, _ = bob_channel.revoke_current_commitment()
-        fwdPkg = alice_channel.receive_revocation(bobRevocation2)
+        alice_channel.receive_revocation(bobRevocation2)
-        # Alice should now be able to forward the settlement HTLC to
-        # any down stream peers.
-        self.assertEqual(fwdPkg.adds, [] , "alice should be forwarding an add HTLC, instead forwarding %s: %s"% (len(fwdPkg.adds), fwdPkg.adds))
-        self.assertEqual(len(fwdPkg.settle_fails), 1, "alice should be forwarding one settle/fails HTLC, instead forwarding: %s"% len(fwdPkg.settle_fails))
 
         # At this point, Bob should have 6 BTC settled, with Alice still having
         # 4 BTC. Alice's channel should show 1 BTC sent and Bob's channel