-
Notifications
You must be signed in to change notification settings - Fork 63
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
Conversation
8ecc392
to
bfc137a
Compare
9fa5736
to
def40d0
Compare
Now ready to review, just told me if you feel we need more detail. |
There was a problem hiding this 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
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'" |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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}]' |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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...
7e519ca
to
9f170d3
Compare
9f170d3
to
eb1e013
Compare
There was a problem hiding this 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!
xref #375