Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enforce no_std for all? protocol/v2 crates #1230

Merged
merged 9 commits into from
Nov 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion protocols/v2/binary-sv2/binary-sv2/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ serde_sv2 = {version = "^1.0.0", path = "../serde-sv2", optional = true}
serde = { version = "1.0.89", features = ["derive", "alloc"], default-features = false, optional = true }
binary_codec_sv2 = {version = "^1.0.0", path = "../no-serde-sv2/codec", optional = true}
derive_codec_sv2 = {version = "^1.0.0", path = "../no-serde-sv2/derive_codec", optional = true}
tracing = {version = "0.1"}
tracing = { version = "0.1", default-features = false }

[features]
default = ["core"]
Expand Down
3 changes: 3 additions & 0 deletions protocols/v2/binary-sv2/binary-sv2/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# binary_sv2

`binary_sv2` is a Rust `no_std` crate
9 changes: 7 additions & 2 deletions protocols/v2/binary-sv2/binary-sv2/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
// TODO unify errors from serde_sv2 and no-serde-sv2
//

#![no_std]
plebhash marked this conversation as resolved.
Show resolved Hide resolved

#[macro_use]
extern crate alloc;

use core::convert::TryInto;

#[cfg(feature = "with_serde")]
Expand Down Expand Up @@ -34,6 +39,7 @@ pub fn u256_from_int<V: Into<u64>>(value: V) -> U256<'static> {
#[cfg(test)]
mod test {
use super::*;
use alloc::vec::Vec;

mod test_struct {
use super::*;
Expand Down Expand Up @@ -748,7 +754,6 @@ mod test {
}
mod test_sv2_option_none {
use super::*;
use core::convert::TryInto;

#[derive(Deserialize, Serialize, PartialEq, Debug, Clone)]
struct Test<'decoder> {
Expand Down
3 changes: 3 additions & 0 deletions protocols/v2/binary-sv2/no-serde-sv2/codec/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# binary_codec_sv2

`binary_codec_sv2` is a Rust `no_std` crate
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::{
Error,
};
use alloc::vec::Vec;
use std::convert::TryFrom;
use core::convert::TryFrom;
#[cfg(not(feature = "no_std"))]
use std::io::{Cursor, Read};

Expand Down
2 changes: 2 additions & 0 deletions protocols/v2/binary-sv2/no-serde-sv2/codec/src/codec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ mod impls;
#[cfg(feature = "with_buffer_pool")]
use buffer_sv2::Slice;

use alloc::vec::Vec;

/// Return the encoded byte size or a `Decodable`
pub trait SizeHint {
fn size_hint(data: &[u8], offset: usize) -> Result<usize, Error>;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
//! Copy data types
use crate::{codec::Fixed, datatypes::Sv2DataType, Error};

use alloc::vec::Vec;
use core::convert::{TryFrom, TryInto};

#[cfg(not(feature = "no_std"))]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ pub use non_copy_data_types::{
B0255, B032, B064K, U256,
};

use alloc::vec::Vec;
use core::convert::TryInto;
#[cfg(not(feature = "no_std"))]
use std::io::{Error as E, Read, Write};

use std::convert::TryInto;

pub trait Sv2DataType<'a>: Sized + SizeHint + GetSize + TryInto<FieldMarker> {
fn from_bytes_(data: &'a mut [u8]) -> Result<Self, Error> {
Self::size_hint(data, 0)?;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ use crate::{
datatypes::Sv2DataType,
Error,
};
use core::convert::TryFrom;
use std::convert::TryInto;

use alloc::vec::Vec;
use core::convert::{TryFrom, TryInto};
#[cfg(not(feature = "no_std"))]
use std::io::{Error as E, Read, Write};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
#[cfg(feature = "prop_test")]
use quickcheck::{Arbitrary, Gen};

use alloc::string::String;
#[cfg(feature = "prop_test")]
use alloc::vec::Vec;

mod inner;
mod seq_inner;

Expand Down Expand Up @@ -28,7 +32,6 @@ impl<'decoder> From<[u8; 32]> for U256<'decoder> {
}
}

#[cfg(not(feature = "with_serde"))]
#[cfg(feature = "prop_test")]
impl<'a> U256<'a> {
pub fn from_gen(g: &mut Gen) -> Self {
Expand All @@ -40,7 +43,6 @@ impl<'a> U256<'a> {
}
}

#[cfg(not(feature = "with_serde"))]
#[cfg(feature = "prop_test")]
impl<'a> B016M<'a> {
pub fn from_gen(g: &mut Gen) -> Self {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ impl_into_encodable_field_for_seq!(B064K<'a>);
impl_into_encodable_field_for_seq!(B016M<'a>);

#[cfg(feature = "prop_test")]
impl<'a, T> std::convert::TryFrom<Seq0255<'a, T>> for Vec<T> {
impl<'a, T> core::convert::TryFrom<Seq0255<'a, T>> for Vec<T> {
type Error = &'static str;
fn try_from(v: Seq0255<'a, T>) -> Result<Self, Self::Error> {
if v.0.len() > 255 {
Expand All @@ -292,7 +292,7 @@ impl<'a, T> std::convert::TryFrom<Seq0255<'a, T>> for Vec<T> {
}

#[cfg(feature = "prop_test")]
impl<'a, T> std::convert::TryFrom<Seq064K<'a, T>> for Vec<T> {
impl<'a, T> core::convert::TryFrom<Seq064K<'a, T>> for Vec<T> {
type Error = &'static str;
fn try_from(v: Seq064K<'a, T>) -> Result<Self, Self::Error> {
if v.0.len() > 64 {
Expand Down
23 changes: 14 additions & 9 deletions protocols/v2/binary-sv2/no-serde-sv2/codec/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@
//! Seq0255 <-> SEQ0_255[T]
//! Seq064K <-> SEQ0_64K[T]
//! ```

#![cfg_attr(feature = "no_std", no_std)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can now use #![no_std], and I'm confident that we are not using any std-compliant methods in binary_codec_sv2 in any of the top-level crates that depend on it. Since almost all of our crates are no_std-compliant, makes sense to have no-serde-sv2 to no-std only. cc: @rrybarczyk

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I asked about this in this comment.

Current binary_codec_sv2 has a broken std support, do we want to remove it or fix it ?

Look like you propose to remove it. It is also my preference.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also think we should enforce no_std on the protocols crates. So let's remove.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR looks good to me and is ready to go. We can enforce the no-std restriction during our protocol refactoring effort, once we have more concrete reasoning for removing std-related imports. However, I’m fine with either approach.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. Let's confirm in the dev meet before we approve.


#[cfg(not(feature = "no_std"))]
use std::io::{Error as E, ErrorKind};

Expand All @@ -35,6 +38,8 @@ pub use crate::codec::{
Fixed, GetSize, SizeHint,
};

use alloc::vec::Vec;

#[allow(clippy::wrong_self_convention)]
pub fn to_bytes<T: Encodable + GetSize>(src: T) -> Result<Vec<u8>, Error> {
let mut result = vec![0_u8; src.get_size()];
Expand Down Expand Up @@ -281,7 +286,7 @@ impl From<&[u8]> for CVec {
// the std lib)
let len = buffer.len();
let ptr = buffer.as_mut_ptr();
std::mem::forget(buffer);
core::mem::forget(buffer);

CVec {
data: ptr,
Expand All @@ -296,7 +301,7 @@ impl From<&[u8]> for CVec {
/// # Safety
#[no_mangle]
pub unsafe extern "C" fn cvec_from_buffer(data: *const u8, len: usize) -> CVec {
let input = std::slice::from_raw_parts(data, len);
let input = core::slice::from_raw_parts(data, len);

let mut buffer: Vec<u8> = vec![0; len];
buffer.copy_from_slice(input);
Expand All @@ -305,7 +310,7 @@ pub unsafe extern "C" fn cvec_from_buffer(data: *const u8, len: usize) -> CVec {
// cause UB, but it may be unsound due to unclear (to me, at least) guarantees of the std lib)
let len = buffer.len();
let ptr = buffer.as_mut_ptr();
std::mem::forget(buffer);
core::mem::forget(buffer);

CVec {
data: ptr,
Expand Down Expand Up @@ -360,7 +365,7 @@ impl<'a, const A: bool, const B: usize, const C: usize, const D: usize>
let len = inner.len();
let cap = inner.capacity();
let ptr = inner.as_mut_ptr();
std::mem::forget(inner);
core::mem::forget(inner);

(ptr, len, cap)
}
Expand All @@ -371,7 +376,7 @@ impl<'a, const A: bool, const B: usize, const C: usize, const D: usize>
let len = inner.len();
let cap = inner.capacity();
let ptr = inner.as_mut_ptr();
std::mem::forget(inner);
core::mem::forget(inner);

(ptr, len, cap)
}
Expand All @@ -393,7 +398,7 @@ pub unsafe extern "C" fn init_cvec2() -> CVec2 {
// cause UB, but it may be unsound due to unclear (to me, at least) guarantees of the std lib)
let len = buffer.len();
let ptr = buffer.as_mut_ptr();
std::mem::forget(buffer);
core::mem::forget(buffer);

CVec2 {
data: ptr,
Expand All @@ -412,7 +417,7 @@ pub unsafe extern "C" fn cvec2_push(cvec2: &mut CVec2, cvec: CVec) {

let len = buffer.len();
let ptr = buffer.as_mut_ptr();
std::mem::forget(buffer);
core::mem::forget(buffer);

cvec2.data = ptr;
cvec2.len = len;
Expand All @@ -428,7 +433,7 @@ impl<'a, T: Into<CVec>> From<Seq0255<'a, T>> for CVec2 {
let len = v.len();
let capacity = v.capacity();
let data = v.as_mut_ptr();
std::mem::forget(v);
core::mem::forget(v);
Self {
data,
len,
Expand All @@ -445,7 +450,7 @@ impl<'a, T: Into<CVec>> From<Seq064K<'a, T>> for CVec2 {
let len = v.len();
let capacity = v.capacity();
let data = v.as_mut_ptr();
std::mem::forget(v);
core::mem::forget(v);
Self {
data,
len,
Expand Down
3 changes: 3 additions & 0 deletions protocols/v2/binary-sv2/no-serde-sv2/derive_codec/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# derive_codec_sv2

`derive_codec_sv2` is a Rust `no_std` crate
9 changes: 9 additions & 0 deletions protocols/v2/binary-sv2/no-serde-sv2/derive_codec/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,13 @@
#![no_std]
plebhash marked this conversation as resolved.
Show resolved Hide resolved

extern crate alloc;
extern crate proc_macro;

use alloc::{
format,
string::{String, ToString},
vec::Vec,
};
use core::iter::FromIterator;
use proc_macro::{Group, TokenStream, TokenTree};

Expand Down
3 changes: 0 additions & 3 deletions protocols/v2/binary-sv2/serde-sv2/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,5 @@ keywords = ["stratum", "mining", "bitcoin", "protocol"]
serde = { version = "1.0.89", features = ["derive", "alloc"], default-features = false }
buffer_sv2 = {version = "^1.0.0", path = "../../../../utils/buffer"}

[features]
no_std = []

[package.metadata.docs.rs]
all-features = true
3 changes: 3 additions & 0 deletions protocols/v2/binary-sv2/serde-sv2/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# serde_sv2

`serde_sv2` is a Rust `no_std` crate
2 changes: 1 addition & 1 deletion protocols/v2/binary-sv2/serde-sv2/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@
//! [rkyv1]: https://docs.rs/rkyv/0.4.3/rkyv
//! [rkyv2]: https://davidkoloski.me/blog/rkyv-is-faster-than/

#![cfg_attr(feature = "no_std", no_std)]
#![no_std]
plebhash marked this conversation as resolved.
Show resolved Hide resolved

#[macro_use]
extern crate alloc;
Expand Down
3 changes: 0 additions & 3 deletions protocols/v2/const-sv2/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,5 @@ keywords = ["stratum", "mining", "bitcoin", "protocol"]

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[features]
no_std = []

[package.metadata.docs.rs]
all-features = true
2 changes: 1 addition & 1 deletion protocols/v2/const-sv2/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
[![rustc+](https://img.shields.io/badge/rustc-1.75.0%2B-lightgrey.svg)](https://blog.rust-lang.org/2023/12/28/Rust-1.75.0.html)
[![license](https://img.shields.io/badge/license-MIT%2FApache--2.0-blue.svg)](https://github.com/stratum-mining/stratum/blob/main/LICENSE.md)

`const_sv2` is a Rust crate that provides essential constants for the Sv2 (Stratum V2) protocol. These constants are crucial for message framing, encryption, and protocol-specific identifiers across various Sv2 components, including Mining, Job Declaration, and Template Distribution protocols.
`const_sv2` is a Rust `no_std` crate that provides essential constants for the Sv2 (Stratum V2) protocol. These constants are crucial for message framing, encryption, and protocol-specific identifiers across various Sv2 components, including Mining, Job Declaration, and Template Distribution protocols.

## Key Capabilities

Expand Down
2 changes: 1 addition & 1 deletion protocols/v2/const-sv2/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
//! `channel_id`. In this case, the first 4 bytes of the payload represent the
//! `channel_id` the message is destined for.

#![cfg_attr(feature = "no_std", no_std)]
#![no_std]

/// Identifier for the extension_type field in the SV2 frame, indicating no
/// extensions.
Expand Down
3 changes: 1 addition & 2 deletions protocols/v2/framing-sv2/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ binary_sv2 = { version = "^1.0.0", path = "../../../protocols/v2/binary-sv2/bina
buffer_sv2 = { version = "^1.0.0", path = "../../../utils/buffer", optional=true }

[features]
no_std = []
with_serde = ["binary_sv2/with_serde", "serde", "buffer_sv2/with_serde"]
with_serde = ["binary_sv2/with_serde", "serde", "buffer_sv2?/with_serde"]
with_buffer_pool = ["binary_sv2/with_buffer_pool", "buffer_sv2"]

[package.metadata.docs.rs]
Expand Down
3 changes: 3 additions & 0 deletions protocols/v2/framing-sv2/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# framing_sv2

`framing_sv2` is a Rust `no_std` crate
3 changes: 2 additions & 1 deletion protocols/v2/framing-sv2/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
//! The `with_serde` feature flag is only used for the Message Generator, and deprecated for any
//! other kind of usage. It will likely be fully deprecated in the future.

#![cfg_attr(feature = "no_std", no_std)]
#![no_std]

extern crate alloc;

/// SV2 framing types
Expand Down
4 changes: 2 additions & 2 deletions utils/buffer/src/buffer_pool/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,7 @@
"{} {} {}",
self.inner_memory.raw_offset, self.inner_memory.raw_len, self.inner_memory.len
);
let mut res = self.inner_memory.get_data_owned(shared_state, mode);
let res = self.inner_memory.get_data_owned(shared_state, mode);
self.pool_back
.set_len_from_inner_memory(self.inner_memory.len);
println!(
Expand Down Expand Up @@ -684,7 +684,7 @@
impl<T: Buffer> Drop for BufferPool<T> {
fn drop(&mut self) {
while self.shared_state.load(Ordering::Relaxed) != 0 {
std::hint::spin_loop();
core::hint::spin_loop();

Check warning on line 687 in utils/buffer/src/buffer_pool/mod.rs

View check run for this annotation

Codecov / codecov/patch

utils/buffer/src/buffer_pool/mod.rs#L687

Added line #L687 was not covered by tests
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion utils/buffer/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//#![cfg_attr(not(feature = "debug"), no_std)]
#![cfg_attr(not(feature = "debug"), no_std)]
//#![feature(backtrace)]

mod buffer;
Expand Down
Loading