Decouple wants outputs v1 and v2 typestates#992
Decouple wants outputs v1 and v2 typestates#992arminsabouri wants to merge 2 commits intopayjoin:masterfrom
Conversation
Pull Request Test Coverage Report for Build 17239269861Details
💛 - Coveralls |
Introduces `core::receive::WantsOutputs` as a common abstraction to decouple v1 and v2 typestates.
arminsabouri
force-pushed
the
decouple-wantsoutsputs
branch
from
b0940fb to
9e40b55
Compare
| payjoin_psbt: wants_outputs.payjoin_psbt, | ||
| params: wants_outputs.params, | ||
| change_vout: wants_outputs.change_vout, | ||
| receiver_inputs: vec![], |
There was a problem hiding this comment.
Note that receiver inputs is the only field that is different between WantsOutputs and WantsInputs. I'd like to spend some time to see if we can consolidate a shared abstraction for the two typestates.
There was a problem hiding this comment.
I am not longer convinced a shared abstraction between WantsOutputs and WantsInputs achieves any stronger typesaftey or simplifies dx. In fact it would complicated matters bc order of operations matter. i.e outputs must be added before inputs. We could come back to this idea of a TransactionConstruction shared abstraction. However, at the present its not mission critical.
Using bip77 nomenclature.
arminsabouri
force-pushed
the
decouple-wantsoutsputs
branch
from
9e40b55 to
ad8285d
Compare
arminsabouri
changed the title
| params: Params, | ||
| change_vout: usize, | ||
| owned_vouts: Vec<usize>, | ||
| pub(crate) inner: crate::receive::WantsOutputs, |
There was a problem hiding this comment.
I'm curious why we even need an explicit v1::WantsOutputs struct that wraps crate::receive::WantsOutputs. Would we need changes if crate::receive::WantsOutputs (and other core state types) were merely re-exported as part of the v1 module when feature = "v1" ?
| #[cfg(feature = "v1")] | ||
| use crate::output_substitution::OutputSubstitution; |
There was a problem hiding this comment.
If you want to export this as receive::v1::OutputSubstitution the right place to do that is in /exclusive/mod.rs because everything exported there is re-exported as
#[cfg(feature = "v1")]
pub use exclusive::*;Then you don't need an additional feature flag
|
Dan is taking over this work in #998 . Closing |
3 participants
Pull Request Checklist
Please confirm the following before requesting review: