Conversation
de6d53e to
8affb5d
Compare
cli/src/commands/tx/finalize.rs
Outdated
|
|
||
| let sigs = extract_ordered_sigs(redeem_script, &input.partial_sigs, i)?; | ||
|
|
||
| eprintln!("sig[0]: {}", hex::encode(&sigs[0])); |
| (secret_key, public_key) | ||
| } | ||
|
|
||
| /// Generate a 2-of-2 multisig address from a list of public keys |
There was a problem hiding this comment.
Why did you delete these comments?
core/src/utils.rs
Outdated
| pub fn generate_2of2_multisig_address_elements( | ||
| pubkeys: &[PublicKey], | ||
| address_params: &'static elements::AddressParams, | ||
| use_p2sh: bool, |
There was a problem hiding this comment.
Flags like use_p2sh: bool are generally confusing for devs, and bad API in general, if it’s true, what exactly is the other caset? It’s clearer to do this with an enum that explicitly defines the possible cases (e.g., 2–3 variants)
service/src/handlers/tweak.rs
Outdated
| pub struct TweakResponse { | ||
| pub cmr_hex: String, | ||
| pub tweaked_public_key_hex: String, | ||
| pub untweaked_public_key_hex: String, |
| .extract_tx() | ||
| .context("Failed to extract transaction from PSET")?; | ||
|
|
||
| // For each input, build the witness from partial signatures |
There was a problem hiding this comment.
Don't go around just deleting comments
cli/src/commands/tx/finalize.rs
Outdated
| if input.partial_sigs.len() < 2 { | ||
| return Err(anyhow::anyhow!( | ||
| "Input {} requires 2 signatures but only has {}", | ||
| let (is_p2sh, is_p2wsh, is_p2tr) = { |
There was a problem hiding this comment.
Confusing, just do:
let script_pubkey = _;
if script_pubkey.is_p2sh() {
# foo
}
# continue
if script_pubkey.is_v1_p2tr() {
# bar
}Using fewer else if and else is generally better, and makes the code easier to read. This is dev CLI we can somethimes drop the error case
service/src/handlers/sign_pset.rs
Outdated
| elements_env, | ||
| state.elements_network, | ||
| // precalculate inside new scope to prevent script cloning because pset will be mutually borrowed | ||
| let (is_p2sh, is_p2wsh, is_p2tr) = { |
There was a problem hiding this comment.
Do the same as in finalize in CLI, you can validate that it's one of the three supported types during request validation.
Changes
Add P2SH and P2TR transactions to service