From b8130d23a6ee0ce91c44728a67ea99e9a885ce28 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Tue, 27 Oct 2020 09:59:30 +1100 Subject: [PATCH 1/5] Only break if Bob has requested amounts already We don't want Bob to be able to crash us by sending an out of order message. Only break if Bob has not requested amounts. --- swap/src/alice.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/swap/src/alice.rs b/swap/src/alice.rs index 523fb3e1..2a5b8f66 100644 --- a/swap/src/alice.rs +++ b/swap/src/alice.rs @@ -51,10 +51,13 @@ pub async fn swap( swarm.send_amounts(channel, p); } OutEvent::Message0(msg) => { - debug!("Got message0 from Bob"); - // TODO: Do this in a more Rusty/functional way. - message0 = msg; - break; + // We don't want Bob to be able to crash us by sending an out of + // order message. Keep looping if Bob has not requested amounts. + if last_amounts.is_some() { + // TODO: We should verify the amounts and notify Bob if they have changed. + message0 = msg; + break; + } } other => panic!("Unexpected event: {:?}", other), }; From 8f5a989ad16679d0464b2cb3bdd2a4a46421b8cd Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Tue, 27 Oct 2020 10:05:20 +1100 Subject: [PATCH 2/5] Use 'amounts' for local varibale name --- swap/src/alice.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/swap/src/alice.rs b/swap/src/alice.rs index 2a5b8f66..775020dd 100644 --- a/swap/src/alice.rs +++ b/swap/src/alice.rs @@ -46,9 +46,11 @@ pub async fn swap( } OutEvent::Request(amounts::OutEvent::Btc { btc, channel }) => { debug!("Got request from Bob to swap {}", btc); - let p = calculate_amounts(btc); - last_amounts = Some(p); - swarm.send_amounts(channel, p); + let amounts = calculate_amounts(btc); + // TODO: We cache the last amounts returned, this needs improving along with + // verification of message 0. + last_amounts = Some(amounts); + swarm.send_amounts(channel, amounts); } OutEvent::Message0(msg) => { // We don't want Bob to be able to crash us by sending an out of From 6be5d64c1c7f5e743ed8062530b1922c24520f54 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Tue, 27 Oct 2020 10:05:52 +1100 Subject: [PATCH 3/5] Remove stale, ugly, type conversion --- swap/src/alice.rs | 4 ---- swap/src/bob.rs | 4 ---- 2 files changed, 8 deletions(-) diff --git a/swap/src/alice.rs b/swap/src/alice.rs index 775020dd..210a2451 100644 --- a/swap/src/alice.rs +++ b/swap/src/alice.rs @@ -70,10 +70,6 @@ pub async fn swap( None => unreachable!("should have amounts by here"), }; - let xmr = monero::Amount::from_piconero(xmr.as_piconero()); - // TODO: This should be the Amount exported by xmr_btc. - let btc = ::bitcoin::Amount::from_sat(btc.as_sat()); - // TODO: Pass this in using let rng = &mut OsRng; let state0 = State0::new( diff --git a/swap/src/bob.rs b/swap/src/bob.rs index 91e88cc1..1d7d08ad 100644 --- a/swap/src/bob.rs +++ b/swap/src/bob.rs @@ -69,10 +69,6 @@ where other => panic!("unexpected event: {:?}", other), }; - // FIXME: Too many `bitcoin` crates/modules. - let xmr = xmr_btc::monero::Amount::from_piconero(xmr.as_piconero()); - let btc = ::bitcoin::Amount::from_sat(btc.as_sat()); - // TODO: Pass this in using let rng = &mut OsRng; let state0 = State0::new( From 194a19cf1dd17cabda249e1a3bbaea3a332ca470 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Tue, 27 Oct 2020 10:09:53 +1100 Subject: [PATCH 4/5] Add todos --- swap/src/alice.rs | 4 ++++ swap/src/bob.rs | 7 ++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/swap/src/alice.rs b/swap/src/alice.rs index 210a2451..b0553bd3 100644 --- a/swap/src/alice.rs +++ b/swap/src/alice.rs @@ -28,6 +28,9 @@ use xmr_btc::{alice::State0, bob, monero}; pub type Swarm = libp2p::Swarm; +// TODO: After we have done some testing replace all the 'panic's with log +// statements or error returns. + // FIXME: This whole function is horrible, needs total re-write. pub async fn swap( listen: Multiaddr, @@ -83,6 +86,7 @@ pub async fn swap( ); swarm.set_state0(state0.clone()); + // TODO: Can we verify message 0 before calling this so we never fail? let state1 = state0.receive(message0).expect("failed to receive msg 0"); let (state2, channel) = match swarm.next().await { diff --git a/swap/src/bob.rs b/swap/src/bob.rs index 1d7d08ad..c563146e 100644 --- a/swap/src/bob.rs +++ b/swap/src/bob.rs @@ -83,8 +83,9 @@ where swarm.send_message0(alice.clone(), state0.next_message(rng)); let state1 = match swarm.next().await { OutEvent::Message0(msg) => { - state0.receive(&wallet, msg).await? // TODO: More graceful error - // handling. + // TODO: Verify the response message before calling receive() and handle any + // error gracefully. + state0.receive(&wallet, msg).await? } other => panic!("unexpected event: {:?}", other), }; @@ -92,7 +93,7 @@ where swarm.send_message1(alice.clone(), state1.next_message()); let state2 = match swarm.next().await { OutEvent::Message1(msg) => { - state1.receive(msg)? // TODO: More graceful error handling. + state1.receive(msg)? // TODO: Same as above. } other => panic!("unexpected event: {:?}", other), }; From b22f265cf345e894d555a9b7d33d04efc485e8c4 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Tue, 27 Oct 2020 10:13:59 +1100 Subject: [PATCH 5/5] Send back an empty response to Message2 Alice does not respond with anything when receiving message 2 from Bob. We don't want to leave Bob's request/response protocol waiting so send an empty response back. --- swap/src/alice/message2.rs | 8 +++++++- swap/src/bob.rs | 11 ++++------ swap/src/bob/message2.rs | 30 ++++++++++------------------ swap/src/network/request_response.rs | 1 + 4 files changed, 23 insertions(+), 27 deletions(-) diff --git a/swap/src/alice/message2.rs b/swap/src/alice/message2.rs index 57b43bd4..a8704c0d 100644 --- a/swap/src/alice/message2.rs +++ b/swap/src/alice/message2.rs @@ -71,11 +71,17 @@ impl NetworkBehaviourEventProcess> fn inject_event(&mut self, event: RequestResponseEvent) { match event { RequestResponseEvent::Message { - message: RequestResponseMessage::Request { request, .. }, + message: + RequestResponseMessage::Request { + request, channel, .. + }, .. } => match request { BobToAlice::Message2(msg) => { self.events.push_back(OutEvent::Msg(msg)); + // Send back empty response so that the request/response protocol completes. + let msg = AliceToBob::EmptyResponse; + self.rr.send_response(channel, msg); } other => debug!("got request: {:?}", other), }, diff --git a/swap/src/bob.rs b/swap/src/bob.rs index c563146e..a72380c9 100644 --- a/swap/src/bob.rs +++ b/swap/src/bob.rs @@ -21,7 +21,7 @@ use crate::{ peer_tracker::{self, PeerTracker}, transport, TokioExecutor, }, - Cmd, Rsp, SwapAmounts, PUNISH_TIMELOCK, REFUND_TIMELOCK, + Cmd, Never, Rsp, SwapAmounts, PUNISH_TIMELOCK, REFUND_TIMELOCK, }; use xmr_btc::{ alice, @@ -134,7 +134,6 @@ pub enum OutEvent { Amounts(SwapAmounts), Message0(alice::Message0), Message1(alice::Message1), - Message2(alice::Message2), } impl From for OutEvent { @@ -171,11 +170,9 @@ impl From for OutEvent { } } -impl From for OutEvent { - fn from(event: message2::OutEvent) -> Self { - match event { - message2::OutEvent::Msg(msg) => OutEvent::Message2(msg), - } +impl From for OutEvent { + fn from(_: Never) -> Self { + panic!("this never happens") } } diff --git a/swap/src/bob/message2.rs b/swap/src/bob/message2.rs index aa8be6bc..e8fa295b 100644 --- a/swap/src/bob/message2.rs +++ b/swap/src/bob/message2.rs @@ -7,28 +7,23 @@ use libp2p::{ NetworkBehaviour, PeerId, }; use std::{ - collections::VecDeque, task::{Context, Poll}, time::Duration, }; use tracing::{debug, error}; -use crate::network::request_response::{AliceToBob, BobToAlice, Codec, Protocol, TIMEOUT}; -use xmr_btc::{alice, bob}; - -#[derive(Debug)] -pub enum OutEvent { - Msg(alice::Message2), -} +use crate::{ + network::request_response::{AliceToBob, BobToAlice, Codec, Protocol, TIMEOUT}, + Never, +}; +use xmr_btc::bob; /// A `NetworkBehaviour` that represents sending message 2 to Alice. #[derive(NetworkBehaviour)] -#[behaviour(out_event = "OutEvent", poll_method = "poll")] +#[behaviour(out_event = "Never", poll_method = "poll")] #[allow(missing_debug_implementations)] pub struct Message2 { rr: RequestResponse, - #[behaviour(ignore)] - events: VecDeque, } impl Message2 { @@ -37,15 +32,13 @@ impl Message2 { let _id = self.rr.send_request(&alice, msg); } + // TODO: Do we need a custom implementation if we are not bubbling any out + // events? fn poll( &mut self, _: &mut Context<'_>, _: &mut impl PollParameters, - ) -> Poll, OutEvent>> { - if let Some(event) = self.events.pop_front() { - return Poll::Ready(NetworkBehaviourAction::GenerateEvent(event)); - } - + ) -> Poll, Never>> { Poll::Pending } } @@ -62,7 +55,6 @@ impl Default for Message2 { vec![(Protocol, ProtocolSupport::Full)], config, ), - events: Default::default(), } } } @@ -78,8 +70,8 @@ impl NetworkBehaviourEventProcess> message: RequestResponseMessage::Response { response, .. }, .. } => match response { - AliceToBob::Message2(msg) => self.events.push_back(OutEvent::Msg(msg)), - other => debug!("got response: {:?}", other), + AliceToBob::EmptyResponse => debug!("Alice correctly responded to message 2"), + other => debug!("unexpected response: {:?}", other), }, RequestResponseEvent::InboundFailure { error, .. } => { error!("Inbound failure: {:?}", error); diff --git a/swap/src/network/request_response.rs b/swap/src/network/request_response.rs index 709013b6..f8d44deb 100644 --- a/swap/src/network/request_response.rs +++ b/swap/src/network/request_response.rs @@ -35,6 +35,7 @@ pub enum AliceToBob { Amounts(SwapAmounts), Message0(alice::Message0), Message1(alice::Message1), + EmptyResponse, // This is sent back as response to Message2 from Bob. Message2(alice::Message2), }