Merge pull request #28 from comit-network/re-work-swap-functions

Re work swap functions

swap/src/alice.rs

@@ -28,6 +28,9 @@ use xmr_btc::{alice::State0, bob, monero};
 
 pub type Swarm = libp2p::Swarm<Alice>;
 
+// 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,
@@ -46,16 +49,21 @@ 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) => {
-                debug!("Got message0 from Bob");
-                // TODO: Do this in a more Rusty/functional way.
+                // 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),
         };
     }
@@ -65,10 +73,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 <R: RngCore + CryptoRng>
     let rng = &mut OsRng;
     let state0 = State0::new(
@@ -82,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 {

swap/src/alice/message2.rs

@@ -71,11 +71,17 @@ impl NetworkBehaviourEventProcess<RequestResponseEvent<BobToAlice, AliceToBob>>
     fn inject_event(&mut self, event: RequestResponseEvent<BobToAlice, AliceToBob>) {
         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),
             },

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,
@@ -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 <R: RngCore + CryptoRng>
     let rng = &mut OsRng;
     let state0 = State0::new(
@@ -87,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),
     };
@@ -96,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),
     };
@@ -137,7 +134,6 @@ pub enum OutEvent {
     Amounts(SwapAmounts),
     Message0(alice::Message0),
     Message1(alice::Message1),
-    Message2(alice::Message2),
 }
 
 impl From<peer_tracker::OutEvent> for OutEvent {
@@ -174,11 +170,9 @@ impl From<message1::OutEvent> for OutEvent {
     }
 }
 
-impl From<message2::OutEvent> for OutEvent {
-    fn from(event: message2::OutEvent) -> Self {
-        match event {
-            message2::OutEvent::Msg(msg) => OutEvent::Message2(msg),
-        }
+impl From<Never> for OutEvent {
+    fn from(_: Never) -> Self {
+        panic!("this never happens")
     }
 }
 

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<Codec>,
-    #[behaviour(ignore)]
-    events: VecDeque<OutEvent>,
 }
 
 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<NetworkBehaviourAction<RequestProtocol<Codec>, OutEvent>> {
-        if let Some(event) = self.events.pop_front() {
-            return Poll::Ready(NetworkBehaviourAction::GenerateEvent(event));
-        }
-
+    ) -> Poll<NetworkBehaviourAction<RequestProtocol<Codec>, 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<RequestResponseEvent<BobToAlice, AliceToBob>>
                 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);

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),
 }