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

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)
   }
 }
 

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],

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) ::

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)
+  }
 }

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,