From bf533162677b024d1fb359e5159217e8239d1a81 Mon Sep 17 00:00:00 2001 From: t-bast Date: Tue, 19 Jan 2021 17:04:06 +0100 Subject: [PATCH 1/5] Add a maximum fee threshold for anchor outputs With anchor outputs, the actual feerate for the commit tx can be decided when broadcasting the tx by using CPFP on the anchor. That means we don't need to constantly keep the channel feerate close to what's happening on-chain. We just need a feerate that's good enough to get the tx to propagate through the bitcoin network. We set the upper threshold to 10 sat/byte, which is what lnd does as well. We let the feerate be lower than that when possible, but do note that depending on your configured `feerate-tolerance`, that means you can still experience some force-close events because of feerate mismatch. --- .../eclair/blockchain/fee/FeeEstimator.scala | 46 ++++++- .../fr/acinq/eclair/channel/Channel.scala | 19 ++- .../fr/acinq/eclair/channel/Commitments.scala | 12 +- .../fr/acinq/eclair/channel/Helpers.scala | 24 +--- .../main/scala/fr/acinq/eclair/io/Peer.scala | 2 +- .../blockchain/fee/FeeEstimatorSpec.scala | 108 +++++++++++++++++ .../states/StateTestsHelperMethods.scala | 9 +- .../channel/states/e/NormalStateSpec.scala | 112 +++++++++++++++--- .../channel/states/f/ShutdownStateSpec.scala | 24 +++- .../states/g/NegotiatingStateSpec.scala | 5 +- .../channel/states/h/ClosingStateSpec.scala | 3 + .../scala/fr/acinq/eclair/io/PeerSpec.scala | 21 +++- 12 files changed, 322 insertions(+), 63 deletions(-) create mode 100644 eclair-core/src/test/scala/fr/acinq/eclair/blockchain/fee/FeeEstimatorSpec.scala diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/fee/FeeEstimator.scala b/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/fee/FeeEstimator.scala index 2a54f0261b..351d327d76 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/fee/FeeEstimator.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/fee/FeeEstimator.scala @@ -17,6 +17,9 @@ package fr.acinq.eclair.blockchain.fee import fr.acinq.bitcoin.Crypto.PublicKey +import fr.acinq.bitcoin.{Satoshi, SatoshiLong} +import fr.acinq.eclair.blockchain.CurrentFeerates +import fr.acinq.eclair.channel.ChannelVersion trait FeeEstimator { // @formatter:off @@ -25,10 +28,51 @@ trait FeeEstimator { // @formatter:on } +object FeeEstimator { + /** When using anchor outputs, we only need to set a feerate that allows the tx to propagate: we will use CPFP to speed up confirmation if needed. */ + val AnchorOutputMaxCommitFeerate = FeeratePerKw(FeeratePerByte(10 sat)) +} + case class FeeTargets(fundingBlockTarget: Int, commitmentBlockTarget: Int, mutualCloseBlockTarget: Int, claimMainBlockTarget: Int) -case class FeerateTolerance(ratioLow: Double, ratioHigh: Double) +case class FeerateTolerance(ratioLow: Double, ratioHigh: Double) { + /** + * @param channelVersion channel version + * @param networkFeerate reference fee rate (value we estimate from our view of the network) + * @param proposedFeerate fee rate proposed (new proposal through update_fee or previous proposal used in our current commit tx) + * @return true if the difference between proposed and reference fee rates is too high. + */ + def isFeeDiffTooHigh(channelVersion: ChannelVersion, networkFeerate: FeeratePerKw, proposedFeerate: FeeratePerKw): Boolean = { + if (channelVersion.hasAnchorOutputs) { + proposedFeerate < networkFeerate * ratioLow || FeeEstimator.AnchorOutputMaxCommitFeerate * ratioHigh < proposedFeerate + } else { + proposedFeerate < networkFeerate * ratioLow || networkFeerate * ratioHigh < proposedFeerate + } + } +} case class OnChainFeeConf(feeTargets: FeeTargets, feeEstimator: FeeEstimator, closeOnOfflineMismatch: Boolean, updateFeeMinDiffRatio: Double, private val defaultFeerateTolerance: FeerateTolerance, private val perNodeFeerateTolerance: Map[PublicKey, FeerateTolerance]) { + def maxFeerateMismatchFor(nodeId: PublicKey): FeerateTolerance = perNodeFeerateTolerance.getOrElse(nodeId, defaultFeerateTolerance) + + /** To avoid spamming our peers with fee updates every time there's a small variation, we only update the fee when the difference exceeds a given ratio. */ + def shouldUpdateFee(currentFeeratePerKw: FeeratePerKw, nextFeeratePerKw: FeeratePerKw): Boolean = + currentFeeratePerKw.toLong == 0 || Math.abs((currentFeeratePerKw.toLong - nextFeeratePerKw.toLong).toDouble / currentFeeratePerKw.toLong) > updateFeeMinDiffRatio + + /** + * Get the feerate that should apply to a channel commitment transaction: + * - if we're using anchor outputs, we use a feerate that allows network propagation of the commit tx: we will use CPFP to speed up confirmation if needed + * - otherwise we use a feerate that should get the commit tx confirmed in the configured number of blocks + */ + def getCommitmentFeerate(channelVersion: ChannelVersion, channelCapacity: Satoshi, currentFeerates_opt: Option[CurrentFeerates]): FeeratePerKw = { + val networkFeerate = currentFeerates_opt match { + case Some(currentFeerates) => currentFeerates.feeratesPerKw.feePerBlock(feeTargets.commitmentBlockTarget) + case None => feeEstimator.getFeeratePerKw(feeTargets.commitmentBlockTarget) + } + if (channelVersion.hasAnchorOutputs) { + networkFeerate.min(FeeEstimator.AnchorOutputMaxCommitFeerate) + } else { + networkFeerate + } + } } diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Channel.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Channel.scala index 893d8e696b..775a2b47ae 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Channel.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Channel.scala @@ -161,7 +161,7 @@ class Channel(val nodeParams: NodeParams, val wallet: EclairWallet, remoteNodeId startWith(WAIT_FOR_INIT_INTERNAL, Nothing) when(WAIT_FOR_INIT_INTERNAL)(handleExceptions { - case Event(initFunder@INPUT_INIT_FUNDER(temporaryChannelId, fundingSatoshis, pushMsat, initialFeeratePerKw, fundingTxFeeratePerKw, initialRelayFees_opt, localParams, remote, _, channelFlags, channelVersion), Nothing) => + case Event(initFunder@INPUT_INIT_FUNDER(temporaryChannelId, fundingSatoshis, pushMsat, initialFeeratePerKw, fundingTxFeeratePerKw, _, localParams, remote, _, channelFlags, channelVersion), Nothing) => context.system.eventStream.publish(ChannelCreated(self, peer, remoteNodeId, isFunder = true, temporaryChannelId, initialFeeratePerKw, Some(fundingTxFeeratePerKw))) activeConnection = remote val fundingPubKey = keyManager.fundingPublicKey(localParams.fundingKeyPath).publicKey @@ -301,7 +301,7 @@ class Channel(val nodeParams: NodeParams, val wallet: EclairWallet, remoteNodeId when(WAIT_FOR_OPEN_CHANNEL)(handleExceptions { case Event(open: OpenChannel, d@DATA_WAIT_FOR_OPEN_CHANNEL(INPUT_INIT_FUNDEE(_, localParams, _, remoteInit, channelVersion))) => log.info("received OpenChannel={}", open) - Helpers.validateParamsFundee(nodeParams, localParams.features, open, remoteNodeId) match { + Helpers.validateParamsFundee(nodeParams, localParams.features, channelVersion, open, remoteNodeId) match { case Left(t) => handleLocalError(t, d, Some(open)) case _ => context.system.eventStream.publish(ChannelCreated(self, peer, remoteNodeId, isFunder = false, open.temporaryChannelId, open.feeratePerKw, None)) @@ -1585,8 +1585,8 @@ class Channel(val nodeParams: NodeParams, val wallet: EclairWallet, remoteNodeId // we send it (if needed) when reconnected. if (d.commitments.localParams.isFunder) { val currentFeeratePerKw = d.commitments.localCommit.spec.feeratePerKw - val networkFeeratePerKw = nodeParams.onChainFeeConf.feeEstimator.getFeeratePerKw(nodeParams.onChainFeeConf.feeTargets.commitmentBlockTarget) - if (Helpers.shouldUpdateFee(currentFeeratePerKw, networkFeeratePerKw, nodeParams.onChainFeeConf.updateFeeMinDiffRatio)) { + val networkFeeratePerKw = nodeParams.onChainFeeConf.getCommitmentFeerate(d.commitments.channelVersion, d.commitments.capacity, None) + if (nodeParams.onChainFeeConf.shouldUpdateFee(currentFeeratePerKw, networkFeeratePerKw)) { self ! CMD_UPDATE_FEE(networkFeeratePerKw, commit = true) } } @@ -1840,12 +1840,11 @@ class Channel(val nodeParams: NodeParams, val wallet: EclairWallet, remoteNodeId } def handleCurrentFeerate(c: CurrentFeerates, d: HasCommitments) = { - val networkFeeratePerKw = c.feeratesPerKw.feePerBlock(target = nodeParams.onChainFeeConf.feeTargets.commitmentBlockTarget) + val networkFeeratePerKw = nodeParams.onChainFeeConf.getCommitmentFeerate(d.commitments.channelVersion, d.commitments.capacity, Some(c)) val currentFeeratePerKw = d.commitments.localCommit.spec.feeratePerKw - val shouldUpdateFee = d.commitments.localParams.isFunder && - Helpers.shouldUpdateFee(currentFeeratePerKw, networkFeeratePerKw, nodeParams.onChainFeeConf.updateFeeMinDiffRatio) + val shouldUpdateFee = d.commitments.localParams.isFunder && nodeParams.onChainFeeConf.shouldUpdateFee(currentFeeratePerKw, networkFeeratePerKw) val shouldClose = !d.commitments.localParams.isFunder && - Helpers.isFeeDiffTooHigh(networkFeeratePerKw, currentFeeratePerKw, nodeParams.onChainFeeConf.maxFeerateMismatchFor(d.commitments.remoteNodeId)) && + nodeParams.onChainFeeConf.maxFeerateMismatchFor(d.commitments.remoteNodeId).isFeeDiffTooHigh(d.commitments.channelVersion, networkFeeratePerKw, currentFeeratePerKw) && d.commitments.hasPendingOrProposedHtlcs // we close only if we have HTLCs potentially at risk if (shouldUpdateFee) { self ! CMD_UPDATE_FEE(networkFeeratePerKw, commit = true) @@ -1865,11 +1864,11 @@ class Channel(val nodeParams: NodeParams, val wallet: EclairWallet, remoteNodeId * @return */ def handleOfflineFeerate(c: CurrentFeerates, d: HasCommitments) = { - val networkFeeratePerKw = c.feeratesPerKw.feePerBlock(target = nodeParams.onChainFeeConf.feeTargets.commitmentBlockTarget) + val networkFeeratePerKw = nodeParams.onChainFeeConf.getCommitmentFeerate(d.commitments.channelVersion, d.commitments.capacity, Some(c)) val currentFeeratePerKw = d.commitments.localCommit.spec.feeratePerKw // if the network fees are too high we risk to not be able to confirm our current commitment val shouldClose = networkFeeratePerKw > currentFeeratePerKw && - Helpers.isFeeDiffTooHigh(networkFeeratePerKw, currentFeeratePerKw, nodeParams.onChainFeeConf.maxFeerateMismatchFor(d.commitments.remoteNodeId)) && + nodeParams.onChainFeeConf.maxFeerateMismatchFor(d.commitments.remoteNodeId).isFeeDiffTooHigh(d.commitments.channelVersion, networkFeeratePerKw, currentFeeratePerKw) && d.commitments.hasPendingOrProposedHtlcs // we close only if we have HTLCs potentially at risk if (shouldClose) { if (nodeParams.onChainFeeConf.closeOnOfflineMismatch) { 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 0334353cb4..5b5e6aa7a6 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 @@ -276,8 +276,8 @@ object Commitments { // we allowed mismatches between our feerates and our remote's as long as commitments didn't contain any HTLC at risk // we need to verify that we're not disagreeing on feerates anymore before offering new HTLCs - val localFeeratePerKw = feeConf.feeEstimator.getFeeratePerKw(target = feeConf.feeTargets.commitmentBlockTarget) - if (Helpers.isFeeDiffTooHigh(localFeeratePerKw, commitments.localCommit.spec.feeratePerKw, feeConf.maxFeerateMismatchFor(commitments.remoteNodeId))) { + val localFeeratePerKw = feeConf.getCommitmentFeerate(commitments.channelVersion, commitments.capacity, None) + if (feeConf.maxFeerateMismatchFor(commitments.remoteNodeId).isFeeDiffTooHigh(commitments.channelVersion, localFeeratePerKw, commitments.localCommit.spec.feeratePerKw)) { return Left(FeerateTooDifferent(commitments.channelId, localFeeratePerKw = localFeeratePerKw, remoteFeeratePerKw = commitments.localCommit.spec.feeratePerKw)) } @@ -337,8 +337,8 @@ object Commitments { // we allowed mismatches between our feerates and our remote's as long as commitments didn't contain any HTLC at risk // we need to verify that we're not disagreeing on feerates anymore before accepting new HTLCs - val localFeeratePerKw = feeConf.feeEstimator.getFeeratePerKw(target = feeConf.feeTargets.commitmentBlockTarget) - if (Helpers.isFeeDiffTooHigh(localFeeratePerKw, commitments.localCommit.spec.feeratePerKw, feeConf.maxFeerateMismatchFor(commitments.remoteNodeId))) { + val localFeeratePerKw = feeConf.getCommitmentFeerate(commitments.channelVersion, commitments.capacity, None) + if (feeConf.maxFeerateMismatchFor(commitments.remoteNodeId).isFeeDiffTooHigh(commitments.channelVersion, localFeeratePerKw, commitments.localCommit.spec.feeratePerKw)) { return Left(FeerateTooDifferent(commitments.channelId, localFeeratePerKw = localFeeratePerKw, remoteFeeratePerKw = commitments.localCommit.spec.feeratePerKw)) } @@ -484,9 +484,9 @@ object Commitments { Left(FeerateTooSmall(commitments.channelId, remoteFeeratePerKw = fee.feeratePerKw)) } else { Metrics.RemoteFeeratePerKw.withoutTags().record(fee.feeratePerKw.toLong) - val localFeeratePerKw = feeConf.feeEstimator.getFeeratePerKw(target = feeConf.feeTargets.commitmentBlockTarget) + val localFeeratePerKw = feeConf.getCommitmentFeerate(commitments.channelVersion, commitments.capacity, None) log.info("remote feeratePerKw={}, local feeratePerKw={}, ratio={}", fee.feeratePerKw, localFeeratePerKw, fee.feeratePerKw.toLong.toDouble / localFeeratePerKw.toLong) - if (Helpers.isFeeDiffTooHigh(localFeeratePerKw, fee.feeratePerKw, feeConf.maxFeerateMismatchFor(commitments.remoteNodeId)) && commitments.hasPendingOrProposedHtlcs) { + if (feeConf.maxFeerateMismatchFor(commitments.remoteNodeId).isFeeDiffTooHigh(commitments.channelVersion, localFeeratePerKw, fee.feeratePerKw) && commitments.hasPendingOrProposedHtlcs) { Left(FeerateTooDifferent(commitments.channelId, localFeeratePerKw = localFeeratePerKw, remoteFeeratePerKw = fee.feeratePerKw)) } else { // NB: we check that the funder can afford this new fee even if spec allows to do it at next signature diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala index 040f7d456e..a037935fd4 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala @@ -22,7 +22,7 @@ import fr.acinq.bitcoin.Script._ import fr.acinq.bitcoin._ import fr.acinq.eclair._ import fr.acinq.eclair.blockchain.EclairWallet -import fr.acinq.eclair.blockchain.fee.{FeeEstimator, FeeTargets, FeeratePerKw, FeerateTolerance} +import fr.acinq.eclair.blockchain.fee.{FeeEstimator, FeeTargets, FeeratePerKw} import fr.acinq.eclair.channel.Channel.REFRESH_CHANNEL_UPDATE_INTERVAL import fr.acinq.eclair.crypto.Generators import fr.acinq.eclair.crypto.keymanager.ChannelKeyManager @@ -81,7 +81,7 @@ object Helpers { /** * Called by the fundee */ - def validateParamsFundee(nodeParams: NodeParams, features: Features, open: OpenChannel, remoteNodeId: PublicKey): Either[ChannelException, Unit] = { + def validateParamsFundee(nodeParams: NodeParams, features: Features, channelVersion: ChannelVersion, open: OpenChannel, remoteNodeId: PublicKey): Either[ChannelException, Unit] = { // BOLT #2: if the chain_hash value, within the open_channel, message is set to a hash of a chain that is unknown to the receiver: // MUST reject the channel. if (nodeParams.chainHash != open.chainHash) return Left(InvalidChainHash(open.temporaryChannelId, local = nodeParams.chainHash, remote = open.chainHash)) @@ -114,8 +114,8 @@ object Helpers { } // BOLT #2: The receiving node MUST fail the channel if: it considers feerate_per_kw too small for timely processing or unreasonably large. - val localFeeratePerKw = nodeParams.onChainFeeConf.feeEstimator.getFeeratePerKw(target = nodeParams.onChainFeeConf.feeTargets.commitmentBlockTarget) - if (isFeeDiffTooHigh(localFeeratePerKw, open.feeratePerKw, nodeParams.onChainFeeConf.maxFeerateMismatchFor(remoteNodeId))) return Left(FeerateTooDifferent(open.temporaryChannelId, localFeeratePerKw, open.feeratePerKw)) + val localFeeratePerKw = nodeParams.onChainFeeConf.getCommitmentFeerate(channelVersion, open.fundingSatoshis, None) + if (nodeParams.onChainFeeConf.maxFeerateMismatchFor(remoteNodeId).isFeeDiffTooHigh(channelVersion, localFeeratePerKw, open.feeratePerKw)) return Left(FeerateTooDifferent(open.temporaryChannelId, localFeeratePerKw, open.feeratePerKw)) // only enforce dust limit check on mainnet if (nodeParams.chainHash == Block.LivenetGenesisBlock.hash) { if (open.dustLimitSatoshis < Channel.MIN_DUSTLIMIT) return Left(DustLimitTooSmall(open.temporaryChannelId, open.dustLimitSatoshis, Channel.MIN_DUSTLIMIT)) @@ -178,22 +178,6 @@ object Helpers { delay } - /** - * To avoid spamming our peers with fee updates every time there's a small variation, we only update the fee when the - * difference exceeds a given ratio (updateFeeMinDiffRatio). - */ - def shouldUpdateFee(currentFeeratePerKw: FeeratePerKw, nextFeeratePerKw: FeeratePerKw, updateFeeMinDiffRatio: Double): Boolean = - currentFeeratePerKw.toLong == 0 || Math.abs((currentFeeratePerKw.toLong - nextFeeratePerKw.toLong).toDouble / currentFeeratePerKw.toLong) > updateFeeMinDiffRatio - - /** - * @param referenceFeePerKw reference fee rate per kiloweight - * @param currentFeePerKw current fee rate per kiloweight - * @param maxFeerateMismatch maximum fee rate mismatch tolerated - * @return true if the difference between proposed and reference fee rates is too high. - */ - def isFeeDiffTooHigh(referenceFeePerKw: FeeratePerKw, currentFeePerKw: FeeratePerKw, maxFeerateMismatch: FeerateTolerance): Boolean = - currentFeePerKw < referenceFeePerKw * maxFeerateMismatch.ratioLow || referenceFeePerKw * maxFeerateMismatch.ratioHigh < currentFeePerKw - /** * @param remoteFeeratePerKw remote fee rate per kiloweight * @return true if the remote fee rate is too small diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/io/Peer.scala b/eclair-core/src/main/scala/fr/acinq/eclair/io/Peer.scala index c60e2f23d8..b03acb2f4e 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/io/Peer.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/io/Peer.scala @@ -126,7 +126,7 @@ class Peer(val nodeParams: NodeParams, remoteNodeId: PublicKey, watcher: ActorRe val (channel, localParams) = createNewChannel(nodeParams, d.localFeatures, funder = true, c.fundingSatoshis, origin_opt = Some(sender), channelVersion) c.timeout_opt.map(openTimeout => context.system.scheduler.scheduleOnce(openTimeout.duration, channel, Channel.TickChannelOpenTimeout)(context.dispatcher)) val temporaryChannelId = randomBytes32 - val channelFeeratePerKw = nodeParams.onChainFeeConf.feeEstimator.getFeeratePerKw(target = nodeParams.onChainFeeConf.feeTargets.commitmentBlockTarget) + val channelFeeratePerKw = nodeParams.onChainFeeConf.getCommitmentFeerate(channelVersion, c.fundingSatoshis, None) val fundingTxFeeratePerKw = c.fundingTxFeeratePerKw_opt.getOrElse(nodeParams.onChainFeeConf.feeEstimator.getFeeratePerKw(target = nodeParams.onChainFeeConf.feeTargets.fundingBlockTarget)) log.info(s"requesting a new channel with fundingSatoshis=${c.fundingSatoshis}, pushMsat=${c.pushMsat} and fundingFeeratePerByte=${c.fundingTxFeeratePerKw_opt} temporaryChannelId=$temporaryChannelId localParams=$localParams") channel ! INPUT_INIT_FUNDER(temporaryChannelId, c.fundingSatoshis, c.pushMsat, channelFeeratePerKw, fundingTxFeeratePerKw, c.initialRelayFees_opt, localParams, d.peerConnection, d.remoteInit, c.channelFlags.getOrElse(nodeParams.channelFlags), channelVersion) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/fee/FeeEstimatorSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/fee/FeeEstimatorSpec.scala new file mode 100644 index 0000000000..7c920ffbbf --- /dev/null +++ b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/fee/FeeEstimatorSpec.scala @@ -0,0 +1,108 @@ +/* + * Copyright 2021 ACINQ SAS + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package fr.acinq.eclair.blockchain.fee + +import fr.acinq.bitcoin.SatoshiLong +import fr.acinq.eclair.TestConstants.TestFeeEstimator +import fr.acinq.eclair.blockchain.CurrentFeerates +import fr.acinq.eclair.channel.ChannelVersion +import org.scalatest.funsuite.AnyFunSuite + +class FeeEstimatorSpec extends AnyFunSuite { + + test("should update fee when diff ratio exceeded") { + val feeConf = OnChainFeeConf(FeeTargets(1, 1, 1, 1), new TestFeeEstimator(), closeOnOfflineMismatch = true, updateFeeMinDiffRatio = 0.1, FeerateTolerance(0.5, 2.0), Map.empty) + assert(!feeConf.shouldUpdateFee(FeeratePerKw(1000 sat), FeeratePerKw(1000 sat))) + assert(!feeConf.shouldUpdateFee(FeeratePerKw(1000 sat), FeeratePerKw(900 sat))) + assert(!feeConf.shouldUpdateFee(FeeratePerKw(1000 sat), FeeratePerKw(1100 sat))) + assert(feeConf.shouldUpdateFee(FeeratePerKw(1000 sat), FeeratePerKw(899 sat))) + assert(feeConf.shouldUpdateFee(FeeratePerKw(1000 sat), FeeratePerKw(1101 sat))) + } + + test("get commitment feerate") { + val feeEstimator = new TestFeeEstimator() + val channelVersion = ChannelVersion.STANDARD + val feeConf = OnChainFeeConf(FeeTargets(1, 2, 1, 1), feeEstimator, closeOnOfflineMismatch = true, updateFeeMinDiffRatio = 0.1, FeerateTolerance(0.5, 2.0), Map.empty) + + feeEstimator.setFeerate(FeeratesPerKw.single(FeeratePerKw(10000 sat)).copy(blocks_2 = FeeratePerKw(5000 sat))) + assert(feeConf.getCommitmentFeerate(channelVersion, 100000 sat, None) === FeeratePerKw(5000 sat)) + + val currentFeerates = CurrentFeerates(FeeratesPerKw.single(FeeratePerKw(10000 sat)).copy(blocks_2 = FeeratePerKw(4000 sat))) + assert(feeConf.getCommitmentFeerate(channelVersion, 100000 sat, Some(currentFeerates)) === FeeratePerKw(4000 sat)) + } + + test("get commitment feerate (anchor outputs)") { + val feeEstimator = new TestFeeEstimator() + val channelVersion = ChannelVersion.ANCHOR_OUTPUTS + val feeConf = OnChainFeeConf(FeeTargets(1, 2, 1, 1), feeEstimator, closeOnOfflineMismatch = true, updateFeeMinDiffRatio = 0.1, FeerateTolerance(0.5, 2.0), Map.empty) + + feeEstimator.setFeerate(FeeratesPerKw.single(FeeratePerKw(10000 sat)).copy(blocks_2 = FeeEstimator.AnchorOutputMaxCommitFeerate / 2)) + assert(feeConf.getCommitmentFeerate(channelVersion, 100000 sat, None) === FeeEstimator.AnchorOutputMaxCommitFeerate / 2) + + feeEstimator.setFeerate(FeeratesPerKw.single(FeeratePerKw(10000 sat)).copy(blocks_2 = FeeEstimator.AnchorOutputMaxCommitFeerate * 2)) + assert(feeConf.getCommitmentFeerate(channelVersion, 100000 sat, None) === FeeEstimator.AnchorOutputMaxCommitFeerate) + + val currentFeerates1 = CurrentFeerates(FeeratesPerKw.single(FeeratePerKw(10000 sat)).copy(blocks_2 = FeeEstimator.AnchorOutputMaxCommitFeerate / 2)) + assert(feeConf.getCommitmentFeerate(channelVersion, 100000 sat, Some(currentFeerates1)) === FeeEstimator.AnchorOutputMaxCommitFeerate / 2) + + val currentFeerates2 = CurrentFeerates(FeeratesPerKw.single(FeeratePerKw(10000 sat)).copy(blocks_2 = FeeEstimator.AnchorOutputMaxCommitFeerate * 1.5)) + feeEstimator.setFeerate(FeeratesPerKw.single(FeeratePerKw(10000 sat)).copy(blocks_2 = FeeEstimator.AnchorOutputMaxCommitFeerate / 2)) + assert(feeConf.getCommitmentFeerate(channelVersion, 100000 sat, Some(currentFeerates2)) === FeeEstimator.AnchorOutputMaxCommitFeerate) + } + + test("fee difference too high") { + val tolerance = FeerateTolerance(ratioLow = 0.5, ratioHigh = 4.0) + val channelVersion = ChannelVersion.STANDARD + val testCases = Seq( + (FeeratePerKw(500 sat), FeeratePerKw(500 sat), false), + (FeeratePerKw(500 sat), FeeratePerKw(250 sat), false), + (FeeratePerKw(500 sat), FeeratePerKw(249 sat), true), + (FeeratePerKw(500 sat), FeeratePerKw(200 sat), true), + (FeeratePerKw(249 sat), FeeratePerKw(500 sat), false), + (FeeratePerKw(250 sat), FeeratePerKw(500 sat), false), + (FeeratePerKw(250 sat), FeeratePerKw(1000 sat), false), + (FeeratePerKw(250 sat), FeeratePerKw(1001 sat), true), + (FeeratePerKw(250 sat), FeeratePerKw(1500 sat), true), + ) + testCases.foreach { case (networkFeerate, proposedFeerate, expected) => + assert(tolerance.isFeeDiffTooHigh(channelVersion, networkFeerate, proposedFeerate) === expected) + } + } + + test("fee difference too high (anchor outputs)") { + val tolerance = FeerateTolerance(ratioLow = 0.5, ratioHigh = 4.0) + val channelVersion = ChannelVersion.ANCHOR_OUTPUTS + assert(FeeEstimator.AnchorOutputMaxCommitFeerate === FeeratePerKw(2500 sat)) + val testCases = Seq( + (FeeratePerKw(500 sat), FeeratePerKw(500 sat), false), + (FeeratePerKw(500 sat), FeeratePerKw(2500 sat), false), + (FeeratePerKw(500 sat), FeeratePerKw(10000 sat), false), + (FeeratePerKw(500 sat), FeeratePerKw(10001 sat), true), + (FeeratePerKw(2500 sat), FeeratePerKw(10000 sat), false), + (FeeratePerKw(2500 sat), FeeratePerKw(10001 sat), true), + (FeeratePerKw(2500 sat), FeeratePerKw(1250 sat), false), + (FeeratePerKw(2500 sat), FeeratePerKw(1249 sat), true), + (FeeratePerKw(2500 sat), FeeratePerKw(1000 sat), true), + (FeeratePerKw(1000 sat), FeeratePerKw(500 sat), false), + (FeeratePerKw(1000 sat), FeeratePerKw(499 sat), true), + ) + testCases.foreach { case (networkFeerate, proposedFeerate, expected) => + assert(tolerance.isFeeDiffTooHigh(channelVersion, networkFeerate, proposedFeerate) === expected) + } + } + +} 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 7b630d45bc..045177420c 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 @@ -22,7 +22,7 @@ import fr.acinq.bitcoin.Crypto.PublicKey import fr.acinq.bitcoin.{ByteVector32, Crypto, SatoshiLong, ScriptFlags, Transaction} import fr.acinq.eclair.TestConstants.{Alice, Bob, TestFeeEstimator} import fr.acinq.eclair.blockchain._ -import fr.acinq.eclair.blockchain.fee.FeeTargets +import fr.acinq.eclair.blockchain.fee.{FeeEstimator, FeeTargets} import fr.acinq.eclair.channel._ import fr.acinq.eclair.payment.OutgoingPacket import fr.acinq.eclair.payment.OutgoingPacket.Upstream @@ -111,10 +111,15 @@ trait StateTestsHelperMethods extends TestKitBase { } else { (Alice.channelParams, Bob.channelParams, ChannelVersion.STANDARD) } + val initialFeeratePerKw = if (tags.contains(StateTestsTags.AnchorOutputs)) { + FeeEstimator.AnchorOutputMaxCommitFeerate + } else { + TestConstants.feeratePerKw + } val aliceInit = Init(aliceParams.features) val bobInit = Init(bobParams.features) - alice ! INPUT_INIT_FUNDER(ByteVector32.Zeroes, TestConstants.fundingSatoshis, pushMsat, TestConstants.feeratePerKw, TestConstants.feeratePerKw, None, aliceParams, alice2bob.ref, bobInit, channelFlags, channelVersion) + alice ! INPUT_INIT_FUNDER(ByteVector32.Zeroes, TestConstants.fundingSatoshis, pushMsat, initialFeeratePerKw, TestConstants.feeratePerKw, None, aliceParams, alice2bob.ref, bobInit, channelFlags, channelVersion) bob ! INPUT_INIT_FUNDEE(ByteVector32.Zeroes, bobParams, bob2alice.ref, aliceInit, channelVersion) alice2bob.expectMsgType[OpenChannel] alice2bob.forward(bob) 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 1147d6002d..96855edbdf 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 @@ -25,7 +25,7 @@ import fr.acinq.eclair.TestConstants.{Alice, Bob} import fr.acinq.eclair.UInt64.Conversions._ import fr.acinq.eclair._ import fr.acinq.eclair.blockchain._ -import fr.acinq.eclair.blockchain.fee.{FeeratePerKw, FeeratesPerKw} +import fr.acinq.eclair.blockchain.fee.{FeeEstimator, FeeratePerByte, FeeratePerKw, FeeratesPerKw} import fr.acinq.eclair.channel.Channel._ import fr.acinq.eclair.channel._ import fr.acinq.eclair.channel.states.{StateTestsBase, StateTestsTags} @@ -228,7 +228,7 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with val add = CMD_ADD_HTLC(sender.ref, initialState.commitments.availableBalanceForSend + 1.msat, randomBytes32, CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), TestConstants.emptyOnionPacket, localOrigin(sender.ref)) alice ! add - val error = InsufficientFunds(channelId(alice), amount = add.amount, missing = 0 sat, reserve = 20000 sat, fees = 13620 sat) + val error = InsufficientFunds(channelId(alice), amount = add.amount, missing = 0 sat, reserve = 20000 sat, fees = 3900 sat) sender.expectMsg(RES_ADD_FAILED(add, error, Some(initialState.channelUpdate))) alice2bob.expectNoMsg(200 millis) } @@ -495,7 +495,7 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with alice2bob.forward(bob, UpdateAddHtlc(ByteVector32.Zeroes, 1, 300000000 msat, randomBytes32, CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), TestConstants.emptyOnionPacket)) alice2bob.forward(bob, UpdateAddHtlc(ByteVector32.Zeroes, 2, 100000000 msat, randomBytes32, CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), TestConstants.emptyOnionPacket)) val error = bob2alice.expectMsgType[Error] - assert(new String(error.data.toArray) === InsufficientFunds(channelId(bob), amount = 100000000 msat, missing = 37060 sat, reserve = 20000 sat, fees = 17060 sat).getMessage) + assert(new String(error.data.toArray) === InsufficientFunds(channelId(bob), amount = 100000000 msat, missing = 24760 sat, reserve = 20000 sat, fees = 4760 sat).getMessage) awaitCond(bob.stateName == CLOSING) // channel should be advertised as down assert(channelUpdateListener.expectMsgType[LocalChannelDown].channelId === bob.stateData.asInstanceOf[DATA_CLOSING].channelId) @@ -1600,7 +1600,8 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with test("recv UpdateFee (anchor outputs)", Tag(StateTestsTags.AnchorOutputs)) { f => import f._ val initialData = bob.stateData.asInstanceOf[DATA_NORMAL] - val fee = UpdateFee(ByteVector32.Zeroes, FeeratePerKw(8000 sat)) + assert(initialData.commitments.localCommit.spec.feeratePerKw === FeeEstimator.AnchorOutputMaxCommitFeerate) + val fee = UpdateFee(ByteVector32.Zeroes, FeeEstimator.AnchorOutputMaxCommitFeerate * 0.8) bob ! fee awaitCond(bob.stateData == initialData.copy(commitments = initialData.commitments.copy(remoteChanges = initialData.commitments.remoteChanges.copy(proposed = initialData.commitments.remoteChanges.proposed :+ fee), remoteNextHtlcId = 0))) } @@ -1645,33 +1646,28 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with bob2blockchain.expectMsgType[WatchConfirmed] } - test("recv UpdateFee (sender can't afford it) (anchor outputs)", Tag(StateTestsTags.AnchorOutputs)) { f => + test("recv UpdateFee (sender can't afford it, anchor outputs)", Tag(StateTestsTags.AnchorOutputs)) { f => import f._ val tx = bob.stateData.asInstanceOf[DATA_NORMAL].commitments.localCommit.publishableTxs.commitTx.tx // This feerate is just above the threshold: (800000 (alice balance) - 20000 (reserve) - 660 (anchors)) / 1124 (commit tx weight) = 693363 - val fee = UpdateFee(ByteVector32.Zeroes, FeeratePerKw(693364 sat)) - // we first update the feerates so that we don't trigger a 'fee too different' error - bob.feeEstimator.setFeerate(FeeratesPerKw.single(fee.feeratePerKw)) - bob ! fee + bob ! UpdateFee(ByteVector32.Zeroes, FeeratePerKw(693364 sat)) val error = bob2alice.expectMsgType[Error] assert(new String(error.data.toArray) === CannotAffordFees(channelId(bob), missing = 1 sat, reserve = 20000 sat, fees = 780001 sat).getMessage) awaitCond(bob.stateName == CLOSING) // channel should be advertised as down assert(channelUpdateListener.expectMsgType[LocalChannelDown].channelId === bob.stateData.asInstanceOf[DATA_CLOSING].channelId) bob2blockchain.expectMsg(PublishAsap(tx)) // commit tx - //bob2blockchain.expectMsgType[PublishAsap] // main delayed (removed because of the high fees) + bob2blockchain.expectMsgType[PublishAsap] // main delayed bob2blockchain.expectMsgType[WatchConfirmed] } test("recv UpdateFee (local/remote feerates are too different)") { f => import f._ - bob.feeEstimator.setFeerate(FeeratesPerKw(FeeratePerKw(500 sat), FeeratePerKw(1000 sat), FeeratePerKw(2000 sat), FeeratePerKw(6000 sat), FeeratePerKw(12000 sat), FeeratePerKw(36000 sat), FeeratePerKw(72000 sat), FeeratePerKw(140000 sat), FeeratePerKw(160000 sat))) - val tx = bob.stateData.asInstanceOf[DATA_NORMAL].commitments.localCommit.publishableTxs.commitTx.tx - val localFeerate = bob.feeEstimator.getFeeratePerKw(bob.feeTargets.commitmentBlockTarget) - assert(localFeerate === FeeratePerKw(2000 sat)) - val remoteFeerate = FeeratePerKw(4000 sat) - bob ! UpdateFee(ByteVector32.Zeroes, remoteFeerate) + val initialState = bob.stateData.asInstanceOf[DATA_NORMAL] + val commitTx = initialState.commitments.localCommit.publishableTxs.commitTx.tx + assert(initialState.commitments.localCommit.spec.feeratePerKw === TestConstants.feeratePerKw) + alice2bob.send(bob, UpdateFee(ByteVector32.Zeroes, TestConstants.feeratePerKw * 3)) bob2alice.expectNoMsg(250 millis) // we don't close because the commitment doesn't contain any HTLC // when we try to add an HTLC, we still disagree on the feerate so we close @@ -1681,7 +1677,49 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with awaitCond(bob.stateName == CLOSING) // channel should be advertised as down assert(channelUpdateListener.expectMsgType[LocalChannelDown].channelId === bob.stateData.asInstanceOf[DATA_CLOSING].channelId) - bob2blockchain.expectMsg(PublishAsap(tx)) + bob2blockchain.expectMsg(PublishAsap(commitTx)) + bob2blockchain.expectMsgType[PublishAsap] + bob2blockchain.expectMsgType[WatchConfirmed] + } + + test("recv UpdateFee (remote feerate is too high, anchor outputs)", Tag(StateTestsTags.AnchorOutputs)) { f => + import f._ + + val initialState = bob.stateData.asInstanceOf[DATA_NORMAL] + val commitTx = initialState.commitments.localCommit.publishableTxs.commitTx.tx + assert(initialState.commitments.localCommit.spec.feeratePerKw === FeeEstimator.AnchorOutputMaxCommitFeerate) + alice2bob.send(bob, UpdateFee(initialState.channelId, FeeEstimator.AnchorOutputMaxCommitFeerate * 3)) + bob2alice.expectNoMsg(250 millis) // we don't close because the commitment doesn't contain any HTLC + + // when we try to add an HTLC, we still disagree on the feerate so we close + alice2bob.send(bob, UpdateAddHtlc(ByteVector32.Zeroes, 0, 2500000 msat, randomBytes32, CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), TestConstants.emptyOnionPacket)) + val error = bob2alice.expectMsgType[Error] + assert(new String(error.data.toArray).contains("local/remote feerates are too different")) + awaitCond(bob.stateName == CLOSING) + // channel should be advertised as down + assert(channelUpdateListener.expectMsgType[LocalChannelDown].channelId === bob.stateData.asInstanceOf[DATA_CLOSING].channelId) + bob2blockchain.expectMsg(PublishAsap(commitTx)) + bob2blockchain.expectMsgType[PublishAsap] + bob2blockchain.expectMsgType[WatchConfirmed] + } + + test("recv UpdateFee (remote feerate is too small, anchor outputs)", Tag(StateTestsTags.AnchorOutputs)) { f => + import f._ + + val initialState = bob.stateData.asInstanceOf[DATA_NORMAL] + val commitTx = initialState.commitments.localCommit.publishableTxs.commitTx.tx + assert(initialState.commitments.localCommit.spec.feeratePerKw === FeeEstimator.AnchorOutputMaxCommitFeerate) + alice2bob.send(bob, UpdateFee(initialState.channelId, FeeratePerKw(FeeratePerByte(2 sat)))) + bob2alice.expectNoMsg(250 millis) // we don't close because the commitment doesn't contain any HTLC + + // when we try to add an HTLC, we still disagree on the feerate so we close + alice2bob.send(bob, UpdateAddHtlc(ByteVector32.Zeroes, 0, 2500000 msat, randomBytes32, CltvExpiryDelta(144).toCltvExpiry(currentBlockHeight), TestConstants.emptyOnionPacket)) + val error = bob2alice.expectMsgType[Error] + assert(new String(error.data.toArray).contains("local/remote feerates are too different")) + awaitCond(bob.stateName == CLOSING) + // channel should be advertised as down + assert(channelUpdateListener.expectMsgType[LocalChannelDown].channelId === bob.stateData.asInstanceOf[DATA_CLOSING].channelId) + bob2blockchain.expectMsg(PublishAsap(commitTx)) bob2blockchain.expectMsgType[PublishAsap] bob2blockchain.expectMsgType[WatchConfirmed] } @@ -2106,6 +2144,14 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with alice2bob.expectMsg(UpdateFee(initialState.commitments.channelId, event.feeratesPerKw.feePerBlock(Alice.nodeParams.onChainFeeConf.feeTargets.commitmentBlockTarget))) } + test("recv CurrentFeerate (when funder, triggers an UpdateFee, anchor outputs)", Tag(StateTestsTags.AnchorOutputs)) { f => + import f._ + val initialState = alice.stateData.asInstanceOf[DATA_NORMAL] + assert(initialState.commitments.localCommit.spec.feeratePerKw === FeeEstimator.AnchorOutputMaxCommitFeerate) + alice ! CurrentFeerates(FeeratesPerKw.single(FeeEstimator.AnchorOutputMaxCommitFeerate / 2)) + alice2bob.expectMsg(UpdateFee(initialState.commitments.channelId, FeeEstimator.AnchorOutputMaxCommitFeerate / 2)) + } + test("recv CurrentFeerate (when funder, doesn't trigger an UpdateFee)") { f => import f._ val event = CurrentFeerates(FeeratesPerKw.single(FeeratePerKw(10010 sat))) @@ -2113,6 +2159,14 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with alice2bob.expectNoMsg(500 millis) } + test("recv CurrentFeerate (when funder, doesn't trigger an UpdateFee, anchor outputs)", Tag(StateTestsTags.AnchorOutputs)) { f => + import f._ + val initialState = alice.stateData.asInstanceOf[DATA_NORMAL] + assert(initialState.commitments.localCommit.spec.feeratePerKw === FeeEstimator.AnchorOutputMaxCommitFeerate) + alice ! CurrentFeerates(FeeratesPerKw.single(FeeEstimator.AnchorOutputMaxCommitFeerate * 2)) + alice2bob.expectNoMsg(500 millis) + } + test("recv CurrentFeerate (when fundee, commit-fee/network-fee are close)") { f => import f._ val event = CurrentFeerates(FeeratesPerKw.single(FeeratePerKw(11000 sat))) @@ -2136,6 +2190,30 @@ class NormalStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with awaitCond(bob.stateName == CLOSING) } + test("recv CurrentFeerate (when fundee, commit-fee/network-fee are very different, with HTLCs, anchor outputs)", Tag(StateTestsTags.AnchorOutputs)) { f => + import f._ + + // We start with a feerate lower than the 10 sat/byte threshold. + alice.feeEstimator.setFeerate(FeeratesPerKw.single(FeeEstimator.AnchorOutputMaxCommitFeerate / 2)) + bob.feeEstimator.setFeerate(FeeratesPerKw.single(FeeEstimator.AnchorOutputMaxCommitFeerate / 2)) + alice ! CMD_UPDATE_FEE(FeeEstimator.AnchorOutputMaxCommitFeerate / 2) + alice2bob.expectMsgType[UpdateFee] + alice2bob.forward(bob) + addHtlc(10000000 msat, alice, bob, alice2bob, bob2alice) + crossSign(alice, bob, alice2bob, bob2alice) + assert(bob.stateData.asInstanceOf[DATA_NORMAL].commitments.localCommit.spec.feeratePerKw === FeeEstimator.AnchorOutputMaxCommitFeerate / 2) + + // The network fees spike, so Bob closes the channel. + bob.feeEstimator.setFeerate(FeeratesPerKw.single(FeeEstimator.AnchorOutputMaxCommitFeerate * 2)) + val event = CurrentFeerates(FeeratesPerKw.single(FeeEstimator.AnchorOutputMaxCommitFeerate * 2)) + bob ! event + bob2alice.expectMsgType[Error] + bob2blockchain.expectMsgType[PublishAsap] // commit tx + bob2blockchain.expectMsgType[PublishAsap] // main delayed + bob2blockchain.expectMsgType[WatchConfirmed] + awaitCond(bob.stateName == CLOSING) + } + test("recv CurrentFeerate (when fundee, commit-fee/network-fee are very different, without HTLCs)") { f => import f._ diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/f/ShutdownStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/f/ShutdownStateSpec.scala index cf5f809d29..2e4e4f5025 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/f/ShutdownStateSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/f/ShutdownStateSpec.scala @@ -20,9 +20,9 @@ import akka.testkit.TestProbe import fr.acinq.bitcoin.Crypto.PrivateKey import fr.acinq.bitcoin.{ByteVector32, ByteVector64, Crypto, SatoshiLong, ScriptFlags, Transaction} import fr.acinq.eclair.blockchain._ -import fr.acinq.eclair.blockchain.fee.{FeeratePerKw, FeeratesPerKw} +import fr.acinq.eclair.blockchain.fee.{FeeEstimator, FeeratePerKw, FeeratesPerKw} import fr.acinq.eclair.channel._ -import fr.acinq.eclair.channel.states.StateTestsBase +import fr.acinq.eclair.channel.states.{StateTestsBase, StateTestsTags} import fr.acinq.eclair.payment.OutgoingPacket.Upstream import fr.acinq.eclair.payment._ import fr.acinq.eclair.payment.relay.Relayer._ @@ -30,8 +30,8 @@ import fr.acinq.eclair.router.Router.ChannelHop import fr.acinq.eclair.wire.Onion.FinalLegacyPayload import fr.acinq.eclair.wire.{CommitSig, Error, FailureMessageCodecs, PermanentChannelFailure, RevokeAndAck, Shutdown, UpdateAddHtlc, UpdateFailHtlc, UpdateFailMalformedHtlc, UpdateFee, UpdateFulfillHtlc} import fr.acinq.eclair.{CltvExpiry, CltvExpiryDelta, MilliSatoshiLong, TestConstants, TestKitBaseClass, randomBytes32} -import org.scalatest.Outcome import org.scalatest.funsuite.FixtureAnyFunSuiteLike +import org.scalatest.{Outcome, Tag} import scodec.bits.ByteVector import java.util.UUID @@ -52,7 +52,7 @@ class ShutdownStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike wit val setup = init() import setup._ within(30 seconds) { - reachNormal(setup) + reachNormal(setup, test.tags) val sender = TestProbe() // alice sends an HTLC to bob val h1 = Crypto.sha256(r1) @@ -645,6 +645,14 @@ class ShutdownStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike wit alice2bob.expectMsg(UpdateFee(initialState.commitments.channelId, event.feeratesPerKw.feePerBlock(alice.feeTargets.commitmentBlockTarget))) } + test("recv CurrentFeerate (when funder, triggers an UpdateFee, anchor outputs)", Tag(StateTestsTags.AnchorOutputs)) { f => + import f._ + val initialState = alice.stateData.asInstanceOf[DATA_SHUTDOWN] + assert(initialState.commitments.localCommit.spec.feeratePerKw === FeeEstimator.AnchorOutputMaxCommitFeerate) + alice ! CurrentFeerates(FeeratesPerKw.single(FeeEstimator.AnchorOutputMaxCommitFeerate / 2)) + alice2bob.expectMsg(UpdateFee(initialState.commitments.channelId, FeeEstimator.AnchorOutputMaxCommitFeerate / 2)) + } + test("recv CurrentFeerate (when funder, doesn't trigger an UpdateFee)") { f => import f._ val event = CurrentFeerates(FeeratesPerKw.single(FeeratePerKw(10010 sat))) @@ -652,6 +660,14 @@ class ShutdownStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike wit alice2bob.expectNoMsg(500 millis) } + test("recv CurrentFeerate (when funder, doesn't trigger an UpdateFee, anchor outputs)", Tag(StateTestsTags.AnchorOutputs)) { f => + import f._ + val initialState = alice.stateData.asInstanceOf[DATA_SHUTDOWN] + assert(initialState.commitments.localCommit.spec.feeratePerKw === FeeEstimator.AnchorOutputMaxCommitFeerate) + alice ! CurrentFeerates(FeeratesPerKw.single(FeeEstimator.AnchorOutputMaxCommitFeerate * 2)) + alice2bob.expectNoMsg(500 millis) + } + test("recv CurrentFeerate (when fundee, commit-fee/network-fee are close)") { f => import f._ val event = CurrentFeerates(FeeratesPerKw.single(FeeratePerKw(11000 sat))) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/g/NegotiatingStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/g/NegotiatingStateSpec.scala index 88a67314c6..8f1dc1f352 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/g/NegotiatingStateSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/g/NegotiatingStateSpec.scala @@ -67,6 +67,9 @@ class NegotiatingStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike if (test.tags.contains("fee2")) { alice.feeEstimator.setFeerate(FeeratesPerKw.single(FeeratePerKw(4316 sat))) bob.feeEstimator.setFeerate(FeeratesPerKw.single(FeeratePerKw(4316 sat))) + } else if (test.tags.contains(StateTestsTags.AnchorOutputs)) { + alice.feeEstimator.setFeerate(FeeratesPerKw.single(FeeratePerKw(1250 sat))) + bob.feeEstimator.setFeerate(FeeratesPerKw.single(FeeratePerKw(1250 sat))) } else { alice.feeEstimator.setFeerate(FeeratesPerKw.single(FeeratePerKw(5000 sat))) bob.feeEstimator.setFeerate(FeeratesPerKw.single(FeeratePerKw(5000 sat))) @@ -100,7 +103,7 @@ class NegotiatingStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike val initialState = alice.stateData.asInstanceOf[DATA_NEGOTIATING] bob2alice.forward(alice) val aliceCloseSig2 = alice2bob.expectMsgType[ClosingSigned] - // BOLT 2: If the receiver [doesn't agree with the fee] it SHOULD propose a value strictly between the received fee-satoshis and its previously-sent fee-satoshis + // BOLT 2: If the receiver doesn't agree with the fee it SHOULD propose a value strictly between the received fee-satoshis and its previously-sent fee-satoshis assert(aliceCloseSig2.feeSatoshis < aliceCloseSig1.feeSatoshis && aliceCloseSig2.feeSatoshis > bobCloseSig1.feeSatoshis) awaitCond(alice.stateData.asInstanceOf[DATA_NEGOTIATING].closingTxProposed.last.map(_.localClosingSigned) == initialState.closingTxProposed.last.map(_.localClosingSigned) :+ aliceCloseSig2) val Some(closingTx) = alice.stateData.asInstanceOf[DATA_NEGOTIATING].bestUnpublishedClosingTx_opt diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/h/ClosingStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/h/ClosingStateSpec.scala index 7d2febd40e..fbccc4992b 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/h/ClosingStateSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/h/ClosingStateSpec.scala @@ -612,6 +612,9 @@ class ClosingStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with test("recv BITCOIN_TX_CONFIRMED (remote commit, anchor outputs)", Tag(StateTestsTags.AnchorOutputs)) { f => import f._ + // Set feerates below the 10 sat/byte anchor outputs threshold to ensure a fee negotiation round-trip takes place. + alice.feeEstimator.setFeerate(FeeratesPerKw.single(FeeratePerKw(1200 sat))) + bob.feeEstimator.setFeerate(FeeratesPerKw.single(FeeratePerKw(1200 sat))) mutualClose(alice, bob, alice2bob, bob2alice, alice2blockchain, bob2blockchain) val initialState = alice.stateData.asInstanceOf[DATA_CLOSING] assert(initialState.commitments.channelVersion === ChannelVersion.ANCHOR_OUTPUTS) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/io/PeerSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/io/PeerSpec.scala index 2bea88ec6b..67a64aec3a 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/io/PeerSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/io/PeerSpec.scala @@ -23,9 +23,10 @@ import com.google.common.net.HostAndPort import fr.acinq.bitcoin.Crypto.PublicKey import fr.acinq.bitcoin.{Block, Btc, SatoshiLong, Script} import fr.acinq.eclair.FeatureSupport.Optional -import fr.acinq.eclair.Features.{StaticRemoteKey, Wumbo} +import fr.acinq.eclair.Features.{AnchorOutputs, StaticRemoteKey, Wumbo} import fr.acinq.eclair.TestConstants._ import fr.acinq.eclair._ +import fr.acinq.eclair.blockchain.fee.{FeeEstimator, FeeratesPerKw} import fr.acinq.eclair.blockchain.{EclairWallet, TestWallet} import fr.acinq.eclair.channel._ import fr.acinq.eclair.io.Peer._ @@ -56,6 +57,7 @@ class PeerSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with Paralle val aliceParams = TestConstants.Alice.nodeParams .modify(_.features).setToIf(test.tags.contains("static_remotekey"))(Features(Set(ActivatedFeature(StaticRemoteKey, Optional)))) .modify(_.features).setToIf(test.tags.contains("wumbo"))(Features(Set(ActivatedFeature(Wumbo, Optional)))) + .modify(_.features).setToIf(test.tags.contains("anchor_outputs"))(Features(Set(ActivatedFeature(StaticRemoteKey, Optional), ActivatedFeature(AnchorOutputs, Optional)))) .modify(_.maxFundingSatoshis).setToIf(test.tags.contains("high-max-funding-satoshis"))(Btc(0.9)) .modify(_.autoReconnect).setToIf(test.tags.contains("auto_reconnect"))(true) @@ -324,6 +326,23 @@ class PeerSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with Paralle } } + test("use correct on-chain fee rates when spawning a channel (anchor outputs)", Tag("anchor_outputs")) { f => + import f._ + + val probe = TestProbe() + system.eventStream.subscribe(probe.ref, classOf[ChannelCreated]) + connect(remoteNodeId, peer, peerConnection, remoteInit = wire.Init(Features(Set(ActivatedFeature(StaticRemoteKey, Optional), ActivatedFeature(AnchorOutputs, Optional))))) + + // We ensure the current network feerate is higher than the default anchor output feerate. + val feeEstimator = nodeParams.onChainFeeConf.feeEstimator.asInstanceOf[TestFeeEstimator] + feeEstimator.setFeerate(FeeratesPerKw.single(FeeEstimator.AnchorOutputMaxCommitFeerate * 2)) + probe.send(peer, Peer.OpenChannel(remoteNodeId, 15000 sat, 0 msat, None, None, None, None)) + + val channelCreated = probe.expectMsgType[ChannelCreated] + assert(channelCreated.initialFeeratePerKw == FeeEstimator.AnchorOutputMaxCommitFeerate) + assert(channelCreated.fundingTxFeeratePerKw.get == feeEstimator.getFeeratePerKw(nodeParams.onChainFeeConf.feeTargets.fundingBlockTarget)) + } + test("use correct final script if option_static_remotekey is negotiated", Tag("static_remotekey")) { f => import f._ From 65e1cd4637c70b7568250e7abd30283f99c4e944 Mon Sep 17 00:00:00 2001 From: t-bast Date: Tue, 19 Jan 2021 18:13:56 +0100 Subject: [PATCH 2/5] Fix anchor outputs closing fee requirements When using anchor outputs, the mutual close fee is allowed to be greater than the commit tx fee, because we're targeting a specific confirmation window. --- .../main/scala/fr/acinq/eclair/channel/Helpers.scala | 10 +++++++--- .../eclair/channel/states/g/NegotiatingStateSpec.scala | 7 +++---- .../eclair/channel/states/h/ClosingStateSpec.scala | 3 --- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala index a037935fd4..c8262e808e 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala @@ -427,8 +427,12 @@ object Helpers { def firstClosingFee(commitments: Commitments, localScriptPubkey: ByteVector, remoteScriptPubkey: ByteVector, feeEstimator: FeeEstimator, feeTargets: FeeTargets)(implicit log: LoggingAdapter): Satoshi = { val requestedFeerate = feeEstimator.getFeeratePerKw(feeTargets.mutualCloseBlockTarget) - // we "MUST set fee_satoshis less than or equal to the base fee of the final commitment transaction" - val feeratePerKw = requestedFeerate.min(commitments.localCommit.spec.feeratePerKw) + val feeratePerKw = if (commitments.channelVersion.hasAnchorOutputs) { + requestedFeerate + } else { + // we "MUST set fee_satoshis less than or equal to the base fee of the final commitment transaction" + requestedFeerate.min(commitments.localCommit.spec.feeratePerKw) + } firstClosingFee(commitments, localScriptPubkey, remoteScriptPubkey, feeratePerKw) } @@ -456,7 +460,7 @@ object Helpers { def checkClosingSignature(keyManager: ChannelKeyManager, commitments: Commitments, localScriptPubkey: ByteVector, remoteScriptPubkey: ByteVector, remoteClosingFee: Satoshi, remoteClosingSig: ByteVector64)(implicit log: LoggingAdapter): Either[ChannelException, Transaction] = { import commitments._ val lastCommitFeeSatoshi = commitments.commitInput.txOut.amount - commitments.localCommit.publishableTxs.commitTx.tx.txOut.map(_.amount).sum - if (remoteClosingFee > lastCommitFeeSatoshi) { + if (remoteClosingFee > lastCommitFeeSatoshi && !commitments.channelVersion.hasAnchorOutputs) { log.error(s"remote proposed a commit fee higher than the last commitment fee: remoteClosingFeeSatoshi=${remoteClosingFee.toLong} lastCommitFeeSatoshi=$lastCommitFeeSatoshi") Left(InvalidCloseFee(commitments.channelId, remoteClosingFee)) } else { diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/g/NegotiatingStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/g/NegotiatingStateSpec.scala index 8f1dc1f352..c7c5ce21da 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/g/NegotiatingStateSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/g/NegotiatingStateSpec.scala @@ -21,10 +21,11 @@ import akka.testkit.TestProbe import fr.acinq.bitcoin.{ByteVector32, ByteVector64, SatoshiLong} import fr.acinq.eclair.TestConstants.Bob import fr.acinq.eclair.blockchain._ -import fr.acinq.eclair.blockchain.fee.{FeeratePerKw, FeeratesPerKw} +import fr.acinq.eclair.blockchain.fee.{FeeEstimator, FeeratePerKw, FeeratesPerKw} import fr.acinq.eclair.channel.Helpers.Closing import fr.acinq.eclair.channel._ import fr.acinq.eclair.channel.states.{StateTestsBase, StateTestsTags} +import fr.acinq.eclair.transactions.Transactions import fr.acinq.eclair.wire.{ClosingSigned, Error, Shutdown} import fr.acinq.eclair.{CltvExpiry, MilliSatoshiLong, TestConstants, TestKitBaseClass} import org.scalatest.funsuite.FixtureAnyFunSuiteLike @@ -67,9 +68,6 @@ class NegotiatingStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike if (test.tags.contains("fee2")) { alice.feeEstimator.setFeerate(FeeratesPerKw.single(FeeratePerKw(4316 sat))) bob.feeEstimator.setFeerate(FeeratesPerKw.single(FeeratePerKw(4316 sat))) - } else if (test.tags.contains(StateTestsTags.AnchorOutputs)) { - alice.feeEstimator.setFeerate(FeeratesPerKw.single(FeeratePerKw(1250 sat))) - bob.feeEstimator.setFeerate(FeeratesPerKw.single(FeeratePerKw(1250 sat))) } else { alice.feeEstimator.setFeerate(FeeratesPerKw.single(FeeratePerKw(5000 sat))) bob.feeEstimator.setFeerate(FeeratesPerKw.single(FeeratePerKw(5000 sat))) @@ -108,6 +106,7 @@ class NegotiatingStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike awaitCond(alice.stateData.asInstanceOf[DATA_NEGOTIATING].closingTxProposed.last.map(_.localClosingSigned) == initialState.closingTxProposed.last.map(_.localClosingSigned) :+ aliceCloseSig2) val Some(closingTx) = alice.stateData.asInstanceOf[DATA_NEGOTIATING].bestUnpublishedClosingTx_opt assert(closingTx.txOut.length === 2) // NB: in the anchor outputs case, anchors are removed from the closing tx + assert(aliceCloseSig2.feeSatoshis > Transactions.weight2fee(FeeEstimator.AnchorOutputMaxCommitFeerate, closingTx.weight())) // NB: closing fee is allowed to be higher than commit tx fee when using anchor outputs } test("recv ClosingSigned (theirCloseFee != ourCloseFee)") { diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/h/ClosingStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/h/ClosingStateSpec.scala index fbccc4992b..7d2febd40e 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/h/ClosingStateSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/h/ClosingStateSpec.scala @@ -612,9 +612,6 @@ class ClosingStateSpec extends TestKitBaseClass with FixtureAnyFunSuiteLike with test("recv BITCOIN_TX_CONFIRMED (remote commit, anchor outputs)", Tag(StateTestsTags.AnchorOutputs)) { f => import f._ - // Set feerates below the 10 sat/byte anchor outputs threshold to ensure a fee negotiation round-trip takes place. - alice.feeEstimator.setFeerate(FeeratesPerKw.single(FeeratePerKw(1200 sat))) - bob.feeEstimator.setFeerate(FeeratesPerKw.single(FeeratePerKw(1200 sat))) mutualClose(alice, bob, alice2bob, bob2alice, alice2blockchain, bob2blockchain) val initialState = alice.stateData.asInstanceOf[DATA_CLOSING] assert(initialState.commitments.channelVersion === ChannelVersion.ANCHOR_OUTPUTS) From 52dab1d876d06f8bba111d19442c8f72e8a01cf4 Mon Sep 17 00:00:00 2001 From: t-bast Date: Tue, 19 Jan 2021 18:54:12 +0100 Subject: [PATCH 3/5] Fix fee mismatch without htlc We allow disagreeing on fees while the channel doesn't contain any htlc, because no funds can be at risk in that case. But we used the latest signed fee when adding a new HTLC, whereas we must also take into account the latest proposed (unsigned) fee. --- .../main/scala/fr/acinq/eclair/channel/Commitments.scala | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) 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 5b5e6aa7a6..a0f6eb1cce 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 @@ -276,8 +276,10 @@ object Commitments { // we allowed mismatches between our feerates and our remote's as long as commitments didn't contain any HTLC at risk // we need to verify that we're not disagreeing on feerates anymore before offering new HTLCs + // NB: there may be a pending update_fee that hasn't been signed yet that needs to be taken into account + val currentFeeratePerKw = commitments.remoteChanges.proposed.collect { case f: UpdateFee => f.feeratePerKw }.lastOption.getOrElse(commitments.localCommit.spec.feeratePerKw) val localFeeratePerKw = feeConf.getCommitmentFeerate(commitments.channelVersion, commitments.capacity, None) - if (feeConf.maxFeerateMismatchFor(commitments.remoteNodeId).isFeeDiffTooHigh(commitments.channelVersion, localFeeratePerKw, commitments.localCommit.spec.feeratePerKw)) { + if (feeConf.maxFeerateMismatchFor(commitments.remoteNodeId).isFeeDiffTooHigh(commitments.channelVersion, localFeeratePerKw, currentFeeratePerKw)) { return Left(FeerateTooDifferent(commitments.channelId, localFeeratePerKw = localFeeratePerKw, remoteFeeratePerKw = commitments.localCommit.spec.feeratePerKw)) } @@ -337,8 +339,10 @@ object Commitments { // we allowed mismatches between our feerates and our remote's as long as commitments didn't contain any HTLC at risk // we need to verify that we're not disagreeing on feerates anymore before accepting new HTLCs + // NB: there may be a pending update_fee that hasn't been signed yet that needs to be taken into account + val currentFeeratePerKw = commitments.remoteChanges.proposed.collect { case f: UpdateFee => f.feeratePerKw }.lastOption.getOrElse(commitments.localCommit.spec.feeratePerKw) val localFeeratePerKw = feeConf.getCommitmentFeerate(commitments.channelVersion, commitments.capacity, None) - if (feeConf.maxFeerateMismatchFor(commitments.remoteNodeId).isFeeDiffTooHigh(commitments.channelVersion, localFeeratePerKw, commitments.localCommit.spec.feeratePerKw)) { + if (feeConf.maxFeerateMismatchFor(commitments.remoteNodeId).isFeeDiffTooHigh(commitments.channelVersion, localFeeratePerKw, currentFeeratePerKw)) { return Left(FeerateTooDifferent(commitments.channelId, localFeeratePerKw = localFeeratePerKw, remoteFeeratePerKw = commitments.localCommit.spec.feeratePerKw)) } From 11eaa284c4299878b6eb2f2437037aad6bc0636a Mon Sep 17 00:00:00 2001 From: t-bast Date: Mon, 15 Feb 2021 18:11:22 +0100 Subject: [PATCH 4/5] Update fee estimator comment --- .../scala/fr/acinq/eclair/blockchain/fee/FeeEstimator.scala | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/fee/FeeEstimator.scala b/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/fee/FeeEstimator.scala index 351d327d76..8a3b305ee8 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/fee/FeeEstimator.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/fee/FeeEstimator.scala @@ -62,7 +62,10 @@ case class OnChainFeeConf(feeTargets: FeeTargets, feeEstimator: FeeEstimator, cl /** * Get the feerate that should apply to a channel commitment transaction: * - if we're using anchor outputs, we use a feerate that allows network propagation of the commit tx: we will use CPFP to speed up confirmation if needed - * - otherwise we use a feerate that should get the commit tx confirmed in the configured number of blocks + * - otherwise we use a feerate that should get the commit tx confirmed within the configured block target + * + * @param channelVersion channel version + * @param currentFeerates_opt if provided, will be used to compute the most up-to-date network fee, otherwise we rely on the fee estimator */ def getCommitmentFeerate(channelVersion: ChannelVersion, channelCapacity: Satoshi, currentFeerates_opt: Option[CurrentFeerates]): FeeratePerKw = { val networkFeerate = currentFeerates_opt match { From 0602beefd487eda7148a7741d43aeae95cf0bbbd Mon Sep 17 00:00:00 2001 From: t-bast Date: Wed, 17 Feb 2021 10:42:56 +0100 Subject: [PATCH 5/5] fixup! Fix fee mismatch without htlc --- .../fr/acinq/eclair/channel/Commitments.scala | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) 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 a0f6eb1cce..57b706f172 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 @@ -276,11 +276,12 @@ object Commitments { // we allowed mismatches between our feerates and our remote's as long as commitments didn't contain any HTLC at risk // we need to verify that we're not disagreeing on feerates anymore before offering new HTLCs - // NB: there may be a pending update_fee that hasn't been signed yet that needs to be taken into account - val currentFeeratePerKw = commitments.remoteChanges.proposed.collect { case f: UpdateFee => f.feeratePerKw }.lastOption.getOrElse(commitments.localCommit.spec.feeratePerKw) + // NB: there may be a pending update_fee that hasn't been applied yet that needs to be taken into account val localFeeratePerKw = feeConf.getCommitmentFeerate(commitments.channelVersion, commitments.capacity, None) - if (feeConf.maxFeerateMismatchFor(commitments.remoteNodeId).isFeeDiffTooHigh(commitments.channelVersion, localFeeratePerKw, currentFeeratePerKw)) { - return Left(FeerateTooDifferent(commitments.channelId, localFeeratePerKw = localFeeratePerKw, remoteFeeratePerKw = commitments.localCommit.spec.feeratePerKw)) + val remoteFeeratePerKw = commitments.localCommit.spec.feeratePerKw +: commitments.remoteChanges.all.collect { case f: UpdateFee => f.feeratePerKw } + remoteFeeratePerKw.find(feerate => feeConf.maxFeerateMismatchFor(commitments.remoteNodeId).isFeeDiffTooHigh(commitments.channelVersion, localFeeratePerKw, feerate)) match { + case Some(feerate) => return Left(FeerateTooDifferent(commitments.channelId, localFeeratePerKw = localFeeratePerKw, remoteFeeratePerKw = feerate)) + case None => } // let's compute the current commitment *as seen by them* with this change taken into account @@ -339,11 +340,12 @@ object Commitments { // we allowed mismatches between our feerates and our remote's as long as commitments didn't contain any HTLC at risk // we need to verify that we're not disagreeing on feerates anymore before accepting new HTLCs - // NB: there may be a pending update_fee that hasn't been signed yet that needs to be taken into account - val currentFeeratePerKw = commitments.remoteChanges.proposed.collect { case f: UpdateFee => f.feeratePerKw }.lastOption.getOrElse(commitments.localCommit.spec.feeratePerKw) + // NB: there may be a pending update_fee that hasn't been applied yet that needs to be taken into account val localFeeratePerKw = feeConf.getCommitmentFeerate(commitments.channelVersion, commitments.capacity, None) - if (feeConf.maxFeerateMismatchFor(commitments.remoteNodeId).isFeeDiffTooHigh(commitments.channelVersion, localFeeratePerKw, currentFeeratePerKw)) { - return Left(FeerateTooDifferent(commitments.channelId, localFeeratePerKw = localFeeratePerKw, remoteFeeratePerKw = commitments.localCommit.spec.feeratePerKw)) + val remoteFeeratePerKw = commitments.localCommit.spec.feeratePerKw +: commitments.remoteChanges.all.collect { case f: UpdateFee => f.feeratePerKw } + remoteFeeratePerKw.find(feerate => feeConf.maxFeerateMismatchFor(commitments.remoteNodeId).isFeeDiffTooHigh(commitments.channelVersion, localFeeratePerKw, feerate)) match { + case Some(feerate) => return Left(FeerateTooDifferent(commitments.channelId, localFeeratePerKw = localFeeratePerKw, remoteFeeratePerKw = feerate)) + case None => } // let's compute the current commitment *as seen by us* including this change