-
Notifications
You must be signed in to change notification settings - Fork 377
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
Interactive Transaction Construction #2419
Interactive Transaction Construction #2419
Conversation
690418c
to
0dd26b9
Compare
We likely want Which brings up the question, since state transition calls can fail/do nothing for two reasons: (a) Invalid data; causing negotiation to abort or (b) Illegal state transition, do we want to always return It makes sense for (a) to return an |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for putting this out, I’ll have a look on the latest state of the specification to refresh my mind before to do another round.
I think in term of interface you have few questions like how we integrate with our signing interface in sign/mod.rs
if this is yielding PSBTs and how we’re broadcasting txns once we’re NegotiationComplete
(like do we throw them in a refactored-as-an-interface’s OnchainTxHandler
to keep broadcasting interface unified ?).
edited last comment - mis-copy past a link from the opened PR on dust exposure, sorry. |
What does the spec dictate happen to channel when it receives a message that it isn't expected in the current state? Here we may want to indicate an Separately, when sending a message, the an illegal state transition would indicate a programmer error, I'd imagine. |
Already reviewed back the dual-funding spec proposal and I’ll have a look on the splicing one. Still the gist to look on. |
pushed a new commit (241b837) that encapsulates some of the recent work that @wpaulino and myself have been working on: Users define inputs/outputs they want to contribute upfront
This is mostly there. Account for tx_complete as an ack mechanism
These two things combine to require some pretty significant reworking of how some of our API works, but we think we are just a few days out from getting it to a point where it will be good for @dunxen to play with it + start writing tests! |
Reviewed the splicing draft so far, though there is a lot of imprecision on the interactions with channel type. Still have to review the gist on the architecture of the state machine. |
1b17e0f
to
477c620
Compare
477c620
to
94d8200
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added few comments, found during early test writing
Is this good for rebasing on now? :) In the Azores but will have some time to start on some end-to-end V2 establishment flows. Will also provide feedback. |
looks good, issues are minor |
Some early tests & small fixes can be seen here: https://github.com/lightningdevkit/rust-lightning/compare/main...optout21:rust-lightning:interact-tx2?expand=1 (no PR yet as it's still very preliminary) |
059af71
to
bce504b
Compare
f08c112
to
1cedde6
Compare
@coderabbitai pause |
1cedde6
to
86777ee
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just tagged some comments that may have been missed. Will take another pass.
lightning/src/ln/interactivetxs.rs
Outdated
fn local_tx_add_input(&mut self, msg: &msgs::TxAddInput) { | ||
let tx = msg.prevtx.clone().into_transaction(); | ||
let input = TxIn { | ||
previous_output: OutPoint { | ||
txid: tx.txid(), | ||
vout: msg.prevtx_out, | ||
}, | ||
sequence: Sequence(msg.sequence), | ||
..Default::default() | ||
}; | ||
debug_assert!((msg.prevtx_out as usize) < tx.output.len()); | ||
let prev_output = &tx.output[msg.prevtx_out as usize]; | ||
self.prevtx_outpoints.insert(input.previous_output.clone()); | ||
self.inputs.insert(msg.serial_id, TxInputWithPrevOutput { | ||
input, | ||
prev_output: prev_output.clone(), | ||
}); | ||
} | ||
|
||
fn local_tx_add_output(&mut self, msg: &msgs::TxAddOutput) { | ||
self.outputs.insert(msg.serial_id, TxOut { | ||
value: msg.sats, | ||
script_pubkey: msg.script.clone(), | ||
}); | ||
} | ||
|
||
fn local_tx_remove_input(&mut self, msg: &msgs::TxRemoveInput) { | ||
self.inputs.remove(&msg.serial_id); | ||
} | ||
|
||
fn local_tx_remove_output(&mut self, msg: &msgs::TxRemoveOutput) { | ||
self.outputs.remove(&msg.serial_id); | ||
} |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
95bef6c
to
5e25aad
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This basically LGTM, but I'll note that I think we ended up substantially over-engineering this - the "our turn to send a message" state is actually never persistent after a call into the InteractiveTxConstructor
as far as I can tell - we always immediately respond. Thus, the whole internal state being designed to represent who's turn it is seems like a confusing abstraction - the reality is any time we look at the InteractiveTxConstructor
its the counterparty's turn! I'm fine with landing this as-is cause we need to make progress and its wayyyy too late to be raising this, but I had the thought so I figured I'd note it.
Ok(tx_to_validate) | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A high-level comment here would, I think, be useful. Something talking about how the protocol is a back-and-forth, so we store different states based on who's turn it is to talk, etc. Then some comments on the macros that explain what the macro is defining.
lightning/src/ln/interactivetxs.rs
Outdated
(constructor, message_send) | ||
} | ||
|
||
fn do_sent_state_transition(&mut self) -> Result<InteractiveTxMessageSend, AbortReason> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we call this, like, maybe_send_message or something that makes a bit clearer what its doing?
Luckily it's easy to simplify in future without affecting consumers of |
5e25aad
to
6869c49
Compare
$ git diff-tree -U1 5e25aad 6869c49
diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs
index c1e7216b..5b26a75e 100644
--- a/lightning/src/ln/interactivetxs.rs
+++ b/lightning/src/ln/interactivetxs.rs
@@ -394,3 +394,26 @@ impl NegotiationContext {
-/// Channel states that can send & receive `tx_(add|remove)_(input|output)` and `tx_complete`
+// The interactive transaction construction protocol allows two peers to collaboratively build a
+// transaction for broadcast.
+//
+// The protocol is turn-based, so we define different states here that we store depending on whose
+// turn it is to send the next message. The states are defined so that their types ensure we only
+// perform actions (only send messages) via defined state transitions that do not violate the
+// protocol.
+//
+// An example of a full negotiation and associated states follows:
+//
+// +------------+ +------------------+---- Holder state after message sent/received ----+
+// | |--(1)- tx_add_input ---->| | MsgSentChange +
+// | |<-(2)- tx_complete ------| | ReceivedTxComplete +
+// | |--(3)- tx_add_output --->| | MsgSentChange +
+// | |<-(4)- tx_complete ------| | ReceivedTxComplete +
+// | |--(5)- tx_add_input ---->| | MsgSentChange +
+// | Holder |<-(6)- tx_add_input -----| Counterparty | MsgReceivedChange +
+// | |--(7)- tx_remove_output >| | MsgSentChange +
+// | |<-(8)- tx_add_output ----| | MsgReceivedChange +
+// | |--(9)- tx_complete ----->| | SentTxComplete +
+// | |<-(10) tx_complete ------| | NegotiationComplete +
+// +------------+ +------------------+--------------------------------------------------+
+
+/// Negotiation states that can send & receive `tx_(add|remove)_(input|output)` and `tx_complete`
trait State {}
@@ -408,2 +431,4 @@ trait MsgReceivedState: State {
+// This macro is a helper for implementing the above state traits for various states subsequently
+// defined below the macro.
macro_rules! define_state {
@@ -466,2 +491,4 @@ trait StateTransition<NewState: State, TransitionData> {
+// This macro helps define the legal transitions between the states above by implementing
+// the `StateTransition` trait for each of the states that follow this declaration.
macro_rules! define_state_transitions {
@@ -542,2 +569,5 @@ impl Default for StateMachine {
+// The `StateMachine` internally executes the actual transition between two states and keeps
+// track of the current state. This macro defines _how_ those state transitions happen to
+// update the internal state.
macro_rules! define_state_machine_transitions {
@@ -645,2 +675,3 @@ pub enum InteractiveTxMessageSend {
+// This macro executes a state machine transition based on a provided action.
macro_rules! do_state_transition {
@@ -705,3 +736,3 @@ impl InteractiveTxConstructor {
let message_send = if is_initiator {
- match constructor.do_sent_state_transition() {
+ match constructor.maybe_send_message() {
Ok(msg_send) => Some(msg_send),
@@ -721,3 +752,3 @@ impl InteractiveTxConstructor {
- fn do_sent_state_transition(&mut self) -> Result<InteractiveTxMessageSend, AbortReason> {
+ fn maybe_send_message(&mut self) -> Result<InteractiveTxMessageSend, AbortReason> {
// We first attempt to send inputs we want to add, then outputs. Once we are done sending
@@ -754,3 +785,3 @@ impl InteractiveTxConstructor {
do_state_transition!(self, received_tx_add_input, msg)?;
- self.do_sent_state_transition()
+ self.maybe_send_message()
}
@@ -761,3 +792,3 @@ impl InteractiveTxConstructor {
do_state_transition!(self, received_tx_remove_input, msg)?;
- self.do_sent_state_transition()
+ self.maybe_send_message()
}
@@ -768,3 +799,3 @@ impl InteractiveTxConstructor {
do_state_transition!(self, received_tx_add_output, msg)?;
- self.do_sent_state_transition()
+ self.maybe_send_message()
}
@@ -775,3 +806,3 @@ impl InteractiveTxConstructor {
do_state_transition!(self, received_tx_remove_output, msg)?;
- self.do_sent_state_transition()
+ self.maybe_send_message()
}
@@ -784,3 +815,3 @@ impl InteractiveTxConstructor {
StateMachine::ReceivedTxComplete(_) => {
- let msg_send = self.do_sent_state_transition()?;
+ let msg_send = self.maybe_send_message()?;
let negotiated_tx = match &self.state_machine { |
We initially designed it to have each |
Right, I think my point is there are no "real" state transitions here - we receive a message and always respond, we never actually spend any time in the "our turn to send a message" state, we just always send a message. |
The top commit should be squashed, no? We generally don't want to have commits fixing things in previous commits in the same PR. |
6869c49
to
f279112
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Largely LGTM. Feel free to handle comments in the follow-up, where needed.
lightning/src/ln/interactivetxs.rs
Outdated
impl NegotiationContext { | ||
fn is_serial_id_valid_for_counterparty(&self, serial_id: &SerialId) -> bool { | ||
// A received `SerialId`'s parity must match the role of the counterparty. | ||
self.holder_is_initiator == !serial_id.is_for_initiator() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be easier to read if SerialIdExt
also had a is_for_counterparty
method.
lightning/src/ln/interactivetxs.rs
Outdated
|
||
define_state!( | ||
MSG_SENT_STATE, | ||
MsgSentChange, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Naming would be more consistent with SentTxComplete
and a bit more readable IMO if this were named SentChangeMsg
. Likewise for others.
entropy_source: &ES, channel_id: ChannelId, feerate_sat_per_kw: u32, is_initiator: bool, | ||
tx_locktime: AbsoluteLockTime, inputs_to_contribute: Vec<(TxIn, TransactionU16LenLimited)>, | ||
outputs_to_contribute: Vec<TxOut>, to_remote_value: u64, | ||
) -> (Self, Option<InteractiveTxMessageSend>) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While this is internal code, it would be nice to have some documentation including what the return values are for.
lightning/src/ln/interactivetxs.rs
Outdated
|
||
pub fn handle_tx_complete( | ||
&mut self, msg: &msgs::TxComplete, | ||
) -> Result<(Option<InteractiveTxMessageSend>, Option<Transaction>), AbortReason> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Likewise here. Though an enum might even be more appropriate given (None, None)
isn't possible.
lightning/src/ln/interactivetxs.rs
Outdated
outputs_a: Vec<TxOut>, | ||
inputs_b: Vec<(TxIn, TransactionU16LenLimited)>, | ||
outputs_b: Vec<TxOut>, | ||
expect_error: Option<AbortReason>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we also check who caused the error?
lightning/src/ln/interactivetxs.rs
Outdated
final_tx_b = final_tx; | ||
}, | ||
Err(abort_reason) => { | ||
assert_eq!(Some(abort_reason), session.expect_error); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we check that there are no more messages left to be sent? Otherwise, the error may occur at an unexpected time but not cause the test to fail. Maybe a break
would suffice if you take
the expect_error
?
lightning/src/ln/interactivetxs.rs
Outdated
StateMachine::ReceivedTxComplete(_) => { | ||
let msg_send = self.maybe_send_message()?; | ||
let negotiated_tx = match &self.state_machine { | ||
StateMachine::NegotiationComplete(s) => Some(s.0.clone()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using Option<&Transaction>
to avoid cloning if possible at the call site.
f279112
to
316d860
Compare
Latest changes. I'll look investigate some of the outstanding feedback I didn't include here for a follow-up. $ git diff-tree -U1 f279112 316d860
diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs
index 5b26a75e..9bb32d5b 100644
--- a/lightning/src/ln/interactivetxs.rs
+++ b/lightning/src/ln/interactivetxs.rs
@@ -42,2 +42,3 @@ trait SerialIdExt {
fn is_for_initiator(&self) -> bool;
+ fn is_for_non_initiator(&self) -> bool;
}
@@ -48,2 +49,6 @@ impl SerialIdExt for SerialId {
}
+
+ fn is_for_non_initiator(&self) -> bool {
+ !self.is_for_initiator()
+ }
}
@@ -87,3 +92,3 @@ struct NegotiationContext {
feerate_sat_per_kw: u32,
- to_remote_value: u64,
+ to_remote_value_satoshis: u64,
}
@@ -93,3 +98,7 @@ impl NegotiationContext {
// A received `SerialId`'s parity must match the role of the counterparty.
- self.holder_is_initiator == !serial_id.is_for_initiator()
+ self.holder_is_initiator == serial_id.is_for_non_initiator()
+ }
+
+ fn total_input_and_output_count(&self) -> usize {
+ self.inputs.len().saturating_add(self.outputs.len())
}
@@ -328,3 +337,3 @@ impl NegotiationContext {
// to the total input value.
- if counterparty_inputs_value.saturating_add(self.to_remote_value)
+ if counterparty_inputs_value.saturating_add(self.to_remote_value_satoshis)
< counterparty_outputs_value
@@ -405,9 +414,9 @@ impl NegotiationContext {
// +------------+ +------------------+---- Holder state after message sent/received ----+
-// | |--(1)- tx_add_input ---->| | MsgSentChange +
+// | |--(1)- tx_add_input ---->| | SentChangeMsg +
// | |<-(2)- tx_complete ------| | ReceivedTxComplete +
-// | |--(3)- tx_add_output --->| | MsgSentChange +
+// | |--(3)- tx_add_output --->| | SentChangeMsg +
// | |<-(4)- tx_complete ------| | ReceivedTxComplete +
-// | |--(5)- tx_add_input ---->| | MsgSentChange +
+// | |--(5)- tx_add_input ---->| | SentChangeMsg +
// | Holder |<-(6)- tx_add_input -----| Counterparty | MsgReceivedChange +
-// | |--(7)- tx_remove_output >| | MsgSentChange +
+// | |--(7)- tx_remove_output >| | SentChangeMsg +
// | |<-(8)- tx_add_output ----| | MsgReceivedChange +
@@ -434,3 +443,3 @@ trait MsgReceivedState: State {
macro_rules! define_state {
- (MSG_SENT_STATE, $state: ident, $doc: expr) => {
+ (SENT_MSG_STATE, $state: ident, $doc: expr) => {
define_state!($state, NegotiationContext, $doc);
@@ -442,3 +451,3 @@ macro_rules! define_state {
};
- (MSG_RECEIVED_STATE, $state: ident, $doc: expr) => {
+ (RECEIVED_MSG_STATE, $state: ident, $doc: expr) => {
define_state!($state, NegotiationContext, $doc);
@@ -459,4 +468,4 @@ macro_rules! define_state {
define_state!(
- MSG_SENT_STATE,
- MsgSentChange,
+ SENT_MSG_STATE,
+ SentChangeMsg,
"We have sent a message to the counterparty that has affected our negotiation state."
@@ -464,3 +473,3 @@ define_state!(
define_state!(
- MSG_SENT_STATE,
+ SENT_MSG_STATE,
SentTxComplete,
@@ -469,3 +478,3 @@ define_state!(
define_state!(
- MSG_RECEIVED_STATE,
+ RECEIVED_MSG_STATE,
MsgReceivedChange,
@@ -474,3 +483,3 @@ define_state!(
define_state!(
- MSG_RECEIVED_STATE,
+ RECEIVED_MSG_STATE,
ReceivedTxComplete,
@@ -494,3 +503,3 @@ trait StateTransition<NewState: State, TransitionData> {
macro_rules! define_state_transitions {
- (MSG_SENT_STATE, [$(DATA $data: ty, TRANSITION $transition: ident),+]) => {
+ (SENT_MSG_STATE, [$(DATA $data: ty, TRANSITION $transition: ident),+]) => {
$(
@@ -505,9 +514,9 @@ macro_rules! define_state_transitions {
};
- (MSG_RECEIVED_STATE, [$(DATA $data: ty, TRANSITION $transition: ident),+]) => {
+ (RECEIVED_MSG_STATE, [$(DATA $data: ty, TRANSITION $transition: ident),+]) => {
$(
- impl<S: MsgReceivedState> StateTransition<MsgSentChange, $data> for S {
- fn transition(self, data: $data) -> StateTransitionResult<MsgSentChange> {
+ impl<S: MsgReceivedState> StateTransition<SentChangeMsg, $data> for S {
+ fn transition(self, data: $data) -> StateTransitionResult<SentChangeMsg> {
let mut context = self.into_negotiation_context();
context.$transition(data);
- Ok(MsgSentChange(context))
+ Ok(SentChangeMsg(context))
}
@@ -535,3 +544,3 @@ macro_rules! define_state_transitions {
// to respond.
-define_state_transitions!(MSG_SENT_STATE, [
+define_state_transitions!(SENT_MSG_STATE, [
DATA &msgs::TxAddInput, TRANSITION received_tx_add_input,
@@ -543,3 +552,3 @@ define_state_transitions!(MSG_SENT_STATE, [
// respond.
-define_state_transitions!(MSG_RECEIVED_STATE, [
+define_state_transitions!(RECEIVED_MSG_STATE, [
DATA &msgs::TxAddInput, TRANSITION sent_tx_add_input,
@@ -549,3 +558,3 @@ define_state_transitions!(MSG_RECEIVED_STATE, [
]);
-define_state_transitions!(TX_COMPLETE, MsgSentChange, ReceivedTxComplete);
+define_state_transitions!(TX_COMPLETE, SentChangeMsg, ReceivedTxComplete);
define_state_transitions!(TX_COMPLETE, MsgReceivedChange, SentTxComplete);
@@ -555,3 +564,3 @@ enum StateMachine {
Indeterminate,
- MsgSentChange(MsgSentChange),
+ SentChangeMsg(SentChangeMsg),
MsgReceivedChange(MsgReceivedChange),
@@ -591,3 +600,3 @@ impl StateMachine {
feerate_sat_per_kw: u32, is_initiator: bool, tx_locktime: AbsoluteLockTime,
- to_remote_value: u64,
+ to_remote_value_satoshis: u64,
) -> Self {
@@ -602,3 +611,3 @@ impl StateMachine {
feerate_sat_per_kw,
- to_remote_value,
+ to_remote_value_satoshis,
};
@@ -607,3 +616,3 @@ impl StateMachine {
} else {
- Self::MsgSentChange(MsgSentChange(context))
+ Self::SentChangeMsg(SentChangeMsg(context))
}
@@ -613,7 +622,7 @@ impl StateMachine {
define_state_machine_transitions!(sent_tx_add_input, &msgs::TxAddInput, [
- FROM MsgReceivedChange, TO MsgSentChange,
- FROM ReceivedTxComplete, TO MsgSentChange
+ FROM MsgReceivedChange, TO SentChangeMsg,
+ FROM ReceivedTxComplete, TO SentChangeMsg
]);
define_state_machine_transitions!(received_tx_add_input, &msgs::TxAddInput, [
- FROM MsgSentChange, TO MsgReceivedChange,
+ FROM SentChangeMsg, TO MsgReceivedChange,
FROM SentTxComplete, TO MsgReceivedChange
@@ -623,7 +632,7 @@ impl StateMachine {
define_state_machine_transitions!(sent_tx_add_output, &msgs::TxAddOutput, [
- FROM MsgReceivedChange, TO MsgSentChange,
- FROM ReceivedTxComplete, TO MsgSentChange
+ FROM MsgReceivedChange, TO SentChangeMsg,
+ FROM ReceivedTxComplete, TO SentChangeMsg
]);
define_state_machine_transitions!(received_tx_add_output, &msgs::TxAddOutput, [
- FROM MsgSentChange, TO MsgReceivedChange,
+ FROM SentChangeMsg, TO MsgReceivedChange,
FROM SentTxComplete, TO MsgReceivedChange
@@ -633,7 +642,7 @@ impl StateMachine {
define_state_machine_transitions!(sent_tx_remove_input, &msgs::TxRemoveInput, [
- FROM MsgReceivedChange, TO MsgSentChange,
- FROM ReceivedTxComplete, TO MsgSentChange
+ FROM MsgReceivedChange, TO SentChangeMsg,
+ FROM ReceivedTxComplete, TO SentChangeMsg
]);
define_state_machine_transitions!(received_tx_remove_input, &msgs::TxRemoveInput, [
- FROM MsgSentChange, TO MsgReceivedChange,
+ FROM SentChangeMsg, TO MsgReceivedChange,
FROM SentTxComplete, TO MsgReceivedChange
@@ -643,7 +652,7 @@ impl StateMachine {
define_state_machine_transitions!(sent_tx_remove_output, &msgs::TxRemoveOutput, [
- FROM MsgReceivedChange, TO MsgSentChange,
- FROM ReceivedTxComplete, TO MsgSentChange
+ FROM MsgReceivedChange, TO SentChangeMsg,
+ FROM ReceivedTxComplete, TO SentChangeMsg
]);
define_state_machine_transitions!(received_tx_remove_output, &msgs::TxRemoveOutput, [
- FROM MsgSentChange, TO MsgReceivedChange,
+ FROM SentChangeMsg, TO MsgReceivedChange,
FROM SentTxComplete, TO MsgReceivedChange
@@ -657,3 +666,3 @@ impl StateMachine {
define_state_machine_transitions!(received_tx_complete, &msgs::TxComplete, [
- FROM MsgSentChange, TO ReceivedTxComplete,
+ FROM SentChangeMsg, TO ReceivedTxComplete,
FROM SentTxComplete, TO NegotiationComplete
@@ -701,7 +710,21 @@ where
+pub enum HandleTxCompleteValue {
+ SendTxMessage(InteractiveTxMessageSend),
+ SendTxComplete((InteractiveTxMessageSend, Transaction)),
+ NegotiationComplete(Transaction),
+}
+
impl InteractiveTxConstructor {
+ /// Instantiates a new `InteractiveTxConstructor`.
+ ///
+ /// If this is for a dual_funded channel then the `to_remote_value_satoshis` parameter should be set
+ /// to zero.
+ ///
+ /// A tuple is returned containing the newly instantiate `InteractiveTxConstructor` and optionally
+ /// an initial wrapped `Tx_` message which the holder needs to send to the counterparty.
pub fn new<ES: Deref>(
entropy_source: &ES, channel_id: ChannelId, feerate_sat_per_kw: u32, is_initiator: bool,
- tx_locktime: AbsoluteLockTime, inputs_to_contribute: Vec<(TxIn, TransactionU16LenLimited)>,
- outputs_to_contribute: Vec<TxOut>, to_remote_value: u64,
+ funding_tx_locktime: AbsoluteLockTime,
+ inputs_to_contribute: Vec<(TxIn, TransactionU16LenLimited)>,
+ outputs_to_contribute: Vec<TxOut>, to_remote_value_satoshis: u64,
) -> (Self, Option<InteractiveTxMessageSend>)
@@ -710,4 +733,8 @@ impl InteractiveTxConstructor {
{
- let state_machine =
- StateMachine::new(feerate_sat_per_kw, is_initiator, tx_locktime, to_remote_value);
+ let state_machine = StateMachine::new(
+ feerate_sat_per_kw,
+ is_initiator,
+ funding_tx_locktime,
+ to_remote_value_satoshis,
+ );
let mut inputs_to_contribute: Vec<(SerialId, TxIn, TransactionU16LenLimited)> =
@@ -811,3 +838,3 @@ impl InteractiveTxConstructor {
&mut self, msg: &msgs::TxComplete,
- ) -> Result<(Option<InteractiveTxMessageSend>, Option<Transaction>), AbortReason> {
+ ) -> Result<HandleTxCompleteValue, AbortReason> {
do_state_transition!(self, received_tx_complete, msg)?;
@@ -816,5 +843,9 @@ impl InteractiveTxConstructor {
let msg_send = self.maybe_send_message()?;
- let negotiated_tx = match &self.state_machine {
- StateMachine::NegotiationComplete(s) => Some(s.0.clone()),
- StateMachine::MsgSentChange(_) => None, // We either had an input or output to contribute.
+ return match &self.state_machine {
+ StateMachine::NegotiationComplete(s) => {
+ Ok(HandleTxCompleteValue::SendTxComplete((msg_send, s.0.clone())))
+ },
+ StateMachine::SentChangeMsg(_) => {
+ Ok(HandleTxCompleteValue::SendTxMessage(msg_send))
+ }, // We either had an input or output to contribute.
_ => {
@@ -824,5 +855,6 @@ impl InteractiveTxConstructor {
};
- Ok((Some(msg_send), negotiated_tx))
},
- StateMachine::NegotiationComplete(s) => Ok((None, Some(s.0.clone()))),
+ StateMachine::NegotiationComplete(s) => {
+ Ok(HandleTxCompleteValue::NegotiationComplete(s.0.clone()))
+ },
_ => {
@@ -843,4 +875,4 @@ mod tests {
use crate::ln::interactivetxs::{
- generate_holder_serial_id, AbortReason, InteractiveTxConstructor, InteractiveTxMessageSend,
- MAX_INPUTS_OUTPUTS_COUNT, MAX_RECEIVED_TX_ADD_INPUT_COUNT,
+ generate_holder_serial_id, AbortReason, HandleTxCompleteValue, InteractiveTxConstructor,
+ InteractiveTxMessageSend, MAX_INPUTS_OUTPUTS_COUNT, MAX_RECEIVED_TX_ADD_INPUT_COUNT,
MAX_RECEIVED_TX_ADD_OUTPUT_COUNT,
@@ -888,2 +920,12 @@ mod tests {
+ #[derive(Debug, PartialEq, Eq)]
+ enum ErrorCulprit {
+ NodeA,
+ NodeB,
+ // Some error values are only checked at the end of the negotiation and are not easy to attribute
+ // to a particular party. Both parties would indicate an `AbortReason` in this case.
+ // e.g. Exceeded max inputs and outputs after negotiation.
+ Indeterminate,
+ }
+
struct TestSession {
@@ -893,3 +935,3 @@ mod tests {
outputs_b: Vec<TxOut>,
- expect_error: Option<AbortReason>,
+ expect_error: Option<(AbortReason, ErrorCulprit)>,
}
@@ -948,3 +990,11 @@ mod tests {
InteractiveTxMessageSend::TxComplete(msg) => {
- for_constructor.handle_tx_complete(&msg)
+ for_constructor.handle_tx_complete(&msg).map(|value| match value {
+ HandleTxCompleteValue::SendTxMessage(msg_send) => {
+ (Some(msg_send), None)
+ },
+ HandleTxCompleteValue::SendTxComplete((msg_send, tx)) => {
+ (Some(msg_send), Some(tx))
+ },
+ HandleTxCompleteValue::NegotiationComplete(tx) => (None, Some(tx)),
+ })
},
@@ -966,3 +1016,10 @@ mod tests {
Err(abort_reason) => {
- assert_eq!(Some(abort_reason), session.expect_error);
+ let error_culprit = match abort_reason {
+ AbortReason::ExceededNumberOfInputsOrOutputs => {
+ ErrorCulprit::Indeterminate
+ },
+ _ => ErrorCulprit::NodeA,
+ };
+ assert_eq!(Some((abort_reason, error_culprit)), session.expect_error);
+ assert!(message_send_b.is_none());
return;
@@ -978,3 +1035,10 @@ mod tests {
Err(abort_reason) => {
- assert_eq!(Some(abort_reason), session.expect_error);
+ let error_culprit = match abort_reason {
+ AbortReason::ExceededNumberOfInputsOrOutputs => {
+ ErrorCulprit::Indeterminate
+ },
+ _ => ErrorCulprit::NodeB,
+ };
+ assert_eq!(Some((abort_reason, error_culprit)), session.expect_error);
+ assert!(message_send_a.is_none());
return;
@@ -1102,3 +1166,3 @@ mod tests {
outputs_b: vec![],
- expect_error: Some(AbortReason::InsufficientFees),
+ expect_error: Some((AbortReason::InsufficientFees, ErrorCulprit::NodeA)),
});
@@ -1110,3 +1174,3 @@ mod tests {
outputs_b: vec![],
- expect_error: Some(AbortReason::OutputsValueExceedsInputsValue),
+ expect_error: Some((AbortReason::OutputsValueExceedsInputsValue, ErrorCulprit::NodeA)),
});
@@ -1126,3 +1190,3 @@ mod tests {
outputs_b: vec![],
- expect_error: Some(AbortReason::InsufficientFees),
+ expect_error: Some((AbortReason::InsufficientFees, ErrorCulprit::NodeA)),
});
@@ -1134,3 +1198,3 @@ mod tests {
outputs_b: generate_outputs(&[100_000]),
- expect_error: Some(AbortReason::InsufficientFees),
+ expect_error: Some((AbortReason::InsufficientFees, ErrorCulprit::NodeB)),
});
@@ -1171,3 +1235,3 @@ mod tests {
outputs_b: vec![],
- expect_error: Some(AbortReason::PrevTxOutInvalid),
+ expect_error: Some((AbortReason::PrevTxOutInvalid, ErrorCulprit::NodeA)),
});
@@ -1185,3 +1249,3 @@ mod tests {
outputs_b: vec![],
- expect_error: Some(AbortReason::IncorrectInputSequenceValue),
+ expect_error: Some((AbortReason::IncorrectInputSequenceValue, ErrorCulprit::NodeA)),
});
@@ -1198,3 +1262,3 @@ mod tests {
outputs_b: vec![],
- expect_error: Some(AbortReason::PrevTxOutInvalid),
+ expect_error: Some((AbortReason::PrevTxOutInvalid, ErrorCulprit::NodeA)),
});
@@ -1211,3 +1275,3 @@ mod tests {
outputs_b: vec![],
- expect_error: Some(AbortReason::PrevTxOutInvalid),
+ expect_error: Some((AbortReason::PrevTxOutInvalid, ErrorCulprit::NodeB)),
});
@@ -1219,3 +1283,3 @@ mod tests {
outputs_b: vec![],
- expect_error: Some(AbortReason::ReceivedTooManyTxAddInputs),
+ expect_error: Some((AbortReason::ReceivedTooManyTxAddInputs, ErrorCulprit::NodeA)),
});
@@ -1229,3 +1293,3 @@ mod tests {
outputs_b: vec![],
- expect_error: Some(AbortReason::DuplicateSerialId),
+ expect_error: Some((AbortReason::DuplicateSerialId, ErrorCulprit::NodeA)),
},
@@ -1239,3 +1303,3 @@ mod tests {
outputs_b: vec![],
- expect_error: Some(AbortReason::ReceivedTooManyTxAddOutputs),
+ expect_error: Some((AbortReason::ReceivedTooManyTxAddOutputs, ErrorCulprit::NodeA)),
});
@@ -1247,3 +1311,3 @@ mod tests {
outputs_b: vec![],
- expect_error: Some(AbortReason::BelowDustLimit),
+ expect_error: Some((AbortReason::BelowDustLimit, ErrorCulprit::NodeA)),
});
@@ -1255,3 +1319,3 @@ mod tests {
outputs_b: vec![],
- expect_error: Some(AbortReason::ExceededMaximumSatsAllowed),
+ expect_error: Some((AbortReason::ExceededMaximumSatsAllowed, ErrorCulprit::NodeA)),
});
@@ -1263,3 +1327,3 @@ mod tests {
outputs_b: vec![],
- expect_error: Some(AbortReason::InvalidOutputScript),
+ expect_error: Some((AbortReason::InvalidOutputScript, ErrorCulprit::NodeA)),
});
@@ -1273,3 +1337,3 @@ mod tests {
outputs_b: vec![],
- expect_error: Some(AbortReason::DuplicateSerialId),
+ expect_error: Some((AbortReason::DuplicateSerialId, ErrorCulprit::NodeA)),
},
@@ -1284,3 +1348,3 @@ mod tests {
outputs_b: vec![],
- expect_error: Some(AbortReason::OutputsValueExceedsInputsValue),
+ expect_error: Some((AbortReason::OutputsValueExceedsInputsValue, ErrorCulprit::NodeA)),
});
@@ -1293,3 +1357,6 @@ mod tests {
outputs_b: vec![],
- expect_error: Some(AbortReason::ExceededNumberOfInputsOrOutputs),
+ expect_error: Some((
+ AbortReason::ExceededNumberOfInputsOrOutputs,
+ ErrorCulprit::Indeterminate,
+ )),
});
@@ -1301,3 +1368,6 @@ mod tests {
outputs_b: vec![],
- expect_error: Some(AbortReason::ExceededNumberOfInputsOrOutputs),
+ expect_error: Some((
+ AbortReason::ExceededNumberOfInputsOrOutputs,
+ ErrorCulprit::Indeterminate,
+ )),
}); |
…tructor` This implements the interactive construction protocol described at https://github.com/lightning/bolts/blob/78e5a6b066d3a8e235931dfc06aa325337874749/02-peer-protocol.md?plain=1#L92. Our implementation includes a state machine with typed states and transitions to ensure consumers have compile-time assurances that the protocol is upheld. States are tracked as in the `StateMachine` enum and can take on all possible states during the negotiation. The states are further divided into two categories, namely by the two traits they implement, either `ReceivedMsgState` or `SentMsgState`. The defined `StateTransitions` enforce the transitions that `ReceivedMsgState`, `SentMsgState`, and the `_TxComplete`s can go through. Co-authored-by: Wilmer Paulino <[email protected]> Co-authored-by: Duncan Dean <[email protected]> Co-authored-by: Jurvis Tan <[email protected]>
$ git diff-tree -U1 316d860 c56198ad
diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs
index 9bb32d5b..94311a36 100644
--- a/lightning/src/ln/interactivetxs.rs
+++ b/lightning/src/ln/interactivetxs.rs
@@ -419,5 +419,5 @@ impl NegotiationContext {
// | |--(5)- tx_add_input ---->| | SentChangeMsg +
-// | Holder |<-(6)- tx_add_input -----| Counterparty | MsgReceivedChange +
+// | Holder |<-(6)- tx_add_input -----| Counterparty | ReceivedChangeMsg +
// | |--(7)- tx_remove_output >| | SentChangeMsg +
-// | |<-(8)- tx_add_output ----| | MsgReceivedChange +
+// | |<-(8)- tx_add_output ----| | ReceivedChangeMsg +
// | |--(9)- tx_complete ----->| | SentTxComplete +
@@ -431,3 +431,3 @@ trait State {}
/// a response.
-trait MsgSentState: State {
+trait SentMsgState: State {
fn into_negotiation_context(self) -> NegotiationContext;
@@ -436,3 +436,3 @@ trait MsgSentState: State {
/// Category of states that our counterparty has put us in after we receive a message from them.
-trait MsgReceivedState: State {
+trait ReceivedMsgState: State {
fn into_negotiation_context(self) -> NegotiationContext;
@@ -445,3 +445,3 @@ macro_rules! define_state {
define_state!($state, NegotiationContext, $doc);
- impl MsgSentState for $state {
+ impl SentMsgState for $state {
fn into_negotiation_context(self) -> NegotiationContext {
@@ -453,3 +453,3 @@ macro_rules! define_state {
define_state!($state, NegotiationContext, $doc);
- impl MsgReceivedState for $state {
+ impl ReceivedMsgState for $state {
fn into_negotiation_context(self) -> NegotiationContext {
@@ -479,3 +479,3 @@ define_state!(
RECEIVED_MSG_STATE,
- MsgReceivedChange,
+ ReceivedChangeMsg,
"We have received a message from the counterparty that has affected our negotiation state."
@@ -505,7 +505,7 @@ macro_rules! define_state_transitions {
$(
- impl<S: MsgSentState> StateTransition<MsgReceivedChange, $data> for S {
- fn transition(self, data: $data) -> StateTransitionResult<MsgReceivedChange> {
+ impl<S: SentMsgState> StateTransition<ReceivedChangeMsg, $data> for S {
+ fn transition(self, data: $data) -> StateTransitionResult<ReceivedChangeMsg> {
let mut context = self.into_negotiation_context();
context.$transition(data)?;
- Ok(MsgReceivedChange(context))
+ Ok(ReceivedChangeMsg(context))
}
@@ -516,3 +516,3 @@ macro_rules! define_state_transitions {
$(
- impl<S: MsgReceivedState> StateTransition<SentChangeMsg, $data> for S {
+ impl<S: ReceivedMsgState> StateTransition<SentChangeMsg, $data> for S {
fn transition(self, data: $data) -> StateTransitionResult<SentChangeMsg> {
@@ -559,3 +559,3 @@ define_state_transitions!(RECEIVED_MSG_STATE, [
define_state_transitions!(TX_COMPLETE, SentChangeMsg, ReceivedTxComplete);
-define_state_transitions!(TX_COMPLETE, MsgReceivedChange, SentTxComplete);
+define_state_transitions!(TX_COMPLETE, ReceivedChangeMsg, SentTxComplete);
@@ -565,3 +565,3 @@ enum StateMachine {
SentChangeMsg(SentChangeMsg),
- MsgReceivedChange(MsgReceivedChange),
+ ReceivedChangeMsg(ReceivedChangeMsg),
SentTxComplete(SentTxComplete),
@@ -614,3 +614,3 @@ impl StateMachine {
if is_initiator {
- Self::MsgReceivedChange(MsgReceivedChange(context))
+ Self::ReceivedChangeMsg(ReceivedChangeMsg(context))
} else {
@@ -622,3 +622,3 @@ impl StateMachine {
define_state_machine_transitions!(sent_tx_add_input, &msgs::TxAddInput, [
- FROM MsgReceivedChange, TO SentChangeMsg,
+ FROM ReceivedChangeMsg, TO SentChangeMsg,
FROM ReceivedTxComplete, TO SentChangeMsg
@@ -626,4 +626,4 @@ impl StateMachine {
define_state_machine_transitions!(received_tx_add_input, &msgs::TxAddInput, [
- FROM SentChangeMsg, TO MsgReceivedChange,
- FROM SentTxComplete, TO MsgReceivedChange
+ FROM SentChangeMsg, TO ReceivedChangeMsg,
+ FROM SentTxComplete, TO ReceivedChangeMsg
]);
@@ -632,3 +632,3 @@ impl StateMachine {
define_state_machine_transitions!(sent_tx_add_output, &msgs::TxAddOutput, [
- FROM MsgReceivedChange, TO SentChangeMsg,
+ FROM ReceivedChangeMsg, TO SentChangeMsg,
FROM ReceivedTxComplete, TO SentChangeMsg
@@ -636,4 +636,4 @@ impl StateMachine {
define_state_machine_transitions!(received_tx_add_output, &msgs::TxAddOutput, [
- FROM SentChangeMsg, TO MsgReceivedChange,
- FROM SentTxComplete, TO MsgReceivedChange
+ FROM SentChangeMsg, TO ReceivedChangeMsg,
+ FROM SentTxComplete, TO ReceivedChangeMsg
]);
@@ -642,3 +642,3 @@ impl StateMachine {
define_state_machine_transitions!(sent_tx_remove_input, &msgs::TxRemoveInput, [
- FROM MsgReceivedChange, TO SentChangeMsg,
+ FROM ReceivedChangeMsg, TO SentChangeMsg,
FROM ReceivedTxComplete, TO SentChangeMsg
@@ -646,4 +646,4 @@ impl StateMachine {
define_state_machine_transitions!(received_tx_remove_input, &msgs::TxRemoveInput, [
- FROM SentChangeMsg, TO MsgReceivedChange,
- FROM SentTxComplete, TO MsgReceivedChange
+ FROM SentChangeMsg, TO ReceivedChangeMsg,
+ FROM SentTxComplete, TO ReceivedChangeMsg
]);
@@ -652,3 +652,3 @@ impl StateMachine {
define_state_machine_transitions!(sent_tx_remove_output, &msgs::TxRemoveOutput, [
- FROM MsgReceivedChange, TO SentChangeMsg,
+ FROM ReceivedChangeMsg, TO SentChangeMsg,
FROM ReceivedTxComplete, TO SentChangeMsg
@@ -656,4 +656,4 @@ impl StateMachine {
define_state_machine_transitions!(received_tx_remove_output, &msgs::TxRemoveOutput, [
- FROM SentChangeMsg, TO MsgReceivedChange,
- FROM SentTxComplete, TO MsgReceivedChange
+ FROM SentChangeMsg, TO ReceivedChangeMsg,
+ FROM SentTxComplete, TO ReceivedChangeMsg
]);
@@ -662,3 +662,3 @@ impl StateMachine {
define_state_machine_transitions!(sent_tx_complete, &msgs::TxComplete, [
- FROM MsgReceivedChange, TO SentTxComplete,
+ FROM ReceivedChangeMsg, TO SentTxComplete,
FROM ReceivedTxComplete, TO NegotiationComplete
@@ -712,3 +712,3 @@ pub enum HandleTxCompleteValue {
SendTxMessage(InteractiveTxMessageSend),
- SendTxComplete((InteractiveTxMessageSend, Transaction)),
+ SendTxComplete(InteractiveTxMessageSend, Transaction),
NegotiationComplete(Transaction),
@@ -845,3 +845,3 @@ impl InteractiveTxConstructor {
StateMachine::NegotiationComplete(s) => {
- Ok(HandleTxCompleteValue::SendTxComplete((msg_send, s.0.clone())))
+ Ok(HandleTxCompleteValue::SendTxComplete(msg_send, s.0.clone()))
},
@@ -994,3 +994,3 @@ mod tests {
},
- HandleTxCompleteValue::SendTxComplete((msg_send, tx)) => {
+ HandleTxCompleteValue::SendTxComplete(msg_send, tx) => {
(Some(msg_send), Some(tx)) |
316d860
to
c56198a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a few comments here I think we need to handle in a followup, but its long since time to unblock the interactive constructor work here, so we should land it!
return Err(AbortReason::ExceededNumberOfInputsOrOutputs); | ||
} | ||
|
||
// TODO: How do we enforce their fees cover the witness without knowing its expected length? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have some way to accomplish this? Or should we return the increase (or decrease) in counterparty value from the builder somehow so that callers can do this check?
fee_for_weight(self.feerate_sat_per_kw, tx_common_fields_weight); | ||
required_counterparty_contribution_fee += tx_common_fields_fee; | ||
} | ||
if counterparty_fees_contributed < required_counterparty_contribution_fee { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wont this always fail for a splice-out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm thinking we need to distinguish shared vs unilateral inputs and outputs within the constructor to make these checks easier to reason about. I'll propose something in the followup I'm busy with.
Creating an interactive tx constructor based on the interactive-tx spec.
Implementation
We created an
InteractiveTxConstructor
, which itself houses a state machine (state_machine
), with the temporary channel id (channel_id
), alongside a set of inputs and outputs that a user would need to define up front prior to commencing negotiation.Given the complexity of number of states and possible transitions, we use types wherever we can to achieve compile-time assurances that we never allow any invalid state transitions to occur.
Tracking States
StateMachine
itself is an enum with a few states it could take on:LocalChange
– state we land when we send a message to our counterparty.RemoteChange
– state we land when we receive some arbitrary message from our counterparty that is awaiting our responseLocalTxComplete
– When we have sent atx_complete
message and are waiting for the counterparty's response.RemoteTxComplete
– When our counterparty has sent ustx_complete
and is waiting for ours/additional inputs outputs.NegotiationComplete
– We exchanged consecutivetx_complete
with the counterparty.NegotiationAborted
– The negotiation has failed and cannot be continued.States and Traits
These states implement traits that we leverage on later to define valid state transitions. The traits (and their implementors) are:
LocalState (trait) – Category of states where we have sent some message to the counterparty, and we are waiting for a response (
LocalChange
,LocalTxComplete
)RemoteState (trait) – Category of states that our counterparty has put us in after we receive a message from them. (
RemoteChange
,RemoteTxComplete
).You may lookup these definitions by searching usages of the
define_state!
macro.Defining
StateTransition
sUsing the groupings we defined above, that allows us to give them a common set of behaviors without needing to repeat ourselves.
LocalStates can... handle transitions upon receiving
TxAddInput
,TxRemoveInput
,TxAddOutput
,TxRemoveOutput
and they transition us toRemoteChange
.RemoteStates can... handle the same transitions as
LocalState
s, except they transition back toLocalState
.And some additional state transitions:
LocalChange can... handle transitions upon receiving
TxComplete
, transitions us toRemoteTxComplete
RemoteChange can... handle transitions upon receiving
TxComplete
, transitions us toLocalTxComplete
We define these possible state transitions using the
define_state_transitions!
macroDefining State Machine Transitions
StateTransition
s define transitions between specific states for local and remote states, depending on the data type it receives. However, we still need to be able to handle transitions for the entire state machine. Rather than handling data-specific responses, we still need to orchestrate overall state transitions.That's what this section is about.
For this, we lean on the
define_state_transitions!
macro, whose calls define the following behaviours:When state machine is in a LocalChange or RemoteChange state:
For each message type (
tx_add_input
,tx_add_output
,tx_remove_input
,tx_remove_output
), there are two sets of transitions:RemoteChange
toLocalChange
, and fromRemoteTxComplete
toLocalChange
for local transitions (handled by $to_local_transition).LocalChange
toRemoteChange
, and fromLocalTxComplete
toRemoteChange
for remote transitions (handled by $to_remote_transition).When the state machine is in RemoteChange:
Upon receiving
tx_complete
, it should transition toLocalTxComplete
. If the state isRemoteTxComplete
, it should transition toNegotiationComplete
When the state machine is in LocalChange:
Upon receiving
tx_complete
, it should transition toRemoteTxComplete
. If the state isLocalTxComplete
, it should transition toNegotiationComplete
Testing
Our test paradigm prioritizes ensuring that we abort when the spec says we should. Based on our implementation of it, the spec only prescribes abort behaviour under the circumstances where peers could land in a scenario where the final transaction would be invalid. For example, it explicitly tells us to abort when a user provides an input/output with things like the wrong sequence type, a final transaction that is too heavy, or has too many inputs and outputs, etc.
However, it doesn't really say what Alice should do if, for example, Bob sends
tx_complete
to Alice, but sendstx_add_input
(or any other message) again before Alice responds. In those cases of receiving unexpected messages, in favor of caution, we have made our own choice to abort the negotiation.In other words, we test the overall
InteractiveTxConstructor
outcomes, but do not currently provide coverage/assertions of the individual state transitions themselves inInteractiveTxStateMachine
.Additional Notes to Reviewers
Our testing paradigm currently has a few blind spots: