Merge pull request #7542 from bitromortac/2109-dust-limit

Implement recent spec changes regarding collab channel close outputs
3 years ago · 5787f3ab74
6 changed files with 91 additions and 18 deletions
--- a/electrum/bitcoin.py
+++ b/electrum/bitcoin.py
@ -349,8 +349,12 @@ def relayfee(network: 'Network' = None) -> int:


 # 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
+# and https://github.com/lightningnetwork/lightning-rfc/blob/7e3dce42cbe4fa4592320db6a4e06c26bb99122b/03-transactions.md#dust-limits
+DUST_LIMIT_P2PKH = 546
+DUST_LIMIT_P2SH = 540
+DUST_LIMIT_UNKNOWN_SEGWIT = 354
+DUST_LIMIT_P2WSH = 330
+DUST_LIMIT_P2WPKH = 294


 def dust_threshold(network: 'Network' = None) -> int:
--- a/electrum/lnpeer.py
+++ b/electrum/lnpeer.py
@ -495,6 +495,9 @@ class Peer(Logger):
        self.lnworker.peer_closed(self)
        self.got_disconnected.set()

+    def is_shutdown_anysegwit(self):
+        return self.features.supports(LnFeatures.OPTION_SHUTDOWN_ANYSEGWIT_OPT)
+
    def is_static_remotekey(self):
        return self.features.supports(LnFeatures.OPTION_STATIC_REMOTEKEY_OPT)

@ -531,7 +534,7 @@ class Peer(Logger):
            static_remotekey = bfh(wallet.get_public_key(addr))
        else:
            static_remotekey = None
-        dust_limit_sat = bitcoin.DUST_LIMIT_DEFAULT_SAT_LEGACY
+        dust_limit_sat = bitcoin.DUST_LIMIT_P2PKH
        reserve_sat = max(funding_sat // 100, dust_limit_sat)
        # for comparison of defaults, see
        # https://github.com/ACINQ/eclair/blob/afa378fbb73c265da44856b4ad0f2128a88ae6c6/eclair-core/src/main/resources/reference.conf#L66
@ -1343,6 +1346,8 @@ class Peer(Logger):
        #        - for example; atm we forward first and then persist "forwarding_info",
        #          so if we segfault in-between and restart, we might forward an HTLC twice...
        #          (same for trampoline forwarding)
+        #        - we could check for the exposure to dust HTLCs, see:
+        #          https://github.com/ACINQ/eclair/pull/1985
        forwarding_enabled = self.network.config.get('lightning_forward_payments', False)
        if not forwarding_enabled:
            self.logger.info(f"forwarding is disabled. failing htlc.")
@ -1692,11 +1697,15 @@ class Peer(Logger):
        if their_upfront_scriptpubkey:
            if not (their_scriptpubkey == their_upfront_scriptpubkey):
                raise UpfrontShutdownScriptViolation("remote didn't use upfront shutdown script it commited to in channel opening")
-        # BOLT-02 restrict the scriptpubkey to some templates:
-        if not (match_script_against_template(their_scriptpubkey, transaction.SCRIPTPUBKEY_TEMPLATE_WITNESS_V0)
-                or match_script_against_template(their_scriptpubkey, transaction.SCRIPTPUBKEY_TEMPLATE_P2SH)
-                or match_script_against_template(their_scriptpubkey, transaction.SCRIPTPUBKEY_TEMPLATE_P2PKH)):
-            raise Exception(f'scriptpubkey in received shutdown message does not conform to any template: {their_scriptpubkey.hex()}')
+        else:
+            # BOLT-02 restrict the scriptpubkey to some templates:
+            if self.is_shutdown_anysegwit() and match_script_against_template(their_scriptpubkey, transaction.SCRIPTPUBKEY_TEMPLATE_ANYSEGWIT):
+                pass
+            elif match_script_against_template(their_scriptpubkey, transaction.SCRIPTPUBKEY_TEMPLATE_WITNESS_V0):
+                pass
+            else:
+                raise Exception(f'scriptpubkey in received shutdown message does not conform to any template: {their_scriptpubkey.hex()}')
+
        chan_id = chan.channel_id
        if chan_id in self.shutdown_received:
            self.shutdown_received[chan_id].set_result(payload)
@ -1753,9 +1762,9 @@ class Peer(Logger):
        # BOLT2: The sending node MUST set fee less than or equal to the base fee of the final ctx
        max_fee = chan.get_latest_fee(LOCAL if is_local else REMOTE)
        our_fee = min(our_fee, max_fee)
-        drop_remote = False
+        drop_to_remote = False
        def send_closing_signed():
-            our_sig, closing_tx = chan.make_closing_tx(our_scriptpubkey, their_scriptpubkey, fee_sat=our_fee, drop_remote=drop_remote)
+            our_sig, closing_tx = chan.make_closing_tx(our_scriptpubkey, their_scriptpubkey, fee_sat=our_fee, drop_remote=drop_to_remote)
            self.send_message('closing_signed', channel_id=chan.channel_id, fee_satoshis=our_fee, signature=our_sig)
        def verify_signature(tx, sig):
            their_pubkey = chan.config[REMOTE].multisig_key.pubkey
@ -1776,13 +1785,22 @@ class Peer(Logger):
            # verify their sig: they might have dropped their output
            our_sig, closing_tx = chan.make_closing_tx(our_scriptpubkey, their_scriptpubkey, fee_sat=their_fee, drop_remote=False)
            if verify_signature(closing_tx, their_sig):
-                drop_remote = False
+                drop_to_remote = False
            else:
                our_sig, closing_tx = chan.make_closing_tx(our_scriptpubkey, their_scriptpubkey, fee_sat=their_fee, drop_remote=True)
                if verify_signature(closing_tx, their_sig):
-                    drop_remote = True
+                    drop_to_remote = True
                else:
+                    # this can happen if we consider our output too valuable to drop,
+                    # but the remote drops it because it violates their dust limit
                    raise Exception('failed to verify their signature')
+            # at this point we know how the closing tx looks like
+            # check that their output is above their scriptpubkey's network dust limit
+            if not drop_to_remote:
+                to_remote_idx = closing_tx.get_output_idxs_from_scriptpubkey(their_scriptpubkey.hex()).pop()
+                to_remote_amount = closing_tx.outputs()[to_remote_idx].value
+                transaction.check_scriptpubkey_template_and_dust(their_scriptpubkey, to_remote_amount)
+
            # Agree if difference is lower or equal to one (see below)
            if abs(our_fee - their_fee) < 2:
                our_fee = their_fee
--- a/electrum/lnutil.py
+++ b/electrum/lnutil.py
@ -40,6 +40,7 @@ COMMITMENT_TX_WEIGHT = 724
 HTLC_OUTPUT_WEIGHT = 172

 LN_MAX_FUNDING_SAT = pow(2, 24) - 1
+DUST_LIMIT_MAX = 1000

 # dummy address for fee estimation of funding tx
 def ln_dummy_address():
@ -103,10 +104,10 @@ class ChannelConfig(StoredObject):
            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
-        # 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:
+        if self.dust_limit_sat < bitcoin.DUST_LIMIT_UNKNOWN_SEGWIT:
            raise Exception(f"{conf_name}. dust limit too low: {self.dust_limit_sat} sat")
+        if self.dust_limit_sat > DUST_LIMIT_MAX:
+            raise Exception(f"{conf_name}. dust limit too high: {self.dust_limit_sat} sat")
        if self.reserve_sat > funding_sat // 100:
            raise Exception(f"{conf_name}. reserve too high: {self.reserve_sat}, funding_sat: {funding_sat}")
        if self.htlc_minimum_msat > 1_000:
@ -1026,6 +1027,12 @@ class LnFeatures(IntFlag):
    _ln_feature_contexts[OPTION_TRAMPOLINE_ROUTING_REQ] = (LNFC.INIT | LNFC.NODE_ANN | LNFC.INVOICE)
    _ln_feature_contexts[OPTION_TRAMPOLINE_ROUTING_OPT] = (LNFC.INIT | LNFC.NODE_ANN | LNFC.INVOICE)

+    OPTION_SHUTDOWN_ANYSEGWIT_REQ = 1 << 26
+    OPTION_SHUTDOWN_ANYSEGWIT_OPT = 1 << 27
+
+    _ln_feature_contexts[OPTION_SHUTDOWN_ANYSEGWIT_REQ] = (LNFC.INIT | LNFC.NODE_ANN)
+    _ln_feature_contexts[OPTION_SHUTDOWN_ANYSEGWIT_OPT] = (LNFC.INIT | LNFC.NODE_ANN)
+
    # temporary
    OPTION_TRAMPOLINE_ROUTING_REQ_ECLAIR = 1 << 50
    OPTION_TRAMPOLINE_ROUTING_OPT_ECLAIR = 1 << 51
@ -1114,6 +1121,7 @@ LN_FEATURES_IMPLEMENTED = (
        | LnFeatures.PAYMENT_SECRET_OPT | LnFeatures.PAYMENT_SECRET_REQ
        | LnFeatures.BASIC_MPP_OPT | LnFeatures.BASIC_MPP_REQ
        | LnFeatures.OPTION_TRAMPOLINE_ROUTING_OPT | LnFeatures.OPTION_TRAMPOLINE_ROUTING_REQ
+        | LnFeatures.OPTION_SHUTDOWN_ANYSEGWIT_OPT | LnFeatures.OPTION_SHUTDOWN_ANYSEGWIT_REQ
 )


--- a/electrum/lnworker.py
+++ b/electrum/lnworker.py
@ -175,7 +175,8 @@ LNWALLET_FEATURES = BASE_FEATURES\
    | LnFeatures.OPTION_STATIC_REMOTEKEY_REQ\
    | LnFeatures.GOSSIP_QUERIES_REQ\
    | LnFeatures.BASIC_MPP_OPT\
-    | LnFeatures.OPTION_TRAMPOLINE_ROUTING_OPT
+    | LnFeatures.OPTION_TRAMPOLINE_ROUTING_OPT\
+    | LnFeatures.OPTION_SHUTDOWN_ANYSEGWIT_OPT\

 LNGOSSIP_FEATURES = BASE_FEATURES\
    | LnFeatures.GOSSIP_QUERIES_OPT\
--- a/electrum/tests/test_transaction.py
+++ b/electrum/tests/test_transaction.py
@ -3,7 +3,8 @@ from typing import NamedTuple, Union
 from electrum import transaction, bitcoin
 from electrum.transaction import (convert_raw_tx_to_hex, tx_from_any, Transaction,
                                  PartialTransaction, TxOutpoint, PartialTxInput,
-                                  PartialTxOutput, Sighash)
+                                  PartialTxOutput, Sighash, match_script_against_template,
+                                  SCRIPTPUBKEY_TEMPLATE_ANYSEGWIT)
 from electrum.util import bh2u, bfh
 from electrum.bitcoin import (deserialize_privkey, opcodes,
                              construct_script, construct_witness)
@ -78,6 +79,13 @@ class TestBCDataStream(ElectrumTestCase):


 class TestTransaction(ElectrumTestCase):
+    def test_match_against_script_template(self):
+        script = bfh(construct_script([opcodes.OP_5, bytes(29)]))
+        self.assertTrue(match_script_against_template(script, SCRIPTPUBKEY_TEMPLATE_ANYSEGWIT))
+        script = bfh(construct_script([opcodes.OP_NOP, bytes(30)]))
+        self.assertFalse(match_script_against_template(script, SCRIPTPUBKEY_TEMPLATE_ANYSEGWIT))
+        script = bfh(construct_script([opcodes.OP_0, bytes(50)]))
+        self.assertFalse(match_script_against_template(script, SCRIPTPUBKEY_TEMPLATE_ANYSEGWIT))

    def test_tx_update_signatures(self):
        tx = tx_from_any("cHNidP8BAFUBAAAAASpcmpT83pj1WBzQAWLGChOTbOt1OJ6mW/OGM7Qk60AxAAAAAAD/////AUBCDwAAAAAAGXapFCMKw3g0BzpCFG8R74QUrpKf6q/DiKwAAAAAAAAA")
@ -927,7 +935,7 @@ class TestSighashTypes(ElectrumTestCase):
    txin = PartialTxInput(prevout=prevout)
    txin.nsequence=0xffffffff
    txin.script_type='p2sh-p2wsh'
-    txin.witness_script = bfh('56210307b8ae49ac90a048e9b53357a2354b3334e9c8bee813ecb98e99a7e07e8c3ba32103b28f0c28bfab54554ae8c658ac5c3e0ce6e79ad336331f78c428dd43eea8449b21034b8113d703413d57761b8b9781957b8c0ac1dfe69f492580ca4195f50376ba4a21033400f6afecb833092a9a21cfdf1ed1376e58c5d1f47de74683123987e967a8f42103a6d48b1131e94ba04d9737d61acdaa1322008af9602b3b14862c07a1789aac162102d8b661b0b3302ee2f162b09e07a55ad5dfbe673a9f01d9f0c19617681024306b56ae') 
+    txin.witness_script = bfh('56210307b8ae49ac90a048e9b53357a2354b3334e9c8bee813ecb98e99a7e07e8c3ba32103b28f0c28bfab54554ae8c658ac5c3e0ce6e79ad336331f78c428dd43eea8449b21034b8113d703413d57761b8b9781957b8c0ac1dfe69f492580ca4195f50376ba4a21033400f6afecb833092a9a21cfdf1ed1376e58c5d1f47de74683123987e967a8f42103a6d48b1131e94ba04d9737d61acdaa1322008af9602b3b14862c07a1789aac162102d8b661b0b3302ee2f162b09e07a55ad5dfbe673a9f01d9f0c19617681024306b56ae')
    txin.redeem_script = bfh('0020a16b5755f7f6f96dbd65f5f0d6ab9418b89af4b1f14a1bb8a09062c35f0dcb54')
    txin._trusted_value_sats = 987654321
    #Output of Transaction
--- a/electrum/transaction.py
+++ b/electrum/transaction.py
@ -431,7 +431,23 @@ class OPPushDataGeneric:
               or (isinstance(item, type) and issubclass(item, cls))


+class OPGeneric:
+    def __init__(self, matcher: Callable=None):
+        if matcher is not None:
+            self.matcher = matcher
+
+    def match(self, op) -> bool:
+        return self.matcher(op)
+
+    @classmethod
+    def is_instance(cls, item):
+        # accept objects that are instances of this class
+        # or other classes that are subclasses
+        return isinstance(item, cls) \
+               or (isinstance(item, type) and issubclass(item, cls))
+
 OPPushDataPubkey = OPPushDataGeneric(lambda x: x in (33, 65))
+OP_ANYSEGWIT_VERSION = OPGeneric(lambda x: x in list(range(opcodes.OP_1, opcodes.OP_16 + 1)))

 SCRIPTPUBKEY_TEMPLATE_P2PKH = [opcodes.OP_DUP, opcodes.OP_HASH160,
                               OPPushDataGeneric(lambda x: x == 20),
@ -440,6 +456,22 @@ SCRIPTPUBKEY_TEMPLATE_P2SH = [opcodes.OP_HASH160, OPPushDataGeneric(lambda x: x
 SCRIPTPUBKEY_TEMPLATE_WITNESS_V0 = [opcodes.OP_0, OPPushDataGeneric(lambda x: x in (20, 32))]
 SCRIPTPUBKEY_TEMPLATE_P2WPKH = [opcodes.OP_0, OPPushDataGeneric(lambda x: x == 20)]
 SCRIPTPUBKEY_TEMPLATE_P2WSH = [opcodes.OP_0, OPPushDataGeneric(lambda x: x == 32)]
+SCRIPTPUBKEY_TEMPLATE_ANYSEGWIT = [OP_ANYSEGWIT_VERSION, OPPushDataGeneric(lambda x: x in list(range(2, 40 + 1)))]
+
+
+def check_scriptpubkey_template_and_dust(scriptpubkey, amount: Optional[int]):
+    if match_script_against_template(scriptpubkey, SCRIPTPUBKEY_TEMPLATE_P2PKH):
+        dust_limit = bitcoin.DUST_LIMIT_P2PKH
+    elif match_script_against_template(scriptpubkey, SCRIPTPUBKEY_TEMPLATE_P2SH):
+        dust_limit = bitcoin.DUST_LIMIT_P2SH
+    elif match_script_against_template(scriptpubkey, SCRIPTPUBKEY_TEMPLATE_P2WSH):
+        dust_limit = bitcoin.DUST_LIMIT_P2WSH
+    elif match_script_against_template(scriptpubkey, SCRIPTPUBKEY_TEMPLATE_P2WPKH):
+        dust_limit = bitcoin.DUST_LIMIT_P2WPKH
+    else:
+        raise Exception(f'scriptpubkey does not conform to any template: {scriptpubkey.hex()}')
+    if amount < dust_limit:
+        raise Exception(f'amount ({amount}) is below dust limit for scriptpubkey type ({dust_limit})')


 def match_script_against_template(script, template) -> bool:
@ -459,6 +491,8 @@ def match_script_against_template(script, template) -> bool:
        script_item = script[i]
        if OPPushDataGeneric.is_instance(template_item) and template_item.check_data_len(script_item[0]):
            continue
+        if OPGeneric.is_instance(template_item) and template_item.match(script_item[0]):
+            continue
        if template_item != script_item[0]:
            return False
    return True