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.

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

JoinMarketQt: Use restartForScan callback in selectWallet #342

Merged
merged 1 commit into from
Apr 14, 2019
Merged

JoinMarketQt: Use restartForScan callback in selectWallet #342

merged 1 commit into from
Apr 14, 2019

Conversation

kristapsk
Copy link
Member

Fixes #341.

@AdamISZ
Copy link
Member

AdamISZ commented Apr 14, 2019

Many thanks for this work. Just to note what I understand here:

I believe this is the correct solution to the issue described in #341 , in that when a wallet is loaded for the first time on a Core instance (e.g. taken from another machine), it requires a rescan because those addresses are not imported here. However, unlike the case of a newly generated wallet (see the JMMainWindow.generateWallet function in joinmarket-qt.py), we didn't previously use the restartForScan callback as the value of restart_cb passed into the wallet sync process, meaning that when the function add_watchonly_addresses was being called in blockchaininterface.py it was being called with a null restart_cb, which in that function is interpreted basically as meaning this is a CLI application and so we can safely print a message and quit. But since it's not a CLI application that's not the right behaviour, and so @kristapsk 's solution here is the correct one: call loadWalletFromBlockchain with restart_cb set to self.restartForScan, exactly as for the newly generated wallet case.

@AdamISZ
Copy link
Member

AdamISZ commented Apr 14, 2019

For context, it's important to note that the information message (same whether on CLI or Qt) given to the user is:

"restart Bitcoin Core with -rescan if you're "
"recovering an existing wallet from backup seed\n"
"Otherwise just restart this joinmarket application."

so in the case we're addressing here, a rescan on Core is actually needed whereas in the new wallet generation case it's not.

@AdamISZ
Copy link
Member

AdamISZ commented Apr 14, 2019

Tested OK with a slight kludge: I set a different rpc_wallet_file to the one in which my test wallet was created, and successfully got the info message and the shutdown as expected. tACK.

@AdamISZ AdamISZ merged commit 22467f4 into JoinMarket-Org:master Apr 14, 2019
AdamISZ added a commit that referenced this pull request Apr 14, 2019
22467f4 Use restartForScan callback in selectWallet (Kristaps Kaupe)
@kristapsk kristapsk deleted the selectwallet-restartforscan branch April 14, 2019 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unhandled error on opening wallet created on different computer in JoinMarketQt
2 participants