electrum

Electrum Bitcoin wallet
git clone https://git.parazyd.org/electrum
Log | Files | Refs | Submodules

commit fc39295d20ee0ad90a944efb63369459a9cd9a19
parent 947af921268341251e1a8d521905e1f7e3a96f19
Author: SomberNight <somber.night@protonmail.com>
Date:   Mon,  8 Jun 2020 21:17:23 +0200

lnpeer: review safety check re channel open flow, and tweak params

Diffstat:
Melectrum/bitcoin.py | 5+++++
Melectrum/lnpeer.py | 126++++++++++++++++++++++++++++++++++++++++++-------------------------------------
Melectrum/lnutil.py | 42+++++++++++++++++++++++++++++++++---------
3 files changed, 105 insertions(+), 68 deletions(-)

diff --git a/electrum/bitcoin.py b/electrum/bitcoin.py @@ -312,6 +312,11 @@ def relayfee(network: 'Network' = None) -> int: return fee +# see https://github.com/bitcoin/bitcoin/blob/a62f0ed64f8bbbdfe6467ac5ce92ef5b5222d1bd/src/policy/policy.cpp#L14 +DUST_LIMIT_DEFAULT_SAT_LEGACY = 546 +DUST_LIMIT_DEFAULT_SAT_SEGWIT = 294 + + def dust_threshold(network: 'Network' = None) -> int: """Returns the dust limit in satoshis.""" # Change <= dust threshold is added to the tx fee diff --git a/electrum/lnpeer.py b/electrum/lnpeer.py @@ -40,10 +40,10 @@ from .lnutil import (Outpoint, LocalConfig, RECEIVED, UpdateAddHtlc, LOCAL, REMOTE, HTLCOwner, generate_keypair, LnKeyFamily, ln_compare_features, privkey_to_pubkey, MIN_FINAL_CLTV_EXPIRY_ACCEPTED, 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, + RemoteMisbehaving, DEFAULT_TO_SELF_DELAY, NBLOCK_OUR_CLTV_EXPIRY_DELTA, format_short_channel_id, ShortChannelID, - IncompatibleLightningFeatures, derive_payment_secret_from_payment_preimage) + IncompatibleLightningFeatures, derive_payment_secret_from_payment_preimage, + LN_MAX_FUNDING_SAT, calc_fees_for_commitment_tx) from .lnutil import FeeUpdate, channel_id_from_funding_tx from .lntransport import LNTransport, LNTransportBase from .lnmsg import encode_msg, decode_msg @@ -504,17 +504,18 @@ class Peer(Logger): channel_seed=channel_seed, static_remotekey=static_remotekey, to_self_delay=DEFAULT_TO_SELF_DELAY, - dust_limit_sat=546, + dust_limit_sat=bitcoin.DUST_LIMIT_DEFAULT_SAT_LEGACY, max_htlc_value_in_flight_msat=funding_sat * 1000, max_accepted_htlcs=5, initial_msat=initial_msat, - reserve_sat=546, + reserve_sat=funding_sat // 100, funding_locked_received=False, was_announced=False, current_commitment_signature=None, current_htlc_signatures=b'', htlc_minimum_msat=1, ) + local_config.validate_params(funding_sat=funding_sat) return local_config def temporarily_reserve_funding_tx_change_address(func): @@ -548,6 +549,12 @@ class Peer(Logger): await asyncio.wait_for(self.initialized, LN_P2P_NETWORK_TIMEOUT) feerate = self.lnworker.current_feerate_per_kw() local_config = self.make_local_config(funding_sat, push_msat, LOCAL) + if funding_sat > LN_MAX_FUNDING_SAT: + raise Exception(f"MUST set funding_satoshis to less than 2^24 satoshi. {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: {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}") # for the first commitment transaction per_commitment_secret_first = get_per_commitment_secret_from_seed(local_config.per_commitment_secret_seed, RevocationStore.START_INDEX) @@ -580,41 +587,31 @@ class Peer(Logger): raise Exception(f"minimum depth too low, {funding_txn_minimum_depth}") if funding_txn_minimum_depth > 30: raise Exception(f"minimum depth too high, {funding_txn_minimum_depth}") - remote_dust_limit_sat = payload['dust_limit_satoshis'] - remote_reserve_sat = self.validate_remote_reserve(payload["channel_reserve_satoshis"], remote_dust_limit_sat, funding_sat) - if remote_dust_limit_sat > remote_reserve_sat: - raise Exception(f"Remote Lightning peer reports dust_limit_sat > reserve_sat which is a BOLT-02 protocol violation.") - htlc_min = payload['htlc_minimum_msat'] - if htlc_min > MAXIMUM_HTLC_MINIMUM_MSAT_ACCEPTED: - raise Exception(f"Remote Lightning peer reports htlc_minimum_msat={htlc_min} mSAT," + - f" which is above Electrums required maximum limit of that parameter ({MAXIMUM_HTLC_MINIMUM_MSAT_ACCEPTED} mSAT).") - remote_max = payload['max_htlc_value_in_flight_msat'] - if remote_max < MINIMUM_MAX_HTLC_VALUE_IN_FLIGHT_ACCEPTED: - raise Exception(f"Remote Lightning peer reports max_htlc_value_in_flight_msat at only {remote_max} mSAT" + - f" which is below Electrums required minimum ({MINIMUM_MAX_HTLC_VALUE_IN_FLIGHT_ACCEPTED} mSAT).") - max_accepted_htlcs = payload["max_accepted_htlcs"] - if max_accepted_htlcs > 483: - raise Exception("Remote Lightning peer reports max_accepted_htlcs > 483, which is a BOLT-02 protocol violation.") - remote_to_self_delay = payload['to_self_delay'] - if remote_to_self_delay > MAXIMUM_REMOTE_TO_SELF_DELAY_ACCEPTED: - raise Exception(f"Remote Lightning peer reports to_self_delay={remote_to_self_delay}," + - f" which is above Electrums required maximum ({MAXIMUM_REMOTE_TO_SELF_DELAY_ACCEPTED})") remote_config = RemoteConfig( payment_basepoint=OnlyPubkeyKeypair(payload['payment_basepoint']), multisig_key=OnlyPubkeyKeypair(payload["funding_pubkey"]), htlc_basepoint=OnlyPubkeyKeypair(payload['htlc_basepoint']), delayed_basepoint=OnlyPubkeyKeypair(payload['delayed_payment_basepoint']), revocation_basepoint=OnlyPubkeyKeypair(payload['revocation_basepoint']), - to_self_delay=remote_to_self_delay, - dust_limit_sat=remote_dust_limit_sat, - max_htlc_value_in_flight_msat=remote_max, - max_accepted_htlcs=max_accepted_htlcs, + to_self_delay=payload['to_self_delay'], + dust_limit_sat=payload['dust_limit_satoshis'], + max_htlc_value_in_flight_msat=payload['max_htlc_value_in_flight_msat'], + max_accepted_htlcs=payload["max_accepted_htlcs"], initial_msat=push_msat, - reserve_sat = remote_reserve_sat, - htlc_minimum_msat = htlc_min, + reserve_sat=payload["channel_reserve_satoshis"], + htlc_minimum_msat=payload['htlc_minimum_msat'], next_per_commitment_point=remote_per_commitment_point, current_per_commitment_point=None, ) + 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") # replace dummy output in funding tx redeem_script = funding_output_script(local_config, remote_config) funding_address = bitcoin.redeem_script_to_address('p2wsh', redeem_script) @@ -678,14 +675,51 @@ class Peer(Logger): return StoredDict(chan_dict, None, []) async def on_open_channel(self, payload): - # payload['channel_flags'] if payload['chain_hash'] != constants.net.rev_genesis_bytes(): raise Exception('wrong chain_hash') funding_sat = payload['funding_satoshis'] push_msat = payload['push_msat'] - feerate = payload['feerate_per_kw'] + 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. {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: {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}") + remote_config = RemoteConfig( + payment_basepoint=OnlyPubkeyKeypair(payload['payment_basepoint']), + multisig_key=OnlyPubkeyKeypair(payload['funding_pubkey']), + htlc_basepoint=OnlyPubkeyKeypair(payload['htlc_basepoint']), + delayed_basepoint=OnlyPubkeyKeypair(payload['delayed_payment_basepoint']), + revocation_basepoint=OnlyPubkeyKeypair(payload['revocation_basepoint']), + to_self_delay=payload['to_self_delay'], + dust_limit_sat=payload['dust_limit_satoshis'], + max_htlc_value_in_flight_msat=payload['max_htlc_value_in_flight_msat'], + max_accepted_htlcs=payload['max_accepted_htlcs'], + initial_msat=funding_sat * 1000 - push_msat, + reserve_sat=payload['channel_reserve_satoshis'], + htlc_minimum_msat=payload['htlc_minimum_msat'], + next_per_commitment_point=payload['first_per_commitment_point'], + current_per_commitment_point=None, + ) + 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). # for the first commitment transaction per_commitment_secret_first = get_per_commitment_secret_from_seed(local_config.per_commitment_secret_seed, RevocationStore.START_INDEX) @@ -696,7 +730,7 @@ class Peer(Logger): dust_limit_satoshis=local_config.dust_limit_sat, max_htlc_value_in_flight_msat=local_config.max_htlc_value_in_flight_msat, channel_reserve_satoshis=local_config.reserve_sat, - htlc_minimum_msat=1000, + htlc_minimum_msat=local_config.htlc_minimum_msat, minimum_depth=min_depth, to_self_delay=local_config.to_self_delay, max_accepted_htlcs=local_config.max_accepted_htlcs, @@ -711,25 +745,6 @@ class Peer(Logger): funding_idx = funding_created['funding_output_index'] funding_txid = bh2u(funding_created['funding_txid'][::-1]) channel_id, funding_txid_bytes = channel_id_from_funding_tx(funding_txid, funding_idx) - remote_balance_sat = funding_sat * 1000 - push_msat - remote_dust_limit_sat = payload['dust_limit_satoshis'] # TODO validate - remote_reserve_sat = self.validate_remote_reserve(payload['channel_reserve_satoshis'], remote_dust_limit_sat, funding_sat) - remote_config = RemoteConfig( - payment_basepoint=OnlyPubkeyKeypair(payload['payment_basepoint']), - multisig_key=OnlyPubkeyKeypair(payload['funding_pubkey']), - htlc_basepoint=OnlyPubkeyKeypair(payload['htlc_basepoint']), - delayed_basepoint=OnlyPubkeyKeypair(payload['delayed_payment_basepoint']), - revocation_basepoint=OnlyPubkeyKeypair(payload['revocation_basepoint']), - to_self_delay=payload['to_self_delay'], - dust_limit_sat=remote_dust_limit_sat, - max_htlc_value_in_flight_msat=payload['max_htlc_value_in_flight_msat'], # TODO validate - max_accepted_htlcs=payload['max_accepted_htlcs'], # TODO validate - initial_msat=remote_balance_sat, - reserve_sat = remote_reserve_sat, - htlc_minimum_msat=payload['htlc_minimum_msat'], # TODO validate - next_per_commitment_point=payload['first_per_commitment_point'], - current_per_commitment_point=None, - ) constraints = ChannelConstraints(capacity=funding_sat, is_initiator=False, funding_txn_minimum_depth=min_depth) outpoint = Outpoint(funding_txid, funding_idx) chan_dict = self.create_channel_storage(channel_id, outpoint, local_config, remote_config, constraints) @@ -752,13 +767,6 @@ class Peer(Logger): chan.set_state(ChannelState.OPENING) self.lnworker.add_new_channel(chan) - def validate_remote_reserve(self, remote_reserve_sat: int, dust_limit: int, funding_sat: int) -> int: - if remote_reserve_sat < dust_limit: - raise Exception('protocol violation: reserve < dust_limit') - if remote_reserve_sat > funding_sat/100: - raise Exception(f'reserve too high: {remote_reserve_sat}, funding_sat: {funding_sat}') - return remote_reserve_sat - async def trigger_force_close(self, channel_id): await self.initialized latest_point = 0 diff --git a/electrum/lnutil.py b/electrum/lnutil.py @@ -81,6 +81,36 @@ class Config(StoredObject): reserve_sat = attr.ib(type=int) htlc_minimum_msat = attr.ib(type=int) + def validate_params(self, *, funding_sat: int) -> None: + for key in ( + self.payment_basepoint, + self.multisig_key, + self.htlc_basepoint, + self.delayed_basepoint, + self.revocation_basepoint + ): + if not (len(key.pubkey) == 33 and ecc.ECPubkey.is_pubkey_bytes(key.pubkey)): + raise Exception("invalid pubkey in channel config") + if self.reserve_sat < self.dust_limit_sat: + raise Exception("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 + # but other implementations are checking against this value too; also let's be conservative + if self.dust_limit_sat < bitcoin.DUST_LIMIT_DEFAULT_SAT_LEGACY: + raise Exception(f"dust limit too low: {self.dust_limit_sat} sat") + if self.reserve_sat > funding_sat // 100: + raise Exception(f'reserve too high: {self.reserve_sat}, funding_sat: {funding_sat}') + if self.htlc_minimum_msat > 1_000: + raise Exception(f"htlc_minimum_msat too high: {self.htlc_minimum_msat} msat") + if self.max_accepted_htlcs < 1: + raise Exception(f"max_accepted_htlcs too low: {self.max_accepted_htlcs}") + if self.max_accepted_htlcs > 483: + raise Exception(f"max_accepted_htlcs too high: {self.max_accepted_htlcs}") + if self.to_self_delay > MAXIMUM_REMOTE_TO_SELF_DELAY_ACCEPTED: + raise Exception(f"to_self_delay too high: {self.to_self_delay} > {MAXIMUM_REMOTE_TO_SELF_DELAY_ACCEPTED}") + if self.max_htlc_value_in_flight_msat < min(funding_sat, 100_000_000): + raise Exception(f"max_htlc_value_in_flight_msat is too small: {self.max_htlc_value_in_flight_msat}") + + @attr.s class LocalConfig(Config): channel_seed = attr.ib(type=bytes, converter=hex_to_bytes) # type: Optional[bytes] @@ -252,12 +282,14 @@ class NotFoundChanAnnouncementForUpdate(Exception): pass class PaymentFailure(UserFacingException): pass # TODO make some of these values configurable? -DEFAULT_TO_SELF_DELAY = 144 +DEFAULT_TO_SELF_DELAY = 7 * 144 REDEEM_AFTER_DOUBLE_SPENT_DELAY = 30 CHANNEL_OPENING_TIMEOUT = 24*60*60 +MIN_FUNDING_SAT = 200_000 + ##### CLTV-expiry-delta-related values # see https://github.com/lightningnetwork/lightning-rfc/blob/master/02-peer-protocol.md#cltv_expiry_delta-selection @@ -283,14 +315,6 @@ OUR_FEE_PROPORTIONAL_MILLIONTHS = 1 NBLOCK_CLTV_EXPIRY_TOO_FAR_INTO_FUTURE = 28 * 144 - -# When we open a channel, the remote peer has to support at least this -# value of mSATs in HTLCs accumulated on the channel, or we refuse opening. -# Number is based on observed testnet limit https://github.com/spesmilo/electrum/issues/5032 -MINIMUM_MAX_HTLC_VALUE_IN_FLIGHT_ACCEPTED = 19_800 * 1000 - -MAXIMUM_HTLC_MINIMUM_MSAT_ACCEPTED = 1000 - MAXIMUM_REMOTE_TO_SELF_DELAY_ACCEPTED = 2016 class RevocationStore: