From c0af66599055116ac21e40c5c0bb144a1d99a46a Mon Sep 17 00:00:00 2001 From: Fabrice Drouin Date: Tue, 19 Feb 2019 11:06:36 +0100 Subject: [PATCH] Electrum fixes and improvements (#873) * Electrum: don't ask for merkle proofs for unconfirmed txs * Electrum: clear status when we get disconnected and still have pending history transactions When we get disconnected and have script hashes for which we still have pending connections, clear the script hash status. When we reconnect we will ask for its history again, this way we won't miss anything. Since we rotate keys it should not result in heavy traffic (script hashes have few history items). * Electrum: represent and persist block heights as 32 bits signed ints Int.MaxValue is about 40,000 years of block which should be enough, and it will fix the encoding problem users on testnet when there's a reorg and one of their txs has a height of -1. Side-note: changing the wallet codec can be done easily: if we cannot read and decode persisted data we just start with an empty wallet and retrieve all wallet data from electrum servers, and once it's ready it will be encoded with the new codec and saved. * Electrum persistence: include a version number It provides a clean way, when upgrading the app, of choosing whether to keep the same version and start from the last persisted wallet (if the persistence format has not been changed or is compatible with the old one), or to change the version and force starting from an empty wallet and downloading all wallet items from Electrum servers. * ElectrumClient: remove useless buffer --- .../blockchain/electrum/ElectrumClient.scala | 10 +++---- .../blockchain/electrum/ElectrumWallet.scala | 29 ++++++++++++------- .../electrum/db/sqlite/SqliteWalletDb.scala | 16 +++++++--- .../ElectrumWalletSimulatedClientSpec.scala | 16 ++++++++++ .../db/sqlite/SqliteWalletDbSpec.scala | 6 ++-- 5 files changed, 56 insertions(+), 21 deletions(-) diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/electrum/ElectrumClient.scala b/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/electrum/ElectrumClient.scala index 8f3fb6879..bb5376ef7 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/electrum/ElectrumClient.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/electrum/ElectrumClient.scala @@ -282,12 +282,12 @@ class ElectrumClient(serverAddress: InetSocketAddress, ssl: SSL)(implicit val ec val (height, header) = parseBlockHeader(json.result) log.debug("connected to server={}, tip={} height={}", serverAddress, header.hash, height) statusListeners.map(_ ! ElectrumReady(height, header, serverAddress)) - context become connected(ctx, height, header, "", Map()) + context become connected(ctx, height, header, Map()) case AddStatusListener(actor) => statusListeners += actor } - def connected(ctx: ChannelHandlerContext, height: Int, tip: BlockHeader, buffer: String, requests: Map[String, (Request, ActorRef)]): Receive = { + def connected(ctx: ChannelHandlerContext, height: Int, tip: BlockHeader, requests: Map[String, (Request, ActorRef)]): Receive = { case AddStatusListener(actor) => statusListeners += actor actor ! ElectrumReady(height, tip, serverAddress) @@ -308,7 +308,7 @@ class ElectrumClient(serverAddress: InetSocketAddress, ssl: SSL)(implicit val ec context watch actor case _ => () } - context become connected(ctx, height, tip, buffer, requests + (curReqId -> (request, sender()))) + context become connected(ctx, height, tip, requests + (curReqId -> (request, sender()))) case Right(json: JsonRPCResponse) => requests.get(json.id) match { @@ -319,7 +319,7 @@ class ElectrumClient(serverAddress: InetSocketAddress, ssl: SSL)(implicit val ec case None => log.warning("server={} could not find requestor for reqId=${} response={}", serverAddress, json.id, json) } - context become connected(ctx, height, tip, buffer, requests - json.id) + context become connected(ctx, height, tip, requests - json.id) case Left(response: HeaderSubscriptionResponse) => headerSubscriptions.map(_ ! response) @@ -329,7 +329,7 @@ class ElectrumClient(serverAddress: InetSocketAddress, ssl: SSL)(implicit val ec case HeaderSubscriptionResponse(height, newtip) => log.info("server={} new tip={}", serverAddress, newtip) - context become connected(ctx, height, newtip, buffer, requests) + context become connected(ctx, height, newtip, requests) } } diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/electrum/ElectrumWallet.scala b/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/electrum/ElectrumWallet.scala index b2c3aad8f..d1c0570d9 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/electrum/ElectrumWallet.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/electrum/ElectrumWallet.scala @@ -127,8 +127,13 @@ class ElectrumWallet(seed: BinaryData, client: ActorRef, params: ElectrumWallet. pendingTransactionRequests = Set(), pendingTransactions = persisted.pendingTransactions, lastReadyMessage = None) - case _ => - log.info("starting with a default wallet") + case Success(None) => + log.info(s"wallet db is empty, starting with a default wallet") + val firstAccountKeys = (0 until params.swipeRange).map(i => derivePrivateKey(accountMaster, i)).toVector + val firstChangeKeys = (0 until params.swipeRange).map(i => derivePrivateKey(changeMaster, i)).toVector + Data(params, blockchain1, firstAccountKeys, firstChangeKeys) + case Failure(exception) => + log.info(s"cannot read wallet db ($exception), starting with a default wallet") val firstAccountKeys = (0 until params.swipeRange).map(i => derivePrivateKey(accountMaster, i)).toVector val firstChangeKeys = (0 until params.swipeRange).map(i => derivePrivateKey(changeMaster, i)).toVector Data(params, blockchain1, firstAccountKeys, firstChangeKeys) @@ -331,9 +336,11 @@ class ElectrumWallet(seed: BinaryData, client: ActorRef, params: ElectrumWallet. case (Some(previousHeight), height) if previousHeight != height => // there was a reorg context.system.eventStream.publish(TransactionConfidenceChanged(txid, confirmations, data.computeTimestamp(txid, params.walletDb))) - downloadHeadersIfMissing(height.toInt) - client ! GetMerkle(txid, height.toInt) - case (Some(previousHeight), height) if previousHeight == height && data.proofs.get(txid).isEmpty => + if (height > 0) { + downloadHeadersIfMissing(height.toInt) + client ! GetMerkle(txid, height.toInt) + } + case (Some(previousHeight), height) if previousHeight == height && height > 0 && data.proofs.get(txid).isEmpty => downloadHeadersIfMissing(height.toInt) client ! GetMerkle(txid, height.toInt) case (Some(previousHeight), height) if previousHeight == height => @@ -440,7 +447,10 @@ class ElectrumWallet(seed: BinaryData, client: ActorRef, params: ElectrumWallet. case Event(ElectrumClient.ElectrumDisconnected, data) => log.info(s"wallet got disconnected") + // remove status for each script hash for which we have pending requests + // this will make us query script hash history for these script hashes again when we reconnect goto(DISCONNECTED) using data.copy( + status = data.status -- data.pendingHistoryRequests, pendingHistoryRequests = Set(), pendingTransactionRequests = Set(), pendingHeadersRequests = Set(), @@ -650,8 +660,7 @@ object ElectrumWallet { * @param blockchain blockchain * @param accountKeys account keys * @param changeKeys change keys - * @param status script hash -> status; "" means that the script hash has not been used - * yet + * @param status script hash -> status; "" means that the script hash has not been used yet * @param transactions wallet transactions * @param heights transactions heights * @param history script hash -> history @@ -665,7 +674,7 @@ object ElectrumWallet { changeKeys: Vector[ExtendedPrivateKey], status: Map[BinaryData, String], transactions: Map[BinaryData, Transaction], - heights: Map[BinaryData, Long], + heights: Map[BinaryData, Int], history: Map[BinaryData, List[ElectrumClient.TransactionHistoryItem]], proofs: Map[BinaryData, GetMerkleResponse], locks: Set[Transaction], @@ -1000,7 +1009,7 @@ object ElectrumWallet { } history + (scriptHash -> entry) } - this.copy(locks = this.locks - tx, transactions = this.transactions + (tx.txid -> tx), heights = this.heights + (tx.txid -> 0L), history = history1) + this.copy(locks = this.locks - tx, transactions = this.transactions + (tx.txid -> tx), heights = this.heights + (tx.txid -> 0), history = history1) } /** @@ -1038,7 +1047,7 @@ object ElectrumWallet { changeKeysCount: Int, status: Map[BinaryData, String], transactions: Map[BinaryData, Transaction], - heights: Map[BinaryData, Long], + heights: Map[BinaryData, Int], history: Map[BinaryData, List[ElectrumClient.TransactionHistoryItem]], proofs: Map[BinaryData, GetMerkleResponse], pendingTransactions: List[Transaction], diff --git a/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/electrum/db/sqlite/SqliteWalletDb.scala b/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/electrum/db/sqlite/SqliteWalletDb.scala index 4a4aa960d..150b4b2fb 100644 --- a/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/electrum/db/sqlite/SqliteWalletDb.scala +++ b/eclair-core/src/main/scala/fr/acinq/eclair/blockchain/electrum/db/sqlite/SqliteWalletDb.scala @@ -160,10 +160,10 @@ object SqliteWalletDb { (wire: BitVector) => statusListCodec.decode(wire).map(_.map(_.toMap)) ) - val heightsListCodec: Codec[List[(BinaryData, Long)]] = listOfN(uint16, binarydata(32) ~ uint32) + val heightsListCodec: Codec[List[(BinaryData, Int)]] = listOfN(uint16, binarydata(32) ~ int32) - val heightsCodec: Codec[Map[BinaryData, Long]] = Codec[Map[BinaryData, Long]]( - (map: Map[BinaryData, Long]) => heightsListCodec.encode(map.toList), + val heightsCodec: Codec[Map[BinaryData, Int]] = Codec[Map[BinaryData, Int]]( + (map: Map[BinaryData, Int]) => heightsListCodec.encode(map.toList), (wire: BitVector) => heightsListCodec.decode(wire).map(_.map(_.toMap)) ) @@ -194,9 +194,17 @@ object SqliteWalletDb { (wire: BitVector) => proofsListCodec.decode(wire).map(_.map(_.toMap)) ) + /** + * change this value + * -if the new codec is incompatible with the old one + * - OR if you want to force a full sync from Electrum servers + */ + val version = 0x0000 + val persistentDataCodec: Codec[PersistentData] = ( - ("accountKeysCount" | int32) :: + ("version" | constant(BitVector.fromInt(version))) :: ("accountKeysCount" | int32) :: + ("changeKeysCount" | int32) :: ("status" | statusCodec) :: ("transactions" | transactionsCodec) :: ("heights" | heightsCodec) :: diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/electrum/ElectrumWalletSimulatedClientSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/electrum/ElectrumWalletSimulatedClientSpec.scala index d85d60271..e2538ec69 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/electrum/ElectrumWalletSimulatedClientSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/electrum/ElectrumWalletSimulatedClientSpec.scala @@ -229,4 +229,20 @@ class ElectrumWalletSimulatedClientSpec extends TestKit(ActorSystem("test")) wit } wallet ! HeaderSubscriptionResponse(wallet.stateData.blockchain.tip.height + 1, bad) } + + test("clear status when we have pending history requests") { + while (client.msgAvailable) { + client.receiveOne(100 milliseconds) + } + // tell wallet that there is something for our first account key + val scriptHash = ElectrumWallet.computeScriptHashFromPublicKey(wallet.stateData.accountKeys(0).publicKey) + wallet ! ScriptHashSubscriptionResponse(scriptHash, "010101") + client.expectMsg(GetScriptHashHistory(scriptHash)) + assert(wallet.stateData.status(scriptHash) == "010101") + + // disconnect wallet + wallet ! ElectrumDisconnected + awaitCond(wallet.stateName == ElectrumWallet.DISCONNECTED) + assert(wallet.stateData.status.get(scriptHash).isEmpty) + } } diff --git a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/electrum/db/sqlite/SqliteWalletDbSpec.scala b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/electrum/db/sqlite/SqliteWalletDbSpec.scala index c45342831..60b89a4ea 100644 --- a/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/electrum/db/sqlite/SqliteWalletDbSpec.scala +++ b/eclair-core/src/test/scala/fr/acinq/eclair/blockchain/electrum/db/sqlite/SqliteWalletDbSpec.scala @@ -75,7 +75,9 @@ class SqliteWalletDbSpec extends FunSuite { 0L ) - def randomHistoryItem = ElectrumClient.TransactionHistoryItem(random.nextInt(1000000), randomBytes(32)) + def randomHeight = if (random.nextBoolean()) random.nextInt(500000) else -1 + + def randomHistoryItem = ElectrumClient.TransactionHistoryItem(randomHeight, randomBytes(32)) def randomHistoryItems = (0 to random.nextInt(100)).map(_ => randomHistoryItem).toList @@ -89,7 +91,7 @@ class SqliteWalletDbSpec extends FunSuite { changeKeysCount = 10, status = (for (i <- 0 until random.nextInt(100)) yield randomBytes(32) -> random.nextInt(100000).toHexString).toMap, transactions = transactions.map(tx => tx.hash -> tx).toMap, - heights = transactions.map(tx => tx.hash -> random.nextInt(500000).toLong).toMap, + heights = transactions.map(tx => tx.hash -> randomHeight).toMap, history = (for (i <- 0 until random.nextInt(100)) yield randomBytes(32) -> randomHistoryItems).toMap, proofs = (for (i <- 0 until random.nextInt(100)) yield randomBytes(32) -> randomProof).toMap, pendingTransactions = transactions.toList,