-
Notifications
You must be signed in to change notification settings - Fork 2
Add unit tests #14
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
Add unit tests #14
Conversation
53bfa4c
to
d751661
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.
This is AWESOME!
Network.REGTEST | ||
) | ||
val descriptor2: Descriptor = Descriptor( | ||
"wpkh(tprv8ZgxMBicQKsPf2qfrEygW6fdYseJDDrVnDv26PH5BHdvSuG6ecCbHqLVof9yZcMoM31z9ur3tTYbSnr1WBqbGX97CbXcmp5H6qeMpyvx35B/86h/1h/1h/0/*)", |
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.
Since this is for WPKH I would think the path should be m/84 not m/86.
I am a bit surprised this worked.
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.
Same for the rest as well
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.
Ooops yep.
The reason why it works is that these bips are simply conventions. You can use any path you want. And this is what makes those strings dangerous to build manually; we see mistakes like this everywhere, even with very advanced bitcoin devs. This is why BDK developed the descriptor templates.
Good catch!
Network.TESTNET | ||
) | ||
val descriptor3: Descriptor = Descriptor( | ||
"tr(tprv8ZgxMBicQKsPf2qfrEygW6fdYseJDDrVnDv26PH5BHdvSuG6ecCbHqLVof9yZcMoM31z9ur3tTYbSnr1WBqbGX97CbXcmp5H6qeMpyvx35B/86h/1h/1h/0/*)", |
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.
Should we make the actual string of the paths constants too. I imagine we will be repeating them every where maybe in new tests too. (Thinking out loud)
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.
Yeah I actually like that idea. Let me push some new stuff to potentially clean this up.
ccb0227
to
da92a03
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 da92a03
da92a03
to
d9fa0ba
Compare
This is just to break the tests ice and add some unit tests. I'll rename the "Offline" tests since for now they will all be independent of any outside networks like Signet or Testnet 4.