Set remote dust limit lower bound (#294)

We are slowly dropping support for non-segwit outputs, as proposed in
lightning/bolts#894

We can thus safely allow dust limits all the way down to 354 satoshis.

In very rare cases where dust_limit_satoshis is negotiated to a low value,
our peer may generate closing txs that will not correctly relay on the
bitcoin network due to dust relay policies.

When that happens, we detect it and force-close instead of completing the
mutual close flow.

src/commonMain/kotlin/fr/acinq/lightning/channel/Channel.kt

@@ -3011,8 +3011,10 @@ object Channel {
 
     // We may need to rely on our peer's commit tx in certain cases (backup/restore) so we must ensure their transactions
     // can propagate through the bitcoin network (assuming bitcoin core nodes with default policies).
-    // A minimal spend of a p2wsh output is 110 bytes and bitcoin core's dust-relay-fee is 3000 sat/kb, which amounts to 330 sat.
-    val MIN_DUST_LIMIT = 330.sat
+    // The various dust limits enforced by the bitcoin network are summarized here:
+    // https://github.com/lightningnetwork/lightning-rfc/blob/master/03-transactions.md#dust-limits
+    // A dust limit of 354 sat ensures all segwit outputs will relay with default relay policies.
+    val MIN_DUST_LIMIT = 354.sat
 
     // we won't exchange more than this many signatures when negotiating the closing fee
     const val MAX_NEGOTIATION_ITERATIONS = 20

src/commonMain/kotlin/fr/acinq/lightning/channel/ChannelException.kt

@@ -48,6 +48,7 @@ data class InvalidCommitmentSignature              (override val channelId: Byte
 data class InvalidHtlcSignature                    (override val channelId: ByteVector32, val tx: Transaction) : ChannelException(channelId, "invalid htlc signature: tx=$tx")
 data class InvalidCloseSignature                   (override val channelId: ByteVector32, val tx: Transaction) : ChannelException(channelId, "invalid close signature: tx=$tx")
 data class InvalidCloseFee                         (override val channelId: ByteVector32, val fee: Satoshi) : ChannelException(channelId, "invalid close fee: fee_satoshis=$fee")
+data class InvalidCloseAmountBelowDust             (override val channelId: ByteVector32, val tx: Transaction) : ChannelException(channelId, "invalid closing tx: some outputs are below dust: tx=$tx")
 data class HtlcSigCountMismatch                    (override val channelId: ByteVector32, val expected: Int, val actual: Int) : ChannelException(channelId, "htlc sig count mismatch: expected=$expected actual: $actual")
 data class ForcedLocalCommit                       (override val channelId: ByteVector32) : ChannelException(channelId, "forced local commit")
 data class UnexpectedHtlcId                        (override val channelId: ByteVector32, val expected: Long, val actual: Long) : ChannelException(channelId, "unexpected htlc id: expected=$expected actual=$actual")

src/commonMain/kotlin/fr/acinq/lightning/channel/Helpers.kt

@@ -451,10 +451,32 @@ object Helpers {
             remoteClosingSig: ByteVector64
         ): Either<ChannelException, ClosingTx> {
             val (closingTx, closingSigned) = makeClosingTx(keyManager, commitments, localScriptPubkey, remoteScriptPubkey, remoteClosingFee)
-            val signedClosingTx = Transactions.addSigs(closingTx, commitments.localParams.channelKeys.fundingPubKey, commitments.remoteParams.fundingPubKey, closingSigned.signature, remoteClosingSig)
-            return when (Transactions.checkSpendable(signedClosingTx)) {
-                is Try.Success -> Either.Right(signedClosingTx)
-                is Try.Failure -> Either.Left(InvalidCloseSignature(commitments.channelId, signedClosingTx.tx))
+            return if (checkClosingDustAmounts(closingTx)) {
+                val signedClosingTx = Transactions.addSigs(closingTx, commitments.localParams.channelKeys.fundingPubKey, commitments.remoteParams.fundingPubKey, closingSigned.signature, remoteClosingSig)
+                when (Transactions.checkSpendable(signedClosingTx)) {
+                    is Try.Success -> Either.Right(signedClosingTx)
+                    is Try.Failure -> Either.Left(InvalidCloseSignature(commitments.channelId, signedClosingTx.tx))
+                }
+            } else {
+                Either.Left(InvalidCloseAmountBelowDust(commitments.channelId, closingTx.tx))
+            }
+        }
+
+        /**
+         * Check that all closing outputs are above bitcoin's dust limit for their script type, otherwise there is a risk
+         * that the closing transaction will not be relayed to miners' mempool and will not confirm.
+         * The various dust limits are detailed in https://github.com/lightningnetwork/lightning-rfc/blob/master/03-transactions.md#dust-limits
+         */
+        fun checkClosingDustAmounts(closingTx: ClosingTx): Boolean {
+            return closingTx.tx.txOut.all { txOut ->
+                val publicKeyScript = txOut.publicKeyScript.toByteArray()
+                when {
+                    Script.isPay2pkh(publicKeyScript) -> txOut.amount >= 546.sat
+                    Script.isPay2sh(publicKeyScript) -> txOut.amount >= 540.sat
+                    Script.isPay2wpkh(publicKeyScript) -> txOut.amount >= 294.sat
+                    Script.isPay2wsh(publicKeyScript) -> txOut.amount >= 330.sat
+                    else -> false
+                }
             }
         }
 

src/commonTest/kotlin/fr/acinq/lightning/channel/HelpersTestsCommon.kt

@@ -4,10 +4,16 @@ import fr.acinq.bitcoin.*
 import fr.acinq.bitcoin.Bitcoin.computeP2PkhAddress
 import fr.acinq.bitcoin.Bitcoin.computeP2ShOfP2WpkhAddress
 import fr.acinq.bitcoin.Bitcoin.computeP2WpkhAddress
+import fr.acinq.lightning.Lightning.randomKey
+import fr.acinq.lightning.channel.Helpers.Closing.checkClosingDustAmounts
 import fr.acinq.lightning.tests.utils.LightningTestSuite
+import fr.acinq.lightning.transactions.Transactions
+import fr.acinq.lightning.utils.sat
 import fr.acinq.secp256k1.Hex
 import kotlin.test.Test
 import kotlin.test.assertEquals
+import kotlin.test.assertFalse
+import kotlin.test.assertTrue
 
 class HelpersTestsCommon : LightningTestSuite() {
 
@@ -30,14 +36,14 @@ class HelpersTestsCommon : LightningTestSuite() {
         }
 
         listOf(
-                Triple("0014d0b19277b0f76c9512f26d77573fd31a8fd15fc7", Block.TestnetGenesisBlock.hash, "tb1q6zceyaas7akf2yhjd4m4w07nr28azh78gw79kk"),
-                Triple("00203287047df2aa7aade3f394790a9c9d6f9235943f48a012e8a9f2c3300ca4f2d1", Block.TestnetGenesisBlock.hash, "tb1qx2rsgl0j4fa2mclnj3us48yad7frt9plfzsp969f7tpnqr9y7tgsyprxej"),
-                Triple("76a914b17deefe2feab87fef7221cf806bb8ca61f00fa188ac", Block.TestnetGenesisBlock.hash, "mwhSm2SHhRhd19KZyaQLgJyAtCLnkbzWbf"),
-                Triple("a914d3cf9d04f4ecc36df8207b300e46bc6775fc84c087", Block.TestnetGenesisBlock.hash, "2NCZBGzKadAnLv1ijAqhrKavMuqvxqu18yY"),
-                Triple("00145cb882efd643b7d63ae133e4d5e88e10bd5a20d7", Block.LivenetGenesisBlock.hash, "bc1qtjug9m7kgwmavwhpx0jdt6ywzz745gxhxwyn8u"),
-                Triple("00208c2865c87ffd33fc5d698c7df9cf2d0fb39d93103c637a06dea32c848ebc3e1d", Block.LivenetGenesisBlock.hash, "bc1q3s5xtjrll5elchtf337lnnedp7eemycs833h5pk75vkgfr4u8cws3ytg02"),
-                Triple("76a914536ffa992491508dca0354e52f32a3a7a679a53a88ac", Block.LivenetGenesisBlock.hash, "18cBEMRxXHqzWWCxZNtU91F5sbUNKhL5PX"),
-                Triple("a91481b9ac6a59b53927da7277b5ad5460d781b365d987", Block.LivenetGenesisBlock.hash, "3DWwX7NYjnav66qygrm4mBCpiByjammaWy"),
+            Triple("0014d0b19277b0f76c9512f26d77573fd31a8fd15fc7", Block.TestnetGenesisBlock.hash, "tb1q6zceyaas7akf2yhjd4m4w07nr28azh78gw79kk"),
+            Triple("00203287047df2aa7aade3f394790a9c9d6f9235943f48a012e8a9f2c3300ca4f2d1", Block.TestnetGenesisBlock.hash, "tb1qx2rsgl0j4fa2mclnj3us48yad7frt9plfzsp969f7tpnqr9y7tgsyprxej"),
+            Triple("76a914b17deefe2feab87fef7221cf806bb8ca61f00fa188ac", Block.TestnetGenesisBlock.hash, "mwhSm2SHhRhd19KZyaQLgJyAtCLnkbzWbf"),
+            Triple("a914d3cf9d04f4ecc36df8207b300e46bc6775fc84c087", Block.TestnetGenesisBlock.hash, "2NCZBGzKadAnLv1ijAqhrKavMuqvxqu18yY"),
+            Triple("00145cb882efd643b7d63ae133e4d5e88e10bd5a20d7", Block.LivenetGenesisBlock.hash, "bc1qtjug9m7kgwmavwhpx0jdt6ywzz745gxhxwyn8u"),
+            Triple("00208c2865c87ffd33fc5d698c7df9cf2d0fb39d93103c637a06dea32c848ebc3e1d", Block.LivenetGenesisBlock.hash, "bc1q3s5xtjrll5elchtf337lnnedp7eemycs833h5pk75vkgfr4u8cws3ytg02"),
+            Triple("76a914536ffa992491508dca0354e52f32a3a7a679a53a88ac", Block.LivenetGenesisBlock.hash, "18cBEMRxXHqzWWCxZNtU91F5sbUNKhL5PX"),
+            Triple("a91481b9ac6a59b53927da7277b5ad5460d781b365d987", Block.LivenetGenesisBlock.hash, "3DWwX7NYjnav66qygrm4mBCpiByjammaWy"),
         ).forEach {
             assertEquals(
                 Helpers.Closing.btcAddressFromScriptPubKey(
@@ -48,4 +54,30 @@ class HelpersTestsCommon : LightningTestSuite() {
             )
         }
     }
+
+    @Test
+    fun `check closing tx amounts above dust`() {
+        val p2pkhBelowDust = listOf(TxOut(545.sat, Script.pay2pkh(randomKey().publicKey())))
+        val p2shBelowDust = listOf(TxOut(539.sat, Script.pay2sh(Hex.decode("0000000000000000000000000000000000000000"))))
+        val p2wpkhBelowDust = listOf(TxOut(293.sat, Script.pay2wpkh(randomKey().publicKey())))
+        val p2wshBelowDust = listOf(TxOut(329.sat, Script.pay2wsh(Hex.decode("0000000000000000000000000000000000000000"))))
+        val allOutputsAboveDust = listOf(
+            TxOut(546.sat, Script.pay2pkh(randomKey().publicKey())),
+            TxOut(540.sat, Script.pay2sh(Hex.decode("0000000000000000000000000000000000000000"))),
+            TxOut(294.sat, Script.pay2wpkh(randomKey().publicKey())),
+            TxOut(330.sat, Script.pay2wsh(Hex.decode("0000000000000000000000000000000000000000"))),
+        )
+
+        fun toClosingTx(txOut: List<TxOut>): Transactions.TransactionWithInputInfo.ClosingTx {
+            val input = Transactions.InputInfo(OutPoint(ByteVector32.Zeroes, 0), TxOut(1000.sat, listOf()), listOf())
+            return Transactions.TransactionWithInputInfo.ClosingTx(input, Transaction(2, listOf(), txOut, 0), null)
+        }
+
+        assertTrue(checkClosingDustAmounts(toClosingTx(allOutputsAboveDust)))
+        assertFalse(checkClosingDustAmounts(toClosingTx(p2pkhBelowDust)))
+        assertFalse(checkClosingDustAmounts(toClosingTx(p2shBelowDust)))
+        assertFalse(checkClosingDustAmounts(toClosingTx(p2wpkhBelowDust)))
+        assertFalse(checkClosingDustAmounts(toClosingTx(p2wshBelowDust)))
+    }
+
 }

src/commonTest/kotlin/fr/acinq/lightning/channel/states/WaitForAcceptChannelTestsCommon.kt

@@ -41,8 +41,8 @@ class WaitForAcceptChannelTestsCommon : LightningTestSuite() {
     @Test
     fun `recv AcceptChannel (dust limit too low)`() {
         val (alice, _, accept) = init()
-        // we don't want their dust limit to be below 330
-        val lowDustLimitSatoshis = 329.sat
+        // we don't want their dust limit to be below 354
+        val lowDustLimitSatoshis = 353.sat
         // but we only enforce it on mainnet
         val aliceMainnet = alice.copy(staticParams = alice.staticParams.copy(nodeParams = alice.staticParams.nodeParams.copy(chainHash = Block.LivenetGenesisBlock.hash)))
         val (alice1, actions1) = aliceMainnet.process(ChannelEvent.MessageReceived(accept.copy(dustLimitSatoshis = lowDustLimitSatoshis)))