From fc6fab499cdad4b1cfc13bb62f3a4ce944582e74 Mon Sep 17 00:00:00 2001 From: Bastien Teinturier <31281497+t-bast@users.noreply.github.com> Date: Wed, 20 Oct 2021 15:11:12 +0200 Subject: [PATCH] Set remote dust limit lower bound (#294) We are slowly dropping support for non-segwit outputs, as proposed in https://github.com/lightningnetwork/lightning-rfc/pull/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. --- .../fr/acinq/lightning/channel/Channel.kt | 6 ++- .../lightning/channel/ChannelException.kt | 1 + .../fr/acinq/lightning/channel/Helpers.kt | 30 ++++++++++-- .../lightning/channel/HelpersTestsCommon.kt | 48 +++++++++++++++---- .../states/WaitForAcceptChannelTestsCommon.kt | 4 +- 5 files changed, 73 insertions(+), 16 deletions(-) diff --git a/src/commonMain/kotlin/fr/acinq/lightning/channel/Channel.kt b/src/commonMain/kotlin/fr/acinq/lightning/channel/Channel.kt index b2c76ea86..f1708a67c 100644 --- a/src/commonMain/kotlin/fr/acinq/lightning/channel/Channel.kt +++ b/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 diff --git a/src/commonMain/kotlin/fr/acinq/lightning/channel/ChannelException.kt b/src/commonMain/kotlin/fr/acinq/lightning/channel/ChannelException.kt index 479530431..82c5de461 100644 --- a/src/commonMain/kotlin/fr/acinq/lightning/channel/ChannelException.kt +++ b/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") diff --git a/src/commonMain/kotlin/fr/acinq/lightning/channel/Helpers.kt b/src/commonMain/kotlin/fr/acinq/lightning/channel/Helpers.kt index 865b8f0b9..a5b2e5eb3 100644 --- a/src/commonMain/kotlin/fr/acinq/lightning/channel/Helpers.kt +++ b/src/commonMain/kotlin/fr/acinq/lightning/channel/Helpers.kt @@ -451,10 +451,32 @@ object Helpers { remoteClosingSig: ByteVector64 ): Either { 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 + } } } diff --git a/src/commonTest/kotlin/fr/acinq/lightning/channel/HelpersTestsCommon.kt b/src/commonTest/kotlin/fr/acinq/lightning/channel/HelpersTestsCommon.kt index 803392507..8374125a2 100644 --- a/src/commonTest/kotlin/fr/acinq/lightning/channel/HelpersTestsCommon.kt +++ b/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): 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))) + } + } \ No newline at end of file diff --git a/src/commonTest/kotlin/fr/acinq/lightning/channel/states/WaitForAcceptChannelTestsCommon.kt b/src/commonTest/kotlin/fr/acinq/lightning/channel/states/WaitForAcceptChannelTestsCommon.kt index 467f02ae0..3dac3db2d 100644 --- a/src/commonTest/kotlin/fr/acinq/lightning/channel/states/WaitForAcceptChannelTestsCommon.kt +++ b/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)))