Large x expiry values throw IllegalArgumentException in relativeExpiry or make Bolt11Invoice.fromString reject the invoice

Open
#3,415 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
52/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
scala

Research direction

Start in eclair-core/src/main/scala/fr/acinq/eclair/payment/Bolt11Invoice.scala, especially relativeExpiry around line 71 and the expiry bit-length check around line 342. Trace how expiry is decoded and represented, then inspect the related invoice tests. Done means both provided invoices decode and relativeExpiry and isExpired return values without throwing.

Written by the indexing model from the issue text.

Description

With x from 9,223,372,037 up to 2^60 - 1, Bolt11Invoice.fromString decodes the invoice but relativeExpiry and isExpired throw IllegalArgumentException. With x from 2^60 to 2^64 - 1, Bolt11Invoice.fromString rejects the invoice with invoice expiry must be smaller than 2^64. This happens on eclair-core master at 06716fce and in v0.14.5.

The two cases below, added to eclair-core/src/test/scala/fr/acinq/eclair/payment/Bolt11InvoiceSpec.scala, build the invoices from the BOLT 11 cup of coffee example and change only the x field.

// BOLT 11 example "Please send $3 for a cup of coffee to the same peer, within one minute", whose x field is 60 seconds.
val cupOfCoffee: Bolt11Invoice = Bolt11Invoice.fromString("lnbc2500u1pvjluezsp5zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zygspp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdq5xysxxatsyp3k7enxv4jsxqzpu9qrsgquk0rl77nj30yxdy8j9vdx85fkpmdla2087ne0xh8nhedh8w27kyke0lp53ut353s06fv3qfegext0eh0ymjpf39tuven09sam30g4vgpfna3rh").get

test("expiry of 9,223,372,037 seconds") {
  val invoice = cupOfCoffee.copy(tags = cupOfCoffee.tags.map {
    case _: Expiry => Expiry(9_223_372_037L)
    case tag => tag
  }).sign(priv)
  assert(!Bolt11Invoice.fromString(invoice.toString).get.isExpired())
}

test("expiry of 2^60 seconds") {
  // Expiry would reject 2^60 because its minimal encoding is 13 groups, so the x field holds its 65 bits directly.
  val xField = BitVector.fromInt(6, 5) ++ BitVector.fromInt(13, 10) ++ long2bits(1L << 60)
  val hrp = s"${cupOfCoffee.prefix}${Amount.encode(cupOfCoffee.amount_opt)}"
  val data = BitVector.fromLong(cupOfCoffee.createdAt.toLong, 35) ++ cupOfCoffee.tags.map {
    case _: Expiry => xField
    case tag => Codecs.taggedFieldCodec.encode(tag).require
  }.reduce(_ ++ _)
  val hash = Crypto.sha256(ByteVector.view(hrp.getBytes) ++ data.toByteVector)
  val sig = Crypto.sign(hash, priv)
  val recid: Byte = if (Crypto.recoverPublicKey(sig, hash, 0) == nodeId) 0 else 1
  val int5s = scodec.codecs.list(scodec.codecs.ubyte(5)).decode(data ++ (sig.bytes :+ recid).bits).require.value.map(b => java.lang.Byte.valueOf(b)).toArray
  val invoice = fr.acinq.bitcoin.Bech32.encode(hrp, int5s, fr.acinq.bitcoin.Bech32.Encoding.Bech32)
  assert(!Bolt11Invoice.fromString(invoice).get.isExpired())
}

./mvnw test -pl eclair-core -Dsuites='*Bolt11InvoiceSpec' reports both cases failing.

- expiry of 9,223,372,037 seconds *** FAILED ***
  java.lang.IllegalArgumentException: requirement failed: Duration is limited to +-(2^63-1)ns (ca. 292 years)
  at fr.acinq.eclair.payment.Bolt11Invoice.relativeExpiry(Bolt11Invoice.scala:71)
- expiry of 2^60 seconds *** FAILED ***
  java.lang.IllegalArgumentException: requirement failed: invoice expiry must be smaller than 2^64
  at fr.acinq.eclair.payment.Bolt11Invoice$Expiry.<init>(Bolt11Invoice.scala:342)
Tests: succeeded 37, failed 2, canceled 0, ignored 0, pending 0

relativeExpiry at Bolt11Invoice.scala:71 builds a FiniteDuration from the expiry in seconds, so values from 9,223,372,037 up to 2^60 - 1 are too large for Duration. The check at line 342 counts the field bits, and the minimal 2^60 takes 13 groups, or 65 bits. fromString therefore rejects values from 2^60 to 2^64 - 1 even though they are below 2^64.

Both invoices should decode, and relativeExpiry and isExpired should return values for them.

Dominant language
Scala
Stars
1.3k
Forks
285
Avg merge
20h 22m
Merged PRs (30d)
28

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from ACINQ/eclair

All issues in ACINQ/eclair

Similar issues

More Scala issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.