Update to a dev version latest bitcoin master 0e2e55971275da64ceb62e8991a0a5fa962cb8b1 #289
Conversation
sanket1729
changed the title
sanket1729
force-pushed
the
update_upstream
branch
from
949465e to
d54bbc5
Compare
sanket1729
force-pushed
the
update_upstream
branch
from
d54bbc5 to
f3c38b8
Compare
| @@ -83,11 +83,11 @@ fn main() { | |||
| 0xa9, 0x14, 0x92, 0x09, 0xa8, 0xf9, 0x0c, 0x58, 0x4b, 0xb5, 0x97, 0x4d, 0x58, 0x68, 0x72, | |||
| 0x49, 0xe5, 0x32, 0xde, 0x59, 0xf4, 0xbc, 0x87, | |||
| ]); | |||
|
|
|||
| let wit = transaction.input[0].witness.to_vec(); | |||
| let mut interpreter = miniscript::Interpreter::from_txdata( | |||
There was a problem hiding this comment.
What do you think to change the parameter type in from_txdata (and in inner::from_txdata) to &Witness avoiding the step of converting to vec?
If I am looking correctly it's used only for iteration and we have Witness::iter which doesn't allocate
There was a problem hiding this comment.
Yes, this is an already planned change in a follow-up PR that changes the interpreter API for taproot support. Wanted to keep things simple here (as in just resolving compile errors).
There was a problem hiding this comment.
Does it make sense for psbt FINAL_SCRIPT_WITNESS to be Witness type instead of Vec<Vec<u8>>
There was a problem hiding this comment.
Added a follow up commit and discovered that rust-miniscript needs rust-bitcoin/rust-bitcoin#774 for a cleaner non-allocating API.
sanket1729
changed the title
sanket1729
force-pushed
the
update_upstream
branch
from
7a10263 to
aa36494
Compare
|
@apoelstra, this can help move things forward on rust-miniscript |
| @@ -7,7 +7,7 @@ description = "Miniscript: a subset of Bitcoin Script designed for analysis" | |||
| license = "CC0-1.0" | |||
|
|
|||
| [features] | |||
| fuzztarget = ["bitcoin/fuzztarget"] | |||
| fuzztarget = [] | |||
There was a problem hiding this comment.
In f3c38b8:
We should drop fuzztarget entirely.
| })?; | ||
| let target = input.sighash_type.unwrap_or(bitcoin::EcdsaSigHashType::All); | ||
| for (key, ecdsa_sig) in &input.partial_sigs { | ||
| let flag = bitcoin::EcdsaSigHashType::from_u32_standard(ecdsa_sig.hash_ty as u32) |
There was a problem hiding this comment.
In f3c38b8:
lol, is this conversion just to check standardness? We should add a method to rust-bitcoin so we can call `ecdsa_sig.hash_tx.is_standard()'.
There was a problem hiding this comment.
Actually, this line is not needed. ecdsa_sig.hash_ty is guaranteed to follow standardness rules.
|
Resolving the comments in follow up PR |
Update to a version of dev rust-bitcoin so that we can prepare for rust-miniscript release. The integration tests are updated to point a personal fork of rust-bitcoincore-rpc to have rust semver type issues.
After we have this, we can make progress on tapscript while we await finalized rust-bitcoin release. After which, we can remove the git dependencies and have crates.io dependencies instead.