Use unsigned comparison for HtlcValueTooHighInFlight

eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelExceptions.scala

@@ -64,7 +64,7 @@ case class UnexpectedHtlcId                    (override val channelId: ByteVect
 case class ExpiryTooSmall                      (override val channelId: ByteVector32, minimum: Long, actual: Long, blockCount: Long) extends ChannelException(channelId, s"expiry too small: minimum=$minimum actual=$actual blockCount=$blockCount")
 case class ExpiryTooBig                        (override val channelId: ByteVector32, maximum: Long, actual: Long, blockCount: Long) extends ChannelException(channelId, s"expiry too big: maximum=$maximum actual=$actual blockCount=$blockCount")
 case class HtlcValueTooSmall                   (override val channelId: ByteVector32, minimum: MilliSatoshi, actual: MilliSatoshi) extends ChannelException(channelId, s"htlc value too small: minimum=$minimum actual=$actual")
-case class HtlcValueTooHighInFlight            (override val channelId: ByteVector32, maximum: UInt64, actual: UInt64) extends ChannelException(channelId, s"in-flight htlcs hold too much value: maximum=$maximum actual=$actual")
+case class HtlcValueTooHighInFlight            (override val channelId: ByteVector32, maximum: UInt64, actual: MilliSatoshi) extends ChannelException(channelId, s"in-flight htlcs hold too much value: maximum=$maximum actual=$actual")
 case class TooManyAcceptedHtlcs                (override val channelId: ByteVector32, maximum: Long) extends ChannelException(channelId, s"too many accepted htlcs: maximum=$maximum")
 case class InsufficientFunds                   (override val channelId: ByteVector32, amount: MilliSatoshi, missing: Satoshi, reserve: Satoshi, fees: Satoshi) extends ChannelException(channelId, s"insufficient funds: missing=$missing reserve=$reserve fees=$fees")
 case class InvalidHtlcPreimage                 (override val channelId: ByteVector32, id: Long) extends ChannelException(channelId, s"invalid htlc preimage for htlc id=$id")

eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelTypes.scala

@@ -193,7 +193,7 @@ final case class DATA_WAIT_FOR_REMOTE_PUBLISH_FUTURE_COMMITMENT(commitments: Com
 final case class LocalParams(nodeId: PublicKey,
                              channelKeyPath: DeterministicWallet.KeyPath,
                              dustLimit: Satoshi,
-                             maxHtlcValueInFlightMsat: UInt64,
+                             maxHtlcValueInFlightMsat: UInt64, // this is not MilliSatoshi because it can exceed the total amount of MilliSatoshi
                              channelReserve: Satoshi,
                              htlcMinimum: MilliSatoshi,
                              toSelfDelay: Int,
@@ -205,7 +205,7 @@ final case class LocalParams(nodeId: PublicKey,
 
 final case class RemoteParams(nodeId: PublicKey,
                               dustLimit: Satoshi,
-                              maxHtlcValueInFlightMsat: UInt64,
+                              maxHtlcValueInFlightMsat: UInt64, // this is not MilliSatoshi because it can exceed the total amount of MilliSatoshi
                               channelReserve: Satoshi,
                               htlcMinimum: MilliSatoshi,
                               toSelfDelay: Int,

eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala

@@ -17,6 +17,7 @@
 package fr.acinq.eclair.channel
 
 import akka.event.LoggingAdapter
+import com.google.common.primitives.UnsignedLongs
 import fr.acinq.bitcoin.Crypto.{PrivateKey, PublicKey, sha256}
 import fr.acinq.bitcoin.{ByteVector32, ByteVector64, Crypto, Satoshi}
 import fr.acinq.eclair
@@ -148,8 +149,9 @@ object Commitments {
     // the HTLC we are about to create is outgoing, but from their point of view it is incoming
     val outgoingHtlcs = reduced.htlcs.filter(_.direction == IN)
 
-    val htlcValueInFlight = UInt64(outgoingHtlcs.map(_.add.amountMsat).sum.toLong)
-    if (htlcValueInFlight > commitments1.remoteParams.maxHtlcValueInFlightMsat) {
+    val htlcValueInFlight = outgoingHtlcs.map(_.add.amountMsat).sum
+    // we must use unsigned comparison here because 'maxHtlcValueInFlightMsat' is effectively an uint64 and can exceed the capacity of MilliSatoshi class
+    if (htlcValueInFlight.compareUnsigned(commitments1.remoteParams.maxHtlcValueInFlightMsat) > 0) {
       // TODO: this should be a specific UPDATE error
       return Left(HtlcValueTooHighInFlight(commitments.channelId, maximum = commitments1.remoteParams.maxHtlcValueInFlightMsat, actual = htlcValueInFlight))
     }
@@ -183,8 +185,9 @@ object Commitments {
     val reduced = CommitmentSpec.reduce(commitments1.localCommit.spec, commitments1.localChanges.acked, commitments1.remoteChanges.proposed)
     val incomingHtlcs = reduced.htlcs.filter(_.direction == IN)
 
-    val htlcValueInFlight = UInt64(incomingHtlcs.map(_.add.amountMsat).sum.toLong)
-    if (htlcValueInFlight > commitments1.localParams.maxHtlcValueInFlightMsat) {
+    val htlcValueInFlight = incomingHtlcs.map(_.add.amountMsat).sum
+    // we must use unsigned comparison here because 'maxHtlcValueInFlightMsat' is effectively an uint64 and can exceed the capacity of MilliSatoshi class
+    if (htlcValueInFlight.compareUnsigned(commitments1.localParams.maxHtlcValueInFlightMsat) > 0) {
       throw HtlcValueTooHighInFlight(commitments.channelId, maximum = commitments1.localParams.maxHtlcValueInFlightMsat, actual = htlcValueInFlight)
     }
 

eclair-core/src/main/scala/fr/acinq/eclair/package.scala

@@ -17,12 +17,11 @@
 package fr.acinq
 
 import java.security.SecureRandom
-
+import com.google.common.primitives.UnsignedLongs
 import fr.acinq.bitcoin.Crypto.PrivateKey
 import fr.acinq.bitcoin._
 import scodec.Attempt
 import scodec.bits.{BitVector, ByteVector}
-
 import scala.concurrent.duration.Duration
 import scala.util.{Failure, Success, Try}
 
@@ -173,6 +172,7 @@ package object eclair {
     def *(m: Double) = MilliSatoshi((amount * m).toLong)
     def /(d: Long) = MilliSatoshi(amount / d)
     def compare(other: MilliSatoshi): Int = if (amount == other.amount) 0 else if (amount < other.amount) -1 else 1
+    def compareUnsigned(other: UInt64): Int = UnsignedLongs.compare(amount, other.toBigInt.longValue())
     def <= (that: MilliSatoshi): Boolean = compare(that) <= 0
     def >= (that: MilliSatoshi): Boolean = compare(that) >= 0
     def <  (that: MilliSatoshi): Boolean = compare(that) < 0

eclair-core/src/main/scala/fr/acinq/eclair/wire/LightningMessageTypes.scala

@@ -71,7 +71,7 @@ case class OpenChannel(chainHash: ByteVector32,
                        fundingSatoshis: Satoshi,
                        pushMsat: MilliSatoshi,
                        dustLimitSatoshis: Satoshi,
-                       maxHtlcValueInFlightMsat: UInt64,
+                       maxHtlcValueInFlightMsat: UInt64, // this is not MilliSatoshi because it can exceed the total amount of MilliSatoshi
                        channelReserveSatoshis: Satoshi,
                        htlcMinimumMsat: MilliSatoshi,
                        feeratePerKw: Long,
@@ -87,7 +87,7 @@ case class OpenChannel(chainHash: ByteVector32,
 
 case class AcceptChannel(temporaryChannelId: ByteVector32,
                          dustLimitSatoshis: Satoshi,
-                         maxHtlcValueInFlightMsat: UInt64,
+                         maxHtlcValueInFlightMsat: UInt64, // this is not MilliSatoshi because it can exceed the total amount of MilliSatoshi
                          channelReserveSatoshis: Satoshi,
                          htlcMinimumMsat: MilliSatoshi,
                          minimumDepth: Long,

eclair-core/src/test/scala/fr/acinq/eclair/CoinUtilsSpec.scala

@@ -18,10 +18,21 @@ package fr.acinq.eclair
 
 import fr.acinq.bitcoin.{Btc, MilliBtc, Satoshi}
 import org.scalatest.FunSuite
+import scodec.bits._
 
 
 class CoinUtilsSpec  extends FunSuite {
 
+  test("use unsigned comparison when comparing millisatoshis to uint64") {
+    assert(MilliSatoshi(123).compareUnsigned(UInt64(123)) == 0)
+    assert(MilliSatoshi(1234).compareUnsigned(UInt64(123)) > 0)
+    assert(MilliSatoshi(123).compareUnsigned(UInt64(1234)) < 0)
+    assert(MilliSatoshi(123).compareUnsigned(UInt64(hex"ffffffffffffffff")) < 0)
+    assert(MilliSatoshi(-123).compareUnsigned(UInt64(hex"ffffffffffffffff")) < 0)
+    assert(MilliSatoshi(Long.MaxValue).compareUnsigned(UInt64(hex"7fffffffffffffff")) == 0) // 7fffffffffffffff == Long.MaxValue
+    assert(MilliSatoshi(Long.MaxValue).compareUnsigned(UInt64(hex"8000000000000000")) < 0)  // 8000000000000000 == Long.MaxValue + 1
+  }
+
   test("Convert string amount to the correct BtcAmount") {
     val am_btc: MilliSatoshi = CoinUtils.convertStringAmountToMsat("1", BtcUnit.code)
     assert(am_btc == MilliSatoshi(100000000000L))

eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala

@@ -205,7 +205,7 @@ class NormalStateSpec extends TestkitBaseClass with StateTestsHelperMethods {
     val initialState = bob.stateData.asInstanceOf[DATA_NORMAL]
     val add = CMD_ADD_HTLC(MilliSatoshi(151000000), randomBytes32, 400144, TestConstants.emptyOnionPacket, upstream = Left(UUID.randomUUID()))
     sender.send(bob, add)
-    val error = HtlcValueTooHighInFlight(channelId(bob), maximum = 150000000, actual = 151000000)
+    val error = HtlcValueTooHighInFlight(channelId(bob), maximum = 150000000, actual = MilliSatoshi(151000000))
     sender.expectMsg(Failure(AddHtlcFailed(channelId(bob), add.paymentHash, error, Local(add.upstream.left.get, Some(sender.ref)), Some(initialState.channelUpdate), Some(add))))
     bob2alice.expectNoMsg(200 millis)
   }
@@ -381,7 +381,7 @@ class NormalStateSpec extends TestkitBaseClass with StateTestsHelperMethods {
     val tx = alice.stateData.asInstanceOf[DATA_NORMAL].commitments.localCommit.publishableTxs.commitTx.tx
     alice2bob.forward(alice, UpdateAddHtlc(ByteVector32.Zeroes, 0, MilliSatoshi(151000000), randomBytes32, 400144, TestConstants.emptyOnionPacket))
     val error = alice2bob.expectMsgType[Error]
-    assert(new String(error.data.toArray) === HtlcValueTooHighInFlight(channelId(alice), maximum = 150000000, actual = 151000000).getMessage)
+    assert(new String(error.data.toArray) === HtlcValueTooHighInFlight(channelId(alice), maximum = 150000000, actual = MilliSatoshi(151000000)).getMessage)
     awaitCond(alice.stateName == CLOSING)
     // channel should be advertised as down
     assert(channelUpdateListener.expectMsgType[LocalChannelDown].channelId === alice.stateData.asInstanceOf[DATA_CLOSING].channelId)

eclair-core/src/test/scala/fr/acinq/eclair/payment/RelayerSpec.scala

@@ -101,15 +101,15 @@ class RelayerSpec extends TestkitBaseClass {
 
     // channel returns an error
     val origin = Relayed(channelId_ab, originHtlcId = 42, amountIn = MilliSatoshi(1100000), amountOut = MilliSatoshi(1000000))
-    sender.send(relayer, Status.Failure(AddHtlcFailed(channelId_bc_1, paymentHash, HtlcValueTooHighInFlight(channelId_bc_1, UInt64(1000000000L), UInt64(1516977616L)), origin, Some(channelUpdate_bc_1), originalCommand = Some(fwd1.message))))
+    sender.send(relayer, Status.Failure(AddHtlcFailed(channelId_bc_1, paymentHash, HtlcValueTooHighInFlight(channelId_bc_1, UInt64(1000000000L), MilliSatoshi(1516977616L)), origin, Some(channelUpdate_bc_1), originalCommand = Some(fwd1.message))))
 
     // second try
     val fwd2 = register.expectMsgType[Register.ForwardShortId[CMD_ADD_HTLC]]
     assert(fwd2.shortChannelId === channelUpdate_bc.shortChannelId)
     assert(fwd2.message.upstream === Right(add_ab))
 
     // failure again
-    sender.send(relayer, Status.Failure(AddHtlcFailed(channelId_bc, paymentHash, HtlcValueTooHighInFlight(channelId_bc, UInt64(1000000000L), UInt64(1516977616L)), origin, Some(channelUpdate_bc), originalCommand = Some(fwd2.message))))
+    sender.send(relayer, Status.Failure(AddHtlcFailed(channelId_bc, paymentHash, HtlcValueTooHighInFlight(channelId_bc, UInt64(1000000000L), MilliSatoshi(1516977616L)), origin, Some(channelUpdate_bc), originalCommand = Some(fwd2.message))))
 
     // the relayer should give up
     val fwdFail = register.expectMsgType[Register.Forward[CMD_FAIL_HTLC]]