search

rust-bitcoin / rust-psbt-v0

Partially Signed Bitcoin Transaction Format - BIP-174
Creative Commons Zero v1.0 Universal
3 stars 2 forks source link

Is this crate solving the right problem #12

Open tcharding opened 1 month ago

tcharding commented 1 month ago

The idea behind this crate was:

  1. Make dev on PSBT stuff move faster.
  2. Make dev on rust-bitcoin move faster by reducing code/workload.
  3. Facilitate easy migration from 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:

This approach would achieve:

apoelstra commented 1 month ago

Yeah, I like this idea. Very similar to the "extension trait" model we are using for other types.

tcharding commented 1 month ago

Ha yeah, an extension repo.

One other thing, @Kixunil mentioned it before, calling the repo psbt-v0 if we are planing on adding PSBTv2 soon may be a mistake. I do think the crate package name is correct because psbt is taken already on crates.io, FTR we are now in control of psbt-v0 and psbt-v2 (on crates.io). I went with this because of the "drop in replacement" thing but if we are not doing that then we probably should re-think about the name.

apoelstra commented 1 month ago

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.

tcharding commented 1 month ago

v0 is basically deprecated and has been for many years.

Even though the v2 stuff hasn't merged into Core?

ref: https://github.com/bitcoin/bitcoin/pull/21283

apoelstra commented 1 month ago

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.

tcharding commented 1 month ago

How do we want to go about this? I don't think patching the current psbt code is a task worth doing. Would either of these two options work?

  1. Move the whole psbt module to psbt::v0 and just carry it around as is for a few years, we could re-export it using psbt::v0::* and then add psbt::v2 next to it - at some stage changing the default from v0 to v2
  2. Clobber the whole thing and just release one Psbt type in v0.33.0 that supports both bips (and is POD)

For (2) users who do not want to bother learning the new type can use psbt-v0 0.1.0 as a drop in replacement.

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 psbt empty except for the two modules for couple of releases to make it obvious what is going on.

Any better ideas?

tcharding commented 1 month ago

Here is a demo I played around with today: https://github.com/rust-bitcoin/rust-bitcoin/pull/3413

apoelstra commented 1 month ago

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.

pythcoiner commented 1 month ago

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

tcharding commented 1 month ago

Yes from a serialization perspective but for users of v0.32 and before the Psbt type will change significantly. For example we have to decide what to do about the the unsigned_tx, my point being that we still have to decide do we want to try to be gentle on the upgrade and deprecate etc. or can we just go mad and solve the problem without worrying about breaking the API. I'd like to do the latter

apoelstra commented 1 month ago

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.