Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Electrum fixes #873

Merged
merged 6 commits into from
Feb 19, 2019
Merged

Electrum fixes #873

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ae6c353
Electrum: don't ask for merkle proofs for unconfirmed txs
sstone Feb 15, 2019
a58d183
Electrum: clear status when we get disconnected and still have pendin…
sstone Feb 15, 2019
6f753c3
Electrum: represent and persist block heights as 32 bits signed ints
sstone Feb 15, 2019
488cf8b
Electrum persistence: include a version number
sstone Feb 15, 2019
1677e0d
ElectrumClient: remove useless buffer
sstone Feb 15, 2019
8ade76b
Electrum wallet: rephrase comment and fix typo
sstone Feb 18, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions eclair-core/src/main/scala/fr/acinq/eclair/blockchain/electrum/ElectrumClient.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 {
Expand All @@ -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)

Expand All @@ -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)
}
}

Expand Down
29 changes: 19 additions & 10 deletions eclair-core/src/main/scala/fr/acinq/eclair/blockchain/electrum/ElectrumWallet.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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) {
Copy link
Member

@pm47 pm47 Feb 18, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why isn't this check in the downloadHeadersIfMissing function?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because you don't want to download headers, but you also don't want to download merkle proofs.

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 =>
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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
Expand All @@ -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],
Expand Down Expand Up @@ -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)
}

/**
Expand Down Expand Up @@ -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],
Expand Down
16 changes: 12 additions & 4 deletions ...ir-core/src/main/scala/fr/acinq/eclair/blockchain/electrum/db/sqlite/SqliteWalletDb.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
)

Expand Down Expand Up @@ -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) ::
Expand Down
16 changes: 16 additions & 0 deletions ...rc/test/scala/fr/acinq/eclair/blockchain/electrum/ElectrumWalletSimulatedClientSpec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
6 changes: 4 additions & 2 deletions ...ore/src/test/scala/fr/acinq/eclair/blockchain/electrum/db/sqlite/SqliteWalletDbSpec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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,
Expand Down