From 3d7f7dfc8267b319ae2277061cb24e0b728da084 Mon Sep 17 00:00:00 2001 From: ThomasV Date: Tue, 23 Jul 2019 19:23:39 +0200 Subject: [PATCH] revamp fee updates (draft) --- electrum/lnchannel.py | 140 +++++++++++++++---------------- electrum/lnpeer.py | 27 +++--- electrum/lnutil.py | 6 +- electrum/tests/test_lnchannel.py | 40 +++++---- 4 files changed, 109 insertions(+), 104 deletions(-) diff --git a/electrum/lnchannel.py b/electrum/lnchannel.py index be49c89db..46dc06078 100644 --- a/electrum/lnchannel.py +++ b/electrum/lnchannel.py @@ -46,6 +46,7 @@ from .lnutil import (Outpoint, LocalConfig, RemoteConfig, Keypair, OnlyPubkeyKey HTLC_TIMEOUT_WEIGHT, HTLC_SUCCESS_WEIGHT, extract_ctn_from_tx_and_chan, UpdateAddHtlc, funding_output_script, SENT, RECEIVED, LOCAL, REMOTE, HTLCOwner, make_commitment_outputs, ScriptHtlc, PaymentFailure, calc_onchain_fees, RemoteMisbehaving, make_htlc_output_witness_script) +from .lnutil import FeeUpdate from .lnsweep import create_sweeptxs_for_our_ctx, create_sweeptxs_for_their_ctx from .lnsweep import create_sweeptx_for_their_revoked_htlc from .lnhtlc import HTLCManager @@ -63,29 +64,7 @@ class ChannelJsonEncoder(json.JSONEncoder): RevokeAndAck = namedtuple("RevokeAndAck", ["per_commitment_secret", "next_per_commitment_point"]) -class FeeUpdateProgress(Enum): - FUNDEE_SIGNED = auto() - FUNDEE_ACKED = auto() - FUNDER_SIGNED = auto() - -FUNDEE_SIGNED = FeeUpdateProgress.FUNDEE_SIGNED -FUNDEE_ACKED = FeeUpdateProgress.FUNDEE_ACKED -FUNDER_SIGNED = FeeUpdateProgress.FUNDER_SIGNED - -class FeeUpdate(defaultdict): - def __init__(self, chan, rate): - super().__init__(lambda: False) - self.rate = rate - self.chan = chan - - def pending_feerate(self, subject): - if self[FUNDEE_ACKED]: - return self.rate - if subject == REMOTE and self.chan.constraints.is_initiator: - return self.rate - if subject == LOCAL and not self.chan.constraints.is_initiator: - return self.rate - # implicit return None + def decodeAll(d, local): for k, v in d.items(): @@ -112,6 +91,12 @@ def str_bytes_dict_from_save(x): def str_bytes_dict_to_save(x): return {str(k): bh2u(v) for k, v in x.items()} +def deserialize_feeupdate(x): + return FeeUpdate(rate=x['rate'], ctn={LOCAL:x['ctn'][str(int(LOCAL))], REMOTE:x['ctn'][str(int(REMOTE))]}) + +def serialize_feeupdate(x): + return {'rate':x.rate, 'ctn': {int(LOCAL):x.ctn[LOCAL], int(REMOTE):x.ctn[REMOTE]}} + class Channel(Logger): # note: try to avoid naming ctns/ctxs/etc as "current" and "pending". # they are ambiguous. Use "oldest_unrevoked" or "latest" or "next". @@ -126,6 +111,8 @@ class Channel(Logger): return super().diagnostic_name() def __init__(self, state, *, sweep_address=None, name=None, lnworker=None): + self.name = name + Logger.__init__(self) self.lnworker = lnworker self.sweep_address = sweep_address assert 'local_state' not in state @@ -150,6 +137,7 @@ class Channel(Logger): self.short_channel_id_predicted = self.short_channel_id self.onion_keys = str_bytes_dict_from_save(state.get('onion_keys', {})) self.force_closed = state.get('force_closed') + self.fee_updates = [deserialize_feeupdate(x) if type(x) is not FeeUpdate else x for x in state.get('fee_updates')] # populated with initial fee # FIXME this is a tx serialised in the custom electrum partial tx format. # we should not persist txns in this format. we should persist htlcs, and be able to derive @@ -162,10 +150,6 @@ class Channel(Logger): remote_ctn=self.config[REMOTE].ctn, log=log) - self.name = name - Logger.__init__(self) - - self.pending_fee = None self._is_funding_txo_spent = None # "don't know" self._state = None @@ -174,6 +158,43 @@ class Channel(Logger): self.remote_commitment = None self.sweep_info = {} + def pending_fee(self): + """ return FeeUpdate that has not been commited on any side""" + for f in self.fee_updates: + if f.ctn[LOCAL] is None and f.ctn[REMOTE] is None: + return f + + def locally_pending_fee(self): + """ return FeeUpdate that been commited remotely and is still pending locally (should used if we are initiator)""" + for f in self.fee_updates: + if f.ctn[LOCAL] is None and f.ctn[REMOTE] is not None: + return f + + def remotely_pending_fee(self): + """ return FeeUpdate that been commited locally and is still pending remotely (should be used if we are not initiator)""" + for f in self.fee_updates: + if f.ctn[LOCAL] is not None and f.ctn[REMOTE] is None: + return f + + def get_feerate(self, subject, target_ctn): + next_ctn = self.config[subject].ctn + 1 + assert target_ctn <= next_ctn + result = self.fee_updates[0] + for f in self.fee_updates[1:]: + ctn = f.ctn[subject] + if ctn is None: + ctn = next_ctn + if ctn > result.ctn[subject] and ctn <= target_ctn: + best_ctn = ctn + result = f + return result.rate + + def get_current_feerate(self, subject): + return self.get_feerate(subject, self.config[subject].ctn) + + def get_next_feerate(self, subject): + return self.get_feerate(subject, self.config[subject].ctn + 1) + def get_payments(self): out = {} for subject in LOCAL, REMOTE: @@ -377,11 +398,9 @@ class Channel(Logger): current_htlc_signatures=htlc_sigs_string, got_sig_for_next=True) - if self.pending_fee is not None: - if not self.constraints.is_initiator: - self.pending_fee[FUNDEE_SIGNED] = True - if self.constraints.is_initiator and self.pending_fee[FUNDEE_ACKED]: - self.pending_fee[FUNDER_SIGNED] = True + # if a fee update was acked, then we add it locally + f = self.locally_pending_fee() + if f: f.ctn[LOCAL] = next_local_ctn self.set_local_commitment(pending_local_commitment) @@ -418,14 +437,6 @@ class Channel(Logger): new_ctx = self.pending_commitment(LOCAL) assert self.signature_fits(new_ctx) self.set_local_commitment(new_ctx) - - if self.pending_fee is not None: - if (not self.constraints.is_initiator and self.pending_fee[FUNDEE_SIGNED])\ - or (self.constraints.is_initiator and self.pending_fee[FUNDER_SIGNED]): - self.constraints = self.constraints._replace(feerate=self.pending_fee.rate) - self.pending_fee = None - self.logger.info(f"Feerate change complete (initiator: {self.constraints.is_initiator})") - self.hm.send_rev() self.config[LOCAL]=self.config[LOCAL]._replace( ctn=new_ctn, @@ -461,29 +472,19 @@ class Channel(Logger): # FIXME not sure this is correct... but it seems to work # if there are update_add_htlc msgs between commitment_signed and rev_ack, # this might break + next_ctn = self.config[REMOTE].ctn + 1 prev_remote_commitment = self.pending_commitment(REMOTE) self.config[REMOTE].revocation_store.add_next_entry(revocation.per_commitment_secret) ##### start applying fee/htlc changes - if self.pending_fee is not None: - if not self.constraints.is_initiator: - self.pending_fee[FUNDEE_SIGNED] = True - if self.constraints.is_initiator and self.pending_fee[FUNDEE_ACKED]: - self.pending_fee[FUNDER_SIGNED] = True - + f = self.pending_fee() + if f: f.ctn[REMOTE] = next_ctn next_point = self.config[REMOTE].next_per_commitment_point - self.hm.recv_rev() - self.config[REMOTE]=self.config[REMOTE]._replace( - ctn=self.config[REMOTE].ctn + 1, + ctn=next_ctn, current_per_commitment_point=next_point, next_per_commitment_point=revocation.next_per_commitment_point, ) - - if self.pending_fee is not None: - if self.constraints.is_initiator: - self.pending_fee[FUNDEE_ACKED] = True - self.set_remote_commitment() self.remote_commitment_to_be_revoked = prev_remote_commitment @@ -539,7 +540,7 @@ class Channel(Logger): - calc_onchain_fees( # 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)), - self.pending_feerate(subject), + self.get_current_feerate(subject), self.constraints.is_initiator, )[subject] @@ -551,7 +552,7 @@ class Channel(Logger): assert type(direction) is Direction if ctn is None: ctn = self.config[subject].ctn - feerate = self.pending_feerate(subject) + feerate = self.get_feerate(subject, ctn) conf = self.config[subject] if (subject, direction) in [(REMOTE, RECEIVED), (LOCAL, SENT)]: weight = HTLC_SUCCESS_WEIGHT @@ -561,27 +562,18 @@ class Channel(Logger): fee_for_htlc = lambda htlc: htlc.amount_msat // 1000 - (weight * feerate // 1000) return list(filter(lambda htlc: fee_for_htlc(htlc) >= conf.dust_limit_sat, htlcs)) - def pending_feerate(self, subject): - assert type(subject) is HTLCOwner - candidate = self.constraints.feerate - if self.pending_fee is not None: - x = self.pending_fee.pending_feerate(subject) - if x is not None: - candidate = x - return candidate - def pending_commitment(self, subject): assert type(subject) is HTLCOwner this_point = self.config[REMOTE].next_per_commitment_point if subject == REMOTE else self.local_points(offset=1)[1] ctn = self.config[subject].ctn + 1 - feerate = self.pending_feerate(subject) + feerate = self.get_feerate(subject, ctn) return self.make_commitment(subject, this_point, ctn, feerate, True) def current_commitment(self, subject): assert type(subject) is HTLCOwner this_point = self.config[REMOTE].current_per_commitment_point if subject == REMOTE else self.local_points(offset=0)[1] ctn = self.config[subject].ctn - feerate = self.constraints.feerate + feerate = self.get_feerate(subject, ctn) return self.make_commitment(subject, this_point, ctn, feerate, False) def get_current_ctn(self, subject): @@ -636,11 +628,12 @@ class Channel(Logger): return self.constraints.capacity - sum(x[2] for x in self.pending_commitment(LOCAL).outputs()) def update_fee(self, feerate, initiator): - if self.constraints.is_initiator != initiator: - raise Exception("Cannot update_fee: wrong initiator", initiator) - if self.pending_fee is not None: - raise Exception("a fee update is already in progress") - self.pending_fee = FeeUpdate(self, rate=feerate) + f = self.pending_fee() + if f: + f.rate = feerate + return + f = FeeUpdate(rate=feerate, ctn={LOCAL:None, REMOTE:None}) + self.fee_updates.append(f) def to_save(self): to_save = { @@ -652,6 +645,7 @@ class Channel(Logger): "funding_outpoint": self.funding_outpoint, "node_id": self.node_id, "remote_commitment_to_be_revoked": str(self.remote_commitment_to_be_revoked), + "fee_updates": self.fee_updates, "log": self.hm.to_save(), "onion_keys": str_bytes_dict_to_save(self.onion_keys), "force_closed": self.force_closed, @@ -665,6 +659,8 @@ class Channel(Logger): for k, v in to_save_ref.items(): if isinstance(v, tuple): serialized_channel[k] = namedtuples_to_dict(v) + elif k == 'fee_updates': + serialized_channel[k] = [serialize_feeupdate(x) for x in v] else: serialized_channel[k] = v dumped = ChannelJsonEncoder().encode(serialized_channel) diff --git a/electrum/lnpeer.py b/electrum/lnpeer.py index c3b385956..515b4b6e8 100644 --- a/electrum/lnpeer.py +++ b/electrum/lnpeer.py @@ -40,6 +40,7 @@ from .lnutil import (Outpoint, LocalConfig, RECEIVED, UpdateAddHtlc, LightningPeerConnectionClosed, HandshakeFailed, NotFoundChanAnnouncementForUpdate, MINIMUM_MAX_HTLC_VALUE_IN_FLIGHT_ACCEPTED, MAXIMUM_HTLC_MINIMUM_MSAT_ACCEPTED, MAXIMUM_REMOTE_TO_SELF_DELAY_ACCEPTED, RemoteMisbehaving, DEFAULT_TO_SELF_DELAY) +from .lnutil import FeeUpdate from .lnsweep import create_sweeptxs_for_watchtower from .lntransport import LNTransport, LNTransportBase from .lnmsg import encode_msg, decode_msg @@ -537,14 +538,15 @@ class Peer(Logger): # remote commitment transaction channel_id, funding_txid_bytes = channel_id_from_funding_tx(funding_txid, funding_index) chan_dict = { - "node_id": self.pubkey, - "channel_id": channel_id, - "short_channel_id": None, - "funding_outpoint": Outpoint(funding_txid, funding_index), - "remote_config": remote_config, - "local_config": local_config, - "constraints": ChannelConstraints(capacity=funding_sat, is_initiator=True, funding_txn_minimum_depth=funding_txn_minimum_depth, feerate=feerate), - "remote_commitment_to_be_revoked": None, + "node_id": self.pubkey, + "channel_id": channel_id, + "short_channel_id": None, + "funding_outpoint": Outpoint(funding_txid, funding_index), + "remote_config": remote_config, + "local_config": local_config, + "constraints": ChannelConstraints(capacity=funding_sat, is_initiator=True, funding_txn_minimum_depth=funding_txn_minimum_depth), + "fee_updates": [FeeUpdate(rate=feerate, ctn={LOCAL:0, REMOTE:0})], + "remote_commitment_to_be_revoked": None, } chan = Channel(chan_dict, sweep_address=self.lnworker.sweep_address, @@ -630,8 +632,9 @@ class Peer(Logger): revocation_store=their_revocation_store, ), "local_config": local_config, - "constraints": ChannelConstraints(capacity=funding_sat, is_initiator=False, funding_txn_minimum_depth=min_depth, feerate=feerate), + "constraints": ChannelConstraints(capacity=funding_sat, is_initiator=False, funding_txn_minimum_depth=min_depth), "remote_commitment_to_be_revoked": None, + "fee_updates": [FeeUpdate(feerate, ctn={LOCAL:0, REMOTE:0})], } chan = Channel(chan_dict, sweep_address=self.lnworker.sweep_address, @@ -1026,7 +1029,7 @@ class Peer(Logger): # if there are no changes, we will not (and must not) send a new commitment next_htlcs, latest_htlcs = chan.hm.get_htlcs_in_next_ctx(REMOTE), chan.hm.get_htlcs_in_latest_ctx(REMOTE) if (next_htlcs == latest_htlcs - and chan.pending_feerate(REMOTE) == chan.constraints.feerate) \ + and chan.get_next_feerate(REMOTE) == chan.get_current_feerate(REMOTE)) \ or ctn_to_sign == self.sent_commitment_for_ctn_last[chan]: return self.logger.info(f'send_commitment. old number htlcs: {len(latest_htlcs)}, new number htlcs: {len(next_htlcs)}') @@ -1088,7 +1091,7 @@ class Peer(Logger): chan = self.channels[channel_id] # make sure there were changes to the ctx, otherwise the remote peer is misbehaving if (chan.hm.get_htlcs_in_next_ctx(LOCAL) == chan.hm.get_htlcs_in_latest_ctx(LOCAL) - and chan.pending_feerate(LOCAL) == chan.constraints.feerate): + and chan.get_next_feerate(LOCAL) == chan.get_current_feerate(LOCAL)): raise RemoteMisbehaving('received commitment_signed without pending changes') # make sure ctn is new ctn_to_recv = chan.get_current_ctn(LOCAL) + 1 @@ -1289,7 +1292,7 @@ class Peer(Logger): # TODO force close if initiator does not update_fee enough return feerate_per_kw = self.lnworker.current_feerate_per_kw() - chan_fee = chan.pending_feerate(REMOTE) + chan_fee = chan.get_next_feerate(REMOTE) self.logger.info(f"current pending feerate {chan_fee}") self.logger.info(f"new feerate {feerate_per_kw}") if feerate_per_kw < chan_fee / 2: diff --git a/electrum/lnutil.py b/electrum/lnutil.py index 28c798bf9..5339e474f 100644 --- a/electrum/lnutil.py +++ b/electrum/lnutil.py @@ -78,7 +78,9 @@ class RemoteConfig(NamedTuple): current_per_commitment_point: Optional[bytes] -ChannelConstraints = namedtuple("ChannelConstraints", ["capacity", "is_initiator", "funding_txn_minimum_depth", "feerate"]) +FeeUpdate = namedtuple("FeeUpdate", ["rate", "ctn"]) + +ChannelConstraints = namedtuple("ChannelConstraints", ["capacity", "is_initiator", "funding_txn_minimum_depth"]) ScriptHtlc = namedtuple('ScriptHtlc', ['redeem_script', 'htlc']) @@ -359,7 +361,7 @@ def make_htlc_tx_with_open_channel(chan: 'Channel', pcp: bytes, for_us: bool, is_htlc_success = for_us == we_receive script, htlc_tx_output = make_htlc_tx_output( amount_msat = amount_msat, - local_feerate = chan.pending_feerate(LOCAL if for_us else REMOTE), # uses pending feerate.. + local_feerate = chan.get_next_feerate(LOCAL if for_us else REMOTE), revocationpubkey=other_revocation_pubkey, local_delayedpubkey=delayedpubkey, success = is_htlc_success, diff --git a/electrum/tests/test_lnchannel.py b/electrum/tests/test_lnchannel.py index 9a2ba0553..74e33685a 100644 --- a/electrum/tests/test_lnchannel.py +++ b/electrum/tests/test_lnchannel.py @@ -31,6 +31,7 @@ from electrum import lnchannel from electrum import lnutil from electrum import bip32 as bip32_utils from electrum.lnutil import SENT, LOCAL, REMOTE, RECEIVED +from electrum.lnutil import FeeUpdate from electrum.ecc import sig_string_from_der_sig from electrum.logging import console_stderr_handler @@ -92,8 +93,8 @@ def create_channel_state(funding_txid, funding_index, funding_sat, local_feerate capacity=funding_sat, is_initiator=is_initiator, funding_txn_minimum_depth=3, - feerate=local_feerate, ), + "fee_updates": [FeeUpdate(rate=local_feerate, ctn={LOCAL:0, REMOTE:0})], "node_id":other_node_id, "remote_commitment_to_be_revoked": None, 'onion_keys': {}, @@ -527,31 +528,34 @@ class TestChannel(unittest.TestCase): return fee def test_UpdateFeeSenderCommits(self): - old_feerate = self.alice_channel.pending_feerate(LOCAL) - fee = self.alice_to_bob_fee_update() - alice_channel, bob_channel = self.alice_channel, self.bob_channel - self.assertEqual(self.alice_channel.pending_feerate(LOCAL), old_feerate) + old_feerate = alice_channel.get_next_feerate(LOCAL) + + fee = self.alice_to_bob_fee_update() + self.assertEqual(alice_channel.get_next_feerate(LOCAL), old_feerate) + alice_sig, alice_htlc_sigs = alice_channel.sign_next_commitment() - self.assertEqual(self.alice_channel.pending_feerate(LOCAL), old_feerate) + #self.assertEqual(alice_channel.get_next_feerate(LOCAL), old_feerate) bob_channel.receive_new_commitment(alice_sig, alice_htlc_sigs) - self.assertNotEqual(fee, bob_channel.constraints.feerate) + self.assertNotEqual(fee, bob_channel.get_current_feerate(LOCAL)) rev, _ = bob_channel.revoke_current_commitment() - self.assertEqual(fee, bob_channel.constraints.feerate) + self.assertEqual(fee, bob_channel.get_current_feerate(LOCAL)) - bob_sig, bob_htlc_sigs = bob_channel.sign_next_commitment() alice_channel.receive_revocation(rev) + + + bob_sig, bob_htlc_sigs = bob_channel.sign_next_commitment() alice_channel.receive_new_commitment(bob_sig, bob_htlc_sigs) - self.assertNotEqual(fee, alice_channel.constraints.feerate) + self.assertNotEqual(fee, alice_channel.get_current_feerate(LOCAL)) rev, _ = alice_channel.revoke_current_commitment() - self.assertEqual(fee, alice_channel.constraints.feerate) + self.assertEqual(fee, alice_channel.get_current_feerate(LOCAL)) bob_channel.receive_revocation(rev) - self.assertEqual(fee, bob_channel.constraints.feerate) + self.assertEqual(fee, bob_channel.get_current_feerate(LOCAL)) def test_UpdateFeeReceiverCommits(self): @@ -567,20 +571,20 @@ class TestChannel(unittest.TestCase): 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.constraints.feerate) + self.assertNotEqual(fee, bob_channel.get_current_feerate(LOCAL)) bob_revocation, _ = bob_channel.revoke_current_commitment() - self.assertEqual(fee, bob_channel.constraints.feerate) + self.assertEqual(fee, bob_channel.get_current_feerate(LOCAL)) 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.constraints.feerate) + self.assertNotEqual(fee, alice_channel.get_current_feerate(LOCAL)) alice_revocation, _ = alice_channel.revoke_current_commitment() - self.assertEqual(fee, alice_channel.constraints.feerate) + self.assertEqual(fee, alice_channel.get_current_feerate(LOCAL)) bob_channel.receive_revocation(alice_revocation) - self.assertEqual(fee, bob_channel.constraints.feerate) + self.assertEqual(fee, bob_channel.get_current_feerate(LOCAL)) @unittest.skip("broken probably because we havn't implemented detecting when we come out of a situation where we violate reserve") def test_AddHTLCNegativeBalance(self): @@ -798,7 +802,7 @@ class TestDust(unittest.TestCase): paymentPreimage = b"\x01" * 32 paymentHash = bitcoin.sha256(paymentPreimage) - fee_per_kw = alice_channel.constraints.feerate + fee_per_kw = alice_channel.get_current_feerate(LOCAL) self.assertEqual(fee_per_kw, 6000) htlcAmt = 500 + lnutil.HTLC_TIMEOUT_WEIGHT * (fee_per_kw // 1000) self.assertEqual(htlcAmt, 4478)