What the developer wrote
Authored by Bastien Teinturier
Fix flaky DER signature weight test (#3299)
This test was flaky because for the funding output, we provide two
signatures, which means that we need twice the slack for the variable
DER encoding length.
What changed, and why it matters
This commit only adjusts a flaky unit test that checks transaction weight estimates. It widens the allowed tolerance for variable-length ECDSA signatures when two signatures are involved instead of one. There is no change to production code, no security fix, and no vulnerability.
No security action needed. Treat as a normal test-stability improvement.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The diff modifies TransactionsSpec.scala. It adds a sigsCount parameter (default 1) to checkExpectedWeight, and multiplies the existing ±4 weight slack by sigsCount. The funding-input weight check is then called with sigsCount = 2 because the funding output requires two signatures. This is purely a test-calibration change; production transaction-weight logic is untouched.
Changed components
eclair-core/src/test/scala/fr/acinq/eclair/transactions/TransactionsSpec.scalaInspect captured patch +4 / −5
diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/transactions/TransactionsSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/transactions/TransactionsSpec.scala
index e46780a..3d99953 100644
--- a/eclair-core/src/test/scala/fr/acinq/eclair/transactions/TransactionsSpec.scala
+++ b/eclair-core/src/test/scala/fr/acinq/eclair/transactions/TransactionsSpec.scala
@@ -26,7 +26,6 @@ import fr.acinq.eclair._
import fr.acinq.eclair.blockchain.fee.FeeratePerKw
import fr.acinq.eclair.channel.ChannelSpendSignature
import fr.acinq.eclair.crypto.keymanager.{LocalCommitmentKeys, RemoteCommitmentKeys}
-import fr.acinq.eclair.reputation.Reputation
import fr.acinq.eclair.transactions.Scripts._
import fr.acinq.eclair.transactions.Transactions.AnchorOutputsCommitmentFormat.anchorAmount
import fr.acinq.eclair.transactions.Transactions._
@@ -148,13 +147,13 @@ class TransactionsSpec extends AnyFunSuite with Logging {
assert(dummyTx.weight() - dummyTx.copy(txOut = dummyTx.txOut.take(1)).weight() == p2trOutputWeight)
}
- private def checkExpectedWeight(actual: Int, expected: Int, commitmentFormat: CommitmentFormat): Unit = {
+ private def checkExpectedWeight(actual: Int, expected: Int, commitmentFormat: CommitmentFormat, sigsCount: Int = 1): Unit = {
commitmentFormat match {
case _: SimpleTaprootChannelCommitmentFormat => assert(actual == expected)
case _: AnchorOutputsCommitmentFormat =>
// ECDSA signatures are der-encoded, which creates some variability in signature size compared to the baseline.
- assert(actual <= expected + 4)
- assert(actual >= expected - 4)
+ assert(actual <= expected + 4 * sigsCount)
+ assert(actual >= expected - 4 * sigsCount)
}
}
@@ -278,7 +277,7 @@ class TransactionsSpec extends AnyFunSuite with Logging {
commitTx.correctlySpends(Seq(fundingTx), ScriptFlags.STANDARD_SCRIPT_VERIFY_FLAGS)
// We check the expected weight of the commit input:
val commitInputWeight = commitTx.copy(txIn = Seq(commitTx.txIn.head, commitTx.txIn.head)).weight() - commitTx.weight()
- checkExpectedWeight(commitInputWeight, commitmentFormat.fundingInputWeight, commitmentFormat)
+ checkExpectedWeight(commitInputWeight, commitmentFormat.fundingInputWeight, commitmentFormat, sigsCount = 2)
val htlcTxs = makeHtlcTxs(commitTx, outputs, commitmentFormat)
val expiries = htlcTxs.map(tx => tx.htlcId -> tx.htlcExpiry.toLong).toMap
val htlcSuccessTxs = htlcTxs.collect { case tx: UnsignedHtlcSuccessTx => tx }
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.