Skip to content

Commit

Permalink
fix: use new bidirectional pipe logic in oracle tests
Browse files Browse the repository at this point in the history
  • Loading branch information
clabby committed Aug 5, 2024
1 parent 819faee commit 3df1934
Show file tree
Hide file tree
Showing 8 changed files with 69 additions and 96 deletions.
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion bin/host/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ pub mod fetcher;
pub mod kv;
pub mod preimage;
pub mod server;
pub mod types;
pub mod util;

pub use cli::{init_tracing_subscriber, HostCli};
Expand Down
16 changes: 0 additions & 16 deletions bin/host/src/types.rs

This file was deleted.

2 changes: 1 addition & 1 deletion crates/preimage/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ serde = { version = "1.0.203", features = ["derive"], optional = true }

[dev-dependencies]
tokio = { version = "1.38.0", features = ["full"] }
tempfile = "3.10.1"
os_pipe = "1.2.1"

[features]
default = []
Expand Down
53 changes: 20 additions & 33 deletions crates/preimage/src/hint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,39 +105,12 @@ mod test {
extern crate std;

use super::*;
use crate::test_utils::bidirectional_pipe;
use alloc::{sync::Arc, vec::Vec};
use kona_common::FileDescriptor;
use std::{fs::File, os::fd::AsRawFd};
use tempfile::tempfile;
use std::os::fd::AsRawFd;
use tokio::sync::Mutex;

/// Test struct containing the [HintReader] and [HintWriter]. The [File]s are stored in this
/// struct so that they are not dropped until the end of the test.
#[derive(Debug)]
struct ClientAndHost {
hint_writer: HintWriter,
hint_reader: HintReader,
_read_file: File,
_write_file: File,
}

/// Helper for creating a new [HintReader] and [HintWriter] for testing. The file channel is
/// over two temporary files.
fn client_and_host() -> ClientAndHost {
let (read_file, write_file) = (tempfile().unwrap(), tempfile().unwrap());
let (read_fd, write_fd) = (
FileDescriptor::Wildcard(read_file.as_raw_fd().try_into().unwrap()),
FileDescriptor::Wildcard(write_file.as_raw_fd().try_into().unwrap()),
);
let client_handle = PipeHandle::new(read_fd, write_fd);
let host_handle = PipeHandle::new(write_fd, read_fd);

let hint_writer = HintWriter::new(client_handle);
let hint_reader = HintReader::new(host_handle);

ClientAndHost { hint_writer, hint_reader, _read_file: read_file, _write_file: write_file }
}

struct TestRouter {
incoming_hints: Arc<Mutex<Vec<String>>>,
}
Expand All @@ -154,15 +127,26 @@ mod test {
async fn test_hint_client_and_host() {
const MOCK_DATA: &str = "test-hint 0xfacade";

let sys = client_and_host();
let (hint_writer, hint_reader) = (sys.hint_writer, sys.hint_reader);
let incoming_hints = Arc::new(Mutex::new(Vec::new()));
let hint_pipe = bidirectional_pipe().unwrap();

let client = tokio::task::spawn(async move { hint_writer.write(MOCK_DATA).await });
let client = tokio::task::spawn(async move {
let hint_writer = HintWriter::new(PipeHandle::new(
FileDescriptor::Wildcard(hint_pipe.client.read.as_raw_fd() as usize),
FileDescriptor::Wildcard(hint_pipe.client.write.as_raw_fd() as usize),
));

hint_writer.write(MOCK_DATA).await
});
let host = tokio::task::spawn({
let incoming_hints_ref = Arc::clone(&incoming_hints);
async move {
let router = TestRouter { incoming_hints: incoming_hints_ref };

let hint_reader = HintReader::new(PipeHandle::new(
FileDescriptor::Wildcard(hint_pipe.host.read.as_raw_fd() as usize),
FileDescriptor::Wildcard(hint_pipe.host.write.as_raw_fd() as usize),
));
hint_reader.next_hint(&router).await.unwrap();

let mut hints = incoming_hints.lock().await;
Expand All @@ -171,7 +155,10 @@ mod test {
}
});

let (_, h) = tokio::join!(client, host);
let h = tokio::select! {
_ = client => panic!("Client task should not return"),
h = host => h,
};
assert_eq!(h.unwrap(), MOCK_DATA);
}
}
3 changes: 3 additions & 0 deletions crates/preimage/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,6 @@ pub use traits::{
CommsClient, HintReaderServer, HintRouter, HintWriterClient, PreimageFetcher,
PreimageOracleClient, PreimageOracleServer,
};

#[cfg(test)]
mod test_utils;
59 changes: 15 additions & 44 deletions crates/preimage/src/oracle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,48 +132,14 @@ mod test {
extern crate std;

use super::*;
use crate::PreimageKeyType;
use crate::{test_utils::bidirectional_pipe, PreimageKeyType};
use alloc::sync::Arc;
use alloy_primitives::keccak256;
use anyhow::anyhow;
use kona_common::FileDescriptor;
use std::{collections::HashMap, fs::File, os::fd::AsRawFd};
use tempfile::tempfile;
use std::{collections::HashMap, os::fd::AsRawFd};
use tokio::sync::Mutex;

/// Test struct containing the [OracleReader] and a [OracleServer] for the host, plus the open
/// [File]s. The [File]s are stored in this struct so that they are not dropped until the
/// end of the test.
#[derive(Debug)]
struct ClientAndHost {
oracle_reader: OracleReader,
oracle_server: OracleServer,
_read_file: File,
_write_file: File,
}

/// Helper for creating a new [OracleReader] and [OracleServer] for testing. The file channel is
/// over two temporary files.
fn client_and_host() -> ClientAndHost {
let (read_file, write_file) = (tempfile().unwrap(), tempfile().unwrap());
let (read_fd, write_fd) = (
FileDescriptor::Wildcard(read_file.as_raw_fd().try_into().unwrap()),
FileDescriptor::Wildcard(write_file.as_raw_fd().try_into().unwrap()),
);
let client_handle = PipeHandle::new(read_fd, write_fd);
let host_handle = PipeHandle::new(write_fd, read_fd);

let oracle_reader = OracleReader::new(client_handle);
let oracle_server = OracleServer::new(host_handle);

ClientAndHost {
oracle_reader,
oracle_server,
_read_file: read_file,
_write_file: write_file,
}
}

struct TestFetcher {
preimages: Arc<Mutex<HashMap<PreimageKey, Vec<u8>>>>,
}
Expand Down Expand Up @@ -202,20 +168,23 @@ mod test {
Arc::new(Mutex::new(preimages))
};

let sys = client_and_host();
let (oracle_reader, oracle_server) = (sys.oracle_reader, sys.oracle_server);
let preimage_pipe = bidirectional_pipe().unwrap();

let client = tokio::task::spawn(async move {
let oracle_reader = OracleReader::new(PipeHandle::new(
FileDescriptor::Wildcard(preimage_pipe.client.read.as_raw_fd() as usize),
FileDescriptor::Wildcard(preimage_pipe.client.write.as_raw_fd() as usize),
));
let contents_a = oracle_reader.get(key_a).await.unwrap();
let contents_b = oracle_reader.get(key_b).await.unwrap();

// Drop the file descriptors to close the pipe, stopping the host's blocking loop on
// waiting for client requests.
drop(sys);

(contents_a, contents_b)
});
let host = tokio::task::spawn(async move {
let oracle_server = OracleServer::new(PipeHandle::new(
FileDescriptor::Wildcard(preimage_pipe.host.read.as_raw_fd() as usize),
FileDescriptor::Wildcard(preimage_pipe.host.write.as_raw_fd() as usize),
));
let test_fetcher = TestFetcher { preimages: Arc::clone(&preimages) };

loop {
Expand All @@ -225,8 +194,10 @@ mod test {
}
});

let (client, _) = tokio::join!(client, host);
let (contents_a, contents_b) = client.unwrap();
let (contents_a, contents_b) = tokio::select! {
cl = client => cl.unwrap(),
_ = host => panic!("Host exited unexpectedly"),
};
assert_eq!(contents_a, MOCK_DATA_A);
assert_eq!(contents_b, MOCK_DATA_B);
}
Expand Down
29 changes: 29 additions & 0 deletions crates/preimage/src/test_utils.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
//! Test utilities for the `kona-preimage` crate.
use anyhow::{anyhow, Result};
use os_pipe::{PipeReader, PipeWriter};

/// A bidirectional pipe, with a client and host end.
#[derive(Debug)]
pub(crate) struct BidirectionalPipe {
pub(crate) client: Pipe,
pub(crate) host: Pipe,
}

/// A single-direction pipe, with a read and write end.
#[derive(Debug)]
pub(crate) struct Pipe {
pub(crate) read: PipeReader,
pub(crate) write: PipeWriter,
}

/// Creates a [BidirectionalPipe] instance.
pub(crate) fn bidirectional_pipe() -> Result<BidirectionalPipe> {
let (ar, bw) = os_pipe::pipe().map_err(|e| anyhow!("Failed to create pipe: {e}"))?;
let (br, aw) = os_pipe::pipe().map_err(|e| anyhow!("Failed to create pipe: {e}"))?;

Ok(BidirectionalPipe {
client: Pipe { read: ar, write: aw },
host: Pipe { read: br, write: bw },
})
}

0 comments on commit 3df1934

Please sign in to comment.