Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion eclair-core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-scala-scalatest_${scala.version.short}</artifactId>
<version>1.5.9</version>
<version>1.17.5</version>
<scope>test</scope>
</dependency>
<dependency>
Expand Down
9 changes: 6 additions & 3 deletions eclair-core/src/main/scala/fr/acinq/eclair/Logs.scala
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,18 @@ object Logs {
parentPaymentId_opt: Option[UUID] = None,
paymentId_opt: Option[UUID] = None,
paymentHash_opt: Option[ByteVector32] = None,
txPublishId_opt: Option[UUID] = None): Map[String, String] =
txPublishId_opt: Option[UUID] = None,
nodeAlias_opt: Option[String] = None): Map[String, String] =
Seq(
// nb: we preformat MDC values so that there is no white spaces in logs when they are not defined
category_opt.map(l => "category" -> s" ${l.category}"),
remoteNodeId_opt.map(n => "nodeId" -> s" n:$n"), // nb: we preformat MDC values so that there is no white spaces in logs when they are not defined
remoteNodeId_opt.map(n => "nodeId" -> s" n:$n"),
channelId_opt.map(c => "channelId" -> s" c:$c"),
parentPaymentId_opt.map(p => "parentPaymentId" -> s" p:$p"),
paymentId_opt.map(i => "paymentId" -> s" i:$i"),
paymentHash_opt.map(h => "paymentHash" -> s" h:$h"),
txPublishId_opt.map(t => "txPublishId" -> s" t:$t")
txPublishId_opt.map(t => "txPublishId" -> s" t:$t"),
nodeAlias_opt.map(a => "nodeAlias" -> s" a:$a"),
).flatten.toMap

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1884,7 +1884,7 @@ class Channel(val nodeParams: NodeParams, val wallet: OnChainChannelFunder, val
case INPUT_RESTORED(data) => data.channelId
case _ => stateData.channelId
}
Logs.mdc(category_opt, remoteNodeId_opt = Some(remoteNodeId), channelId_opt = Some(id))
Logs.mdc(category_opt, remoteNodeId_opt = Some(remoteNodeId), channelId_opt = Some(id), nodeAlias_opt = Some(nodeParams.alias))
}

// we let the peer decide what to do
Expand Down
2 changes: 1 addition & 1 deletion eclair-core/src/main/scala/fr/acinq/eclair/io/Peer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ class Peer(val nodeParams: NodeParams, remoteNodeId: PublicKey, wallet: OnChainA
}

override def mdc(currentMessage: Any): MDC = {
Logs.mdc(LogCategory(currentMessage), Some(remoteNodeId), Logs.channelId(currentMessage))
Logs.mdc(LogCategory(currentMessage), Some(remoteNodeId), Logs.channelId(currentMessage), nodeAlias_opt = Some(nodeParams.alias))
}

}
Expand Down
17 changes: 14 additions & 3 deletions eclair-core/src/main/scala/fr/acinq/eclair/io/PeerConnection.scala
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

package fr.acinq.eclair.io

import akka.actor.{ActorRef, FSM, OneForOneStrategy, PoisonPill, Props, SupervisorStrategy, Terminated}
import akka.actor.{ActorRef, FSM, OneForOneStrategy, PoisonPill, Props, Stash, SupervisorStrategy, Terminated}
import akka.event.Logging.MDC
import fr.acinq.bitcoin.scalacompat.ByteVector32
import fr.acinq.bitcoin.scalacompat.Crypto.PublicKey
Expand All @@ -28,7 +28,7 @@ import fr.acinq.eclair.remote.EclairInternalsSerializer.RemoteTypes
import fr.acinq.eclair.router.Router._
import fr.acinq.eclair.wire.protocol
import fr.acinq.eclair.wire.protocol._
import fr.acinq.eclair.{FSMDiagnosticActorLogging, Feature, Features, InitFeature, Logs, TimestampMilli, TimestampSecond}
import fr.acinq.eclair.{FSMDiagnosticActorLogging, Features, InitFeature, Logs, TimestampMilli, TimestampSecond}
import scodec.Attempt
import scodec.bits.ByteVector

Expand Down Expand Up @@ -56,7 +56,7 @@ import scala.util.Random
*
* Created by PM on 11/03/2020.
*/
class PeerConnection(keyPair: KeyPair, conf: PeerConnection.Conf, switchboard: ActorRef, router: ActorRef) extends FSMDiagnosticActorLogging[PeerConnection.State, PeerConnection.Data] {
class PeerConnection(keyPair: KeyPair, conf: PeerConnection.Conf, switchboard: ActorRef, router: ActorRef) extends FSMDiagnosticActorLogging[PeerConnection.State, PeerConnection.Data] with Stash {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another pretty good reason not to use TestActorRef is that it is not compatible with Stash (https://stackoverflow.com/a/18377939).

Stash allows us to nicely handle race conditions between messages, instead of delaying them, which creates other race conditions (which is exactly what happened in my attempted fix 2632be3).


import PeerConnection._

Expand Down Expand Up @@ -95,6 +95,11 @@ class PeerConnection(keyPair: KeyPair, conf: PeerConnection.Conf, switchboard: A
log.warning(s"authentication timed out after ${conf.authTimeout}")
d.pendingAuth.origin_opt.foreach(_ ! ConnectionResult.AuthenticationFailed("authentication timed out"))
stop(FSM.Normal)

case Event(_: protocol.Init, _) =>
log.debug("stashing remote init")
stash()
stay()
}

when(BEFORE_INIT) {
Expand All @@ -108,7 +113,13 @@ class PeerConnection(keyPair: KeyPair, conf: PeerConnection.Conf, switchboard: A
}
d.transport ! localInit
startSingleTimer(INIT_TIMER, InitTimeout, conf.initTimeout)
unstashAll() // unstash remote init if it already arrived
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NB: I'm only unstashing right before we are ready to process the remote init.

goto(INITIALIZING) using InitializingData(chainHash, d.pendingAuth, d.remoteNodeId, d.transport, peer, localInit, doSync, d.isPersistent)

case Event(_: protocol.Init, _) =>
log.debug("stashing remote init")
stash()
stay()
}

when(INITIALIZING) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ class ReconnectionTask(nodeParams: NodeParams, remoteNodeId: PublicKey) extends
}

override def mdc(currentMessage: Any): MDC = {
Logs.mdc(Some(LogCategory.CONNECTION), Some(remoteNodeId))
Logs.mdc(Some(LogCategory.CONNECTION), Some(remoteNodeId), nodeAlias_opt = Some(nodeParams.alias))
}

}
Expand Down
11 changes: 6 additions & 5 deletions eclair-core/src/main/scala/fr/acinq/eclair/router/Router.scala
Original file line number Diff line number Diff line change
Expand Up @@ -257,12 +257,13 @@ class Router(val nodeParams: NodeParams, watcher: typed.ActorRef[ZmqWatcher.Comm

override def mdc(currentMessage: Any): MDC = {
val category_opt = LogCategory(currentMessage)
currentMessage match {
case s: SendChannelQuery => Logs.mdc(category_opt, remoteNodeId_opt = Some(s.remoteNodeId))
case prm: PeerRoutingMessage => Logs.mdc(category_opt, remoteNodeId_opt = Some(prm.remoteNodeId))
case lcu: LocalChannelUpdate => Logs.mdc(category_opt, remoteNodeId_opt = Some(lcu.remoteNodeId))
case _ => Logs.mdc(category_opt)
val remoteNodeId_opt = currentMessage match {
case s: SendChannelQuery => Some(s.remoteNodeId)
case prm: PeerRoutingMessage => Some(prm.remoteNodeId)
case lcu: LocalChannelUpdate => Some(lcu.remoteNodeId)
case _ => None
}
Logs.mdc(category_opt, remoteNodeId_opt = remoteNodeId_opt, nodeAlias_opt = Some(nodeParams.alias))
}
}

Expand Down
2 changes: 1 addition & 1 deletion eclair-core/src/test/resources/logback-test.xml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
<appender name="CONSOLE" class="ch.qos.logback.core.ConsoleAppender">
<target>System.out</target>
<encoder>
<pattern>%date{HH:mm:ss.SSS} %highlight(%-5level) %replace(%logger{24}){'\$.*',''}%X{category}%X{nodeId}%X{channelId}%X{paymentHash}%.-11X{parentPaymentId}%.-11X{paymentId}%.-11X{txPublishId} - %msg%ex{12}%n</pattern>
<pattern>%date{HH:mm:ss.SSS} %highlight(%-5level) %replace(%logger{24}){'\$.*',''}%X{category}%.-9X{nodeId}%.-11X{channelId}%.-11X{paymentHash}%.-11X{parentPaymentId}%.-11X{paymentId}%.-11X{txPublishId}%.-11X{nodeAlias} - %msg%ex{12}%n</pattern>
</encoder>
</appender>

Expand Down
22 changes: 11 additions & 11 deletions eclair-core/src/test/scala/fr/acinq/eclair/CltvExpirySpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,15 @@ class CltvExpirySpec extends AnyFunSuite with ParallelTestExecution {

test("cltv expiry delta") {
val d = CltvExpiryDelta(561)
assert(d.toInt === 561)
assert(d.toInt == 561)

// add
assert(d + 5 === CltvExpiryDelta(566))
assert(d + CltvExpiryDelta(5) === CltvExpiryDelta(566))
assert(d + 5 == CltvExpiryDelta(566))
assert(d + CltvExpiryDelta(5) == CltvExpiryDelta(566))

// subtract
assert(d - CltvExpiryDelta(5) === CltvExpiryDelta(556))
assert(d - CltvExpiryDelta(562) === CltvExpiryDelta(-1))
assert(d - CltvExpiryDelta(5) == CltvExpiryDelta(556))
assert(d - CltvExpiryDelta(562) == CltvExpiryDelta(-1))

// compare
assert(d <= CltvExpiryDelta(561))
Expand All @@ -44,18 +44,18 @@ class CltvExpirySpec extends AnyFunSuite with ParallelTestExecution {
assert(d > CltvExpiryDelta(560))

// convert to cltv expiry
assert(d.toCltvExpiry(currentBlockHeight = BlockHeight(1105)) === CltvExpiry(1666))
assert(d.toCltvExpiry(currentBlockHeight = BlockHeight(1106)) === CltvExpiry(1667))
assert(d.toCltvExpiry(currentBlockHeight = BlockHeight(1105)) == CltvExpiry(1666))
assert(d.toCltvExpiry(currentBlockHeight = BlockHeight(1106)) == CltvExpiry(1667))
}

test("cltv expiry") {
val e = CltvExpiry(1105)
assert(e.toLong === 1105)
assert(e.toLong == 1105)

// add
assert(e + CltvExpiryDelta(561) === CltvExpiry(1666))
assert(e - CltvExpiryDelta(561) === CltvExpiry(544))
assert(e - CltvExpiry(561) === CltvExpiryDelta(544))
assert(e + CltvExpiryDelta(561) == CltvExpiry(1666))
assert(e - CltvExpiryDelta(561) == CltvExpiry(544))
assert(e - CltvExpiry(561) == CltvExpiryDelta(544))

// compare
assert(e <= CltvExpiry(1105))
Expand Down
Loading