From e8a74899f37512a0c2b977e7861d1c4af8984213 Mon Sep 17 00:00:00 2001 From: pm47 Date: Mon, 13 Feb 2017 12:04:06 +0100 Subject: [PATCH] now take fees into account when accepting htlcs --- .../fr/acinq/eclair/channel/Commitments.scala | 58 ++++++++++--------- .../eclair/transactions/Transactions.scala | 8 +-- .../channel/states/e/NormalStateSpec.scala | 16 ++--- 3 files changed, 43 insertions(+), 39 deletions(-) diff --git a/eclair-node/src/main/scala/fr/acinq/eclair/channel/Commitments.scala b/eclair-node/src/main/scala/fr/acinq/eclair/channel/Commitments.scala index 692f283e0..7ed14f128 100644 --- a/eclair-node/src/main/scala/fr/acinq/eclair/channel/Commitments.scala +++ b/eclair-node/src/main/scala/fr/acinq/eclair/channel/Commitments.scala @@ -71,28 +71,30 @@ object Commitments extends Logging { throw new RuntimeException(s"counterparty requires a minimum htlc value of ${commitments.remoteParams.htlcMinimumMsat} msat") } - // let's compute the current commitment *as seen by them* - val reduced = CommitmentSpec.reduce(commitments.remoteCommit.spec, commitments.remoteChanges.acked, commitments.localChanges.proposed) + // let's compute the current commitment *as seen by them* with this change taken into account + val add = UpdateAddHtlc(commitments.channelId, commitments.localNextHtlcId, cmd.amountMsat, cmd.expiry, cmd.paymentHash, cmd.onion) + val commitments1 = addLocalProposal(commitments, add).copy(localNextHtlcId = commitments.localNextHtlcId + 1) + val reduced = CommitmentSpec.reduce(commitments1.remoteCommit.spec, commitments1.remoteChanges.acked, commitments1.localChanges.proposed) - val htlcValueInFlight = reduced.htlcs.map(_.add.amountMsat).sum + cmd.amountMsat - if (htlcValueInFlight > commitments.remoteParams.maxHtlcValueInFlightMsat) { - throw new RuntimeException(s"reached counterparty's in-flight htlcs value limit: value=$htlcValueInFlight max=${commitments.remoteParams.maxHtlcValueInFlightMsat}") + val htlcValueInFlight = reduced.htlcs.map(_.add.amountMsat).sum + if (htlcValueInFlight > commitments1.remoteParams.maxHtlcValueInFlightMsat) { + throw new RuntimeException(s"reached counterparty's in-flight htlcs value limit: value=$htlcValueInFlight max=${commitments1.remoteParams.maxHtlcValueInFlightMsat}") } // the HTLC we are about to create is outgoing, but from their point of view it is incoming - val acceptedHtlcs = reduced.htlcs.count(_.direction == IN) + 1 - if (acceptedHtlcs > commitments.remoteParams.maxAcceptedHtlcs) { - throw new RuntimeException(s"reached counterparty's max accepted htlc count limit: value=$acceptedHtlcs max=${commitments.remoteParams.maxAcceptedHtlcs}") + val acceptedHtlcs = reduced.htlcs.count(_.direction == IN) + if (acceptedHtlcs > commitments1.remoteParams.maxAcceptedHtlcs) { + throw new RuntimeException(s"reached counterparty's max accepted htlc count limit: value=$acceptedHtlcs max=${commitments1.remoteParams.maxAcceptedHtlcs}") } - // a node cannot spend pending incoming htlcs, and need to keep funds above the reserve required by the counterparty - val available = reduced.toRemoteMsat - commitments.remoteParams.channelReserveSatoshis * 1000 - if (cmd.amountMsat > available) { - throw new RuntimeException(s"insufficient funds: to-local=${reduced.toRemoteMsat / 1000} reserve=${commitments.remoteParams.channelReserveSatoshis} available=${available / 1000}") + // a node cannot spend pending incoming htlcs, and need to keep funds above the reserve required by the counterparty, after paying the fee + // we look from remote's point of view, so if local is funder remote doesn't pay the fees + val fees = if (commitments1.localParams.isFunder) 0 else Transactions.commitTxFee(Satoshi(commitments1.remoteParams.dustLimitSatoshis), reduced).amount + val missing = reduced.toRemoteMsat / 1000 - commitments1.remoteParams.channelReserveSatoshis - fees + if (missing < 0) { + throw new RuntimeException(s"insufficient funds: missing=${-1 * missing} reserve=${commitments1.remoteParams.channelReserveSatoshis} fees=$fees") } - val add = UpdateAddHtlc(commitments.channelId, commitments.localNextHtlcId, cmd.amountMsat, cmd.expiry, cmd.paymentHash, cmd.onion) - val commitments1 = addLocalProposal(commitments, add).copy(localNextHtlcId = commitments.localNextHtlcId + 1) (commitments1, add) } @@ -113,26 +115,28 @@ object Commitments extends Logging { throw new RuntimeException(s"htlc value too small: min=${commitments.localParams.htlcMinimumMsat}") } - // let's compute the current commitment *as seen by us* including all pending changes - val reduced = CommitmentSpec.reduce(commitments.localCommit.spec, commitments.localChanges.acked, commitments.remoteChanges.proposed) + // let's compute the current commitment *as seen by us* including this change + val commitments1 = addRemoteProposal(commitments, add).copy(remoteNextHtlcId = commitments.remoteNextHtlcId + 1) + val reduced = CommitmentSpec.reduce(commitments1.localCommit.spec, commitments1.localChanges.acked, commitments1.remoteChanges.proposed) - val htlcValueInFlight = reduced.htlcs.map(_.add.amountMsat).sum + add.amountMsat - if (htlcValueInFlight > commitments.localParams.maxHtlcValueInFlightMsat) { - throw new RuntimeException(s"in-flight htlcs hold too much value: value=$htlcValueInFlight max=${commitments.localParams.maxHtlcValueInFlightMsat}") + val htlcValueInFlight = reduced.htlcs.map(_.add.amountMsat).sum + if (htlcValueInFlight > commitments1.localParams.maxHtlcValueInFlightMsat) { + throw new RuntimeException(s"in-flight htlcs hold too much value: value=$htlcValueInFlight max=${commitments1.localParams.maxHtlcValueInFlightMsat}") } - val acceptedHtlcs = reduced.htlcs.count(_.direction == IN) + 1 - if (acceptedHtlcs > commitments.localParams.maxAcceptedHtlcs) { - throw new RuntimeException(s"too many accepted htlcs: value=$acceptedHtlcs max=${commitments.localParams.maxAcceptedHtlcs}") + val acceptedHtlcs = reduced.htlcs.count(_.direction == IN) + if (acceptedHtlcs > commitments1.localParams.maxAcceptedHtlcs) { + throw new RuntimeException(s"too many accepted htlcs: value=$acceptedHtlcs max=${commitments1.localParams.maxAcceptedHtlcs}") } - // a node cannot spend pending incoming htlcs, and need to keep funds above the reserve required by the counterparty - val available = reduced.toRemoteMsat - commitments.localParams.channelReserveSatoshis * 1000 - if (add.amountMsat > available) { - throw new RuntimeException(s"insufficient funds: to-remote=${reduced.toRemoteMsat / 1000} reserve=${commitments.localParams.channelReserveSatoshis} available=${available / 1000}") + // a node cannot spend pending incoming htlcs, and need to keep funds above the reserve required by the counterparty, after paying the fee + val fees = if (commitments1.localParams.isFunder) 0 else Transactions.commitTxFee(Satoshi(commitments1.remoteParams.dustLimitSatoshis), reduced).amount + val missing = reduced.toRemoteMsat / 1000 - commitments1.localParams.channelReserveSatoshis - fees + if (missing < 0) { + throw new RuntimeException(s"insufficient funds: missing=${-1 * missing} reserve=${commitments1.localParams.channelReserveSatoshis} fees=$fees") } - addRemoteProposal(commitments, add).copy(remoteNextHtlcId = commitments.remoteNextHtlcId + 1) + commitments1 } def sendFulfill(commitments: Commitments, cmd: CMD_FULFILL_HTLC): (Commitments, UpdateFulfillHtlc) = { diff --git a/eclair-node/src/main/scala/fr/acinq/eclair/transactions/Transactions.scala b/eclair-node/src/main/scala/fr/acinq/eclair/transactions/Transactions.scala index 74ee7cf31..0fe4f5bf2 100644 --- a/eclair-node/src/main/scala/fr/acinq/eclair/transactions/Transactions.scala +++ b/eclair-node/src/main/scala/fr/acinq/eclair/transactions/Transactions.scala @@ -140,9 +140,9 @@ object Transactions { val commitFee = commitTxFee(localDustLimit, spec) val (toLocalAmount: Satoshi, toRemoteAmount: Satoshi) = (MilliSatoshi(spec.toLocalMsat), MilliSatoshi(spec.toRemoteMsat)) match { - case (local, remote) if localIsFunder && local.compare(commitFee) < 0 => (Satoshi(0), millisatoshi2satoshi(remote)) //TODO: check: can't pay fees! => pay whatever you can + case (local, remote) if localIsFunder && local.compare(commitFee) < 0 => (Satoshi(0), millisatoshi2satoshi(remote)) //TODO: check: can't pay fees! (will happen when UpdateFee is implemented) case (local, remote) if localIsFunder && local.compare(commitFee) >= 0 => (local - commitFee, millisatoshi2satoshi(remote)) - case (local, remote) if !localIsFunder && remote.compare(commitFee) < 0 => (millisatoshi2satoshi(local), Satoshi(0)) //TODO: check: can't pay fees! => pay whatever you can + case (local, remote) if !localIsFunder && remote.compare(commitFee) < 0 => (millisatoshi2satoshi(local), Satoshi(0)) //TODO: check: can't pay fees! (will happen when UpdateFee is implemented) case (local, remote) if !localIsFunder && remote.compare(commitFee) >= 0 => (millisatoshi2satoshi(local), remote - commitFee) } val toLocalDelayedOutput_opt = if (toLocalAmount.compare(localDustLimit) >= 0) Some(TxOut(toLocalAmount, pay2wsh(toLocalDelayed(localRevocationPubkey, toLocalDelay, localDelayedPubkey)))) else None @@ -294,9 +294,9 @@ object Transactions { require(spec.htlcs.size == 0, "there shouldn't be any pending htlcs") val (toLocalAmount: Satoshi, toRemoteAmount: Satoshi) = (MilliSatoshi(spec.toLocalMsat), MilliSatoshi(spec.toRemoteMsat)) match { - case (local, remote) if localIsFunder && local.compare(closingFee) <= 0 => ??? //TODO: can't pay fees! + case (local, remote) if localIsFunder && local.compare(closingFee) <= 0 => ??? //TODO: check: can't pay fees! (will happen when UpdateFee is implemented) case (local, remote) if localIsFunder && local.compare(closingFee) > 0 => (local - closingFee, millisatoshi2satoshi(remote)) - case (local, remote) if !localIsFunder && remote.compare(closingFee) <= 0 => ??? //TODO: can't pay fees! + case (local, remote) if !localIsFunder && remote.compare(closingFee) <= 0 => ??? //TODO: check: can't pay fees! (will happen when UpdateFee is implemented) case (local, remote) if !localIsFunder && remote.compare(closingFee) > 0 => (millisatoshi2satoshi(local), remote - closingFee) } diff --git a/eclair-node/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala b/eclair-node/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala index cd5620d5e..74630b11a 100644 --- a/eclair-node/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala +++ b/eclair-node/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala @@ -112,7 +112,7 @@ class NormalStateSpec extends TestkitBaseClass with StateTestsHelperMethods { within(30 seconds) { val sender = TestProbe() sender.send(alice, CMD_ADD_HTLC(Int.MaxValue, "11" * 32, 400144)) - sender.expectMsg("insufficient funds: to-local=800000 reserve=20000 available=780000") + sender.expectMsg("insufficient funds: missing=1367483 reserve=20000 fees=0") alice2bob.expectNoMsg(200 millis) } } @@ -130,7 +130,7 @@ class NormalStateSpec extends TestkitBaseClass with StateTestsHelperMethods { sender.expectMsg("ok") alice2bob.expectMsgType[UpdateAddHtlc] sender.send(alice, CMD_ADD_HTLC(100000, "33" * 32, 400144)) - sender.expectMsg("insufficient funds: to-local=20000 reserve=20000 available=0") + sender.expectMsg("insufficient funds: missing=100 reserve=20000 fees=0") alice2bob.expectNoMsg(200 millis) } } @@ -145,7 +145,7 @@ class NormalStateSpec extends TestkitBaseClass with StateTestsHelperMethods { sender.expectMsg("ok") alice2bob.expectMsgType[UpdateAddHtlc] sender.send(alice, CMD_ADD_HTLC(500000000, "33" * 32, 400144)) - sender.expectMsg("insufficient funds: to-local=200000 reserve=20000 available=180000") + sender.expectMsg("insufficient funds: missing=320000 reserve=20000 fees=0") alice2bob.expectNoMsg(200 millis) } } @@ -246,7 +246,7 @@ class NormalStateSpec extends TestkitBaseClass with StateTestsHelperMethods { val htlc = UpdateAddHtlc(0, 0, Long.MaxValue, 400144, BinaryData("00112233445566778899aabbccddeeff"), "") alice2bob.forward(bob, htlc) val error = bob2alice.expectMsgType[Error] - assert(new String(error.data) === "insufficient funds: to-remote=800000 reserve=20000 available=780000") + assert(new String(error.data) === "insufficient funds: missing=9223372036083735 reserve=20000 fees=8960") awaitCond(bob.stateName == CLOSING) bob2blockchain.expectMsg(PublishAsap(tx)) bob2blockchain.expectMsgType[WatchConfirmed] @@ -256,12 +256,12 @@ class NormalStateSpec extends TestkitBaseClass with StateTestsHelperMethods { test("recv UpdateAddHtlc (insufficient funds w/ pending htlcs 1/2)") { case (_, bob, alice2bob, bob2alice, _, bob2blockchain, _) => within(30 seconds) { val tx = bob.stateData.asInstanceOf[DATA_NORMAL].commitments.localCommit.publishableTxs.commitTx.tx - alice2bob.forward(bob, UpdateAddHtlc(0, 0, 500000000, 400144, "11" * 32, "")) + alice2bob.forward(bob, UpdateAddHtlc(0, 0, 400000000, 400144, "11" * 32, "")) alice2bob.forward(bob, UpdateAddHtlc(0, 1, 200000000, 400144, "22" * 32, "")) - alice2bob.forward(bob, UpdateAddHtlc(0, 2, 80000000, 400144, "33" * 32, "")) + alice2bob.forward(bob, UpdateAddHtlc(0, 2, 167600000, 400144, "33" * 32, "")) alice2bob.forward(bob, UpdateAddHtlc(0, 3, 10000000, 400144, "44" * 32, "")) val error = bob2alice.expectMsgType[Error] - assert(new String(error.data) === "insufficient funds: to-remote=20000 reserve=20000 available=0") + assert(new String(error.data) === "insufficient funds: missing=11720 reserve=20000 fees=14120") awaitCond(bob.stateName == CLOSING) bob2blockchain.expectMsg(PublishAsap(tx)) bob2blockchain.expectMsgType[WatchConfirmed] @@ -275,7 +275,7 @@ class NormalStateSpec extends TestkitBaseClass with StateTestsHelperMethods { alice2bob.forward(bob, UpdateAddHtlc(0, 1, 300000000, 400144, "22" * 32, "")) alice2bob.forward(bob, UpdateAddHtlc(0, 2, 500000000, 400144, "33" * 32, "")) val error = bob2alice.expectMsgType[Error] - assert(new String(error.data) === "insufficient funds: to-remote=200000 reserve=20000 available=180000") + assert(new String(error.data) === "insufficient funds: missing=332400 reserve=20000 fees=12400") awaitCond(bob.stateName == CLOSING) bob2blockchain.expectMsg(PublishAsap(tx)) bob2blockchain.expectMsgType[WatchConfirmed]