From ec7750211f5963f439a7997ce69907939b061935 Mon Sep 17 00:00:00 2001 From: Bastien Teinturier <31281497+t-bast@users.noreply.github.com> Date: Mon, 20 Apr 2020 16:23:05 +0200 Subject: [PATCH] Send `update_fee` on reconnection (#1383) We update transaction fees at every block (ie every 10 minutes). While this works well when the remote peer is a node that's online for more than 10 minutes, it's an issue for mobile wallets that usually come online for a few minutes and then disconnect. We want to make sure we send these wallet peers an update_fee when one is needed, so we now check for feerate updates on reconnection. Fixes #1381. --- .../fr/acinq/eclair/channel/Channel.scala | 12 ++++++ .../channel/states/e/OfflineStateSpec.scala | 40 ++++++++++++++++--- 2 files changed, 47 insertions(+), 5 deletions(-) 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 94fce73fd..d5a590c8c 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 @@ -1545,9 +1545,21 @@ class Channel(val nodeParams: NodeParams, val wallet: EclairWallet, remoteNodeId () } } + // we will re-enable the channel after some delay to prevent flappy updates in case the connection is unstable setTimer(Reconnected.toString, BroadcastChannelUpdate(Reconnected), 10 seconds, repeat = false) + // We usually handle feerate updates once per block (~10 minutes), but when our remote is a mobile wallet that + // only briefly connects and then disconnects, we may never have the opportunity to send our `update_fee`, so + // 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)) { + self ! CMD_UPDATE_FEE(networkFeeratePerKw, commit = true) + } + } + goto(NORMAL) using d.copy(commitments = commitments1) sending sendQueue } diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/OfflineStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/OfflineStateSpec.scala index 079486a23..0964b31d7 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/OfflineStateSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/OfflineStateSpec.scala @@ -22,7 +22,7 @@ import akka.actor.Status import akka.testkit.{TestActorRef, TestProbe} import fr.acinq.bitcoin.Crypto.PrivateKey import fr.acinq.bitcoin.{ByteVector32, ScriptFlags, Transaction} -import fr.acinq.eclair.TestConstants.Alice +import fr.acinq.eclair.TestConstants.{Alice, TestFeeEstimator} import fr.acinq.eclair.blockchain._ import fr.acinq.eclair.blockchain.fee.FeeratesPerKw import fr.acinq.eclair.channel._ @@ -95,7 +95,6 @@ class OfflineStateSpec extends TestkitBaseClass with StateTestsHelperMethods { TestConstants.Alice.keyManager.channelKeyPath(aliceCommitments.localParams, aliceCommitments.channelVersion), aliceCommitments.localCommit.index) - // a didn't receive any update or sig val ab_reestablish = alice2bob.expectMsg(ChannelReestablish(ab_add_0.channelId, 1, 0, PrivateKey(ByteVector32.Zeroes), aliceCurrentPerCommitmentPoint)) // b didn't receive the sig @@ -208,7 +207,6 @@ class OfflineStateSpec extends TestkitBaseClass with StateTestsHelperMethods { awaitCond(alice.stateName == NORMAL) awaitCond(bob.stateName == NORMAL) - } test("discover that we have a revoked commitment") { f => @@ -262,7 +260,6 @@ class OfflineStateSpec extends TestkitBaseClass with StateTestsHelperMethods { // alice is able to claim its main output val claimMainOutput = alice2blockchain.expectMsgType[PublishAsap].tx Transaction.correctlySpends(claimMainOutput, bobCommitTx :: Nil, ScriptFlags.STANDARD_SCRIPT_VERIFY_FLAGS) - } test("discover that they have a more recent commit than the one we know") { f => @@ -314,7 +311,6 @@ class OfflineStateSpec extends TestkitBaseClass with StateTestsHelperMethods { // alice is able to claim its main output val claimMainOutput = alice2blockchain.expectMsgType[PublishAsap].tx Transaction.correctlySpends(claimMainOutput, bobCommitTx :: Nil, ScriptFlags.STANDARD_SCRIPT_VERIFY_FLAGS) - } test("counterparty lies about having a more recent commitment") { f => @@ -513,6 +509,40 @@ class OfflineStateSpec extends TestkitBaseClass with StateTestsHelperMethods { alice2bob.expectNoMsg() } + test("handle feerate changes while offline (update at reconnection)") { f => + import f._ + val sender = TestProbe() + + // we simulate a disconnection + sender.send(alice, INPUT_DISCONNECTED) + sender.send(bob, INPUT_DISCONNECTED) + awaitCond(alice.stateName == OFFLINE) + awaitCond(bob.stateName == OFFLINE) + + val localFeeratePerKw = alice.stateData.asInstanceOf[DATA_NORMAL].commitments.localCommit.spec.feeratePerKw + val networkFeeratePerKw = 2 * localFeeratePerKw + val networkFeerate = FeeratesPerKw.single(networkFeeratePerKw) + + // Alice ignores feerate changes while offline + alice.underlyingActor.nodeParams.onChainFeeConf.feeEstimator.asInstanceOf[TestFeeEstimator].setFeerate(networkFeerate) + sender.send(alice, CurrentFeerates(networkFeerate)) + alice2blockchain.expectNoMsg() + alice2bob.expectNoMsg() + + // then we reconnect them; Alice should send the feerate changes to Bob + sender.send(alice, INPUT_RECONNECTED(alice2bob.ref, aliceInit, bobInit)) + sender.send(bob, INPUT_RECONNECTED(bob2alice.ref, bobInit, aliceInit)) + + // peers exchange channel_reestablish messages + alice2bob.expectMsgType[ChannelReestablish] + bob2alice.expectMsgType[ChannelReestablish] + // note that we don't forward the channel_reestablish so that only alice reaches NORMAL state, it facilitates the test below + bob2alice.forward(alice) + + alice2bob.expectMsgType[FundingLocked] // since the channel's commitment hasn't been updated, we re-send funding_locked + alice2bob.expectMsg(UpdateFee(channelId(alice), networkFeeratePerKw)) + } + test("handle feerate changes while offline (fundee scenario)") { f => import f._ val sender = TestProbe()