-
Notifications
You must be signed in to change notification settings - Fork 2
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
Is this crate solving the right problem #12
Comments
Yeah, I like this idea. Very similar to the "extension trait" model we are using for other types. |
Ha yeah, an extension repo. One other thing, @Kixunil mentioned it before, calling the repo |
I think if we want to work on PSBT we should exclusively work on v2. v0 is basically deprecated and has been for many years. Meanwhile our existing PSBT v0 code in rust-bitcoin pretty much "just works" for the people using it. |
Even though the v2 stuff hasn't merged into Core? |
It depends -- if you are doing multiparty transaction construction then you pretty-much need V2. (Core doesn't do this and therefore doesn't stand to gain much, which is one reason why a PR like this from the wallet maintainer implementing a critical interop protocol would stay open for 3+ years without merging...) If you're not, then v0 is totally sufficient -- but you also don't need a very elaborate API for it. I suppose we could improve our rust-bitcoin API, and there's value in doing that here, but the value is capped because v0 itself isn't really usable in the generality that it was designed for. |
How do we want to go about this? I don't think patching the current
For (2) users who do not want to bother learning the new type can use I personally don't think its worth doing (1) but I guess it doesn't hurt. If we skipped the re-export and default to v0 I'd be happy. We could keep Any better ideas? |
Here is a demo I played around with today: rust-bitcoin/rust-bitcoin#3413 |
Given that v0 PSBTs should be parseable as v2 (and vice-versa, I think, except that in v2 you aren't required to have an unsigned transaction), I think we should work on unifying the types and having v0 supported as a use case of the v2 type. |
I tend to agree, in almost cases i guess the consumer don't even want to bother choose v0/v2 |
Yes from a serialization perspective but for users of |
I think we can go mad, but we shouldn't deprecate anything in rust-bitcoin til the "go mad" crate is done. Or at least, can handle all the basic v0 stuff. |
The idea behind this crate was:
rust-bitcoin
move faster by reducing code/workload.bitcoin::psbt
to this crate.Because of (3) I suggested using merge script (requires ack from maintainer) but until we get other maintainers all this has done is create more work for @apoelstra and more work for me backporting (#11 was non-trivial to do).
And because we decided not to delete the
bitcoin::psbt
module I'm not sure this crate is solving the correct problem.Perhaps a better approach would be:
bitcoin::psbt
for thePsbt
type and use it for 'basic' stuff e.g., serializationbitcoin::psbt
that is not just POD or 'basic' stuffThis approach would achieve:
psbt
modulebitcoin::psbt
(deprecate at least)bitcoin::psbt
is neglected and a second class citizenbitcoin::psbt
now the purpose is well defined (assuming we can define 'basic')The text was updated successfully, but these errors were encountered: