diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/MilliSatoshi.scala b/eclair-core/src/main/scala/fr/acinq/eclair/MilliSatoshi.scala index ff97d45565..1691125192 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/MilliSatoshi.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/MilliSatoshi.scala @@ -16,6 +16,7 @@ package fr.acinq.eclair +import com.google.common.primitives.UnsignedLongs import fr.acinq.bitcoin.{Btc, BtcAmount, MilliBtc, Satoshi, btc2satoshi, millibtc2satoshi} /** @@ -40,6 +41,7 @@ case class MilliSatoshi(private val underlying: Long) extends Ordered[MilliSatos override def compare(other: MilliSatoshi): Int = underlying.compareTo(other.underlying) // Since BtcAmount is a sealed trait that MilliSatoshi cannot extend, we need to redefine comparison operators. def compare(other: BtcAmount): Int = compare(other.toMilliSatoshi) + def compareUnsigned(other: UInt64): Int = UnsignedLongs.compare(underlying, other.toBigInt.longValue()) def <=(other: BtcAmount): Boolean = compare(other) <= 0 def >=(other: BtcAmount): Boolean = compare(other) >= 0 def <(other: BtcAmount): Boolean = compare(other) < 0 diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelExceptions.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelExceptions.scala index c177c92ad2..af4546ba9e 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelExceptions.scala +++ b/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: CltvExpiry, actual: CltvExpiry, blockCount: Long) extends ChannelException(channelId, s"expiry too small: minimum=$minimum actual=$actual blockCount=$blockCount") case class ExpiryTooBig (override val channelId: ByteVector32, maximum: CltvExpiry, actual: CltvExpiry, 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") diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelTypes.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelTypes.scala index 6facbf7e52..04203ea2bc 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelTypes.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelTypes.scala @@ -192,7 +192,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: CltvExpiryDelta, @@ -204,7 +204,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: CltvExpiryDelta, diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala index f94842645a..d5dea687cb 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Commitments.scala +++ b/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} import fr.acinq.eclair.blockchain.fee.{FeeEstimator, FeeTargets} @@ -145,8 +146,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)) } @@ -180,8 +182,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) } diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/package.scala b/eclair-core/src/main/scala/fr/acinq/eclair/package.scala index 2349b2984c..003f7764fb 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/package.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/package.scala @@ -22,7 +22,6 @@ 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} diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/wire/LightningMessageTypes.scala b/eclair-core/src/main/scala/fr/acinq/eclair/wire/LightningMessageTypes.scala index e7c583b1b8..e2aa4a90d4 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/wire/LightningMessageTypes.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/wire/LightningMessageTypes.scala @@ -72,7 +72,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, @@ -88,7 +88,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, diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/CoinUtilsSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/CoinUtilsSpec.scala index 8e051c2c93..809210524f 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/CoinUtilsSpec.scala +++ b/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)) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala index c38d884811..c66daeedb7 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala @@ -204,7 +204,7 @@ class NormalStateSpec extends TestkitBaseClass with StateTestsHelperMethods { val initialState = bob.stateData.asInstanceOf[DATA_NORMAL] val add = CMD_ADD_HTLC(151000000 msat, randomBytes32, CltvExpiryDelta(144).toCltvExpiry, 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 = 151000000 msat) 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) } @@ -380,7 +380,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, 151000000 msat, randomBytes32, CltvExpiryDelta(144).toCltvExpiry, 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 = 151000000 msat).getMessage) awaitCond(alice.stateName == CLOSING) // channel should be advertised as down assert(channelUpdateListener.expectMsgType[LocalChannelDown].channelId === alice.stateData.asInstanceOf[DATA_CLOSING].channelId) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/payment/RelayerSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/payment/RelayerSpec.scala index 733c69d3d8..534a6101b2 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/payment/RelayerSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/payment/RelayerSpec.scala @@ -102,7 +102,7 @@ class RelayerSpec extends TestkitBaseClass { // channel returns an error val origin = Relayed(channelId_ab, originHtlcId = 42, amountIn = 1100000 msat, amountOut = 1000000 msat) - 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), 1516977616L msat), origin, Some(channelUpdate_bc_1), originalCommand = Some(fwd1.message)))) // second try val fwd2 = register.expectMsgType[Register.ForwardShortId[CMD_ADD_HTLC]] @@ -110,7 +110,7 @@ class RelayerSpec extends TestkitBaseClass { 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), 1516977616L msat), origin, Some(channelUpdate_bc), originalCommand = Some(fwd2.message)))) // the relayer should give up val fwdFail = register.expectMsgType[Register.Forward[CMD_FAIL_HTLC]]