Skip to content

Commit

Permalink
Refactor Framing crate
Browse files Browse the repository at this point in the history
This commit aims to refactor the `framing-sv2` crate. List of changes:
1. Removed `Frame` trait
2. Changed `EitherFrame` enum to `Frame`
3. Removed unused functions from `HandShakeFrame` and `Sv2Frame`
4. Added documentation where missing
5. Made sure there is consistency in function naming in `Frame`, i.e, in
   order to get the header you call `Frame::header`, for payload
   `Frame::payload` and so on.
6. Made sure we dont return mut data from `Frame` get functions(leave it
   to the caller to decide).
7. Fixed all the modules in `protocols` based on the above changes
8. Fixed all the modules in `roles` based on the above changes

For 7 & 8 its mainly removing the `Frame` trait import plus naming fixes
where I renamed `StandardEitherFrame` to `StandardFrame` and other name
fixes and also fixing the `header` and `payload` calls to align with the
new API.
  • Loading branch information
jbesraa committed Jun 18, 2024
1 parent 0caa106 commit 12bec35
Show file tree
Hide file tree
Showing 45 changed files with 601 additions and 529 deletions.
24 changes: 15 additions & 9 deletions benches/benches/src/sv2/criterion_sv2_benchmark.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use codec_sv2::{Frame, StandardEitherFrame, StandardSv2Frame};
use codec_sv2::{StandardFrame, StandardSv2Frame};
use criterion::{black_box, Criterion};
use roles_logic_sv2::{
handlers::{common::ParseUpstreamCommonMessages, mining::ParseUpstreamMiningMessages},
Expand All @@ -20,7 +20,7 @@ use crate::client::{

pub type Message = MiningDeviceMessages<'static>;
pub type StdFrame = StandardSv2Frame<Message>;
pub type EitherFrame = StandardEitherFrame<Message>;
pub type EitherFrame = StandardFrame<Message>;

fn client_sv2_setup_connection(c: &mut Criterion) {
c.bench_function("client_sv2_setup_connection", |b| {
Expand Down Expand Up @@ -53,9 +53,11 @@ fn client_sv2_setup_connection_serialize_deserialize(c: &mut Criterion) {
let mut dst = vec![0; size];
let _serialized = frame.serialize(&mut dst);
b.iter(|| {
let mut frame = StdFrame::from_bytes(black_box(dst.clone().into())).unwrap();
let type_ = frame.get_header().unwrap().msg_type().clone();
let payload = frame.payload();
let frame = StdFrame::from_bytes(black_box(dst.clone().into())).unwrap();
let type_ = frame.header().msg_type();
let payload = frame.payload().unwrap();
let mut payload = payload.to_owned();
let payload = payload.as_mut();
let _ = AnyMessage::try_from((type_, payload)).unwrap();
});
});
Expand Down Expand Up @@ -95,8 +97,10 @@ fn client_sv2_open_channel_serialize_deserialize(c: &mut Criterion) {
frame.serialize(&mut dst);
b.iter(|| {
let mut frame = StdFrame::from_bytes(black_box(dst.clone().into())).unwrap();
let type_ = frame.get_header().unwrap().msg_type().clone();
let payload = frame.payload();
let type_ = frame.header().msg_type();
let payload = frame.payload().unwrap();
let mut payload = payload.to_owned();
let payload = payload.as_mut();
black_box(AnyMessage::try_from((type_, payload)).unwrap());
});
});
Expand Down Expand Up @@ -151,8 +155,10 @@ fn client_sv2_mining_message_submit_standard_serialize_deserialize(c: &mut Crite
|b| {
b.iter(|| {
let mut frame = StdFrame::from_bytes(black_box(dst.clone().into())).unwrap();
let type_ = frame.get_header().unwrap().msg_type().clone();
let payload = frame.payload();
let type_ = frame.header().msg_type();
let payload = frame.payload().unwrap();
let mut payload = payload.to_owned();
let payload = payload.as_mut();
black_box(AnyMessage::try_from((type_, payload)).unwrap());
});
},
Expand Down
22 changes: 14 additions & 8 deletions benches/benches/src/sv2/iai_sv2_benchmark.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use codec_sv2::{Frame, StandardEitherFrame, StandardSv2Frame};
use codec_sv2::{StandardFrame, StandardSv2Frame};
use iai::{black_box, main};
use roles_logic_sv2::{
handlers::{common::ParseUpstreamCommonMessages, mining::ParseUpstreamMiningMessages, SendTo_},
Expand All @@ -18,7 +18,7 @@ use crate::client::{create_client, open_channel, Device, SetupConnectionHandler}

pub type Message = MiningDeviceMessages<'static>;
pub type StdFrame = StandardSv2Frame<Message>;
pub type EitherFrame = StandardEitherFrame<Message>;
pub type EitherFrame = StandardFrame<Message>;

fn client_sv2_setup_connection() {
let address: SocketAddr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)), 34254);
Expand Down Expand Up @@ -46,8 +46,10 @@ fn client_sv2_setup_connection_serialize_deserialize() {
let mut dst = vec![0; size];
frame.serialize(&mut dst);
let mut frame = StdFrame::from_bytes(black_box(dst.clone().into())).unwrap();
let type_ = frame.get_header().unwrap().msg_type().clone();
let payload = frame.payload();
let type_ = frame.header().msg_type();
let payload = frame.payload().unwrap();
let mut payload = payload.to_owned();
let payload = payload.as_mut();
black_box(AnyMessage::try_from((type_, payload)));
}

Expand Down Expand Up @@ -77,8 +79,10 @@ fn client_sv2_open_channel_serialize_deserialize() {
let mut dst = vec![0; size];
frame.serialize(&mut dst);
let mut frame = StdFrame::from_bytes(black_box(dst.clone().into())).unwrap();
let type_ = frame.get_header().unwrap().msg_type().clone();
let payload = frame.payload();
let type_ = frame.header().msg_type();
let payload = frame.payload().unwrap();
let mut payload = payload.to_owned();
let payload = payload.as_mut();
black_box(AnyMessage::try_from((type_, payload)));
}

Expand Down Expand Up @@ -127,8 +131,10 @@ fn client_sv2_mining_message_submit_standard_serialize_deserialize() {
let mut dst = vec![0; size];
frame.serialize(&mut dst);
let mut frame = StdFrame::from_bytes(black_box(dst.clone().into())).unwrap();
let type_ = frame.get_header().unwrap().msg_type().clone();
let payload = frame.payload();
let type_ = frame.header().msg_type();
let payload = frame.payload().unwrap();
let mut payload = payload.to_owned();
let payload = payload.as_mut();
black_box(AnyMessage::try_from((type_, payload)));
}

Expand Down
4 changes: 2 additions & 2 deletions benches/benches/src/sv2/lib/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use bitcoin::{
use async_channel::{Receiver, Sender};
use async_std::channel::unbounded;
use binary_sv2::u256_from_int;
use codec_sv2::{buffer_sv2::Slice, StandardEitherFrame, StandardSv2Frame};
use codec_sv2::{buffer_sv2::Slice, StandardFrame, StandardSv2Frame};
use roles_logic_sv2::{
common_messages_sv2::{Protocol, SetupConnection, SetupConnectionSuccess},
common_properties::{IsMiningUpstream, IsUpstream},
Expand All @@ -23,7 +23,7 @@ use roles_logic_sv2::{
use std::{net::SocketAddr, sync::Arc};
pub type Message = MiningDeviceMessages<'static>;
pub type StdFrame = StandardSv2Frame<Message>;
pub type EitherFrame = StandardEitherFrame<Message>;
pub type EitherFrame = StandardFrame<Message>;

pub fn create_client() -> Device {
let (sender, receiver) = unbounded();
Expand Down
15 changes: 10 additions & 5 deletions examples/interop-cpp/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ mod main_ {

#[cfg(not(feature = "with_serde"))]
mod main_ {
use codec_sv2::{Encoder, Frame, StandardDecoder, StandardSv2Frame};
use codec_sv2::{Encoder, StandardDecoder, StandardSv2Frame};
use common_messages_sv2::{Protocol, SetupConnection, SetupConnectionError};
use const_sv2::{
CHANNEL_BIT_SETUP_CONNECTION, MESSAGE_TYPE_SETUP_CONNECTION,
Expand Down Expand Up @@ -124,10 +124,15 @@ mod main_ {

loop {
let buffer = decoder.writable();
stream.read_exact(buffer).unwrap();
if let Ok(mut f) = decoder.next_frame() {
let msg_type = f.get_header().unwrap().msg_type();
let payload = f.payload();
match stream.read_exact(buffer) {
Ok(_) => {}
Err(_) => continue,
};
if let Ok(f) = decoder.next_frame() {
let msg_type = f.header().msg_type();
let payload = f.payload().unwrap();
let mut payload = payload.to_owned();
let payload = payload.as_mut();
let message: Sv2Message = (msg_type, payload).try_into().unwrap();
match message {
Sv2Message::SetupConnection(_) => panic!(),
Expand Down
18 changes: 12 additions & 6 deletions examples/ping-pong-with-noise/src/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use async_std::{
};
use core::convert::TryInto;

use codec_sv2::{Frame, HandshakeRole, StandardEitherFrame, StandardSv2Frame};
use codec_sv2::{HandshakeRole, StandardFrame, StandardSv2Frame};

use std::time;

Expand All @@ -28,8 +28,8 @@ pub struct Node {
name: String,
last_id: u32,
expected: Expected,
receiver: Receiver<StandardEitherFrame<Message<'static>>>,
sender: Sender<StandardEitherFrame<Message<'static>>>,
receiver: Receiver<StandardFrame<Message<'static>>>,
sender: Sender<StandardFrame<Message<'static>>>,
}

impl Node {
Expand Down Expand Up @@ -95,11 +95,14 @@ impl Node {

fn handle_message(
&mut self,
mut frame: StandardSv2Frame<Message<'static>>,
frame: StandardSv2Frame<Message<'static>>,
) -> Message<'static> {
match self.expected {
Expected::Ping => {
let ping: Result<Ping, _> = from_bytes(frame.payload());
let payload = frame.payload().unwrap();
let mut payload = payload.to_owned();
let payload = payload.as_mut();
let ping: Result<Ping, _> = from_bytes(payload);
match ping {
Ok(ping) => {
println!("Node {} received:", self.name);
Expand All @@ -118,7 +121,10 @@ impl Node {
}
}
Expected::Pong => {
let pong: Result<Pong, _> = from_bytes(frame.payload());
let payload = frame.payload().unwrap();
let mut payload = payload.to_owned();
let payload = payload.as_mut();
let pong: Result<Pong, _> = from_bytes(payload);
match pong {
Ok(pong) => {
println!("Node {} received:", self.name);
Expand Down
14 changes: 10 additions & 4 deletions examples/ping-pong-without-noise/src/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use async_std::{
task,
};

use codec_sv2::{Frame, StandardDecoder, StandardSv2Frame};
use codec_sv2::{StandardDecoder, StandardSv2Frame};

#[derive(Debug)]
enum Expected {
Expand Down Expand Up @@ -83,11 +83,14 @@ impl Node {

fn handle_message(
&mut self,
mut frame: StandardSv2Frame<Message<'static>>,
frame: StandardSv2Frame<Message<'static>>,
) -> Message<'static> {
match self.expected {
Expected::Ping => {
let ping: Result<Ping, _> = from_bytes(frame.payload());
let payload = frame.payload().unwrap();
let mut payload = payload.to_owned();
let payload = payload.as_mut();
let ping: Result<Ping, _> = from_bytes(payload);
match ping {
Ok(ping) => {
println!("Node {} received:", self.name);
Expand All @@ -107,7 +110,10 @@ impl Node {
}
}
Expected::Pong => {
let pong: Result<Pong, _> = from_bytes(frame.payload());
let payload = frame.payload().unwrap();
let mut payload = payload.to_owned();
let payload = payload.as_mut();
let pong: Result<Pong, _> = from_bytes(payload);
match pong {
Ok(pong) => {
println!("Node {} received:", self.name);
Expand Down
4 changes: 2 additions & 2 deletions examples/template-provider-test/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use async_channel::{Receiver, Sender};
use async_std::net::TcpStream;
use codec_sv2::{Frame, StandardEitherFrame, StandardSv2Frame, Sv2Frame};
use codec_sv2::{StandardFrame, StandardSv2Frame, Sv2Frame};
use network_helpers::PlainConnection;
use roles_logic_sv2::{
parsers::{IsSv2Message, TemplateDistribution},
Expand All @@ -13,7 +13,7 @@ use std::{

pub type Message = TemplateDistribution<'static>;
pub type StdFrame = StandardSv2Frame<Message>;
pub type EitherFrame = StandardEitherFrame<Message>;
pub type EitherFrame = StandardFrame<Message>;

#[async_std::main]
async fn main() {
Expand Down
2 changes: 1 addition & 1 deletion protocols/fuzz-tests/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
use libfuzzer_sys::fuzz_target;
use binary_codec_sv2::{Seq064K,U256,B0255,Seq0255};
use binary_codec_sv2::from_bytes;
use codec_sv2::{StandardDecoder,Sv2Frame,Frame};
use codec_sv2::{StandardDecoder,Sv2Frame};
use roles_logic_sv2::parsers::PoolMessages;

type F = Sv2Frame<PoolMessages<'static>,Vec<u8>>;
Expand Down
13 changes: 5 additions & 8 deletions protocols/v2/codec-sv2/src/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ use crate::State;

#[cfg(feature = "noise_sv2")]
pub type StandardNoiseDecoder<T> = WithNoise<Buffer, T>;
pub type StandardEitherFrame<T> = EitherFrame<T, <Buffer as IsBuffer>::Slice>;
pub type StandardFrame<T> = EitherFrame<T, <Buffer as IsBuffer>::Slice>;
pub type StandardSv2Frame<T> = Sv2Frame<T, <Buffer as IsBuffer>::Slice>;
pub type StandardDecoder<T> = WithoutNoise<Buffer, T>;

Expand Down Expand Up @@ -97,10 +97,7 @@ impl<'a, T: Serialize + GetSize + Deserialize<'a>, B: IsBuffer + AeadBuffer> Wit
}

#[inline]
fn decode_noise_frame(
&mut self,
noise_codec: &mut NoiseCodec,
) -> Result<EitherFrame<T, B::Slice>> {
fn decode_noise_frame(&mut self, noise_codec: &mut NoiseCodec) -> Result<EitherFrame<T, B::Slice>> {
match (
IsBuffer::len(&self.noise_buffer),
IsBuffer::len(&self.sv2_buffer),
Expand Down Expand Up @@ -142,7 +139,7 @@ impl<'a, T: Serialize + GetSize + Deserialize<'a>, B: IsBuffer + AeadBuffer> Wit
}
self.sv2_buffer.danger_set_start(0);
let src = self.sv2_buffer.get_data_owned();
let frame = Sv2Frame::<T, B::Slice>::from_bytes_unchecked(src);
let frame = Sv2Frame::<T, B::Slice>::from_bytes(src)?;
Ok(frame.into())
}
}
Expand All @@ -152,7 +149,7 @@ impl<'a, T: Serialize + GetSize + Deserialize<'a>, B: IsBuffer + AeadBuffer> Wit
let src = self.noise_buffer.get_data_owned().as_mut().to_vec();

// below is inffalible as noise frame length has been already checked
let frame = HandShakeFrame::from_bytes_unchecked(src.into());
let frame = HandShakeFrame::from_bytes(src.into());

frame.into()
}
Expand Down Expand Up @@ -203,7 +200,7 @@ impl<T: Serialize + binary_sv2::GetSize, B: IsBuffer> WithoutNoise<B, T> {
0 => {
self.missing_b = Header::SIZE;
let src = self.buffer.get_data_owned();
let frame = Sv2Frame::<T, B::Slice>::from_bytes_unchecked(src);
let frame = Sv2Frame::<T, B::Slice>::from_bytes(src)?;
Ok(frame)
}
_ => {
Expand Down
4 changes: 2 additions & 2 deletions protocols/v2/codec-sv2/src/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ pub use const_sv2::{AEAD_MAC_LEN, SV2_FRAME_CHUNK_SIZE, SV2_FRAME_HEADER_SIZE};
#[cfg(feature = "noise_sv2")]
use core::convert::TryInto;
use core::marker::PhantomData;
use framing_sv2::framing2::Sv2Frame;
#[cfg(feature = "noise_sv2")]
use framing_sv2::framing2::{EitherFrame, HandShakeFrame};
use framing_sv2::framing2::Sv2Frame;
#[allow(unused_imports)]
pub use framing_sv2::header::NOISE_HEADER_ENCRYPTED_SIZE;

Expand Down Expand Up @@ -106,7 +106,7 @@ impl<T: Serialize + GetSize> NoiseEncoder<T> {
error!("Error while encoding 2 frame - while_handshaking: {:?}", e);
Error::FramingError(e)
})?;
let payload = i.get_payload_when_handshaking();
let payload = i.payload().to_vec();
let wrtbl = self.noise_buffer.get_writable(payload.len());
for (i, b) in payload.iter().enumerate() {
wrtbl[i] = *b;
Expand Down
2 changes: 1 addition & 1 deletion protocols/v2/codec-sv2/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ pub mod error;

pub use error::{CError, Error, Result};

pub use decoder::{StandardEitherFrame, StandardSv2Frame};
pub use decoder::{StandardFrame, StandardSv2Frame};

pub use decoder::StandardDecoder;
#[cfg(feature = "noise_sv2")]
Expand Down
3 changes: 0 additions & 3 deletions protocols/v2/framing-sv2/src/error.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
// use crate::framing2::EitherFrame;
use core::fmt;

// pub type FramingResult<T> = core::result::Result<T, Error>;

#[derive(Debug, PartialEq, Eq)]
pub enum Error {
BinarySv2Error(binary_sv2::Error),
Expand Down
Loading

0 comments on commit 12bec35

Please sign in to comment.