lnhtlc: fee update upgrade and passes ReciverCommits and SenderCommits tests, fix NameErrors in lnbase

7 years ago · 03c2b954d9
4 changed files with 245 additions and 147 deletions
--- a/lib/lnbase.py
+++ b/lib/lnbase.py
@ -596,7 +596,8 @@ class Peer(PrintError):
                    current_per_commitment_point=None,
                    amount_msat=remote_amount,
                    revocation_store=their_revocation_store,
-                    next_htlc_id = 0
+                    next_htlc_id = 0,
+                    feerate=local_feerate
                ),
                "local_state": LocalState(
                    ctn = -1,
@ -605,9 +606,10 @@ class Peer(PrintError):
                    next_htlc_id = 0,
                    funding_locked_received = False,
                    was_announced = False,
-                    current_commitment_signature = None
+                    current_commitment_signature = None,
+                    feerate=local_feerate
                ),
-                "constraints": ChannelConstraints(capacity=funding_sat, feerate=local_feerate, is_initiator=True, funding_txn_minimum_depth=funding_txn_minimum_depth)
+                "constraints": ChannelConstraints(capacity=funding_sat, is_initiator=True, funding_txn_minimum_depth=funding_txn_minimum_depth)
        }
        m = HTLCStateMachine(chan)
        sig_64, _ = m.sign_next_commitment()
@ -942,13 +944,13 @@ class Peer(PrintError):

        await self.receive_revoke(chan)

-        m.settle_htlc(payment_preimage, htlc_id)
+        chan.settle_htlc(payment_preimage, htlc_id)
        self.send_message(gen_msg("update_fulfill_htlc", channel_id=channel_id, id=htlc_id, payment_preimage=payment_preimage))

        # remote commitment transaction without htlcs
-        bare_ctx = chan.make_commitment(m.remote_state.ctn + 1, False, m.remote_state.next_per_commitment_point,
-            m.remote_state.amount_msat - expected_received_msat, m.local_state.amount_msat + expected_received_msat)
-        sig_64 = sign_and_get_sig_string(bare_ctx, m.local_config, m.remote_config)
+        bare_ctx = chan.make_commitment(chan.remote_state.ctn + 1, False, chan.remote_state.next_per_commitment_point,
+            chan.remote_state.amount_msat - expected_received_msat, chan.local_state.amount_msat + expected_received_msat)
+        sig_64 = sign_and_get_sig_string(bare_ctx, chan.local_config, chan.remote_config)
        self.send_message(gen_msg("commitment_signed", channel_id=channel_id, signature=sig_64, num_htlcs=0))

        await self.receive_revoke(chan)
--- a/lib/lnhtlc.py
+++ b/lib/lnhtlc.py
@ -12,10 +12,36 @@ from .lnutil import secret_to_pubkey, derive_privkey, derive_pubkey, derive_blin
 from .lnutil import sign_and_get_sig_string
 from .lnutil import make_htlc_tx_with_open_channel, make_commitment, make_received_htlc, make_offered_htlc
 from .lnutil import HTLC_TIMEOUT_WEIGHT, HTLC_SUCCESS_WEIGHT
+from contextlib import contextmanager

 SettleHtlc = namedtuple("SettleHtlc", ["htlc_id"])
 RevokeAndAck = namedtuple("RevokeAndAck", ["per_commitment_secret", "next_per_commitment_point"])

+@contextmanager
+def PendingFeerateApplied(machine):
+    old_local_state = machine.local_state
+    old_remote_state = machine.remote_state
+
+    new_local_feerate = machine.local_state.feerate
+    new_remote_feerate = machine.remote_state.feerate
+
+    if machine.constraints.is_initiator:
+        if machine.pending_fee_update is not None:
+            new_remote_feerate = machine.pending_fee_update
+        if machine.pending_ack_fee_update is not None:
+            new_local_feerate = machine.pending_ack_fee_update
+    else:
+        if machine.pending_fee_update is not None:
+            new_local_feerate = machine.pending_fee_update
+        if machine.pending_ack_fee_update is not None:
+            new_remote_feerate = machine.pending_ack_fee_update
+
+    machine.local_state = machine.local_state._replace(feerate=new_local_feerate)
+    machine.remote_state = machine.remote_state._replace(feerate=new_remote_feerate)
+    yield
+    machine.local_state = old_local_state._replace(feerate=old_local_state.feerate)
+    machine.remote_state = old_remote_state._replace(feerate=old_remote_state.feerate)
+
 class UpdateAddHtlc:
    def __init__(self, amount_msat, payment_hash, cltv_expiry, total_fee):
        self.amount_msat = amount_msat
@ -107,7 +133,11 @@ class HTLCStateMachine(PrintError):

        self.total_msat_sent = 0
        self.total_msat_received = 0
-        self.pending_feerate = None
+        self.pending_fee_update = None
+        self.pending_ack_fee_update = None
+
+        self.local_commitment = self.pending_local_commitment
+        self.remote_commitment = self.pending_remote_commitment

    def add_htlc(self, htlc):
        """
@ -154,30 +184,35 @@ class HTLCStateMachine(PrintError):
            if htlc.l_locked_in is None: htlc.l_locked_in = self.local_state.ctn
        self.print_error("sign_next_commitment")

-        sig_64 = sign_and_get_sig_string(self.remote_commitment, self.local_config, self.remote_config)
+        if self.constraints.is_initiator and self.pending_fee_update:
+            self.pending_ack_fee_update = self.pending_fee_update
+            self.pending_fee_update = None

-        their_remote_htlc_privkey_number = derive_privkey(
-            int.from_bytes(self.local_config.htlc_basepoint.privkey, 'big'),
-            self.remote_state.next_per_commitment_point)
-        their_remote_htlc_privkey = their_remote_htlc_privkey_number.to_bytes(32, 'big')
+        with PendingFeerateApplied(self):
+            sig_64 = sign_and_get_sig_string(self.pending_remote_commitment, self.local_config, self.remote_config)

-        for_us = False
+            their_remote_htlc_privkey_number = derive_privkey(
+                int.from_bytes(self.local_config.htlc_basepoint.privkey, 'big'),
+                self.remote_state.next_per_commitment_point)
+            their_remote_htlc_privkey = their_remote_htlc_privkey_number.to_bytes(32, 'big')

-        htlcsigs = []
-        for we_receive, htlcs in zip([True, False], [self.htlcs_in_remote, self.htlcs_in_local]):
-            assert len(htlcs) <= 1
-            for htlc in htlcs:
-                weight = HTLC_SUCCESS_WEIGHT if we_receive else HTLC_TIMEOUT_WEIGHT
-                if htlc.amount_msat // 1000 - weight * (self.constraints.feerate // 1000) < self.remote_config.dust_limit_sat:
-                    continue
-                original_htlc_output_index = 0
-                args = [self.remote_state.next_per_commitment_point, for_us, we_receive, htlc.amount_msat + htlc.total_fee, htlc.cltv_expiry, htlc.payment_hash, self.remote_commitment, original_htlc_output_index]
-                htlc_tx = make_htlc_tx_with_open_channel(self, *args)
-                sig = bfh(htlc_tx.sign_txin(0, their_remote_htlc_privkey))
-                htlc_sig = ecc.sig_string_from_der_sig(sig[:-1])
-                htlcsigs.append(htlc_sig)
+            for_us = False

-        return sig_64, htlcsigs
+            htlcsigs = []
+            for we_receive, htlcs in zip([True, False], [self.htlcs_in_remote, self.htlcs_in_local]):
+                assert len(htlcs) <= 1
+                for htlc in htlcs:
+                    weight = HTLC_SUCCESS_WEIGHT if we_receive else HTLC_TIMEOUT_WEIGHT
+                    if htlc.amount_msat // 1000 - weight * (self.remote_state.feerate // 1000) < self.remote_config.dust_limit_sat:
+                        continue
+                    original_htlc_output_index = 0
+                    args = [self.remote_state.next_per_commitment_point, for_us, we_receive, htlc.amount_msat + htlc.total_fee, htlc.cltv_expiry, htlc.payment_hash, self.pending_remote_commitment, original_htlc_output_index]
+                    htlc_tx = make_htlc_tx_with_open_channel(self, *args)
+                    sig = bfh(htlc_tx.sign_txin(0, their_remote_htlc_privkey))
+                    htlc_sig = ecc.sig_string_from_der_sig(sig[:-1])
+                    htlcsigs.append(htlc_sig)
+
+            return sig_64, htlcsigs

    def receive_new_commitment(self, sig, htlc_sigs):
        """
@ -197,26 +232,31 @@ class HTLCStateMachine(PrintError):
            if htlc.r_locked_in is None: htlc.r_locked_in = self.remote_state.ctn
        assert len(htlc_sigs) == 0 or type(htlc_sigs[0]) is bytes

-        preimage_hex = self.local_commitment.serialize_preimage(0)
-        pre_hash = Hash(bfh(preimage_hex))
-        if not ecc.verify_signature(self.remote_config.multisig_key.pubkey, sig, pre_hash):
-            raise Exception('failed verifying signature of our updated commitment transaction: ' + str(sig))
+        if not self.constraints.is_initiator:
+            self.pending_ack_fee_update = self.pending_fee_update
+            self.pending_fee_update = None

-        _, this_point, _ = self.points
+        with PendingFeerateApplied(self):
+            preimage_hex = self.pending_local_commitment.serialize_preimage(0)
+            pre_hash = Hash(bfh(preimage_hex))
+            if not ecc.verify_signature(self.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 and len(self.local_commitment.outputs()) == 3:
-            print("CHECKING HTLC SIGS")
-            we_receive = True
-            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, 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.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")
+            if len(self.htlcs_in_remote) > 0 and len(self.pending_local_commitment.outputs()) == 3:
+                print("CHECKING HTLC SIGS")
+                we_receive = True
+                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, this_point, True, we_receive, amount_msat, cltv_expiry, payment_hash, self.pending_local_commitment, 0)
+                pre_hash = Hash(bfh(htlc_tx.serialize_preimage(0)))
+                remote_htlc_pubkey = derive_pubkey(self.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")

-        # TODO check htlc in htlcs_in_local
+            # TODO check htlc in htlcs_in_local

    def revoke_current_commitment(self):
        """
@ -233,18 +273,28 @@ class HTLCStateMachine(PrintError):

        last_secret, this_point, next_point = self.points

-        if self.pending_feerate is not None:
-            new_feerate = self.pending_feerate
-        else:
-            new_feerate = self.constraints.feerate
+        new_feerate = self.local_state.feerate

-        self.local_state=self.local_state._replace(
-            ctn=self.local_state.ctn + 1
+        if not self.constraints.is_initiator and self.pending_fee_update is not None:
+            new_feerate = self.pending_fee_update
+            self.pending_fee_update = None
+            self.pending_ack_fee_update = None
+        elif self.pending_ack_fee_update is not None:
+            new_feerate = self.pending_ack_fee_update
+            self.pending_fee_update = None
+            self.pending_ack_fee_update = None
+
+        self.remote_state=self.remote_state._replace(
+            feerate=new_feerate
        )
-        self.constraints=self.constraints._replace(
+
+        self.local_state=self.local_state._replace(
+            ctn=self.local_state.ctn + 1,
            feerate=new_feerate
        )

+        self.local_commitment = self.pending_local_commitment
+
        return RevokeAndAck(last_secret, next_point), "current htlcs"

    @property
@ -322,11 +372,14 @@ class HTLCStateMachine(PrintError):
            ctn=self.remote_state.ctn + 1,
            current_per_commitment_point=next_point,
            next_per_commitment_point=revocation.next_per_commitment_point,
-            amount_msat=self.remote_state.amount_msat + (sent_this_batch - received_this_batch) + sent_fees - received_fees
+            amount_msat=self.remote_state.amount_msat + (sent_this_batch - received_this_batch) + sent_fees - received_fees,
+            feerate=self.pending_fee_update if self.pending_fee_update is not None else self.remote_state.feerate
        )
        self.local_state=self.local_state._replace(
            amount_msat = self.local_state.amount_msat + (received_this_batch - sent_this_batch) - sent_fees + received_fees
        )
+        self.local_commitment = self.pending_local_commitment
+        self.remote_commitment = self.pending_remote_commitment

    @staticmethod
    def htlcsum(htlcs):
@ -351,7 +404,7 @@ class HTLCStateMachine(PrintError):
        return remote_msat, total_fee_remote, local_msat, total_fee_local

    @property
-    def remote_commitment(self):
+    def pending_remote_commitment(self):
        remote_msat, total_fee_remote, local_msat, total_fee_local = self.amounts()
        assert local_msat >= 0
        assert remote_msat >= 0
@ -364,29 +417,30 @@ class HTLCStateMachine(PrintError):

        trimmed = 0

-        htlcs_in_local = []
-        for htlc in self.htlcs_in_local:
-            if htlc.amount_msat // 1000 - HTLC_SUCCESS_WEIGHT * (self.constraints.feerate // 1000) < self.remote_config.dust_limit_sat:
-                trimmed += htlc.amount_msat // 1000
-                continue
-            htlcs_in_local.append(
-                ( make_received_htlc(local_revocation_pubkey, local_htlc_pubkey, remote_htlc_pubkey, htlc.payment_hash, htlc.cltv_expiry), htlc.amount_msat + htlc.total_fee))
+        with PendingFeerateApplied(self):
+            htlcs_in_local = []
+            for htlc in self.htlcs_in_local:
+                if htlc.amount_msat // 1000 - HTLC_SUCCESS_WEIGHT * (self.remote_state.feerate // 1000) < self.remote_config.dust_limit_sat:
+                    trimmed += htlc.amount_msat // 1000
+                    continue
+                htlcs_in_local.append(
+                    ( make_received_htlc(local_revocation_pubkey, local_htlc_pubkey, remote_htlc_pubkey, htlc.payment_hash, htlc.cltv_expiry), htlc.amount_msat + htlc.total_fee))

-        htlcs_in_remote = []
-        for htlc in self.htlcs_in_remote:
-            if htlc.amount_msat // 1000 - HTLC_TIMEOUT_WEIGHT * (self.constraints.feerate // 1000) < self.remote_config.dust_limit_sat:
-                trimmed += htlc.amount_msat // 1000
-                continue
-            htlcs_in_remote.append(
-                ( make_offered_htlc(local_revocation_pubkey, local_htlc_pubkey, remote_htlc_pubkey, htlc.payment_hash), htlc.amount_msat + htlc.total_fee))
+            htlcs_in_remote = []
+            for htlc in self.htlcs_in_remote:
+                if htlc.amount_msat // 1000 - HTLC_TIMEOUT_WEIGHT * (self.remote_state.feerate // 1000) < self.remote_config.dust_limit_sat:
+                    trimmed += htlc.amount_msat // 1000
+                    continue
+                htlcs_in_remote.append(
+                    ( make_offered_htlc(local_revocation_pubkey, local_htlc_pubkey, remote_htlc_pubkey, htlc.payment_hash), htlc.amount_msat + htlc.total_fee))

-        commit = self.make_commitment(self.remote_state.ctn + 1,
-            False, this_point,
-            remote_msat - total_fee_remote, local_msat - total_fee_local, htlcs_in_local + htlcs_in_remote, trimmed)
-        return commit
+            commit = self.make_commitment(self.remote_state.ctn + 1,
+                False, this_point,
+                remote_msat - total_fee_remote, local_msat - total_fee_local, htlcs_in_local + htlcs_in_remote, trimmed)
+            return commit

    @property
-    def local_commitment(self):
+    def pending_local_commitment(self):
        remote_msat, total_fee_remote, local_msat, total_fee_local = self.amounts()
        assert local_msat >= 0
        assert remote_msat >= 0
@ -399,26 +453,27 @@ class HTLCStateMachine(PrintError):

        trimmed = 0

-        htlcs_in_local = []
-        for htlc in self.htlcs_in_local:
-            if htlc.amount_msat // 1000 - HTLC_TIMEOUT_WEIGHT * (self.constraints.feerate // 1000) < self.local_config.dust_limit_sat:
-                trimmed += htlc.amount_msat // 1000
-                continue
-            htlcs_in_local.append(
-                ( make_offered_htlc(remote_revocation_pubkey, remote_htlc_pubkey, local_htlc_pubkey, htlc.payment_hash), htlc.amount_msat + htlc.total_fee))
+        with PendingFeerateApplied(self):
+            htlcs_in_local = []
+            for htlc in self.htlcs_in_local:
+                if htlc.amount_msat // 1000 - HTLC_TIMEOUT_WEIGHT * (self.local_state.feerate // 1000) < self.local_config.dust_limit_sat:
+                    trimmed += htlc.amount_msat // 1000
+                    continue
+                htlcs_in_local.append(
+                    ( make_offered_htlc(remote_revocation_pubkey, remote_htlc_pubkey, local_htlc_pubkey, htlc.payment_hash), htlc.amount_msat + htlc.total_fee))

-        htlcs_in_remote = []
-        for htlc in self.htlcs_in_remote:
-            if htlc.amount_msat // 1000 - HTLC_SUCCESS_WEIGHT * (self.constraints.feerate // 1000) < self.local_config.dust_limit_sat:
-                trimmed += htlc.amount_msat // 1000
-                continue
-            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 + htlc.total_fee))
+            htlcs_in_remote = []
+            for htlc in self.htlcs_in_remote:
+                if htlc.amount_msat // 1000 - HTLC_SUCCESS_WEIGHT * (self.local_state.feerate // 1000) < self.local_config.dust_limit_sat:
+                    trimmed += htlc.amount_msat // 1000
+                    continue
+                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 + htlc.total_fee))

-        commit = self.make_commitment(self.local_state.ctn + 1,
-            True, this_point,
-            local_msat - total_fee_local, remote_msat - total_fee_remote, htlcs_in_local + htlcs_in_remote, trimmed)
-        return commit
+            commit = self.make_commitment(self.local_state.ctn + 1,
+                True, this_point,
+                local_msat - total_fee_local, remote_msat - total_fee_remote, htlcs_in_local + htlcs_in_remote, trimmed)
+            return commit

    def gen_htlc_indices(self, subject, just_unsettled=True):
        assert subject in ["local", "remote"]
@ -479,10 +534,14 @@ class HTLCStateMachine(PrintError):
        return self.constraints.capacity - sum(x[2] for x in self.local_commitment.outputs())

    def update_fee(self, fee):
-        self.pending_feerate = fee
+        if not self.constraints.is_initiator:
+            raise Exception("only initiator can update_fee, this counterparty is not initiator")
+        self.pending_fee_update = fee

    def receive_update_fee(self, fee):
-        self.pending_feerate = fee
+        if self.constraints.is_initiator:
+            raise Exception("only the non-initiator can receive_update_fee, this counterparty is initiator")
+        self.pending_fee_update = fee

    def to_save(self):
        return {
@ -538,7 +597,7 @@ class HTLCStateMachine(PrintError):
            local_msat,
            remote_msat,
            chan.local_config.dust_limit_sat,
-            chan.constraints.feerate,
+            chan.local_state.feerate if for_us else chan.remote_state.feerate,
            for_us,
            chan.constraints.is_initiator,
            htlcs=htlcs,
--- a/lib/lnutil.py
+++ b/lib/lnutil.py
@ -17,9 +17,9 @@ ChannelConfig = namedtuple("ChannelConfig", [
    "payment_basepoint", "multisig_key", "htlc_basepoint", "delayed_basepoint", "revocation_basepoint",
    "to_self_delay", "dust_limit_sat", "max_htlc_value_in_flight_msat", "max_accepted_htlcs"])
 OnlyPubkeyKeypair = namedtuple("OnlyPubkeyKeypair", ["pubkey"])
-RemoteState = namedtuple("RemoteState", ["ctn", "next_per_commitment_point", "amount_msat", "revocation_store", "current_per_commitment_point", "next_htlc_id"])
-LocalState = namedtuple("LocalState", ["ctn", "per_commitment_secret_seed", "amount_msat", "next_htlc_id", "funding_locked_received", "was_announced", "current_commitment_signature"])
-ChannelConstraints = namedtuple("ChannelConstraints", ["feerate", "capacity", "is_initiator", "funding_txn_minimum_depth"])
+RemoteState = namedtuple("RemoteState", ["ctn", "next_per_commitment_point", "amount_msat", "revocation_store", "current_per_commitment_point", "next_htlc_id", "feerate"])
+LocalState = namedtuple("LocalState", ["ctn", "per_commitment_secret_seed", "amount_msat", "next_htlc_id", "funding_locked_received", "was_announced", "current_commitment_signature", "feerate"])
+ChannelConstraints = namedtuple("ChannelConstraints", ["capacity", "is_initiator", "funding_txn_minimum_depth"])
 #OpenChannel = namedtuple("OpenChannel", ["channel_id", "short_channel_id", "funding_outpoint", "local_config", "remote_config", "remote_state", "local_state", "constraints", "node_id"])

 class RevocationStore:
@ -201,7 +201,7 @@ def make_htlc_tx_with_open_channel(chan, pcp, for_us, we_receive, amount_msat, c
    is_htlc_success = for_us == we_receive
    htlc_tx_output = make_htlc_tx_output(
        amount_msat = amount_msat,
-        local_feerate = chan.constraints.feerate,
+        local_feerate = chan.local_state.feerate if for_us else chan.remote_state.feerate,
        revocationpubkey=revocation_pubkey,
        local_delayedpubkey=delayedpubkey,
        success = is_htlc_success,
--- a/lib/tests/test_lnhtlc.py
+++ b/lib/tests/test_lnhtlc.py
@ -49,7 +49,8 @@ def create_channel_state(funding_txid, funding_index, funding_sat, local_feerate
                current_per_commitment_point=cur,
                amount_msat=remote_amount,
                revocation_store=their_revocation_store,
-                next_htlc_id = 0
+                next_htlc_id = 0,
+                feerate=local_feerate
            ),
            "local_state":lnbase.LocalState(
                ctn = 0,
@ -58,9 +59,10 @@ def create_channel_state(funding_txid, funding_index, funding_sat, local_feerate
                next_htlc_id = 0,
                funding_locked_received=True,
                was_announced=False,
-                current_commitment_signature=None
+                current_commitment_signature=None,
+                feerate=local_feerate
            ),
-            "constraints":lnbase.ChannelConstraints(capacity=funding_sat, feerate=local_feerate, is_initiator=is_initiator, funding_txn_minimum_depth=3),
+            "constraints":lnbase.ChannelConstraints(capacity=funding_sat, is_initiator=is_initiator, funding_txn_minimum_depth=3),
            "node_id":other_node_id
    }

@ -109,19 +111,17 @@ class TestLNBaseHTLCStateMachine(unittest.TestCase):
        else:
            self.assertFalse()

-    def test_SimpleAddSettleWorkflow(self):
-
+    def setUp(self):
        # Create a test channel which will be used for the duration of this
        # unittest. The channel will be funded evenly with Alice having 5 BTC,
        # and Bob having 5 BTC.
-        alice_channel, bob_channel = create_test_channels()
+        self.alice_channel, self.bob_channel = create_test_channels()

-        paymentPreimage = b"\x01" * 32
-        paymentHash = bitcoin.sha256(paymentPreimage)
-        htlcAmt = one_bitcoin_in_msat
-        htlc = lnhtlc.UpdateAddHtlc(
+        self.paymentPreimage = b"\x01" * 32
+        paymentHash = bitcoin.sha256(self.paymentPreimage)
+        self.htlc = lnhtlc.UpdateAddHtlc(
            payment_hash = paymentHash,
-            amount_msat =  htlcAmt,
+            amount_msat =  one_bitcoin_in_msat,
            cltv_expiry =  5,
            total_fee = 0
        )
@ -129,9 +129,13 @@ class TestLNBaseHTLCStateMachine(unittest.TestCase):
        # First Alice adds the outgoing HTLC to her local channel's state
        # update log. Then Alice sends this wire message over to Bob who adds
        # this htlc to his remote state update log.
-        aliceHtlcIndex = alice_channel.add_htlc(htlc)
+        self.aliceHtlcIndex = self.alice_channel.add_htlc(self.htlc)

-        bobHtlcIndex = bob_channel.receive_htlc(htlc)
+        self.bobHtlcIndex = self.bob_channel.receive_htlc(self.htlc)
+
+    def test_SimpleAddSettleWorkflow(self):
+        alice_channel, bob_channel = self.alice_channel, self.bob_channel
+        htlc = self.htlc

        # Next alice commits this change by sending a signature message. Since
        # we expect the messages to be ordered, Bob will receive the HTLC we
@ -192,15 +196,15 @@ class TestLNBaseHTLCStateMachine(unittest.TestCase):
        # them should be exactly the amount of the HTLC.
        self.assertEqual(len(alice_channel.local_commitment.outputs()), 3, "alice should have three commitment outputs, instead have %s"% len(alice_channel.local_commitment.outputs()))
        self.assertEqual(len(bob_channel.local_commitment.outputs()), 3, "bob should have three commitment outputs, instead have %s"% len(bob_channel.local_commitment.outputs()))
-        self.assertOutputExistsByValue(alice_channel.local_commitment, htlcAmt // 1000)
-        self.assertOutputExistsByValue(bob_channel.local_commitment, htlcAmt // 1000)
+        self.assertOutputExistsByValue(alice_channel.local_commitment, htlc.amount_msat // 1000)
+        self.assertOutputExistsByValue(bob_channel.local_commitment, htlc.amount_msat // 1000)

        # Now we'll repeat a similar exchange, this time with Bob settling the
        # HTLC once he learns of the preimage.
-        preimage = paymentPreimage
-        bob_channel.settle_htlc(preimage, bobHtlcIndex)
+        preimage = self.paymentPreimage
+        bob_channel.settle_htlc(preimage, self.bobHtlcIndex)

-        alice_channel.receive_htlc_settle(preimage, aliceHtlcIndex)
+        alice_channel.receive_htlc_settle(preimage, self.aliceHtlcIndex)

        bobSig2, bobHtlcSigs2 = bob_channel.sign_next_commitment()
        alice_channel.receive_new_commitment(bobSig2, bobHtlcSigs2)
@ -234,12 +238,71 @@ class TestLNBaseHTLCStateMachine(unittest.TestCase):
        self.assertEqual(alice_channel.local_update_log, [], "alice's local not updated, should be empty, has %s entries instead"% len(alice_channel.local_update_log))
        self.assertEqual(alice_channel.remote_update_log, [], "alice's remote not updated, should be empty, has %s entries instead"% len(alice_channel.remote_update_log))

+    def alice_to_bob_fee_update(self):
+        fee = 111
+        self.alice_channel.update_fee(fee)
+        self.bob_channel.receive_update_fee(fee)
+        return fee
+
+    def test_UpdateFeeSenderCommits(self):
+        fee = self.alice_to_bob_fee_update()
+
+        alice_channel, bob_channel = self.alice_channel, self.bob_channel
+
+        alice_sig, alice_htlc_sigs = alice_channel.sign_next_commitment()
+        bob_channel.receive_new_commitment(alice_sig, alice_htlc_sigs)
+
+        self.assertNotEqual(fee, bob_channel.local_state.feerate)
+        rev, _ = bob_channel.revoke_current_commitment()
+        self.assertEqual(fee, bob_channel.local_state.feerate)
+
+        bob_sig, bob_htlc_sigs = bob_channel.sign_next_commitment()
+        alice_channel.receive_revocation(rev)
+        alice_channel.receive_new_commitment(bob_sig, bob_htlc_sigs)
+
+        self.assertNotEqual(fee, alice_channel.local_state.feerate)
+        rev, _ = alice_channel.revoke_current_commitment()
+        self.assertEqual(fee, alice_channel.local_state.feerate)
+
+        bob_channel.receive_revocation(rev)
+
+
+    def test_UpdateFeeReceiverCommits(self):
+        fee = self.alice_to_bob_fee_update()
+
+        alice_channel, bob_channel = self.alice_channel, self.bob_channel
+
+        bob_sig, bob_htlc_sigs = bob_channel.sign_next_commitment()
+        alice_channel.receive_new_commitment(bob_sig, bob_htlc_sigs)
+
+        alice_revocation, _ = alice_channel.revoke_current_commitment()
+        bob_channel.receive_revocation(alice_revocation)
+        alice_sig, alice_htlc_sigs = alice_channel.sign_next_commitment()
+        bob_channel.receive_new_commitment(alice_sig, alice_htlc_sigs)
+
+        self.assertNotEqual(fee, bob_channel.local_state.feerate)
+        bob_revocation, _ = bob_channel.revoke_current_commitment()
+        self.assertEqual(fee, bob_channel.local_state.feerate)
+
+        bob_sig, bob_htlc_sigs = bob_channel.sign_next_commitment()
+        alice_channel.receive_revocation(bob_revocation)
+        alice_channel.receive_new_commitment(bob_sig, bob_htlc_sigs)
+
+        self.assertNotEqual(fee, alice_channel.local_state.feerate)
+        alice_revocation, _ = alice_channel.revoke_current_commitment()
+        self.assertEqual(fee, alice_channel.local_state.feerate)
+
+        bob_channel.receive_revocation(alice_revocation)
+
+
+
+class TestLNHTLCDust(unittest.TestCase):
    def test_HTLCDustLimit(self):
        alice_channel, bob_channel = create_test_channels()

        paymentPreimage = b"\x01" * 32
        paymentHash = bitcoin.sha256(paymentPreimage)
-        fee_per_kw = alice_channel.constraints.feerate
+        fee_per_kw = alice_channel.local_state.feerate
        self.assertEqual(fee_per_kw, 6000)
        htlcAmt = 500 + lnutil.HTLC_TIMEOUT_WEIGHT * (fee_per_kw // 1000)
        self.assertEqual(htlcAmt, 4478)
@ -263,32 +326,6 @@ class TestLNBaseHTLCStateMachine(unittest.TestCase):
        self.assertEqual(len(alice_channel.local_commitment.outputs()), 2)
        self.assertEqual(alice_channel.total_msat_sent // 1000, htlcAmt)

-    def test_UpdateFeeSenderCommits(self):
-        alice_channel, bob_channel = create_test_channels()
-
-        paymentPreimage = b"\x01" * 32
-        paymentHash = bitcoin.sha256(paymentPreimage)
-        htlc = lnhtlc.UpdateAddHtlc(
-            payment_hash = paymentHash,
-            amount_msat =  one_bitcoin_in_msat,
-            cltv_expiry =  5, # also in create_test_channels
-            total_fee = 0
-        )
-
-        aliceHtlcIndex = alice_channel.add_htlc(htlc)
-        bobHtlcIndex = bob_channel.receive_htlc(htlc)
-
-        fee = 111
-        alice_channel.update_fee(fee)
-        bob_channel.receive_update_fee(fee)
-
-        alice_sig, alice_htlc_sigs = alice_channel.sign_next_commitment()
-        bob_channel.receive_new_commitment(alice_sig, alice_htlc_sigs)
-        self.assertNotEqual(fee, alice_channel.constraints.feerate)
-        rev, _ = alice_channel.revoke_current_commitment()
-        self.assertEqual(fee, alice_channel.constraints.feerate)
-        bob_channel.receive_revocation(rev)
-
 def force_state_transition(chanA, chanB):
    chanB.receive_new_commitment(*chanA.sign_next_commitment())
    rev, _ = chanB.revoke_current_commitment()
@ -301,7 +338,7 @@ def force_state_transition(chanA, chanB):
 # function provides a simple way to allow test balance assertions to take fee
 # calculations into account.
 def calc_static_fee(numHTLCs):
-  commitWeight = 724
-  htlcWeight   = 172
-  feePerKw     = 24//4 * 1000
-  return feePerKw * (commitWeight + htlcWeight*numHTLCs) // 1000
+    commitWeight = 724
+    htlcWeight   = 172
+    feePerKw     = 24//4 * 1000
+    return feePerKw * (commitWeight + htlcWeight*numHTLCs) // 1000