diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala index 9b6d9b9e9b..78c0e4a090 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala @@ -94,13 +94,16 @@ case class Commitments(channelVersion: ChannelVersion, if (localParams.isFunder) { // The funder always pays the on-chain fees, so we must subtract that from the amount we can send. val commitFees = commitTxFeeMsat(remoteParams.dustLimit, reduced) + // the funder needs to keep an extra reserve to be able to handle fee increase without getting the channel stuck + // (see https://github.com/lightningnetwork/lightning-rfc/issues/728) + val funderFeeReserve = htlcOutputFee(2 * reduced.feeratePerKw) val htlcFees = htlcOutputFee(reduced.feeratePerKw) if (balanceNoFees - commitFees < offeredHtlcTrimThreshold(remoteParams.dustLimit, reduced)) { // htlc will be trimmed - (balanceNoFees - commitFees).max(0 msat) + (balanceNoFees - commitFees - funderFeeReserve).max(0 msat) } else { // htlc will have an output in the commitment tx, so there will be additional fees. - (balanceNoFees - commitFees - htlcFees).max(0 msat) + (balanceNoFees - commitFees - funderFeeReserve - htlcFees).max(0 msat) } } else { // The fundee doesn't pay on-chain fees. @@ -117,13 +120,16 @@ case class Commitments(channelVersion: ChannelVersion, } else { // The funder always pays the on-chain fees, so we must subtract that from the amount we can receive. val commitFees = commitTxFeeMsat(localParams.dustLimit, reduced) + // we expect the funder to keep an extra reserve to be able to handle fee increase without getting the channel stuck + // (see https://github.com/lightningnetwork/lightning-rfc/issues/728) + val funderFeeReserve = htlcOutputFee(2 * reduced.feeratePerKw) val htlcFees = htlcOutputFee(reduced.feeratePerKw) if (balanceNoFees - commitFees < receivedHtlcTrimThreshold(localParams.dustLimit, reduced)) { // htlc will be trimmed - (balanceNoFees - commitFees).max(0 msat) + (balanceNoFees - commitFees - funderFeeReserve).max(0 msat) } else { // htlc will have an output in the commitment tx, so there will be additional fees. - (balanceNoFees - commitFees - htlcFees).max(0 msat) + (balanceNoFees - commitFees - funderFeeReserve - htlcFees).max(0 msat) } } } @@ -185,7 +191,10 @@ object Commitments { // note that the funder pays the fee, so if sender != funder, both sides will have to afford this payment val fees = commitTxFee(commitments1.remoteParams.dustLimit, reduced) - val missingForSender = reduced.toRemote - commitments1.remoteParams.channelReserve - (if (commitments1.localParams.isFunder) fees else 0.sat) + // the funder needs to keep an extra reserve to be able to handle fee increase without getting the channel stuck + // (see https://github.com/lightningnetwork/lightning-rfc/issues/728) + val funderFeeReserve = htlcOutputFee(2 * reduced.feeratePerKw) + val missingForSender = reduced.toRemote - commitments1.remoteParams.channelReserve - (if (commitments1.localParams.isFunder) fees + funderFeeReserve else 0.msat) val missingForReceiver = reduced.toLocal - commitments1.localParams.channelReserve - (if (commitments1.localParams.isFunder) 0.sat else fees) if (missingForSender < 0.msat) { return Failure(InsufficientFunds(commitments.channelId, amount = cmd.amount, missing = -missingForSender.truncateToSatoshi, reserve = commitments1.remoteParams.channelReserve, fees = if (commitments1.localParams.isFunder) fees else 0.sat)) @@ -227,6 +236,8 @@ object Commitments { // note that the funder pays the fee, so if sender != funder, both sides will have to afford this payment val fees = commitTxFee(commitments1.remoteParams.dustLimit, reduced) + // NB: we don't enforce the funderFeeReserve (see sendAdd) because it would confuse a remote funder that doesn't have this mitigation in place + // We could enforce it once we're confident a large portion of the network implements it. val missingForSender = reduced.toRemote - commitments1.localParams.channelReserve - (if (commitments1.localParams.isFunder) 0.sat else fees) val missingForReceiver = reduced.toLocal - commitments1.remoteParams.channelReserve - (if (commitments1.localParams.isFunder) fees else 0.sat) if (missingForSender < 0.sat) { diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/CommitmentsSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/CommitmentsSpec.scala index 0556628a75..ac29fdd540 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/CommitmentsSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/CommitmentsSpec.scala @@ -53,7 +53,9 @@ class CommitmentsSpec extends TestkitBaseClass with StateTestsHelperMethods { test("take additional HTLC fee into account") { f => import f._ - val htlcOutputFee = 1720000 msat + // The fee for a single HTLC is 1720000 msat but the funder keeps an extra reserve to make sure we're able to handle + // an additional HTLC at twice the feerate (hence the multiplier). + val htlcOutputFee = 3 * 1720000 msat val a = 772760000 msat // initial balance alice val ac0 = alice.stateData.asInstanceOf[DATA_NORMAL].commitments val bc0 = bob.stateData.asInstanceOf[DATA_NORMAL].commitments @@ -75,7 +77,8 @@ class CommitmentsSpec extends TestkitBaseClass with StateTestsHelperMethods { import f._ val fee = 1720000 msat // fee due to the additional htlc output - val a = (772760000 msat) - fee // initial balance alice + val funderFeeReserve = fee * 2 // extra reserve to handle future fee increase + val a = (772760000 msat) - fee - funderFeeReserve // initial balance alice val b = 190000000 msat // initial balance bob val p = 42000000 msat // a->b payment @@ -159,7 +162,8 @@ class CommitmentsSpec extends TestkitBaseClass with StateTestsHelperMethods { import f._ val fee = 1720000 msat // fee due to the additional htlc output - val a = (772760000 msat) - fee // initial balance alice + val funderFeeReserve = fee * 2 // extra reserve to handle future fee increase + val a = (772760000 msat) - fee - funderFeeReserve // initial balance alice val b = 190000000 msat // initial balance bob val p = 42000000 msat // a->b payment @@ -243,7 +247,8 @@ class CommitmentsSpec extends TestkitBaseClass with StateTestsHelperMethods { import f._ val fee = 1720000 msat // fee due to the additional htlc output - val a = (772760000 msat) - fee // initial balance alice + val funderFeeReserve = fee * 2 // extra reserve to handle future fee increase + val a = (772760000 msat) - fee - funderFeeReserve // initial balance alice val b = 190000000 msat // initial balance bob val p1 = 10000000 msat // a->b payment val p2 = 20000000 msat // a->b payment @@ -386,6 +391,23 @@ class CommitmentsSpec extends TestkitBaseClass with StateTestsHelperMethods { assert(ac16.availableBalanceForReceive == b + p1 - p3) } + // See https://github.com/lightningnetwork/lightning-rfc/issues/728 + test("funder keeps additional reserve to avoid channel being stuck") { f => + val isFunder = true + val c = CommitmentsSpec.makeCommitments(100000000 msat, 50000000 msat, 2500, 546 sat, isFunder) + val (_, cmdAdd) = makeCmdAdd(c.availableBalanceForSend, randomKey.publicKey, f.currentBlockHeight) + val Success((c1, _)) = sendAdd(c, cmdAdd, Local(UUID.randomUUID, None), f.currentBlockHeight) + assert(c1.availableBalanceForSend === 0.msat) + + // We should be able to handle a fee increase. + val Success((c2, _)) = sendFee(c1, CMD_UPDATE_FEE(3000)) + + // Now we shouldn't be able to send until we receive enough to handle the updated commit tx fee (even trimmed HTLCs shouldn't be sent). + val (_, cmdAdd1) = makeCmdAdd(100 msat, randomKey.publicKey, f.currentBlockHeight) + val Failure(e) = sendAdd(c2, cmdAdd1, Local(UUID.randomUUID, None), f.currentBlockHeight) + assert(e.isInstanceOf[InsufficientFunds]) + } + test("can send availableForSend") { f => for (isFunder <- Seq(true, false)) { val c = CommitmentsSpec.makeCommitments(702000000 msat, 52000000 msat, 2679, 546 sat, isFunder) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala index 3e4196041f..86456652bc 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala @@ -190,7 +190,7 @@ class NormalStateSpec extends TestkitBaseClass with StateTestsHelperMethods { val initialState = alice.stateData.asInstanceOf[DATA_NORMAL] val add = CMD_ADD_HTLC(MilliSatoshi(Int.MaxValue), randomBytes32, CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), TestConstants.emptyOnionPacket, Upstream.Local(UUID.randomUUID())) sender.send(alice, add) - val error = InsufficientFunds(channelId(alice), amount = MilliSatoshi(Int.MaxValue), missing = 1376443 sat, reserve = 20000 sat, fees = 8960 sat) + val error = InsufficientFunds(channelId(alice), amount = MilliSatoshi(Int.MaxValue), missing = 1379883 sat, reserve = 20000 sat, fees = 8960 sat) sender.expectMsg(Failure(AddHtlcFailed(channelId(alice), add.paymentHash, error, Origin.Local(add.upstream.asInstanceOf[Upstream.Local].id, Some(sender.ref)), Some(initialState.channelUpdate), Some(add)))) alice2bob.expectNoMsg(200 millis) } @@ -207,19 +207,27 @@ class NormalStateSpec extends TestkitBaseClass with StateTestsHelperMethods { alice2bob.expectNoMsg(200 millis) } - test("recv CMD_ADD_HTLC (HTLC dips remote funder below reserve)") { f => + test("recv CMD_ADD_HTLC (HTLC dips into remote funder fee reserve)") { f => import f._ val sender = TestProbe() - addHtlc(771000000 msat, alice, bob, alice2bob, bob2alice) + addHtlc(767600000 msat, alice, bob, alice2bob, bob2alice) crossSign(alice, bob, alice2bob, bob2alice) - assert(alice.stateData.asInstanceOf[DATA_NORMAL].commitments.availableBalanceForSend === 40000.msat) + assert(alice.stateData.asInstanceOf[DATA_NORMAL].commitments.availableBalanceForSend === 0.msat) // actual test begins - // at this point alice has the minimal amount to sustain a channel (29000 sat ~= alice reserve + commit fee) - val add = CMD_ADD_HTLC(120000000 msat, randomBytes32, CltvExpiry(400144), TestConstants.emptyOnionPacket, Upstream.Local(UUID.randomUUID())) - sender.send(bob, add) - val error = RemoteCannotAffordFeesForNewHtlc(channelId(bob), add.amount, missing = 1680 sat, 10000 sat, 10680 sat) - sender.expectMsg(Failure(AddHtlcFailed(channelId(bob), add.paymentHash, error, Origin.Local(add.upstream.asInstanceOf[Upstream.Local].id, Some(sender.ref)), Some(bob.stateData.asInstanceOf[DATA_NORMAL].channelUpdate), Some(add)))) + // at this point alice has the minimal amount to sustain a channel + // alice maintains an extra reserve to accommodate for a few more HTLCs, so the first two HTLCs should be allowed + sender.send(bob, CMD_ADD_HTLC(12000000 msat, randomBytes32, CltvExpiry(400144), TestConstants.emptyOnionPacket, Upstream.Local(UUID.randomUUID()))) + sender.expectMsg(ChannelCommandResponse.Ok) + + sender.send(bob, CMD_ADD_HTLC(12500000 msat, randomBytes32, CltvExpiry(400144), TestConstants.emptyOnionPacket, Upstream.Local(UUID.randomUUID()))) + sender.expectMsg(ChannelCommandResponse.Ok) + + // but this one will dip alice below her reserve: we must wait for the two previous HTLCs to settle before sending any more + val failedAdd = CMD_ADD_HTLC(11000000 msat, randomBytes32, CltvExpiry(400144), TestConstants.emptyOnionPacket, Upstream.Local(UUID.randomUUID())) + sender.send(bob, failedAdd) + val error = RemoteCannotAffordFeesForNewHtlc(channelId(bob), failedAdd.amount, missing = 1720 sat, 10000 sat, 14120 sat) + sender.expectMsg(Failure(AddHtlcFailed(channelId(bob), failedAdd.paymentHash, error, Origin.Local(failedAdd.upstream.asInstanceOf[Upstream.Local].id, Some(sender.ref)), Some(bob.stateData.asInstanceOf[DATA_NORMAL].channelUpdate), Some(failedAdd)))) } test("recv CMD_ADD_HTLC (insufficient funds w/ pending htlcs and 0 balance)") { f => @@ -232,7 +240,7 @@ class NormalStateSpec extends TestkitBaseClass with StateTestsHelperMethods { sender.send(alice, CMD_ADD_HTLC(200000000 msat, randomBytes32, CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), TestConstants.emptyOnionPacket, Upstream.Local(UUID.randomUUID()))) sender.expectMsg(ChannelCommandResponse.Ok) alice2bob.expectMsgType[UpdateAddHtlc] - sender.send(alice, CMD_ADD_HTLC(67600000 msat, randomBytes32, CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), TestConstants.emptyOnionPacket, Upstream.Local(UUID.randomUUID()))) + sender.send(alice, CMD_ADD_HTLC(64160000 msat, randomBytes32, CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), TestConstants.emptyOnionPacket, Upstream.Local(UUID.randomUUID()))) sender.expectMsg(ChannelCommandResponse.Ok) alice2bob.expectMsgType[UpdateAddHtlc] val add = CMD_ADD_HTLC(1000000 msat, randomBytes32, CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), TestConstants.emptyOnionPacket, Upstream.Local(UUID.randomUUID())) @@ -254,7 +262,7 @@ class NormalStateSpec extends TestkitBaseClass with StateTestsHelperMethods { alice2bob.expectMsgType[UpdateAddHtlc] val add = CMD_ADD_HTLC(500000000 msat, randomBytes32, CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), TestConstants.emptyOnionPacket, Upstream.Local(UUID.randomUUID())) sender.send(alice, add) - val error = InsufficientFunds(channelId(alice), amount = 500000000 msat, missing = 332400 sat, reserve = 20000 sat, fees = 12400 sat) + val error = InsufficientFunds(channelId(alice), amount = 500000000 msat, missing = 335840 sat, reserve = 20000 sat, fees = 12400 sat) sender.expectMsg(Failure(AddHtlcFailed(channelId(alice), add.paymentHash, error, Origin.Local(add.upstream.asInstanceOf[Upstream.Local].id, Some(sender.ref)), Some(initialState.channelUpdate), Some(add)))) alice2bob.expectNoMsg(200 millis) } @@ -316,7 +324,7 @@ class NormalStateSpec extends TestkitBaseClass with StateTestsHelperMethods { // this is over channel-capacity val add2 = CMD_ADD_HTLC(TestConstants.fundingSatoshis.toMilliSatoshi * 2 / 3, randomBytes32, CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), TestConstants.emptyOnionPacket, Upstream.Local(UUID.randomUUID())) sender.send(alice, add2) - val error = InsufficientFunds(channelId(alice), add2.amount, 564013 sat, 20000 sat, 10680 sat) + val error = InsufficientFunds(channelId(alice), add2.amount, 567453 sat, 20000 sat, 10680 sat) sender.expectMsg(Failure(AddHtlcFailed(channelId(alice), add2.paymentHash, error, Origin.Local(add2.upstream.asInstanceOf[Upstream.Local].id, Some(sender.ref)), Some(initialState.channelUpdate), Some(add2)))) alice2bob.expectNoMsg(200 millis) }