-
Notifications
You must be signed in to change notification settings - Fork 4
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
feat!(strata-cli): X-Only PK of recovery address instead of take back leaf hash #735
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
@@ Coverage Diff @@
## main #735 +/- ##
==========================================
+ Coverage 54.80% 54.91% +0.10%
==========================================
Files 322 322
Lines 36107 36125 +18
==========================================
+ Hits 19788 19837 +49
+ Misses 16319 16288 -31
... and 2 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Commit: 0cf2ea9 SP1 Execution Results
|
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.
Would it be possible to squeeze in this change as well?
https://alpenlabs.atlassian.net/browse/STR-970
Instead of two scripts: N/N
and take back
, this would mean there is a single script and the taproot internal key will be N/N
. I'm guessing this should be straightforward to implement but let me know if it isn't, in which case we can defer it to later. This will also require a tiny change on the strata-bridge
since we no longer need the control block to spend the taproot and only need to tweak the internal key with the script.
Like this in 4a0d3f1 ? |
Yes, I think so. Is there a way to test it though? The output descriptor should be the same as if you constructed the taproot address manually via: let take_back_script = construct_takeback_script(); // <pk>OP_CHECKSIGVERIFY<D>OP_CSV
let taproot_addr = create_taproot_addr(network::Regtest, SpendPath::Both { internal_key: bridge_key, scripts: &[take_back_script] }).unwrap(); |
Vacuum sealed in cf5eb8c |
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.
Approving this tentatively, but the deposit subcommand is being removed in the other PR, so some of this could be removed.
use crate::constants::BRIDGE_MUSIG2_PUBKEY; | ||
|
||
#[test] | ||
fn bridge_in_descriptor_script() { | ||
let bridge_musig2_pubkey = BRIDGE_MUSIG2_PUBKEY.parse::<XOnlyPublicKey>().unwrap(); |
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.
Hold on, what is this pubkey for?
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 the musig2 aggregated key. Arguably, we should construct this ourselves by reading off of the params (not for this PR though).
@delbonis how are we planning to do deposits for Testnet I if not via the |
Description
Deposit Request Transactions (DRT) now need X-Only PK of recovery address instead of take back leaf hash.
This fixes the
strata-cli
DRT metadataOP_RETURN
construction.Type of Change
Notes to Reviewers
The new
OP_RETURN
now is:Checklist
Related Issues
STR-1176