Interactive Transaction Construction #2419
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jurvis
commented
Jul 17, 2023
jurvis
force-pushed
the
2023-03-interactivetxs
branch
from
690418c
to
0dd26b9
Compare
jurvis
commented
Jul 17, 2023
|
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 |
ariard
reviewed
Jul 19, 2023
There was a problem hiding this comment.
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. |
6 tasks
|
Already reviewed back the dual-funding spec proposal and I’ll have a look on the splicing one. Still the gist to look on. |
optout21
reviewed
Aug 30, 2023
optout21
reviewed
Aug 30, 2023
|
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. |
jurvis
force-pushed
the
2023-03-interactivetxs
branch
2 times, most recently
from
1b17e0f
to
477c620
Compare
jurvis
changed the title
jurvis
force-pushed
the
2023-03-interactivetxs
branch
from
477c620
to
94d8200
Compare
optout21
reviewed
Sep 21, 2023
|
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) |
optout21
reviewed
Sep 28, 2023
4 tasks
dunxen
reviewed
Oct 5, 2023
dunxen
reviewed
Oct 12, 2023
jurvis
force-pushed
the
2023-03-interactivetxs
branch
from
059af71
to
bce504b
Compare
dunxen
force-pushed
the
2023-03-interactivetxs
branch
from
f08c112
to
1cedde6
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
@coderabbitai pause |
dunxen
force-pushed
the
2023-03-interactivetxs
branch
from
1cedde6
to
86777ee
Compare
jkczyz
reviewed
Mar 7, 2024
lightning/src/ln/interactivetxs.rs
Outdated
Comment on lines
266
to
320
| 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.
dunxen
force-pushed
the
2023-03-interactivetxs
branch
3 times, most recently
from
95bef6c
to
5e25aad
Compare
TheBlueMatt
reviewed
Mar 12, 2024
There was a problem hiding this comment.
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.
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.
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 |
dunxen
force-pushed
the
2023-03-interactivetxs
branch
from
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. |
dunxen
force-pushed
the
2023-03-interactivetxs
branch
from
6869c49
to
f279112
Compare
jkczyz
previously approved these changes
Mar 13, 2024
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.
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.
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.
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.
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.
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.
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.
Consider using Option<&Transaction> to avoid cloning if possible at the call site.
dunxen
force-pushed
the
2023-03-interactivetxs
branch
from
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,
+ )),
}); |
jkczyz
reviewed
Mar 14, 2024
…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)) |
dunxen
force-pushed
the
2023-03-interactivetxs
branch
from
316d860
to
c56198a
Compare
jkczyz
approved these changes
Mar 14, 2024
TheBlueMatt
approved these changes
Mar 20, 2024
| 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.
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.
Wont this always fail for a splice-out?
There was a problem hiding this comment.
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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
StateMachineitself 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_completemessage and are waiting for the counterparty's response.RemoteTxComplete– When our counterparty has sent ustx_completeand is waiting for ours/additional inputs outputs.NegotiationComplete– We exchanged consecutivetx_completewith 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
StateTransitionsUsing 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,TxRemoveOutputand they transition us toRemoteChange.RemoteStates can... handle the same transitions as
LocalStates, except they transition back toLocalState.And some additional state transitions:
LocalChange can... handle transitions upon receiving
TxComplete, transitions us toRemoteTxCompleteRemoteChange can... handle transitions upon receiving
TxComplete, transitions us toLocalTxCompleteWe define these possible state transitions using the
define_state_transitions!macroDefining State Machine Transitions
StateTransitions 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:RemoteChangetoLocalChange, and fromRemoteTxCompletetoLocalChangefor local transitions (handled by $to_local_transition).LocalChangetoRemoteChange, and fromLocalTxCompletetoRemoteChangefor 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 toNegotiationCompleteWhen the state machine is in LocalChange:
Upon receiving
tx_complete, it should transition toRemoteTxComplete. If the state isLocalTxComplete, it should transition toNegotiationCompleteTesting
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_completeto 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
InteractiveTxConstructoroutcomes, 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: