From cc3395a5bbdbe1b549f35dd522e57754c462cffc Mon Sep 17 00:00:00 2001 From: Pierre-Marie Padiou Date: Mon, 18 Mar 2019 14:39:29 +0100 Subject: [PATCH 1/3] Better logic for sending `channel_updates` (#888) * don't spam with channel_updates at startup Previous logic was very simple but naive: - every time a channel_update changed we would send it out - we would always make a new channel_update with the disabled flag set at startup. In case our node was simply restarted, this resulted in us re-sending a channel_update with the disabled flag set, then a second one with the disabled flag unset a few seconds later, for each public channel. On top of that, this opened way to a bug: if reconnection is very fast, then the two successive channel_update will have the same timestamp, causing the router to not send the second one, which means that the channel would be considered disabled by the network, and excluded from payments. The new logic is as follows: - when we do NORMAL->NORMAL or NORMAL->OFFLINE or OFFLINE->NORMAL, we send out the new channel_update if it has changed - in all other case (e.g. WAIT_FOR_INIT_INTERNAL->OFFLINE) we do nothing As a side effect, if we were connected to a peer, then we shut down eclair, then the peer goes down, then we restart eclair: we will make a new channel_update with the disabled flag set but we won't broadcast it. If someone tries to make a payment to that node, we will return the new channel_update with disabled flag set (and maybe the payer will then broadcast that channel_update). So even in that corner case we are good. * quick reconnection: bump channel_update timestamp In case of a disconnection-reconnection, we first generate a channel_update with disabled bit set, then after we reconnect we generate a second channel_update with disabled bit not set. If this happens very quickly, then both channel_updates will have the same timestamp, and the second one will get ignored by the network. A simple fix is to bump the second timestamp in this case. * set channel_update refresh timer at reconnection We only care about this timer when connected anyway. We also cancel it when disconnecting. This has several advantages: - having a static task resulted in unnecessary refresh if the channel got disconnected/reconnected in between 2 weeks - better repartition of the channel_update refresh over time because at startup all channels were generated at the same time causing all refresh tasks to be synchronized - less overhead for the scheduler (because we cancel refresh task for offline channels (minor, but still) --- .../fr/acinq/eclair/channel/Channel.scala | 32 ++++++++++------ .../channel/states/e/OfflineStateSpec.scala | 38 +++++++++++++++++++ pom.xml | 1 + 3 files changed, 59 insertions(+), 12 deletions(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Channel.scala b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Channel.scala index 671be2385..ed2050a68 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/channel/Channel.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/channel/Channel.scala @@ -97,8 +97,6 @@ class Channel(val nodeParams: NodeParams, wallet: EclairWallet, remoteNodeId: Pu context.system.eventStream.subscribe(self, classOf[CurrentBlockCount]) // this will be used to make sure the current commitment fee is up-to-date context.system.eventStream.subscribe(self, classOf[CurrentFeerates]) - // we need to periodically re-send channel updates, otherwise channel will be considered stale and get pruned by network - setTimer(TickRefreshChannelUpdate.toString, TickRefreshChannelUpdate, timeout = REFRESH_CHANNEL_UPDATE_INTERVAL, repeat = true) /* 8888888 888b 888 8888888 88888888888 @@ -201,9 +199,7 @@ class Channel(val nodeParams: NodeParams, wallet: EclairWallet, remoteNodeId: Pu blockchain ! WatchSpent(self, data.commitments.commitInput.outPoint.txid, data.commitments.commitInput.outPoint.index.toInt, data.commitments.commitInput.txOut.publicKeyScript, BITCOIN_FUNDING_SPENT) blockchain ! WatchLost(self, data.commitments.commitInput.outPoint.txid, nodeParams.minDepthBlocks, BITCOIN_FUNDING_LOST) context.system.eventStream.publish(ShortChannelIdAssigned(self, normal.channelId, normal.channelUpdate.shortChannelId)) - // we rebuild a channel_update for two reasons: - // - we want to reload values from configuration - // - if eclair was previously killed, it might not have had time to publish a channel_update with enable=false + // we rebuild a new channel_update with values from the configuration because they may have changed while eclair was down val channelUpdate = Announcements.makeChannelUpdate(nodeParams.chainHash, nodeParams.privateKey, remoteNodeId, normal.channelUpdate.shortChannelId, nodeParams.expiryDeltaBlocks, normal.commitments.remoteParams.htlcMinimumMsat, normal.channelUpdate.feeBaseMsat, normal.channelUpdate.feeProportionalMillionths, normal.commitments.localCommit.spec.totalFunds, enable = false) @@ -879,6 +875,8 @@ class Channel(val nodeParams: NodeParams, wallet: EclairWallet, remoteNodeId: Pu d.commitments.localChanges.proposed.collect { case add: UpdateAddHtlc => relayer ! Status.Failure(AddHtlcFailed(d.channelId, add.paymentHash, ChannelUnavailable(d.channelId), d.commitments.originChannels(add.id), Some(channelUpdate), None)) } + // disable the channel_update refresh timer + cancelTimer(TickRefreshChannelUpdate.toString) goto(OFFLINE) using d.copy(channelUpdate = channelUpdate) case Event(e: Error, d: DATA_NORMAL) => handleRemoteError(e, d) @@ -1453,7 +1451,13 @@ class Channel(val nodeParams: NodeParams, wallet: EclairWallet, remoteNodeId: Pu } } // re-enable the channel - val channelUpdate = Announcements.makeChannelUpdate(nodeParams.chainHash, nodeParams.privateKey, remoteNodeId, d.shortChannelId, nodeParams.expiryDeltaBlocks, d.commitments.remoteParams.htlcMinimumMsat, d.channelUpdate.feeBaseMsat, d.channelUpdate.feeProportionalMillionths, d.commitments.localCommit.spec.totalFunds, enable = Helpers.aboveReserve(d.commitments)) + val timestamp = Platform.currentTime / 1000 match { + case ts if ts == d.channelUpdate.timestamp => ts + 1 // corner case: in case of quick reconnection, we bump the timestamp of the new channel_update, otherwise it will get ignored by the network + case ts => ts + } + val channelUpdate = Announcements.makeChannelUpdate(nodeParams.chainHash, nodeParams.privateKey, remoteNodeId, d.shortChannelId, nodeParams.expiryDeltaBlocks, d.commitments.remoteParams.htlcMinimumMsat, d.channelUpdate.feeBaseMsat, d.channelUpdate.feeProportionalMillionths, d.commitments.localCommit.spec.totalFunds, enable = Helpers.aboveReserve(d.commitments), timestamp = timestamp) + // we will refresh need to periodically re-send channel updates, otherwise channel will be considered stale and get pruned by network + setTimer(TickRefreshChannelUpdate.toString, TickRefreshChannelUpdate, timeout = REFRESH_CHANNEL_UPDATE_INTERVAL, repeat = true) goto(NORMAL) using d.copy(commitments = commitments1, channelUpdate = channelUpdate) } @@ -1559,7 +1563,7 @@ class Channel(val nodeParams: NodeParams, wallet: EclairWallet, remoteNodeId: Pu // we only care about this event in NORMAL and SHUTDOWN state, and we never unregister to the event stream case Event(CurrentFeerates(_), _) => stay - // we only care about this event in NORMAL state, and the scheduler is never disabled + // we only care about this event in NORMAL state, and the scheduler is not cancelled in some cases (e.g. NORMAL->CLOSING) case Event(TickRefreshChannelUpdate, _) => stay // we receive this when we send command to ourselves @@ -1604,14 +1608,18 @@ class Channel(val nodeParams: NodeParams, wallet: EclairWallet, remoteNodeId: Pu case _ => () } - (stateData, nextStateData) match { - case (d1: DATA_NORMAL, d2: DATA_NORMAL) if d1.channelUpdate == d2.channelUpdate && d1.channelAnnouncement == d2.channelAnnouncement => + (state, nextState, stateData, nextStateData) match { + // ORDER MATTERS! + case (_, _, d1: DATA_NORMAL, d2: DATA_NORMAL) if d1.channelUpdate == d2.channelUpdate && d1.channelAnnouncement == d2.channelAnnouncement => // don't do anything if neither the channel_update nor the channel_announcement didn't change () - case (_, normal: DATA_NORMAL) => - // whenever we go to a state with NORMAL data (can be OFFLINE or NORMAL), we send out the new channel_update (most of the time it will just be to enable/disable the channel) + case (WAIT_FOR_FUNDING_LOCKED | NORMAL | SYNCING, NORMAL | OFFLINE, _, normal: DATA_NORMAL) => + // when we do WAIT_FOR_FUNDING_LOCKED->NORMAL or NORMAL->NORMAL or SYNCING->NORMAL or NORMAL->OFFLINE, we send out the new channel_update (most of the time it will just be to enable/disable the channel) context.system.eventStream.publish(LocalChannelUpdate(self, normal.commitments.channelId, normal.shortChannelId, normal.commitments.remoteParams.nodeId, normal.channelAnnouncement, normal.channelUpdate, normal.commitments)) - case (normal: DATA_NORMAL, _) => + case (_, _, _: DATA_NORMAL, _: DATA_NORMAL) => + // in any other case (e.g. WAIT_FOR_INIT_INTERNAL->OFFLINE) we do nothing + () + case (_, _, normal: DATA_NORMAL, _) => // when we finally leave the NORMAL state (or OFFLINE with NORMAL data) to go to SHUTDOWN/NEGOTIATING/CLOSING/ERR*, we advertise the fact that channel can't be used for payments anymore // if the channel is private we don't really need to tell the counterparty because it is already aware that the channel is being closed context.system.eventStream.publish(LocalChannelDown(self, normal.commitments.channelId, normal.shortChannelId, normal.commitments.remoteParams.nodeId)) diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/OfflineStateSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/OfflineStateSpec.scala index bfd84d675..e1f7842d4 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/OfflineStateSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/OfflineStateSpec.scala @@ -366,4 +366,42 @@ class OfflineStateSpec extends TestkitBaseClass with StateTestsHelperMethods { channelUpdateListener.expectNoMsg(300 millis) } + test("bump timestamp in case of quick reconnection") { f => + import f._ + val sender = TestProbe() + + // we simulate a disconnection + sender.send(alice, INPUT_DISCONNECTED) + sender.send(bob, INPUT_DISCONNECTED) + awaitCond(alice.stateName == OFFLINE) + awaitCond(bob.stateName == OFFLINE) + + // alice and bob announce that their channel is OFFLINE + val channelUpdate_alice_disabled = channelUpdateListener.expectMsgType[LocalChannelUpdate].channelUpdate + val channelUpdate_bob_disabled = channelUpdateListener.expectMsgType[LocalChannelUpdate].channelUpdate + assert(Announcements.isEnabled(channelUpdate_alice_disabled.channelFlags) == false) + assert(Announcements.isEnabled(channelUpdate_bob_disabled.channelFlags) == false) + + // we immediately reconnect them + sender.send(alice, INPUT_RECONNECTED(alice2bob.ref, aliceInit, bobInit)) + sender.send(bob, INPUT_RECONNECTED(bob2alice.ref, bobInit, aliceInit)) + + // peers exchange channel_reestablish messages + alice2bob.expectMsgType[ChannelReestablish] + bob2alice.expectMsgType[ChannelReestablish] + alice2bob.forward(bob) + bob2alice.forward(alice) + + // both nodes reach NORMAL state, and broadcast a new channel_update + val channelUpdate_alice_enabled = channelUpdateListener.expectMsgType[LocalChannelUpdate].channelUpdate + val channelUpdate_bob_enabled = channelUpdateListener.expectMsgType[LocalChannelUpdate].channelUpdate + assert(Announcements.isEnabled(channelUpdate_alice_enabled.channelFlags) == true) + assert(Announcements.isEnabled(channelUpdate_bob_enabled.channelFlags) == true) + + // let's check that the two successive channel_update have a different timestamp + assert(channelUpdate_alice_enabled.timestamp > channelUpdate_alice_disabled.timestamp) + assert(channelUpdate_bob_enabled.timestamp > channelUpdate_bob_disabled.timestamp) + + } + } diff --git a/pom.xml b/pom.xml index 23915e218..ccf8aec34 100644 --- a/pom.xml +++ b/pom.xml @@ -216,6 +216,7 @@ ${project.build.directory} + -Xmx1024m From 4aa7a1ca9f2b0e9a2230ce2a9240299fa7520046 Mon Sep 17 00:00:00 2001 From: Fabrice Drouin Date: Tue, 19 Mar 2019 14:57:03 +0100 Subject: [PATCH 2/3] Upgrade to bitcoin 0.17.1 (#826) Bitcoin Core 0.18 is about to enter RC cycle and should be release soon (initial target was April). It is not compatible with 0.16 (some of the RPC calls that we use have been removed. They're still available in 0.17 but tagged as deprecated). With this PR, eclair will be compatible with 0.17 and the upcoming 0.18, but not with 0.16 any more so it will be a breaking change for some of our users. Supporting the last 2 versions is the right option and we should be ready before 0.18 is actually released (its initial target was April). --- eclair-core/pom.xml | 18 +++--- .../main/scala/fr/acinq/eclair/Setup.scala | 4 +- .../bitcoind/BitcoinCoreWallet.scala | 14 +++-- .../test/resources/integration/bitcoin.conf | 4 +- .../bitcoind/BitcoinCoreWalletSpec.scala | 58 ++++++++++++++++--- .../blockchain/bitcoind/BitcoindService.scala | 2 +- .../bitcoind/ExtendedBitcoinClientSpec.scala | 13 ++++- .../fee/BitcoinCoreFeeProviderSpec.scala | 9 ++- travis/builddeps.sh | 25 -------- 9 files changed, 91 insertions(+), 56 deletions(-) delete mode 100755 travis/builddeps.sh diff --git a/eclair-core/pom.xml b/eclair-core/pom.xml index 679f4a591..22d61f6c7 100644 --- a/eclair-core/pom.xml +++ b/eclair-core/pom.xml @@ -79,10 +79,10 @@ true - https://bitcoin.org/bin/bitcoin-core-0.16.3/bitcoin-0.16.3-x86_64-linux-gnu.tar.gz + https://bitcoin.org/bin/bitcoin-core-0.17.1/bitcoin-0.17.1-x86_64-linux-gnu.tar.gz - c371e383f024c6c45fb255d528a6beec - e6d8ab1f7661a6654fd81e236b9b5fd35a3d4dcb + 724043999e2b5ed0c088e8db34f15d43 + 546ee35d4089c7ccc040a01cdff3362599b8bc53 @@ -93,10 +93,10 @@ - https://bitcoin.org/bin/bitcoin-core-0.16.3/bitcoin-0.16.3-osx64.tar.gz + https://bitcoin.org/bin/bitcoin-core-0.17.1/bitcoin-0.17.1-osx64.tar.gz - bacd87d0c3f65a5acd666e33d094a59e - 62cc5bd9ced610bb9e8d4a854396bfe2139e3d0f + b5a792c6142995faa42b768273a493bd + 8bd51c7024d71de07df381055993e9f472013db8 @@ -107,9 +107,9 @@ - https://bitcoin.org/bin/bitcoin-core-0.16.3/bitcoin-0.16.3-win64.zip - bbde9b1206956d19298034319e9f405e - 85e3dc8a9c6f93b1b20cb79fa5850b5ce81da221 + https://bitcoin.org/bin/bitcoin-core-0.17.1/bitcoin-0.17.1-win64.zip + b0e824e9dd02580b5b01f073f3c89858 + 4e17bad7d08c465b444143a93cd6eb1c95076e3f diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/Setup.scala b/eclair-core/src/main/scala/fr/acinq/eclair/Setup.scala index 9c9425783..ffe2c095d 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/Setup.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/Setup.scala @@ -126,15 +126,13 @@ class Setup(datadir: File, } yield (progress, chainHash, bitcoinVersion, unspentAddresses, blocks, headers) // blocking sanity checks val (progress, chainHash, bitcoinVersion, unspentAddresses, blocks, headers) = await(future, 30 seconds, "bicoind did not respond after 30 seconds") - assert(bitcoinVersion >= 160300, "Eclair requires Bitcoin Core 0.16.3 or higher") + assert(bitcoinVersion >= 170000, "Eclair requires Bitcoin Core 0.17.0 or higher") assert(chainHash == nodeParams.chainHash, s"chainHash mismatch (conf=${nodeParams.chainHash} != bitcoind=$chainHash)") if (chainHash != Block.RegtestGenesisBlock.hash) { assert(unspentAddresses.forall(address => !isPay2PubkeyHash(address)), "Make sure that all your UTXOS are segwit UTXOS and not p2pkh (check out our README for more details)") } assert(progress > 0.999, s"bitcoind should be synchronized (progress=$progress") assert(headers - blocks <= 1, s"bitcoind should be synchronized (headers=$headers blocks=$blocks") - // TODO: add a check on bitcoin version? - Bitcoind(bitcoinClient) case ELECTRUM => val addresses = config.hasPath("electrum") match { diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreWallet.scala b/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreWallet.scala index 9a7f3bc4d..008326fa0 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreWallet.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreWallet.scala @@ -19,10 +19,12 @@ package fr.acinq.eclair.blockchain.bitcoind import fr.acinq.bitcoin._ import fr.acinq.eclair._ import fr.acinq.eclair.blockchain._ -import fr.acinq.eclair.blockchain.bitcoind.rpc.{BitcoinJsonRPCClient, JsonRPCError} +import fr.acinq.eclair.blockchain.bitcoind.rpc.{BitcoinJsonRPCClient, Error, JsonRPCError} import fr.acinq.eclair.transactions.Transactions import grizzled.slf4j.Logging +import org.json4s.DefaultFormats import org.json4s.JsonAST._ +import org.json4s.jackson.Serialization import scodec.bits.ByteVector import scala.concurrent.{ExecutionContext, Future} @@ -47,9 +49,13 @@ class BitcoinCoreWallet(rpcClient: BitcoinJsonRPCClient)(implicit ec: ExecutionC def fundTransaction(tx: Transaction, lockUnspents: Boolean, feeRatePerKw: Long): Future[FundTransactionResponse] = fundTransaction(Transaction.write(tx).toHex, lockUnspents, feeRatePerKw) def signTransaction(hex: String): Future[SignTransactionResponse] = - rpcClient.invoke("signrawtransaction", hex).map(json => { + rpcClient.invoke("signrawtransactionwithwallet", hex).map(json => { val JString(hex) = json \ "hex" val JBool(complete) = json \ "complete" + if (!complete) { + val message = (json \ "errors" \\ classOf[JString]).mkString(",") + throw new JsonRPCError(Error(-1, message)) + } SignTransactionResponse(Transaction.read(hex), complete) }) @@ -74,7 +80,7 @@ class BitcoinCoreWallet(rpcClient: BitcoinJsonRPCClient)(implicit ec: ExecutionC private def signTransactionOrUnlock(tx: Transaction): Future[SignTransactionResponse] = { val f = signTransaction(tx) - // if signature fails (e.g. because wallet is uncrypted) we need to unlock the utxos + // if signature fails (e.g. because wallet is encrypted) we need to unlock the utxos f.recoverWith { case _ => unlockOutpoints(tx.txIn.map(_.outPoint)) .recover { case t: Throwable => logger.warn(s"Cannot unlock failed transaction's UTXOs txid=${tx.txid}", t); t } // no-op, just add a log in case of failure @@ -94,7 +100,7 @@ class BitcoinCoreWallet(rpcClient: BitcoinJsonRPCClient)(implicit ec: ExecutionC // we ask bitcoin core to add inputs to the funding tx, and use the specified change address FundTransactionResponse(unsignedFundingTx, _, fee) <- fundTransaction(partialFundingTx, lockUnspents = true, feeRatePerKw) // now let's sign the funding tx - SignTransactionResponse(fundingTx, _) <- signTransactionOrUnlock(unsignedFundingTx) + SignTransactionResponse(fundingTx, true) <- signTransactionOrUnlock(unsignedFundingTx) // there will probably be a change output, so we need to find which output is ours outputIndex = Transactions.findPubKeyScriptIndex(fundingTx, pubkeyScript, outputsAlreadyUsed = Set.empty, amount_opt = None) _ = logger.debug(s"created funding txid=${fundingTx.txid} outputIndex=$outputIndex fee=$fee") diff --git a/eclair-core/src/test/resources/integration/bitcoin.conf b/eclair-core/src/test/resources/integration/bitcoin.conf index da4dd59a0..29775744a 100644 --- a/eclair-core/src/test/resources/integration/bitcoin.conf +++ b/eclair-core/src/test/resources/integration/bitcoin.conf @@ -1,7 +1,7 @@ regtest=1 +noprinttoconsole=1 server=1 port=28333 -rpcport=28332 rpcuser=foo rpcpassword=bar txindex=1 @@ -9,3 +9,5 @@ zmqpubrawblock=tcp://127.0.0.1:28334 zmqpubrawtx=tcp://127.0.0.1:28335 rpcworkqueue=64 addresstype=bech32 +[regtest] +rpcport=28332 diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreWalletSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreWalletSpec.scala index 2f4adea65..3d0a51029 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreWalletSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreWalletSpec.scala @@ -21,7 +21,7 @@ import akka.actor.Status.Failure import akka.pattern.pipe import akka.testkit.{TestKit, TestProbe} import com.typesafe.config.ConfigFactory -import fr.acinq.bitcoin.{Block, MilliBtc, Satoshi, Script, Transaction} +import fr.acinq.bitcoin.{ByteVector32, Block, MilliBtc, OutPoint, Satoshi, Script, Transaction, TxIn, TxOut} import fr.acinq.eclair.blockchain._ import fr.acinq.eclair.blockchain.bitcoind.BitcoinCoreWallet.FundTransactionResponse import fr.acinq.eclair.blockchain.bitcoind.rpc.{BasicBitcoinJsonRPCClient, JsonRPCError} @@ -41,7 +41,14 @@ import scala.util.{Random, Try} class BitcoinCoreWalletSpec extends TestKit(ActorSystem("test")) with BitcoindService with FunSuiteLike with BeforeAndAfterAll with Logging { - val commonConfig = ConfigFactory.parseMap(Map("eclair.chain" -> "regtest", "eclair.spv" -> false, "eclair.server.public-ips.1" -> "localhost", "eclair.bitcoind.port" -> 28333, "eclair.bitcoind.rpcport" -> 28332, "eclair.bitcoind.zmq" -> "tcp://127.0.0.1:28334", "eclair.router-broadcast-interval" -> "2 second", "eclair.auto-reconnect" -> false)) + val commonConfig = ConfigFactory.parseMap(Map( + "eclair.chain" -> "regtest", + "eclair.spv" -> false, + "eclair.server.public-ips.1" -> "localhost", + "eclair.bitcoind.port" -> 28333, + "eclair.bitcoind.rpcport" -> 28332, + "eclair.router-broadcast-interval" -> "2 second", + "eclair.auto-reconnect" -> false)) val config = ConfigFactory.load(commonConfig).getConfig("eclair") val walletPassword = Random.alphanumeric.take(8).mkString @@ -94,6 +101,39 @@ class BitcoinCoreWalletSpec extends TestKit(ActorSystem("test")) with BitcoindSe } } + test("handle errors when signing transactions") { + val bitcoinClient = new BasicBitcoinJsonRPCClient( + user = config.getString("bitcoind.rpcuser"), + password = config.getString("bitcoind.rpcpassword"), + host = config.getString("bitcoind.host"), + port = config.getInt("bitcoind.rpcport")) + val wallet = new BitcoinCoreWallet(bitcoinClient) + + val sender = TestProbe() + + // create a transaction that spends UTXOs that don't exist + wallet.getFinalAddress.pipeTo(sender.ref) + val address = sender.expectMsgType[String] + val unknownTxids = Seq( + ByteVector32.fromValidHex("01" * 32), + ByteVector32.fromValidHex("02" * 32), + ByteVector32.fromValidHex("03" * 32) + ) + val unsignedTx = Transaction(version = 2, + txIn = Seq( + TxIn(OutPoint(unknownTxids(0), 0), signatureScript = Nil, sequence = TxIn.SEQUENCE_FINAL), + TxIn(OutPoint(unknownTxids(1), 0), signatureScript = Nil, sequence = TxIn.SEQUENCE_FINAL), + TxIn(OutPoint(unknownTxids(2), 0), signatureScript = Nil, sequence = TxIn.SEQUENCE_FINAL) + ), + txOut = TxOut(Satoshi(1000000), addressToPublicKeyScript(address, Block.RegtestGenesisBlock.hash)) :: Nil, + lockTime = 0) + + // signing it should fail, and the error message should contain the txids of the UTXOs that could not be used + wallet.signTransaction(unsignedTx).pipeTo(sender.ref) + val Failure(JsonRPCError(error)) = sender.expectMsgType[Failure] + unknownTxids.foreach(id => assert(error.message.contains(id.toString()))) + } + test("create/commit/rollback funding txes") { val bitcoinClient = new BasicBitcoinJsonRPCClient( user = config.getString("bitcoind.rpcuser"), @@ -141,10 +181,9 @@ class BitcoinCoreWalletSpec extends TestKit(ActorSystem("test")) with BitcoindSe sender.send(bitcoincli, BitcoinReq("getrawtransaction", fundingTxes(2).txid.toString())) assert(sender.expectMsgType[JString](10 seconds).s === fundingTxes(2).toString()) - // NB: bitcoin core doesn't clear the locks when a tx is published + // NB: from 0.17.0 on bitcoin core will clear locks when a tx is published sender.send(bitcoincli, BitcoinReq("listlockunspent")) - assert(sender.expectMsgType[JValue](10 seconds).children.size === 2) - + assert(sender.expectMsgType[JValue](10 seconds).children.size === 0) } test("encrypt wallet") { @@ -177,7 +216,8 @@ class BitcoinCoreWalletSpec extends TestKit(ActorSystem("test")) with BitcoindSe val pubkeyScript = Script.write(Script.pay2wsh(Scripts.multiSig2of2(randomKey.publicKey, randomKey.publicKey))) wallet.makeFundingTx(pubkeyScript, MilliBtc(50), 10000).pipeTo(sender.ref) - assert(sender.expectMsgType[Failure].cause.asInstanceOf[JsonRPCError].error.message.contains("Please enter the wallet passphrase with walletpassphrase first.")) + val error = sender.expectMsgType[Failure].cause.asInstanceOf[JsonRPCError].error + assert(error.message.contains("Please enter the wallet passphrase with walletpassphrase first")) sender.send(bitcoincli, BitcoinReq("listlockunspent")) assert(sender.expectMsgType[JValue](10 seconds).children.size === 0) @@ -212,14 +252,14 @@ class BitcoinCoreWalletSpec extends TestKit(ActorSystem("test")) with BitcoindSe bitcoinClient.invoke("fundrawtransaction", noinputTx1).pipeTo(sender.ref) val json = sender.expectMsgType[JValue] val JString(unsignedtx1) = json \ "hex" - bitcoinClient.invoke("signrawtransaction", unsignedtx1).pipeTo(sender.ref) + bitcoinClient.invoke("signrawtransactionwithwallet", unsignedtx1).pipeTo(sender.ref) val JString(signedTx1) = sender.expectMsgType[JValue] \ "hex" val tx1 = Transaction.read(signedTx1) // let's then generate another tx that double spends the first one val inputs = tx1.txIn.map(txIn => Map("txid" -> txIn.outPoint.txid.toString, "vout" -> txIn.outPoint.index)).toArray bitcoinClient.invoke("createrawtransaction", inputs, Map(address -> tx1.txOut.map(_.amount.toLong).sum * 1.0 / 1e8)).pipeTo(sender.ref) val JString(unsignedtx2) = sender.expectMsgType[JValue] - bitcoinClient.invoke("signrawtransaction", unsignedtx2).pipeTo(sender.ref) + bitcoinClient.invoke("signrawtransactionwithwallet", unsignedtx2).pipeTo(sender.ref) val JString(signedTx2) = sender.expectMsgType[JValue] \ "hex" val tx2 = Transaction.read(signedTx2) @@ -236,4 +276,4 @@ class BitcoinCoreWalletSpec extends TestKit(ActorSystem("test")) with BitcoindSe sender.expectMsg(true) } -} \ No newline at end of file +} diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoindService.scala b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoindService.scala index 42f7f949c..479072f4e 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoindService.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoindService.scala @@ -44,7 +44,7 @@ trait BitcoindService extends Logging { val INTEGRATION_TMP_DIR = new File(TestUtils.BUILD_DIRECTORY, s"integration-${UUID.randomUUID()}") logger.info(s"using tmp dir: $INTEGRATION_TMP_DIR") - val PATH_BITCOIND = new File(TestUtils.BUILD_DIRECTORY, "bitcoin-0.16.3/bin/bitcoind") + val PATH_BITCOIND = new File(TestUtils.BUILD_DIRECTORY, "bitcoin-0.17.1/bin/bitcoind") val PATH_BITCOIND_DATADIR = new File(INTEGRATION_TMP_DIR, "datadir-bitcoin") var bitcoind: Process = null diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/ExtendedBitcoinClientSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/ExtendedBitcoinClientSpec.scala index d730b2bf1..18a08923d 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/ExtendedBitcoinClientSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/ExtendedBitcoinClientSpec.scala @@ -34,7 +34,14 @@ import scala.concurrent.ExecutionContext.Implicits.global class ExtendedBitcoinClientSpec extends TestKit(ActorSystem("test")) with BitcoindService with FunSuiteLike with BeforeAndAfterAll with Logging { - val commonConfig = ConfigFactory.parseMap(Map("eclair.chain" -> "regtest", "eclair.spv" -> false, "eclair.server.public-ips.1" -> "localhost", "eclair.bitcoind.port" -> 28333, "eclair.bitcoind.rpcport" -> 28332, "eclair.bitcoind.zmq" -> "tcp://127.0.0.1:28334", "eclair.router-broadcast-interval" -> "2 second", "eclair.auto-reconnect" -> false)) + val commonConfig = ConfigFactory.parseMap(Map( + "eclair.chain" -> "regtest", + "eclair.spv" -> false, + "eclair.server.public-ips.1" -> "localhost", + "eclair.bitcoind.port" -> 28333, + "eclair.bitcoind.rpcport" -> 28332, + "eclair.router-broadcast-interval" -> "2 second", + "eclair.auto-reconnect" -> false)) val config = ConfigFactory.load(commonConfig).getConfig("eclair") implicit val formats = DefaultFormats @@ -67,7 +74,7 @@ class ExtendedBitcoinClientSpec extends TestKit(ActorSystem("test")) with Bitcoi val json = sender.expectMsgType[JValue] val JString(unsignedtx) = json \ "hex" val JInt(changePos) = json \ "changepos" - bitcoinClient.invoke("signrawtransaction", unsignedtx).pipeTo(sender.ref) + bitcoinClient.invoke("signrawtransactionwithwallet", unsignedtx).pipeTo(sender.ref) val JString(signedTx) = sender.expectMsgType[JValue] \ "hex" val tx = Transaction.read(signedTx) val txid = tx.txid.toString() @@ -92,7 +99,7 @@ class ExtendedBitcoinClientSpec extends TestKit(ActorSystem("test")) with Bitcoi val pos = if (changePos == 0) 1 else 0 bitcoinClient.invoke("createrawtransaction", Array(Map("txid" -> txid, "vout" -> pos)), Map(address -> 5.99999)).pipeTo(sender.ref) val JString(unsignedtx) = sender.expectMsgType[JValue] - bitcoinClient.invoke("signrawtransaction", unsignedtx).pipeTo(sender.ref) + bitcoinClient.invoke("signrawtransactionwithwallet", unsignedtx).pipeTo(sender.ref) val JString(signedTx) = sender.expectMsgType[JValue] \ "hex" signedTx } diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/fee/BitcoinCoreFeeProviderSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/fee/BitcoinCoreFeeProviderSpec.scala index 82c292166..7b9126b2a 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/fee/BitcoinCoreFeeProviderSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/fee/BitcoinCoreFeeProviderSpec.scala @@ -38,7 +38,14 @@ import scala.util.Random class BitcoinCoreFeeProviderSpec extends TestKit(ActorSystem("test")) with BitcoindService with FunSuiteLike with BeforeAndAfterAll with Logging { - val commonConfig = ConfigFactory.parseMap(Map("eclair.chain" -> "regtest", "eclair.spv" -> false, "eclair.server.public-ips.1" -> "localhost", "eclair.bitcoind.port" -> 28333, "eclair.bitcoind.rpcport" -> 28332, "eclair.bitcoind.zmq" -> "tcp://127.0.0.1:28334", "eclair.router-broadcast-interval" -> "2 second", "eclair.auto-reconnect" -> false)) + val commonConfig = ConfigFactory.parseMap(Map( + "eclair.chain" -> "regtest", + "eclair.spv" -> false, + "eclair.server.public-ips.1" -> "localhost", + "eclair.bitcoind.port" -> 28333, + "eclair.bitcoind.rpcport" -> 28332, + "eclair.router-broadcast-interval" -> "2 second", + "eclair.auto-reconnect" -> false)) val config = ConfigFactory.load(commonConfig).getConfig("eclair") val walletPassword = Random.alphanumeric.take(8).mkString diff --git a/travis/builddeps.sh b/travis/builddeps.sh deleted file mode 100755 index bfff183d5..000000000 --- a/travis/builddeps.sh +++ /dev/null @@ -1,25 +0,0 @@ -pushd . -# lightning deps -sudo add-apt-repository -y ppa:chris-lea/libsodium -sudo apt-get update -sudo apt-get install -y libsodium-dev libgmp-dev libsqlite3-dev -cd -git clone https://github.com/luke-jr/libbase58.git -cd libbase58 -./autogen.sh && ./configure && make && sudo make install -# lightning -cd -git clone https://github.com/ElementsProject/lightning.git -cd lightning -git checkout fce9ee29e3c37b4291ebb050e6a687cfaa7df95a -git submodule init -git submodule update -make -# bitcoind -cd -wget https://bitcoin.org/bin/bitcoin-core-0.13.0/bitcoin-0.13.0-x86_64-linux-gnu.tar.gz -echo "bcc1e42d61f88621301bbb00512376287f9df4568255f8b98bc10547dced96c8 bitcoin-0.13.0-x86_64-linux-gnu.tar.gz" > sha256sum.asc -sha256sum -c sha256sum.asc -tar xzvf bitcoin-0.13.0-x86_64-linux-gnu.tar.gz -popd - From 1c9ac1d62d8596b4d6f64992583eb68d5fa9d8c3 Mon Sep 17 00:00:00 2001 From: araspitzu Date: Wed, 20 Mar 2019 10:43:48 +0100 Subject: [PATCH 3/3] Ensure the cost function in path-finding is monotonic (#904) Make sure the cost function in path-finding is monotonic --- .../scala/fr/acinq/eclair/router/Graph.scala | 2 +- .../eclair/router/RouteCalculationSpec.scala | 30 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/router/Graph.scala b/eclair-core/src/main/scala/fr/acinq/eclair/router/Graph.scala index 4392c757c..20a2a8e00 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/router/Graph.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/router/Graph.scala @@ -304,7 +304,7 @@ object Graph { // NB we're guaranteed to have weightRatios and factors > 0 val factor = (cltvFactor * wr.cltvDeltaFactor) + (ageFactor * wr.ageFactor) + (capFactor * wr.capacityFactor) - val edgeWeight = if (isNeighborTarget) prev.weight else edgeCost * factor + val edgeWeight = if (isNeighborTarget) prev.weight else prev.weight + edgeCost * factor RichWeight(cost = edgeCost, length = prev.length + 1, cltv = prev.cltv + channelCltvDelta, weight = edgeWeight) } diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/router/RouteCalculationSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/router/RouteCalculationSpec.scala index 3b270ebc6..2169a9062 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/router/RouteCalculationSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/router/RouteCalculationSpec.scala @@ -837,6 +837,36 @@ class RouteCalculationSpec extends FunSuite { assert(hops2Nodes(routeScoreOptimized) === (a, e) :: (e, f) :: (f, d) :: Nil) } + + test("cost function is monotonic") { + + // This test have a channel (542280x2156x0) that according to heuristics is very convenient but actually useless to reach the target, + // then if the cost function is not monotonic the path-finding breaks because the result path contains a loop. + val updates = List( + ChannelDesc(ShortChannelId("565643x1216x0"), PublicKey(hex"03864ef025fde8fb587d989186ce6a4a186895ee44a926bfc370e2c366597a3f8f"), PublicKey(hex"024655b768ef40951b20053a5c4b951606d4d86085d51238f2c67c7dec29c792ca")) -> ChannelUpdate(ByteVector32.Zeroes.bytes, ByteVector32.Zeroes, ShortChannelId("565643x1216x0"), 0, 1.toByte, 1.toByte, 144, htlcMinimumMsat = 0, feeBaseMsat = 1000, 100, Some(15000000000L)), + ChannelDesc(ShortChannelId("565643x1216x0"), PublicKey(hex"024655b768ef40951b20053a5c4b951606d4d86085d51238f2c67c7dec29c792ca"), PublicKey(hex"03864ef025fde8fb587d989186ce6a4a186895ee44a926bfc370e2c366597a3f8f")) -> ChannelUpdate(ByteVector32.Zeroes.bytes, ByteVector32.Zeroes, ShortChannelId("565643x1216x0"), 0, 1.toByte, 0.toByte, 14, htlcMinimumMsat = 1, 1000, 10, Some(4294967295L)), + ChannelDesc(ShortChannelId("542280x2156x0"), PublicKey(hex"03864ef025fde8fb587d989186ce6a4a186895ee44a926bfc370e2c366597a3f8f"), PublicKey(hex"03cb7983dc247f9f81a0fa2dfa3ce1c255365f7279c8dd143e086ca333df10e278")) -> ChannelUpdate(ByteVector32.Zeroes.bytes, ByteVector32.Zeroes, ShortChannelId("542280x2156x0"), 0, 1.toByte, 1.toByte, 144, htlcMinimumMsat = 1000, feeBaseMsat = 1000, 100, Some(16777000000L)), + ChannelDesc(ShortChannelId("542280x2156x0"), PublicKey(hex"03cb7983dc247f9f81a0fa2dfa3ce1c255365f7279c8dd143e086ca333df10e278"), PublicKey(hex"03864ef025fde8fb587d989186ce6a4a186895ee44a926bfc370e2c366597a3f8f")) -> ChannelUpdate(ByteVector32.Zeroes.bytes, ByteVector32.Zeroes, ShortChannelId("542280x2156x0"), 0, 1.toByte, 0.toByte, 144, htlcMinimumMsat = 1, 667, 1, Some(16777000000L)), + ChannelDesc(ShortChannelId("565779x2711x0"), PublicKey(hex"03864ef025fde8fb587d989186ce6a4a186895ee44a926bfc370e2c366597a3f8f"), PublicKey(hex"036d65409c41ab7380a43448f257809e7496b52bf92057c09c4f300cbd61c50d96")) -> ChannelUpdate(ByteVector32.Zeroes.bytes, ByteVector32.Zeroes, ShortChannelId("565779x2711x0"), 0, 1.toByte, 3.toByte, 144, htlcMinimumMsat = 1, 1000, 100, Some(230000000L)), + ChannelDesc(ShortChannelId("565779x2711x0"), PublicKey(hex"036d65409c41ab7380a43448f257809e7496b52bf92057c09c4f300cbd61c50d96"), PublicKey(hex"03864ef025fde8fb587d989186ce6a4a186895ee44a926bfc370e2c366597a3f8f")) -> ChannelUpdate(ByteVector32.Zeroes.bytes, ByteVector32.Zeroes, ShortChannelId("565779x2711x0"), 0, 1.toByte, 0.toByte, 144, htlcMinimumMsat = 1, 1000, 100, Some(230000000L)) + ).toMap + + val g = DirectedGraph.makeGraph(updates) + + val params = RouteParams(randomize = false, maxFeeBaseMsat = 21000, maxFeePct = 0.03, routeMaxCltv = 1008, routeMaxLength = 6, ratios = Some( + WeightRatios(cltvDeltaFactor = 0.15, ageFactor = 0.35, capacityFactor = 0.5) + )) + val thisNode = PublicKey(hex"036d65409c41ab7380a43448f257809e7496b52bf92057c09c4f300cbd61c50d96") + val targetNode = PublicKey(hex"024655b768ef40951b20053a5c4b951606d4d86085d51238f2c67c7dec29c792ca") + val amount = 351000 + + Globals.blockCount.set(567634) // simulate mainnet block for heuristic + val Success(route) = Router.findRoute(g, thisNode, targetNode, amount, 1, Set.empty, Set.empty, params) + + assert(route.size == 2) + assert(route.last.nextNodeId == targetNode) + } + } object RouteCalculationSpec {