Public commit record
98/100 · Strong
What the developer wrote
Authored by Thomas HUET
Fix flaky `ReputationRecorder` test (#3166)
Sometimes, the test publishes events to the `EventStream` before the `ReputationRecorder` has subscribed, which leads to dropped events.
We fix this by bypassing the `EventStream` in tests.
We also subscribe to `OutgoingHtlcSettled` before subscribing to `OutgoingHtlcAdded` to prevent the unlikely edge case where a HTLC stays pending forever because it settled before `ReputationRecorder` subscribed to `OutgoingHtlcSettled`.
✓ Descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Explains rationale or failure mode✓ Mentions testing or verification✓ Links an issue, advisory, or supporting reference
What changed, and why it matters
This commit fixes a flaky automated test and makes a tiny production-code ordering change. The main code now subscribes to 'HTLC settled' events before 'HTLC added' events, so a very brief race during startup cannot leave a payment stuck. The rest of the change only alters test code to avoid using the event bus in tests, which removes test flakiness. There is no direct evidence this is a security vulnerability or that it was exploited.
Recommended action
No urgent action. Treat as routine test reliability and minor hardening. Reviewers may verify the subscription ordering change does not break normal ReputationRecorder behavior under load.
Security signals we found
01
Race-condition mitigation in event subscription ordering
02
Test-only refactor to bypass EventStream
03
No input validation, cryptographic, or authorization changes
04
No memory safety, overflow, or injection patterns
Evidence from the diff
ReputationRecorder.scala: swaps the order of EventStream.Subscribe calls so OutgoingHtlcSettled is subscribed before OutgoingHtlcAdded. ReputationRecorderSpec.scala: changes tests to send WrappedOutgoingHtlcAdded/WrappedOutgoingHtlcSettled commands directly to the actor instead of publishing via the EventStream, and widens one assertion tolerance from 1.0 to 0.99. The Wrapped* command classes are changed from private to public to allow direct test access. The production change is defensive and race-condition mitigation, not a patch for an exploitable bug.
Changed components
eclair-core/src/main/scala/fr/acinq/eclair/reputation/ReputationRecorder.scalaeclair-core/src/test/scala/fr/acinq/eclair/reputation/ReputationRecorderSpec.scalaInspect captured patch +24 / −24
diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/reputation/ReputationRecorder.scala b/eclair-core/src/main/scala/fr/acinq/eclair/reputation/ReputationRecorder.scala
index f480a92..58224a5 100644
--- a/eclair-core/src/main/scala/fr/acinq/eclair/reputation/ReputationRecorder.scala
+++ b/eclair-core/src/main/scala/fr/acinq/eclair/reputation/ReputationRecorder.scala
@@ -32,14 +32,14 @@ object ReputationRecorder {
// @formatter:off
sealed trait Command
case class GetConfidence(replyTo: ActorRef[Reputation.Score], upstream: Upstream.Hot, downstream_opt: Option[PublicKey], fee: MilliSatoshi, currentBlockHeight: BlockHeight, expiry: CltvExpiry) extends Command
- private case class WrappedOutgoingHtlcAdded(added: OutgoingHtlcAdded) extends Command
- private case class WrappedOutgoingHtlcSettled(settled: OutgoingHtlcSettled) extends Command
+ case class WrappedOutgoingHtlcAdded(added: OutgoingHtlcAdded) extends Command
+ case class WrappedOutgoingHtlcSettled(settled: OutgoingHtlcSettled) extends Command
// @formatter:on
def apply(config: Reputation.Config): Behavior[Command] =
Behaviors.setup(context => {
- context.system.eventStream ! EventStream.Subscribe(context.messageAdapter(WrappedOutgoingHtlcAdded))
context.system.eventStream ! EventStream.Subscribe(context.messageAdapter(WrappedOutgoingHtlcSettled))
+ context.system.eventStream ! EventStream.Subscribe(context.messageAdapter(WrappedOutgoingHtlcAdded))
new ReputationRecorder(config).run()
})
diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/reputation/ReputationRecorderSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/reputation/ReputationRecorderSpec.scala
index 61eeb64..1cbbe2b 100644
--- a/eclair-core/src/test/scala/fr/acinq/eclair/reputation/ReputationRecorderSpec.scala
+++ b/eclair-core/src/test/scala/fr/acinq/eclair/reputation/ReputationRecorderSpec.scala
@@ -64,8 +64,8 @@ class ReputationRecorderSpec extends ScalaTestWithActorTestKit(ConfigFactory.loa
reputationRecorder ! GetConfidence(replyTo.ref, upstream1, Some(nextA), 2000 msat, BlockHeight(0), CltvExpiry(2))
replyTo.expectMessage(Reputation.Score(0.0, 0.0))
val added1 = makeOutgoingHtlcAdded(upstream1, nextA, 2000 msat, CltvExpiry(2))
- testKit.system.eventStream ! EventStream.Publish(added1)
- testKit.system.eventStream ! EventStream.Publish(makeOutgoingHtlcFulfilled(added1.add))
+ reputationRecorder ! WrappedOutgoingHtlcAdded(added1)
+ reputationRecorder ! WrappedOutgoingHtlcSettled(makeOutgoingHtlcFulfilled(added1.add))
val upstream2 = makeChannelUpstream(originNode, 7)
awaitCond({
reputationRecorder ! GetConfidence(replyTo.ref, upstream2, Some(nextB), 1000 msat, BlockHeight(0), CltvExpiry(2))
@@ -73,7 +73,7 @@ class ReputationRecorderSpec extends ScalaTestWithActorTestKit(ConfigFactory.loa
score.incomingConfidence === (2.0 / 4) +- 0.001 && score.outgoingConfidence == 0.0
}, max = 60 seconds)
val added2 = makeOutgoingHtlcAdded(upstream2, nextB, 1000 msat, CltvExpiry(2))
- testKit.system.eventStream ! EventStream.Publish(added2)
+ reputationRecorder ! WrappedOutgoingHtlcAdded(added2)
awaitCond({
reputationRecorder ! GetConfidence(replyTo.ref, makeChannelUpstream(originNode, 7), Some(nextA), 3000 msat, BlockHeight(0), CltvExpiry(2))
val score = replyTo.expectMessageType[Reputation.Score]
@@ -86,9 +86,9 @@ class ReputationRecorderSpec extends ScalaTestWithActorTestKit(ConfigFactory.loa
score.incomingConfidence === (2.0 / 6) +- 0.001 && score.outgoingConfidence == 0.0
})
val added3 = makeOutgoingHtlcAdded(upstream3, nextB, 1000 msat, CltvExpiry(2))
- testKit.system.eventStream ! EventStream.Publish(added3)
- testKit.system.eventStream ! EventStream.Publish(makeOutgoingHtlcFulfilled(added3.add))
- testKit.system.eventStream ! EventStream.Publish(makeOutgoingHtlcFailed(added2.add))
+ reputationRecorder ! WrappedOutgoingHtlcAdded(added3)
+ reputationRecorder ! WrappedOutgoingHtlcSettled(makeOutgoingHtlcFulfilled(added3.add))
+ reputationRecorder ! WrappedOutgoingHtlcSettled(makeOutgoingHtlcFailed(added2.add))
// Not endorsed
awaitCond({
reputationRecorder ! GetConfidence(replyTo.ref, makeChannelUpstream(originNode, 0), Some(nextA), 1000 msat, BlockHeight(0), CltvExpiry(2))
@@ -122,8 +122,8 @@ class ReputationRecorderSpec extends ScalaTestWithActorTestKit(ConfigFactory.loa
score.incomingConfidence == 0.0 && score.outgoingConfidence == 0.0
})
val added1 = makeOutgoingHtlcAdded(upstream1, d, 6000 msat, CltvExpiry(2))
- testKit.system.eventStream ! EventStream.Publish(added1)
- testKit.system.eventStream ! EventStream.Publish(makeOutgoingHtlcFulfilled(added1.add))
+ reputationRecorder ! WrappedOutgoingHtlcAdded(added1)
+ reputationRecorder ! WrappedOutgoingHtlcSettled(makeOutgoingHtlcFulfilled(added1.add))
val upstream2 = Upstream.Hot.Trampoline(makeChannelUpstream(a, 7, 10000 msat) :: makeChannelUpstream(c, 0, 10000 msat) :: Nil)
awaitCond({
reputationRecorder ! GetConfidence(replyTo.ref, upstream2, Some(d), 2000 msat, BlockHeight(0), CltvExpiry(2))
@@ -131,7 +131,7 @@ class ReputationRecorderSpec extends ScalaTestWithActorTestKit(ConfigFactory.loa
score.incomingConfidence === (1.0 / 3) +- 0.001 && score.outgoingConfidence === (6.0 / 10) +- 0.001
}, max = 60 seconds)
val added2 = makeOutgoingHtlcAdded(upstream2, d, 2000 msat, CltvExpiry(2))
- testKit.system.eventStream ! EventStream.Publish(added2)
+ reputationRecorder ! WrappedOutgoingHtlcAdded(added2)
val upstream3 = Upstream.Hot.Trampoline(makeChannelUpstream(a, 0, 10000 msat) :: makeChannelUpstream(b, 7, 15000 msat) :: makeChannelUpstream(b, 7, 5000 msat) :: Nil)
awaitCond({
reputationRecorder ! GetConfidence(replyTo.ref, upstream3, Some(e), 3000 msat, BlockHeight(0), CltvExpiry(2))
@@ -139,9 +139,9 @@ class ReputationRecorderSpec extends ScalaTestWithActorTestKit(ConfigFactory.loa
score.incomingConfidence == 0.0 && score.outgoingConfidence == 0.0
}, max = 60 seconds)
val added3 = makeOutgoingHtlcAdded(upstream3, e, 3000 msat, CltvExpiry(2))
- testKit.system.eventStream ! EventStream.Publish(added3)
- testKit.system.eventStream ! EventStream.Publish(makeOutgoingHtlcFailed(added2.add))
- testKit.system.eventStream ! EventStream.Publish(makeOutgoingHtlcFulfilled(added3.add))
+ reputationRecorder ! WrappedOutgoingHtlcAdded(added3)
+ reputationRecorder ! WrappedOutgoingHtlcSettled(makeOutgoingHtlcFailed(added2.add))
+ reputationRecorder ! WrappedOutgoingHtlcSettled(makeOutgoingHtlcFulfilled(added3.add))
awaitCond({
reputationRecorder ! GetConfidence(replyTo.ref, makeChannelUpstream(a, 7), Some(d), 1000 msat, BlockHeight(0), CltvExpiry(2))
@@ -179,13 +179,13 @@ class ReputationRecorderSpec extends ScalaTestWithActorTestKit(ConfigFactory.loa
for (_ <- 1 to 200) {
val upstream = makeChannelUpstream(originNode, 7)
val added = makeOutgoingHtlcAdded(upstream, nextNode, 10000 msat, CltvExpiry(2))
- testKit.system.eventStream ! EventStream.Publish(added)
- testKit.system.eventStream ! EventStream.Publish(makeOutgoingHtlcFulfilled(added.add))
+ reputationRecorder ! WrappedOutgoingHtlcAdded(added)
+ reputationRecorder ! WrappedOutgoingHtlcSettled(makeOutgoingHtlcFulfilled(added.add))
}
awaitCond({
reputationRecorder ! GetConfidence(replyTo.ref, makeChannelUpstream(originNode, 7), Some(nextNode), 10000 msat, BlockHeight(0), CltvExpiry(2))
val score = replyTo.expectMessageType[Reputation.Score]
- (score.incomingConfidence === 1.0 +- 0.01) && (score.outgoingConfidence === 1.0 +- 0.01)
+ (score.incomingConfidence === 0.99 +- 0.01) && (score.outgoingConfidence === 0.99 +- 0.01)
}, max = 60 seconds)
// HTLCs with lower endorsement don't benefit from this high reputation.
@@ -198,7 +198,7 @@ class ReputationRecorderSpec extends ScalaTestWithActorTestKit(ConfigFactory.loa
for (_ <- 1 to 100) {
val upstream = makeChannelUpstream(originNode, 7)
val added = makeOutgoingHtlcAdded(upstream, nextNode, 10000 msat, CltvExpiry(2))
- testKit.system.eventStream ! EventStream.Publish(added)
+ reputationRecorder ! WrappedOutgoingHtlcAdded(added)
}
awaitCond({
reputationRecorder ! GetConfidence(replyTo.ref, makeChannelUpstream(originNode, 7), Some(nextNode), 10000 msat, BlockHeight(0), CltvExpiry(2))
@@ -216,8 +216,8 @@ class ReputationRecorderSpec extends ScalaTestWithActorTestKit(ConfigFactory.loa
for (node <- Seq(a, b, c)) {
val upstream = makeChannelUpstream(node, 7)
val added = makeOutgoingHtlcAdded(upstream, randomKey().publicKey, 10000000 msat, CltvExpiry(2))
- testKit.system.eventStream ! EventStream.Publish(added)
- testKit.system.eventStream ! EventStream.Publish(makeOutgoingHtlcFulfilled(added.add))
+ reputationRecorder ! WrappedOutgoingHtlcAdded(added)
+ reputationRecorder ! WrappedOutgoingHtlcSettled(makeOutgoingHtlcFulfilled(added.add))
awaitCond({
reputationRecorder ! GetConfidence(replyTo.ref, makeChannelUpstream(node, 7), Some(attacker), 10000 msat, BlockHeight(0), CltvExpiry(2))
val score = replyTo.expectMessageType[Reputation.Score]
@@ -230,8 +230,8 @@ class ReputationRecorderSpec extends ScalaTestWithActorTestKit(ConfigFactory.loa
for (_ <- 1 to 100) {
val upstream = makeChannelUpstream(node, 7)
val added = makeOutgoingHtlcAdded(upstream, attacker, 10000 msat, CltvExpiry(2))
- testKit.system.eventStream ! EventStream.Publish(added)
- testKit.system.eventStream ! EventStream.Publish(makeOutgoingHtlcFulfilled(added.add))
+ reputationRecorder ! WrappedOutgoingHtlcAdded(added)
+ reputationRecorder ! WrappedOutgoingHtlcSettled(makeOutgoingHtlcFulfilled(added.add))
}
}
@@ -240,7 +240,7 @@ class ReputationRecorderSpec extends ScalaTestWithActorTestKit(ConfigFactory.loa
for (_ <- 1 to 50) {
val upstream = makeChannelUpstream(node, 7)
val added = makeOutgoingHtlcAdded(upstream, attacker, 10000 msat, CltvExpiry(2))
- testKit.system.eventStream ! EventStream.Publish(added)
+ reputationRecorder ! WrappedOutgoingHtlcAdded(added)
}
}
awaitCond({
Risk score
Our methodology →Why this scored 11/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.