From 949424ac2a3eb1ebec3e33062bdb21d56cb12fe9 Mon Sep 17 00:00:00 2001 From: sstone Date: Mon, 22 May 2023 15:00:09 +0200 Subject: [PATCH] Rework and simplify tx inclusion proofs We can use the tx position that we compute when we verify the inclusion proof instead of explicitly asking Bitcoin Core for it. We also check that the id of the tx that is returned match what we asked for, and add min-difficulty checks for regtest and testnet. --- eclair-core/src/main/resources/reference.conf | 2 +- .../blockchain/bitcoind/ZmqWatcher.scala | 43 ++++++++----------- .../bitcoind/rpc/BitcoinCoreClient.scala | 28 +++++++----- .../bitcoind/BitcoinCoreClientSpec.scala | 4 +- 4 files changed, 39 insertions(+), 38 deletions(-) diff --git a/eclair-core/src/main/resources/reference.conf b/eclair-core/src/main/resources/reference.conf index 4a0cdb368a..cbc574fc82 100644 --- a/eclair-core/src/main/resources/reference.conf +++ b/eclair-core/src/main/resources/reference.conf @@ -40,7 +40,7 @@ eclair { // - ignore: eclair will leave these utxos locked and start startup-locked-utxos-behavior = "stop" final-pubkey-refresh-delay = 3 seconds - min-difficulty = 387294044 // difficulty of block 600000 + min-difficulty-target = 387294044 // difficulty of block 600000 } node-alias = "eclair" diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/ZmqWatcher.scala b/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/ZmqWatcher.scala index 4cd18fc347..86c58780ea 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/ZmqWatcher.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/ZmqWatcher.scala @@ -416,35 +416,30 @@ private class ZmqWatcher(nodeParams: NodeParams, blockHeight: AtomicLong, client private def checkConfirmed(w: WatchConfirmed[_ <: WatchConfirmedTriggered]): Future[Unit] = { log.debug("checking confirmations of txid={}", w.txId) - def checkConfirmationProof(): Future[Unit] = { - client.getTxConfirmationProof(w.txId).map(headerInfos => { - if (nodeParams.chainHash == Block.LivenetGenesisBlock.hash) { - // 0x1715a35cL = 387294044 = difficulty of block 600000 - val minDiff = Try(context.system.settings.config.getLong("eclair.bitcoind.min-difficulty")).getOrElse(0x1715a35cL) - require(headerInfos.forall(hi => hi.header.bits < minDiff)) - } - }) + val minDifficultyTarget = nodeParams.chainHash match { + case Block.LivenetGenesisBlock.hash => Try(context.system.settings.config.getLong("eclair.bitcoind.min-difficulty-target")).getOrElse(0x1715a35cL) // 0x1715a35cL = 387294044 = difficulty target of block 600000 + case Block.TestnetGenesisBlock.hash => 0x1d00ffffL + case _ => 0x207fffffL } - // NB: this is very inefficient since internally we call `getrawtransaction` three times, but it doesn't really - // matter because this only happens once, when the watched transaction has reached min_depth client.getTxConfirmations(w.txId).flatMap { case Some(confirmations) if confirmations >= w.minDepth => - checkConfirmationProof().andThen(_ => - client.getTransaction(w.txId).flatMap { tx => - client.getTransactionShortId(w.txId).map { - case (height, index) => w match { - case w: WatchFundingConfirmed => context.self ! TriggerEvent(w.replyTo, w, WatchFundingConfirmedTriggered(height, index, tx)) - case w: WatchFundingDeeplyBuried => context.self ! TriggerEvent(w.replyTo, w, WatchFundingDeeplyBuriedTriggered(height, index, tx)) - case w: WatchTxConfirmed => context.self ! TriggerEvent(w.replyTo, w, WatchTxConfirmedTriggered(height, index, tx)) - case w: WatchParentTxConfirmed => context.self ! TriggerEvent(w.replyTo, w, WatchParentTxConfirmedTriggered(height, index, tx)) - case w: WatchAlternativeCommitTxConfirmed => context.self ! TriggerEvent(w.replyTo, w, WatchAlternativeCommitTxConfirmedTriggered(height, index, tx)) - } - } + for { + proof <- client.getTxConfirmationProof(w.txId) + _ = require(proof.confirmations >= confirmations) + _ = require(proof.headerInfos.forall(hi => hi.header.bits <= minDifficultyTarget)) + height = BlockHeight(proof.height) + tx <- client.getTransaction(w.txId) + } yield { + w match { + case w: WatchFundingConfirmed => context.self ! TriggerEvent(w.replyTo, w, WatchFundingConfirmedTriggered(height, proof.txIndex, tx)) + case w: WatchFundingDeeplyBuried => context.self ! TriggerEvent(w.replyTo, w, WatchFundingDeeplyBuriedTriggered(height, proof.txIndex, tx)) + case w: WatchTxConfirmed => context.self ! TriggerEvent(w.replyTo, w, WatchTxConfirmedTriggered(height, proof.txIndex, tx)) + case w: WatchParentTxConfirmed => context.self ! TriggerEvent(w.replyTo, w, WatchParentTxConfirmedTriggered(height, proof.txIndex, tx)) + case w: WatchAlternativeCommitTxConfirmed => context.self ! TriggerEvent(w.replyTo, w, WatchAlternativeCommitTxConfirmedTriggered(height, proof.txIndex, tx)) } - ) - case _ => Future.successful((): Unit) + } + case _ => Future.successful(()) } } - } diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/rpc/BitcoinCoreClient.scala b/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/rpc/BitcoinCoreClient.scala index 6461d049b0..3b0a4b9885 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/rpc/BitcoinCoreClient.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/bitcoind/rpc/BitcoinCoreClient.scala @@ -52,7 +52,11 @@ class BitcoinCoreClient(val rpcClient: BitcoinJsonRPCClient) extends OnChainWall //------------------------- TRANSACTIONS -------------------------// def getTransaction(txid: ByteVector32)(implicit ec: ExecutionContext): Future[Transaction] = - getRawTransaction(txid).map(raw => Transaction.read(raw)) + getRawTransaction(txid).map(raw => { + val tx = Transaction.read(raw) + require(tx.txid == txid, "transaction id mismatch") + tx + }) private def getRawTransaction(txid: ByteVector32)(implicit ec: ExecutionContext): Future[String] = rpcClient.invoke("getrawtransaction", txid).collect { @@ -79,7 +83,7 @@ class BitcoinCoreClient(val rpcClient: BitcoinJsonRPCClient) extends OnChainWall * @param txid transaction id * @return a list of block header information, starting from the block in which the transaction was published, up to the current tip */ - def getTxConfirmationProof(txid: ByteVector32)(implicit ec: ExecutionContext): Future[List[BlockHeaderInfo]] = { + def getTxConfirmationProof(txid: ByteVector32)(implicit ec: ExecutionContext): Future[TxConfirmationProof] = { import KotlinUtils._ /** @@ -103,16 +107,13 @@ class BitcoinCoreClient(val rpcClient: BitcoinJsonRPCClient) extends OnChainWall // that can be used to rebuild the block's merkle root (header, txHashesAndPos) = verifyTxOutProof(proof) // inclusionData contains a header and a list of (txid, position) that can be used to re-build the header's merkle root - // check that the block hash included in the proof matches the block in which the tx was published - Some(blockHash) <- getTxBlockHash(txid) - _ = require(header.blockId.contentEquals(blockHash.toArray), "confirmation proof is not valid (block id mismatch)") - // check that our txid is included in the merkle root of the block it was published in - txids = txHashesAndPos.map { case (txhash, _) => txhash.reverse } - _ = require(txids.contains(txid), "confirmation proof is not valid (txid not found)") + // find the position of txid in the merkle root of the block it was published in + pos_opt = txHashesAndPos.find { case (hash, _) => hash.reverse == txid } map { case (_, pos) => pos } + _ = require(pos_opt.isDefined, "confirmation proof is not valid (txid not found)") // get the block in which our tx was confirmed and all following blocks - headerInfos <- getBlockInfos(blockHash, confirmations_opt.get) - _ = require(headerInfos.head.header.blockId.contentEquals(blockHash.toArray), "block header id mismatch") - } yield headerInfos + headerInfos <- getBlockInfos(header.blockId, confirmations_opt.get) + _ = require(headerInfos.head.header.blockId == header.blockId, "block header id mismatch") + } yield TxConfirmationProof(txid, headerInfos, pos_opt.get) } def getTxOutProof(txid: ByteVector32)(implicit ec: ExecutionContext): Future[ByteVector] = @@ -717,6 +718,11 @@ object BitcoinCoreClient { case class BlockHeaderInfo(header: BlockHeader, confirmation: Long, height: Long, nextBlockHash: Option[ByteVector32]) + case class TxConfirmationProof(txid: ByteVector32, headerInfos: List[BlockHeaderInfo], txIndex: Int) { + val confirmations = headerInfos.size + val height = headerInfos.head.height + } + def toSatoshi(btcAmount: BigDecimal): Satoshi = Satoshi(btcAmount.bigDecimal.scaleByPowerOfTen(8).longValue) } \ No newline at end of file diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreClientSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreClientSpec.scala index fee0920670..b40ef6e811 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreClientSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/bitcoind/BitcoinCoreClientSpec.scala @@ -1379,8 +1379,8 @@ class BitcoinCoreClientSpec extends TestKitBaseClass with BitcoindService with A val check = fr.acinq.bitcoin.Block.verifyTxOutProof(proof.toArray) val header = check.getFirst bitcoinClient.getTxConfirmationProof(tx.txid).pipeTo(sender.ref) - val headerInfos = sender.expectMsgType[List[BlockHeaderInfo]] - assert(header == headerInfos.head.header) + val confirmationProof = sender.expectMsgType[TxConfirmationProof] + assert(header == confirmationProof.headerInfos.head.header) // try again with a bitcoin client that returns a proof that is not valid for our tx but from the same block where it was confirmed bitcoinClient.getTxOutProof(dummyTx.txid).pipeTo(sender.ref)