Skip to content
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

Open
tcharding opened this issue Sep 25, 2024 · 11 comments
Open

Is this crate solving the right problem #12

tcharding opened this issue Sep 25, 2024 · 11 comments

Comments

@tcharding
Copy link
Member

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:

  • Depend on bitcoin::psbt for the Psbt type and use it for 'basic' stuff e.g., serialization
  • Make this crate a layer on top that implements for example roles, signing, a more user friendly API than just setting fields
  • Deprecate anything in bitcoin::psbt that is not just POD or 'basic' stuff

This approach would achieve:

  • Go back to the original idea behind the psbt module
  • Reduce code in bitcoin::psbt (deprecate at least)
  • Stop up from feeling like bitcoin::psbt is neglected and a second class citizen
  • Allow use to 'finish' bitcoin::psbt now the purpose is well defined (assuming we can define 'basic')
  • Allow this crate to innovate and move fast
@apoelstra
Copy link
Member

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

@tcharding
Copy link
Member Author

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
Copy link
Member

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
Copy link
Member Author

v0 is basically deprecated and has been for many years.

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

ref: bitcoin/bitcoin#21283

@apoelstra
Copy link
Member

apoelstra commented Sep 25, 2024

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
Copy link
Member Author

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
Copy link
Member Author

Here is a demo I played around with today: rust-bitcoin/rust-bitcoin#3413

@apoelstra
Copy link
Member

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
Copy link

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
Copy link
Member Author

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants