From 4af103378a5da23936ebd9cd119448096b9dabab Mon Sep 17 00:00:00 2001 From: SomberNight Date: Fri, 24 Sep 2021 19:58:32 +0200 Subject: [PATCH] lnpeer: refactor some checks re open_channel/accept_channel --- electrum/lnpeer.py | 66 +++++++++++----------------------------------- electrum/lnutil.py | 63 ++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 74 insertions(+), 55 deletions(-) diff --git a/electrum/lnpeer.py b/electrum/lnpeer.py index 3f0901e50..316d10e00 100644 --- a/electrum/lnpeer.py +++ b/electrum/lnpeer.py @@ -33,7 +33,7 @@ from .lnonion import (new_onion_packet, OnionFailureCode, calc_hops_data_for_pay OnionFailureCodeMetaFlag) from .lnchannel import Channel, RevokeAndAck, RemoteCtnTooFarInFuture, ChannelState, PeerState from . import lnutil -from .lnutil import (Outpoint, LocalConfig, RECEIVED, UpdateAddHtlc, +from .lnutil import (Outpoint, LocalConfig, RECEIVED, UpdateAddHtlc, ChannelConfig, RemoteConfig, OnlyPubkeyKeypair, ChannelConstraints, RevocationStore, funding_output_script, get_per_commitment_secret_from_seed, secret_to_pubkey, PaymentFailure, LnFeatures, @@ -42,7 +42,6 @@ from .lnutil import (Outpoint, LocalConfig, RECEIVED, UpdateAddHtlc, LightningPeerConnectionClosed, HandshakeFailed, RemoteMisbehaving, ShortChannelID, IncompatibleLightningFeatures, derive_payment_secret_from_payment_preimage, - LN_MAX_FUNDING_SAT, calc_fees_for_commitment_tx, UpfrontShutdownScriptViolation) from .lnutil import FeeUpdate, channel_id_from_funding_tx from .lntransport import LNTransport, LNTransportBase @@ -600,17 +599,6 @@ class Peer(Logger): if not self.lnworker.channel_db and not self.lnworker.is_trampoline_peer(self.pubkey): raise Exception('Not a trampoline node: ' + str(self.their_features)) - if funding_sat > LN_MAX_FUNDING_SAT: - raise Exception( - f"MUST set funding_satoshis to less than 2^24 satoshi. " - f"{funding_sat} sat > {LN_MAX_FUNDING_SAT}") - if push_msat > 1000 * funding_sat: - raise Exception( - f"MUST set push_msat to equal or less than 1000 * funding_satoshis: " - f"{push_msat} msat > {1000 * funding_sat} msat") - if funding_sat < lnutil.MIN_FUNDING_SAT: - raise Exception(f"funding_sat too low: {funding_sat} < {lnutil.MIN_FUNDING_SAT}") - feerate = self.lnworker.current_feerate_per_kw() local_config = self.make_local_config(funding_sat, push_msat, LOCAL) @@ -676,15 +664,13 @@ class Peer(Logger): current_per_commitment_point=None, upfront_shutdown_script=upfront_shutdown_script ) - remote_config.validate_params(funding_sat=funding_sat) - # if channel_reserve_satoshis is less than dust_limit_satoshis within the open_channel message: - # MUST reject the channel. - if remote_config.reserve_sat < local_config.dust_limit_sat: - raise Exception("violated constraint: remote_config.reserve_sat < local_config.dust_limit_sat") - # if channel_reserve_satoshis from the open_channel message is less than dust_limit_satoshis: - # MUST reject the channel. - if local_config.reserve_sat < remote_config.dust_limit_sat: - raise Exception("violated constraint: local_config.reserve_sat < remote_config.dust_limit_sat") + ChannelConfig.cross_validate_params( + local_config=local_config, + remote_config=remote_config, + funding_sat=funding_sat, + is_local_initiator=True, + initial_feerate_per_kw=feerate, + ) # -> funding created # replace dummy output in funding tx @@ -798,16 +784,6 @@ class Peer(Logger): feerate = payload['feerate_per_kw'] # note: we are not validating this temp_chan_id = payload['temporary_channel_id'] local_config = self.make_local_config(funding_sat, push_msat, REMOTE) - if funding_sat > LN_MAX_FUNDING_SAT: - raise Exception( - f"MUST set funding_satoshis to less than 2^24 satoshi. " - f"{funding_sat} sat > {LN_MAX_FUNDING_SAT}") - if push_msat > 1000 * funding_sat: - raise Exception( - f"MUST set push_msat to equal or less than 1000 * funding_satoshis: " - f"{push_msat} msat > {1000 * funding_sat} msat") - if funding_sat < lnutil.MIN_FUNDING_SAT: - raise Exception(f"funding_sat too low: {funding_sat} < {lnutil.MIN_FUNDING_SAT}") upfront_shutdown_script = self.upfront_shutdown_script_from_payload( payload, 'open') @@ -829,26 +805,14 @@ class Peer(Logger): current_per_commitment_point=None, upfront_shutdown_script=upfront_shutdown_script, ) + ChannelConfig.cross_validate_params( + local_config=local_config, + remote_config=remote_config, + funding_sat=funding_sat, + is_local_initiator=False, + initial_feerate_per_kw=feerate, + ) - remote_config.validate_params(funding_sat=funding_sat) - # The receiving node MUST fail the channel if: - # the funder's amount for the initial commitment transaction is not - # sufficient for full fee payment. - if remote_config.initial_msat < calc_fees_for_commitment_tx( - num_htlcs=0, - feerate=feerate, - is_local_initiator=False)[REMOTE]: - raise Exception( - "the funder's amount for the initial commitment transaction " - "is not sufficient for full fee payment") - # The receiving node MUST fail the channel if: - # both to_local and to_remote amounts for the initial commitment transaction are - # less than or equal to channel_reserve_satoshis (see BOLT 3). - if (local_config.initial_msat <= 1000 * payload['channel_reserve_satoshis'] - and remote_config.initial_msat <= 1000 * payload['channel_reserve_satoshis']): - raise Exception( - "both to_local and to_remote amounts for the initial commitment " - "transaction are less than or equal to channel_reserve_satoshis") # note: we ignore payload['channel_flags'], which e.g. contains 'announce_channel'. # Notably if the remote sets 'announce_channel' to True, we will ignore that too, # but we will not play along with actually announcing the channel (so we keep it private). diff --git a/electrum/lnutil.py b/electrum/lnutil.py index 50c35c2fe..7cc1bfb5e 100644 --- a/electrum/lnutil.py +++ b/electrum/lnutil.py @@ -66,7 +66,7 @@ class Keypair(OnlyPubkeyKeypair): privkey = attr.ib(type=bytes, converter=hex_to_bytes) @attr.s -class Config(StoredObject): +class ChannelConfig(StoredObject): # shared channel config fields payment_basepoint = attr.ib(type=OnlyPubkeyKeypair, converter=json_to_keypair) multisig_key = attr.ib(type=OnlyPubkeyKeypair, converter=json_to_keypair) @@ -93,6 +93,14 @@ class Config(StoredObject): ): if not (len(key.pubkey) == 33 and ecc.ECPubkey.is_pubkey_bytes(key.pubkey)): raise Exception(f"{conf_name}. invalid pubkey in channel config") + if funding_sat < MIN_FUNDING_SAT: + raise Exception(f"funding_sat too low: {funding_sat} sat < {MIN_FUNDING_SAT}") + # MUST set funding_satoshis to less than 2^24 satoshi + if funding_sat > LN_MAX_FUNDING_SAT: + raise Exception(f"funding_sat too high: {funding_sat} sat > {LN_MAX_FUNDING_SAT}") + # MUST set push_msat to equal or less than 1000 * funding_satoshis + if not (0 <= self.initial_msat <= 1000 * funding_sat): + raise Exception(f"{conf_name}. insane initial_msat={self.initial_msat}. (funding_sat={funding_sat})") if self.reserve_sat < self.dust_limit_sat: raise Exception(f"{conf_name}. MUST set channel_reserve_satoshis greater than or equal to dust_limit_satoshis") # technically this could be using the lower DUST_LIMIT_DEFAULT_SAT_SEGWIT @@ -106,7 +114,7 @@ class Config(StoredObject): HTLC_MINIMUM_MSAT_MIN = 0 # should be at least 1 really, but apparently some nodes are sending zero... if self.htlc_minimum_msat < HTLC_MINIMUM_MSAT_MIN: raise Exception(f"{conf_name}. htlc_minimum_msat too low: {self.htlc_minimum_msat} msat < {HTLC_MINIMUM_MSAT_MIN}") - if self.max_accepted_htlcs < 1: + if self.max_accepted_htlcs < 5: raise Exception(f"{conf_name}. max_accepted_htlcs too low: {self.max_accepted_htlcs}") if self.max_accepted_htlcs > 483: raise Exception(f"{conf_name}. max_accepted_htlcs too high: {self.max_accepted_htlcs}") @@ -115,9 +123,56 @@ class Config(StoredObject): if self.max_htlc_value_in_flight_msat < min(1000 * funding_sat, 100_000_000): raise Exception(f"{conf_name}. max_htlc_value_in_flight_msat is too small: {self.max_htlc_value_in_flight_msat}") + @classmethod + def cross_validate_params( + cls, + *, + local_config: 'LocalConfig', + remote_config: 'RemoteConfig', + funding_sat: int, + is_local_initiator: bool, # whether we are the funder + initial_feerate_per_kw: int, + ) -> None: + # first we validate the configs separately + local_config.validate_params(funding_sat=funding_sat) + remote_config.validate_params(funding_sat=funding_sat) + # now do tests that need access to both configs + if is_local_initiator: + funder, fundee = LOCAL, REMOTE + funder_config, fundee_config = local_config, remote_config + else: + funder, fundee = REMOTE, LOCAL + funder_config, fundee_config = remote_config, local_config + # if channel_reserve_satoshis is less than dust_limit_satoshis within the open_channel message: + # MUST reject the channel. + if remote_config.reserve_sat < local_config.dust_limit_sat: + raise Exception("violated constraint: remote_config.reserve_sat < local_config.dust_limit_sat") + # if channel_reserve_satoshis from the open_channel message is less than dust_limit_satoshis: + # MUST reject the channel. + if local_config.reserve_sat < remote_config.dust_limit_sat: + raise Exception("violated constraint: local_config.reserve_sat < remote_config.dust_limit_sat") + # The receiving node MUST fail the channel if: + # the funder's amount for the initial commitment transaction is not + # sufficient for full fee payment. + if funder_config.initial_msat < calc_fees_for_commitment_tx( + num_htlcs=0, + feerate=initial_feerate_per_kw, + is_local_initiator=is_local_initiator)[funder]: + raise Exception( + "the funder's amount for the initial commitment transaction " + "is not sufficient for full fee payment") + # The receiving node MUST fail the channel if: + # both to_local and to_remote amounts for the initial commitment transaction are + # less than or equal to channel_reserve_satoshis (see BOLT 3). + if (max(local_config.initial_msat, remote_config.initial_msat) + <= 1000 * max(local_config.reserve_sat, remote_config.reserve_sat)): + raise Exception( + "both to_local and to_remote amounts for the initial commitment " + "transaction are less than or equal to channel_reserve_satoshis") + @attr.s -class LocalConfig(Config): +class LocalConfig(ChannelConfig): channel_seed = attr.ib(type=bytes, converter=hex_to_bytes) # type: Optional[bytes] funding_locked_received = attr.ib(type=bool) was_announced = attr.ib(type=bool) @@ -150,7 +205,7 @@ class LocalConfig(Config): raise Exception(f"{conf_name}. htlc_minimum_msat too low: {self.htlc_minimum_msat} msat < {HTLC_MINIMUM_MSAT_MIN}") @attr.s -class RemoteConfig(Config): +class RemoteConfig(ChannelConfig): next_per_commitment_point = attr.ib(type=bytes, converter=hex_to_bytes) current_per_commitment_point = attr.ib(default=None, type=bytes, converter=hex_to_bytes)