From 10ea7bdc230fedbcaa5a144f6668685dcc48754a Mon Sep 17 00:00:00 2001 From: Pierre-Marie Padiou Date: Fri, 11 Jan 2019 19:16:37 +0100 Subject: [PATCH] Make ping less aggressive (#782) * send pings only after there is less activity * make disconnect-on-ping configurable --- eclair-core/src/main/resources/reference.conf | 4 +- .../scala/fr/acinq/eclair/NodeParams.scala | 4 ++ .../main/scala/fr/acinq/eclair/io/Peer.scala | 55 ++++++++++++------- .../scala/fr/acinq/eclair/TestConstants.scala | 4 ++ 4 files changed, 47 insertions(+), 20 deletions(-) diff --git a/eclair-core/src/main/resources/reference.conf b/eclair-core/src/main/resources/reference.conf index fb44105b0..14d3b6446 100644 --- a/eclair-core/src/main/resources/reference.conf +++ b/eclair-core/src/main/resources/reference.conf @@ -85,6 +85,8 @@ eclair { router-init-timeout = 5 minutes ping-interval = 30 seconds + ping-timeout = 10 seconds // will disconnect if peer takes longer than that to respond + ping-disconnect = true // disconnect if no answer to our pings auto-reconnect = true payment-handler = "local" @@ -92,4 +94,4 @@ eclair { max-pending-payment-requests = 10000000 max-payment-fee = 0.03 // max total fee for outgoing payments, in percentage: sending a payment will not be attempted if the cheapest route found is more expensive than that min-funding-satoshis = 100000 -} \ No newline at end of file +} diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/NodeParams.scala b/eclair-core/src/main/scala/fr/acinq/eclair/NodeParams.scala index 8f398fa79..7f24cd1a9 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/NodeParams.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/NodeParams.scala @@ -68,6 +68,8 @@ case class NodeParams(keyManager: KeyManager, revocationTimeout: FiniteDuration, routerBroadcastInterval: FiniteDuration, pingInterval: FiniteDuration, + pingTimeout: FiniteDuration, + pingDisconnect: Boolean, maxFeerateMismatch: Double, updateFeeMinDiffRatio: Double, autoReconnect: Boolean, @@ -205,6 +207,8 @@ object NodeParams { revocationTimeout = FiniteDuration(config.getDuration("revocation-timeout").getSeconds, TimeUnit.SECONDS), routerBroadcastInterval = FiniteDuration(config.getDuration("router-broadcast-interval").getSeconds, TimeUnit.SECONDS), pingInterval = FiniteDuration(config.getDuration("ping-interval").getSeconds, TimeUnit.SECONDS), + pingTimeout = FiniteDuration(config.getDuration("ping-timeout").getSeconds, TimeUnit.SECONDS), + pingDisconnect = config.getBoolean("ping-disconnect"), maxFeerateMismatch = config.getDouble("max-feerate-mismatch"), updateFeeMinDiffRatio = config.getDouble("update-fee_min-diff-ratio"), autoReconnect = config.getBoolean("auto-reconnect"), 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 51c0d58cc..3af2c9b24 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 @@ -20,7 +20,7 @@ import java.io.ByteArrayInputStream import java.net.InetSocketAddress import java.nio.ByteOrder -import akka.actor.{ActorRef, Cancellable, OneForOneStrategy, PoisonPill, Props, Status, SupervisorStrategy, Terminated} +import akka.actor.{ActorRef, OneForOneStrategy, PoisonPill, Props, Status, SupervisorStrategy, Terminated} import akka.event.Logging.MDC import fr.acinq.bitcoin.Crypto.PublicKey import fr.acinq.bitcoin.{BinaryData, DeterministicWallet, MilliSatoshi, Protocol, Satoshi} @@ -124,7 +124,7 @@ class Peer(nodeParams: NodeParams, remoteNodeId: PublicKey, authenticator: Actor // let's bring existing/requested channels online d.channels.values.toSet[ActorRef].foreach(_ ! INPUT_RECONNECTED(d.transport, d.localInit, remoteInit)) // we deduplicate with toSet because there might be two entries per channel (tmp id and final id) - goto(CONNECTED) using ConnectedData(d.address_opt, d.transport, d.localInit, remoteInit, d.channels.map { case (k: ChannelId, v) => (k, v) }) + goto(CONNECTED) using ConnectedData(d.address_opt, d.transport, d.localInit, remoteInit, d.channels.map { case (k: ChannelId, v) => (k, v) }) forMax(30 seconds) // forMax will trigger a StateTimeout } else { log.warning(s"incompatible features, disconnecting") d.origin_opt.foreach(origin => origin ! Status.Failure(new RuntimeException("incompatible features"))) @@ -157,18 +157,36 @@ class Peer(nodeParams: NodeParams, remoteNodeId: PublicKey, authenticator: Actor } when(CONNECTED) { + case Event(StateTimeout, _: ConnectedData) => + // the first ping is sent after the connection has been quiet for a while + // we don't want to send pings right after connection, because peer will be syncing and may not be able to + // answer to our ping quickly enough, which will make us close the connection + log.debug(s"no messages sent/received for a while, start sending pings") + self ! SendPing + setStateTimeout(CONNECTED, None) // cancels the state timeout (it will be reset with forMax) + stay + case Event(SendPing, d: ConnectedData) => - // no need to use secure random here - val pingSize = Random.nextInt(1000) - val pongSize = Random.nextInt(1000) - val ping = wire.Ping(pongSize, BinaryData("00" * pingSize)) - setTimer(PingTimeout.toString, PingTimeout(ping), 10 seconds, repeat = false) - d.transport ! ping - stay using d.copy(expectedPong_opt = Some(ExpectedPong(ping))) + if (d.expectedPong_opt.isEmpty) { + // no need to use secure random here + val pingSize = Random.nextInt(1000) + val pongSize = Random.nextInt(1000) + val ping = wire.Ping(pongSize, BinaryData("00" * pingSize)) + setTimer(PingTimeout.toString, PingTimeout(ping), nodeParams.pingTimeout, repeat = false) + d.transport ! ping + stay using d.copy(expectedPong_opt = Some(ExpectedPong(ping))) + } else { + log.warning(s"can't send ping, already have one in flight") + stay + } case Event(PingTimeout(ping), d: ConnectedData) => - log.warning(s"no response to ping=$ping, closing connection") - d.transport ! PoisonPill + if (nodeParams.pingDisconnect) { + log.warning(s"no response to ping=$ping, closing connection") + d.transport ! PoisonPill + } else { + log.warning(s"no response to ping=$ping (ignored)") + } stay case Event(ping@wire.Ping(pongLength, _), d: ConnectedData) => @@ -189,7 +207,9 @@ class Peer(nodeParams: NodeParams, remoteNodeId: PublicKey, authenticator: Actor val latency = Platform.currentTime - timestamp log.debug(s"received pong with latency=$latency") cancelTimer(PingTimeout.toString()) - schedulePing() + // pings are sent periodically with some randomization + val nextDelay = nodeParams.pingInterval + secureRandom.nextInt(10).seconds + setTimer(SendPing.toString, SendPing, nextDelay, repeat = false) case None => log.debug(s"received unexpected pong with size=${data.length}") } @@ -417,12 +437,15 @@ class Peer(nodeParams: NodeParams, remoteNodeId: PublicKey, authenticator: Actor case Event(SendPing, _) => stay // we got disconnected in the meantime case Event(_: Pong, _) => stay // we got disconnected before receiving the pong + + case Event(_: PingTimeout, _) => stay // we got disconnected after sending a ping + + case Event(_: Terminated, _) => stay // this channel got closed before having a commitment and we got disconnected (e.g. a funding error occured) } onTransition { case _ -> DISCONNECTED if nodeParams.autoReconnect && nextStateData.address_opt.isDefined => setTimer(RECONNECT_TIMER, Reconnect, 1 second, repeat = false) case DISCONNECTED -> _ if nodeParams.autoReconnect && stateData.address_opt.isDefined => cancelTimer(RECONNECT_TIMER) - case _ -> CONNECTED => schedulePing() } def createNewChannel(nodeParams: NodeParams, funder: Boolean, fundingSatoshis: Long, origin_opt: Option[ActorRef]): (ActorRef, LocalParams) = { @@ -445,12 +468,6 @@ class Peer(nodeParams: NodeParams, remoteNodeId: PublicKey, authenticator: Actor override def mdc(currentMessage: Any): MDC = Logs.mdc(remoteNodeId_opt = Some(remoteNodeId)) - def schedulePing(): Unit = { - // pings are periodically with some randomization - val nextDelay = nodeParams.pingInterval + secureRandom.nextInt(10).seconds - setTimer(SendPing.toString, SendPing, nextDelay, repeat = false) - } - } object Peer { 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 386fda8f4..8167d5178 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/TestConstants.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/TestConstants.scala @@ -74,6 +74,8 @@ object TestConstants { revocationTimeout = 20 seconds, routerBroadcastInterval = 60 seconds, pingInterval = 30 seconds, + pingTimeout = 10 seconds, + pingDisconnect = true, maxFeerateMismatch = 1.5, updateFeeMinDiffRatio = 0.1, autoReconnect = false, @@ -131,6 +133,8 @@ object TestConstants { revocationTimeout = 20 seconds, routerBroadcastInterval = 60 seconds, pingInterval = 30 seconds, + pingTimeout = 10 seconds, + pingDisconnect = true, maxFeerateMismatch = 1.0, updateFeeMinDiffRatio = 0.1, autoReconnect = false,