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

Recover on Bitcoin Core #619

Merged
merged 2 commits into from
Aug 28, 2023
Merged

Conversation

pythcoiner
Copy link
Collaborator

@pythcoiner pythcoiner commented Aug 20, 2023

xref #375

doc/RECOVER.md Outdated Show resolved Hide resolved
@pythcoiner pythcoiner force-pushed the liana_recover branch 3 times, most recently from 9fa5736 to def40d0 Compare August 27, 2023 08:41
@pythcoiner pythcoiner marked this pull request as ready for review August 27, 2023 08:43
@pythcoiner
Copy link
Collaborator Author

Now ready to review, just told me if you feel we need more detail.

Copy link
Member

@darosior darosior left a comment

Choose a reason for hiding this comment

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

Thanks for tackling this. I've kept from throwing nits, i plan on making a second pass on the document for syntax and maybe adding some more details. Just two comments i think are most important.

doc/RECOVER.md Outdated
Comment on lines 48 to 62
output:
```
[
{
"success": false,
"error": {
"code": -5,
"message": "Provided checksum '8ldsjayd' does not match computed checksum 'rgmh0m0p'"
}
},
{
"success": false,
"error": {
"code": -5,
"message": "Provided checksum '8ldsjayd' does not match computed checksum 'u7dlz0cc'"
Copy link
Member

Choose a reason for hiding this comment

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

It would be much cleaner to call getdescriptorinfo and get the checksum from there. Triggering an error on purpose is confusing and error messages are usually unreliable.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

right, i forgot to fix that, prior to finding out there's a bug in importdescriptor (' are dropped and not count as hardened path) the checksum from getdescriptorinfo was not matching, so i decide to go this ugly way instead

doc/RECOVER.md Outdated
We now got the correct checksum, replace wrong ones in the command:

```shell
bitcoin-cli -signet -rpcwallet=liana importdescriptors '[{"desc": "wsh(or_d(pk([a5c6b76e/48h/1h/0h/2h]tpubDF5861hj6vR3iJr3aPjGJz4rNbqDCRujQ21mczzKT5SiedaQqNVgHC8HT9ceyxvMFRoPMx4P6HAcL3NZrUPhRUbwCyj3TKSa64bAfnE3sLh/0/*),and_v(v:pkh([c477fd13/48h/1h/0h/2h]tpubDFn7iPbFqGrTQ2aRACNsUK1MXQR4Z6dYfU2nD1WA9ifSaia642j3Wah4n5pBUEpERNWGJsyv3Dv5qwBabC9TLQrwSboKzukw9wmurGu7XVH/0/*),older(3))))#rgmh0m0p", "range": [0,10000], "timestamp": 1682920310, "active": true, "internal":false}, {"desc": "wsh(or_d(pk([a5c6b76e/48h/1h/0h/2h]tpubDF5861hj6vR3iJr3aPjGJz4rNbqDCRujQ21mczzKT5SiedaQqNVgHC8HT9ceyxvMFRoPMx4P6HAcL3NZrUPhRUbwCyj3TKSa64bAfnE3sLh/1/*),and_v(v:pkh([c477fd13/48h/1h/0h/2h]tpubDFn7iPbFqGrTQ2aRACNsUK1MXQR4Z6dYfU2nD1WA9ifSaia642j3Wah4n5pBUEpERNWGJsyv3Dv5qwBabC9TLQrwSboKzukw9wmurGu7XVH/1/*),older(3))))#u7dlz0cc", "range": [0,10000], "timestamp": 1682920310, "active": true, "internal":true}]'
Copy link
Member

Choose a reason for hiding this comment

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

Why this timestamp? You should discuss the wallet birthdate and rescan.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i don't know where this timestamp from 😲
told me what you think about the fix...

@pythcoiner pythcoiner force-pushed the liana_recover branch 2 times, most recently from 7e519ca to 9f170d3 Compare August 27, 2023 11:20
@darosior darosior changed the title [WIP] Recover on Bitcoin Core Recover on Bitcoin Core Aug 28, 2023
Copy link
Member

@darosior darosior left a comment

Choose a reason for hiding this comment

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

ACK fcfad2d. I'll have a second pass for the syntax and expliciting a couple things. Thanks!

@darosior darosior merged commit 7ba95de into wizardsardine:master Aug 28, 2023
18 checks passed
@pythcoiner pythcoiner deleted the liana_recover branch October 25, 2023 08:28
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.

2 participants