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 3a0f2b49d..da0f32345 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 @@ -236,15 +236,16 @@ object Commitments { } } + // We apply local *and* remote restrictions, to ensure both peers are happy with the resulting number of HTLCs. // NB: we need the `toSeq` because otherwise duplicate amountMsat would be removed (since outgoingHtlcs is a Set). val htlcValueInFlight = outgoingHtlcs.toSeq.map(_.amountMsat).sum - if (commitments1.remoteParams.maxHtlcValueInFlightMsat < htlcValueInFlight) { + if (Seq(commitments1.localParams.maxHtlcValueInFlightMsat, commitments1.remoteParams.maxHtlcValueInFlightMsat).min < htlcValueInFlight) { // TODO: this should be a specific UPDATE error - return Failure(HtlcValueTooHighInFlight(commitments.channelId, maximum = commitments1.remoteParams.maxHtlcValueInFlightMsat, actual = htlcValueInFlight)) + return Failure(HtlcValueTooHighInFlight(commitments.channelId, maximum = Seq(commitments1.localParams.maxHtlcValueInFlightMsat, commitments1.remoteParams.maxHtlcValueInFlightMsat).min, actual = htlcValueInFlight)) } - if (outgoingHtlcs.size > commitments1.remoteParams.maxAcceptedHtlcs) { - return Failure(TooManyAcceptedHtlcs(commitments.channelId, maximum = commitments1.remoteParams.maxAcceptedHtlcs)) + if (Seq(commitments1.localParams.maxAcceptedHtlcs, commitments1.remoteParams.maxAcceptedHtlcs).min < outgoingHtlcs.size) { + return Failure(TooManyAcceptedHtlcs(commitments.channelId, maximum = Seq(commitments1.localParams.maxAcceptedHtlcs, commitments1.remoteParams.maxAcceptedHtlcs).min)) } Success(commitments1, add) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/TestConstants.scala b/eclair-core/src/test/scala/fr/acinq/eclair/TestConstants.scala index 8d47639a6..b15b79b0e 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/TestConstants.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/TestConstants.scala @@ -118,7 +118,7 @@ object TestConstants { closeOnOfflineMismatch = true, updateFeeMinDiffRatio = 0.1 ), - maxHtlcValueInFlightMsat = UInt64(150000000), + maxHtlcValueInFlightMsat = UInt64(500000000), maxAcceptedHtlcs = 100, expiryDeltaBlocks = CltvExpiryDelta(144), fulfillSafetyBeforeTimeoutBlocks = CltvExpiryDelta(6), 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 f0cf9d52c..017ddb584 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 @@ -57,7 +57,7 @@ class CommitmentsSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with } } - test("take additional HTLC fee into account") { f => + test("take additional HTLC fee into account", Tag("no_max_htlc_value_inflight")) { f => import f._ // 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). diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/StateTestsHelperMethods.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/StateTestsHelperMethods.scala index 38d52d96a..66ac284f3 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/StateTestsHelperMethods.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/StateTestsHelperMethods.scala @@ -101,9 +101,12 @@ trait StateTestsHelperMethods extends TestKitBase with FixtureTestSuite with Par .modify(_.channelReserve).setToIf(channelVersion.hasZeroReserve)(0.sat) .modify(_.features).setToIf(channelVersion.hasStaticRemotekey)(Features(Set(ActivatedFeature(Features.StaticRemoteKey, FeatureSupport.Optional)))) .modify(_.staticPaymentBasepoint).setToIf(channelVersion.hasStaticRemotekey)(Some(Helpers.getWalletPaymentBasepoint(wallet))) + .modify(_.maxHtlcValueInFlightMsat).setToIf(tags.contains("no_max_htlc_value_inflight"))(UInt64.MaxValue) + .modify(_.maxHtlcValueInFlightMsat).setToIf(tags.contains("alice_low_max_htlc_value_inflight"))(UInt64(150000000)) val bobParams = Bob.channelParams .modify(_.features).setToIf(channelVersion.hasStaticRemotekey)(Features(Set(ActivatedFeature(Features.StaticRemoteKey, FeatureSupport.Optional)))) .modify(_.staticPaymentBasepoint).setToIf(channelVersion.hasStaticRemotekey)(Some(Helpers.getWalletPaymentBasepoint(wallet))) + .modify(_.maxHtlcValueInFlightMsat).setToIf(tags.contains("no_max_htlc_value_inflight"))(UInt64.MaxValue) val aliceInit = Init(aliceParams.features) val bobInit = Init(bobParams.features) alice ! INPUT_INIT_FUNDER(ByteVector32.Zeroes, fundingSatoshis, pushMsat, TestConstants.feeratePerKw, TestConstants.feeratePerKw, aliceParams, alice2bob.ref, bobInit, channelFlags, channelVersion) 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 f4a8f9ccb..9cab065ba 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 @@ -261,7 +261,7 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with alice2bob.expectNoMsg(200 millis) } - test("recv CMD_ADD_HTLC (HTLC dips into remote funder fee reserve)") { f => + test("recv CMD_ADD_HTLC (HTLC dips into remote funder fee reserve)", Tag("no_max_htlc_value_inflight")) { f => import f._ val sender = TestProbe() addHtlc(767600000 msat, alice, bob, alice2bob, bob2alice) @@ -284,7 +284,7 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with 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 => + test("recv CMD_ADD_HTLC (insufficient funds w/ pending htlcs and 0 balance)", Tag("no_max_htlc_value_inflight")) { f => import f._ val sender = TestProbe() val initialState = alice.stateData.asInstanceOf[DATA_NORMAL] @@ -304,7 +304,7 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with alice2bob.expectNoMsg(200 millis) } - test("recv CMD_ADD_HTLC (insufficient funds w/ pending htlcs 2/2)") { f => + test("recv CMD_ADD_HTLC (insufficient funds w/ pending htlcs 2/2)", Tag("no_max_htlc_value_inflight")) { f => import f._ val sender = TestProbe() val initialState = alice.stateData.asInstanceOf[DATA_NORMAL] @@ -321,10 +321,12 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with alice2bob.expectNoMsg(200 millis) } - test("recv CMD_ADD_HTLC (over max inflight htlc value)") { f => + test("recv CMD_ADD_HTLC (over remote max inflight htlc value)", Tag("alice_low_max_htlc_value_inflight")) { f => import f._ val sender = TestProbe() val initialState = bob.stateData.asInstanceOf[DATA_NORMAL] + assert(initialState.commitments.localParams.maxHtlcValueInFlightMsat === UInt64.MaxValue) + assert(initialState.commitments.remoteParams.maxHtlcValueInFlightMsat === UInt64(150000000)) val add = CMD_ADD_HTLC(151000000 msat, randomBytes32, CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), TestConstants.emptyOnionPacket, Upstream.Local(UUID.randomUUID())) sender.send(bob, add) val error = HtlcValueTooHighInFlight(channelId(bob), maximum = 150000000, actual = 151000000 msat) @@ -332,10 +334,12 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with bob2alice.expectNoMsg(200 millis) } - test("recv CMD_ADD_HTLC (over max inflight htlc value with duplicate amounts)") { f => + test("recv CMD_ADD_HTLC (over remote max inflight htlc value with duplicate amounts)", Tag("alice_low_max_htlc_value_inflight")) { f => import f._ val sender = TestProbe() val initialState = bob.stateData.asInstanceOf[DATA_NORMAL] + assert(initialState.commitments.localParams.maxHtlcValueInFlightMsat === UInt64.MaxValue) + assert(initialState.commitments.remoteParams.maxHtlcValueInFlightMsat === UInt64(150000000)) val add = CMD_ADD_HTLC(75500000 msat, randomBytes32, CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), TestConstants.emptyOnionPacket, Upstream.Local(UUID.randomUUID())) sender.send(bob, add) sender.expectMsg(ChannelCommandResponse.Ok) @@ -347,10 +351,25 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with bob2alice.expectNoMsg(200 millis) } - test("recv CMD_ADD_HTLC (over max accepted htlcs)") { f => + test("recv CMD_ADD_HTLC (over local max inflight htlc value)", Tag("alice_low_max_htlc_value_inflight")) { f => import f._ val sender = TestProbe() val initialState = alice.stateData.asInstanceOf[DATA_NORMAL] + assert(initialState.commitments.localParams.maxHtlcValueInFlightMsat === UInt64(150000000)) + assert(initialState.commitments.remoteParams.maxHtlcValueInFlightMsat === UInt64.MaxValue) + val add = CMD_ADD_HTLC(151000000 msat, randomBytes32, CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), TestConstants.emptyOnionPacket, Upstream.Local(UUID.randomUUID())) + sender.send(alice, add) + val error = HtlcValueTooHighInFlight(channelId(alice), maximum = 150000000, actual = 151000000 msat) + 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) + } + + test("recv CMD_ADD_HTLC (over remote max accepted htlcs)") { f => + import f._ + val sender = TestProbe() + val initialState = alice.stateData.asInstanceOf[DATA_NORMAL] + assert(initialState.commitments.localParams.maxAcceptedHtlcs === 100) + assert(initialState.commitments.remoteParams.maxAcceptedHtlcs === 30) // Bob accepts a maximum of 30 htlcs // Bob accepts a maximum of 30 htlcs for (i <- 0 until 30) { sender.send(alice, CMD_ADD_HTLC(10000000 msat, randomBytes32, CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), TestConstants.emptyOnionPacket, Upstream.Local(UUID.randomUUID()))) @@ -364,7 +383,25 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with alice2bob.expectNoMsg(200 millis) } - test("recv CMD_ADD_HTLC (over capacity)") { f => + test("recv CMD_ADD_HTLC (over local max accepted htlcs)") { f => + import f._ + val sender = TestProbe() + val initialState = bob.stateData.asInstanceOf[DATA_NORMAL] + assert(initialState.commitments.localParams.maxAcceptedHtlcs === 30) // Bob accepts a maximum of 30 htlcs + assert(initialState.commitments.remoteParams.maxAcceptedHtlcs === 100) // Alice accepts more, but Bob will stop at 30 HTLCs + for (_ <- 0 until 30) { + sender.send(bob, CMD_ADD_HTLC(2500000 msat, randomBytes32, CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), TestConstants.emptyOnionPacket, Upstream.Local(UUID.randomUUID()))) + sender.expectMsg(ChannelCommandResponse.Ok) + bob2alice.expectMsgType[UpdateAddHtlc] + } + val add = CMD_ADD_HTLC(10000000 msat, randomBytes32, CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), TestConstants.emptyOnionPacket, Upstream.Local(UUID.randomUUID())) + sender.send(bob, add) + val error = TooManyAcceptedHtlcs(channelId(bob), maximum = 30) + 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)))) + bob2alice.expectNoMsg(200 millis) + } + + test("recv CMD_ADD_HTLC (over capacity)", Tag("no_max_htlc_value_inflight")) { f => import f._ val sender = TestProbe() val initialState = alice.stateData.asInstanceOf[DATA_NORMAL] @@ -538,7 +575,7 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with bob2blockchain.expectMsgType[WatchConfirmed] } - test("recv UpdateAddHtlc (over max inflight htlc value)") { f => + test("recv UpdateAddHtlc (over max inflight htlc value)", Tag("alice_low_max_htlc_value_inflight")) { f => import f._ val tx = alice.stateData.asInstanceOf[DATA_NORMAL].commitments.localCommit.publishableTxs.commitTx.tx alice2bob.forward(alice, UpdateAddHtlc(ByteVector32.Zeroes, 0, 151000000 msat, randomBytes32, CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), TestConstants.emptyOnionPacket))