From 6407644a5dd1f5c6be63f7a7d7f86017d59c9535 Mon Sep 17 00:00:00 2001
From: sgdxbc <sung@comp.nus.edu.sg>
Date: Tue, 9 Jul 2024 11:12:49 +0800
Subject: [PATCH] Update Inline and fix for pbft tests

---
 src/event/combinators.rs | 53 ++++++++++++++++------------------------
 src/pbft/tests.rs        | 37 +++++++++++++++++++---------
 2 files changed, 47 insertions(+), 43 deletions(-)

diff --git a/src/event/combinators.rs b/src/event/combinators.rs
index 647a4fe1..5c8c81bd 100644
--- a/src/event/combinators.rs
+++ b/src/event/combinators.rs
@@ -1,23 +1,36 @@
 use derive_more::{Deref, DerefMut};
 use derive_where::derive_where;
 
-use super::{OnEvent, SendEvent};
+use super::{OnEvent, SendEvent, Submit};
 
+// the alternative definition would be Inline<'a, S, C>(&'a mut S, &'a mut C)
+// (or two distinct lifetimes if preferred)
+// there may be some benefits by not fixing the inners to references in the
+// future, so i have chosen this representation, but there is indeed nothing
+// much for now since the use cases for Inline always takes references
 #[derive(Debug)]
 pub struct Inline<S, C>(pub S, pub C);
 
-// impl<S: OnEvent<C>, C> SendEvent<S::Event> for Inline<S, C> {
-//     fn send(&mut self, event: S::Event) -> anyhow::Result<()> {
-//         self.0.on_event(event, &mut self.1)
-//     }
-// }
-
 impl<S: OnEvent<C>, C> SendEvent<S::Event> for Inline<&'_ mut S, &'_ mut C> {
     fn send(&mut self, event: S::Event) -> anyhow::Result<()> {
         self.0.on_event(event, self.1)
     }
 }
 
+// the original intent was to reuse the SendEvent<UntypedEvent<_, _>> impl of a
+// Inline<Untyped<_, _>, _>
+// that did not work because UntypedEvent<_, _> contains a Box<_ + 'static> so a
+// wrapping pass through UntypedEvent would unnecessarily amplify `'a` to
+// `'static` which is highly undesired for Inline use cases
+// it is possible for UntypedEvent to be `struct UntypedEvent<'a>(Box<_ + 'a>)`
+// but that would complicate a lot just for this niche case, while this direct
+// repeated impl, though conceptually repeating, really takes only trivial code
+impl<'a, S, C> Submit<S, C> for Inline<&'a mut S, &'a mut C> {
+    fn submit(&mut self, work: crate::event::Work<S, C>) -> anyhow::Result<()> {
+        work(self.0, self.1)
+    }
+}
+
 // a bit wild to directly impl on foreign type, hope no conflict to anything
 impl<M> SendEvent<M> for Option<M> {
     fn send(&mut self, event: M) -> anyhow::Result<()> {
@@ -53,30 +66,6 @@ impl<F: FnMut(M) -> N, M, N, E: SendEvent<N>> SendEvent<M> for Map<F, E> {
     }
 }
 
-pub mod work {
-    use crate::event::{OnEvent as _, Submit, Untyped, UntypedEvent};
-
-    pub type Inline<'a, S, C> = super::Inline<Untyped<&'a mut C, &'a mut S>, &'a mut C>;
-
-    impl<'a, S, C> Inline<'a, S, C> {
-        pub fn new_worker(state: &'a mut S, context: &'a mut C) -> Self {
-            Self(Untyped::new(state), context)
-        }
-    }
-
-    // fix to 'static because UntypedEvent takes a Box<_ + 'static>
-    // it is possible for UntypedEvent to be `struct UntypedEvent<'a>(Box<_ + 'a>)` but that is too
-    // overly engineered
-    impl<'a, S: 'static, C: 'static> Submit<S, C> for Inline<'a, S, C> {
-        fn submit(&mut self, work: crate::event::Work<S, C>) -> anyhow::Result<()> {
-            self.0.on_event(
-                UntypedEvent(Box::new(move |state, context| work(*state, *context))),
-                &mut self.1,
-            )
-        }
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use crate::event::Submit as _;
@@ -87,7 +76,7 @@ mod tests {
     fn inline_worker() -> anyhow::Result<()> {
         let mut state = 1;
         let mut context = 0;
-        let mut inline = Inline::new_worker(&mut state, &mut context);
+        let mut inline = Inline(&mut state, &mut context);
         for _ in 0..10 {
             inline.submit(Box::new(move |state, context| {
                 let old_state = *state;
diff --git a/src/pbft/tests.rs b/src/pbft/tests.rs
index c5f633d8..42673cb6 100644
--- a/src/pbft/tests.rs
+++ b/src/pbft/tests.rs
@@ -1,5 +1,3 @@
-use std::marker::PhantomData;
-
 use bytes::Bytes;
 use derive_more::From;
 use derive_where::derive_where;
@@ -8,7 +6,7 @@ use serde::{Deserialize, Serialize};
 use crate::{
     crypto::{Crypto, Verifiable},
     event::{
-        combinators::{work::Inline, Transient},
+        combinators::{Inline, Transient},
         Erase, UntypedEvent,
     },
     model::{NetworkState, ScheduleState},
@@ -86,6 +84,8 @@ pub struct State<W> {
     pub replicas: Vec<(ReplicaState, ReplicaLocalContext)>,
 }
 
+type ReplicaState = replica::State<kvstore::App, Addr>;
+
 #[derive(Debug, Clone)]
 #[derive_where(PartialEq, Eq, Hash)]
 pub struct ClientLocalContext<W> {
@@ -94,7 +94,18 @@ pub struct ClientLocalContext<W> {
     schedule: ScheduleState<Timer>,
 }
 
-type ReplicaState = replica::State<kvstore::App, Addr>;
+struct ClientContextCarrier;
+
+impl<'a, W> client::context::On<ClientContext<'a, W>> for ClientContextCarrier {
+    type Schedule = &'a mut ScheduleState<Timer>;
+}
+
+type ClientContext<'a, W> = client::context::Context<
+    ClientContextCarrier,
+    &'a mut NetworkState<Addr, Message>,
+    &'a mut CloseLoop<W, Option<Invoke<Bytes>>>,
+    Addr,
+>;
 
 #[derive(Debug, Clone)]
 #[derive_where(PartialEq, Eq, Hash)]
@@ -105,16 +116,20 @@ pub struct ReplicaLocalContext {
 }
 
 #[derive(Debug)]
-pub struct ReplicaContextCarrier<'a>(PhantomData<&'a ()>);
-
-impl<'a, C: 'static> replica::context::On<C, ReplicaState> for ReplicaContextCarrier<'a> {
-    type CryptoWorker = Inline<'a, Crypto, Self::CryptoContext>;
-    type CryptoContext = Erase<ReplicaState, C, Transient<UntypedEvent<ReplicaState, C>>>;
-    type Schedule = ScheduleState<Timer>;
+pub struct ReplicaContextCarrier;
+
+impl<'a> replica::context::On<ReplicaContext<'a>, ReplicaState> for ReplicaContextCarrier {
+    type CryptoWorker = Inline<&'a mut Crypto, &'a mut Self::CryptoContext>;
+    type CryptoContext = Erase<
+        ReplicaState,
+        ReplicaContext<'a>,
+        Transient<UntypedEvent<ReplicaState, ReplicaContext<'a>>>,
+    >;
+    type Schedule = &'a mut ScheduleState<Timer>;
 }
 
 pub type ReplicaContext<'a> = replica::context::Context<
-    ReplicaContextCarrier<'a>,
+    ReplicaContextCarrier,
     &'a mut NetworkState<Addr, Message>,
     &'a mut NetworkState<Addr, Message>,
     ReplicaState,