From 27c7c4e26a3eeef3b85c9a392e3abaabef089fa9 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Wed, 4 Oct 2023 05:51:26 +0000 Subject: [PATCH 01/16] Add a `bitcoin_io` crate In order to support standard (de)serialization of structs, the `rust-bitcoin` ecosystem uses the standard `std::io::{Read,Write}` traits. This works great for environments with `std`, however sadly the `std::io` module has not yet been added to the `core` crate. Thus, in `no-std`, the `rust-bitcoin` ecosystem has historically used the `core2` crate to provide copies of the `std::io` module without any major dependencies. Sadly, its one dependency, `memchr`, recently broke our MSRV. Worse, because we didn't want to take on any excess dependencies for `std` builds, `rust-bitcoin` has had to have mutually-exclusive `std` and `no-std` builds. This breaks general assumptions about how features work in Rust, causing substantial pain for applications far downstream of `rust-bitcoin` crates. Here, we add a new `bitcoin_io` crate, making it an unconditional dependency and using its `io` module in the in-repository crates in place of `std::io` and `core2::io`. As it is not substantial additional code, the `hashes` io implementations are no longer feature-gated. This doesn't actually accomplish anything on its own, only adding the new crate which still depends on `core2`. --- Cargo-minimal.lock | 11 ++++++-- Cargo-recent.lock | 11 ++++++-- Cargo.toml | 5 +++- bitcoin/Cargo.toml | 8 +++--- bitcoin/embedded/Cargo.toml | 4 ++- bitcoin/src/lib.rs | 25 ++++++------------ hashes/Cargo.toml | 9 +++---- hashes/embedded/Cargo.toml | 5 +++- hashes/embedded/src/main.rs | 2 +- hashes/extended_tests/schemars/Cargo.toml | 3 +++ hashes/src/lib.rs | 12 +++------ io/Cargo.toml | 25 ++++++++++++++++++ io/src/lib.rs | 32 +++++++++++++++++++++++ 13 files changed, 110 insertions(+), 42 deletions(-) create mode 100644 io/Cargo.toml create mode 100644 io/src/lib.rs diff --git a/Cargo-minimal.lock b/Cargo-minimal.lock index 0b1392fd..9f7011eb 100644 --- a/Cargo-minimal.lock +++ b/Cargo-minimal.lock @@ -38,9 +38,9 @@ dependencies = [ "bech32", "bincode", "bitcoin-internals", + "bitcoin-io", "bitcoin_hashes", "bitcoinconsensus", - "core2", "hex-conservative", "hex_lit", "mutagen", @@ -69,11 +69,18 @@ dependencies = [ "serde", ] +[[package]] +name = "bitcoin-io" +version = "0.1.0" +dependencies = [ + "core2", +] + [[package]] name = "bitcoin_hashes" version = "0.13.0" dependencies = [ - "core2", + "bitcoin-io", "hex-conservative", "schemars", "serde", diff --git a/Cargo-recent.lock b/Cargo-recent.lock index 307e739e..932cd2b5 100644 --- a/Cargo-recent.lock +++ b/Cargo-recent.lock @@ -37,9 +37,9 @@ dependencies = [ "bech32", "bincode", "bitcoin-internals", + "bitcoin-io", "bitcoin_hashes", "bitcoinconsensus", - "core2", "hex-conservative", "hex_lit", "mutagen", @@ -68,11 +68,18 @@ dependencies = [ "serde", ] +[[package]] +name = "bitcoin-io" +version = "0.1.0" +dependencies = [ + "core2", +] + [[package]] name = "bitcoin_hashes" version = "0.13.0" dependencies = [ - "core2", + "bitcoin-io", "hex-conservative", "schemars", "serde", diff --git a/Cargo.toml b/Cargo.toml index 643dc29c..42895c92 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,5 +1,5 @@ [workspace] -members = ["bitcoin", "hashes", "internals", "fuzz"] +members = ["bitcoin", "hashes", "internals", "fuzz", "io"] [patch.crates-io.bitcoin] path = "bitcoin" @@ -9,3 +9,6 @@ path = "hashes" [patch.crates-io.bitcoin-internals] path = "internals" + +[patch.crates-io.bitcoin-io] +path = "io" diff --git a/bitcoin/Cargo.toml b/bitcoin/Cargo.toml index 1f1926a4..5556e3e4 100644 --- a/bitcoin/Cargo.toml +++ b/bitcoin/Cargo.toml @@ -26,8 +26,8 @@ bitcoinconsensus-std = ["bitcoinconsensus/std", "std"] # The no-std feature doesn't disable std - you need to turn off the std feature for that by disabling default. # Instead no-std enables additional features required for this crate to be usable without std. # As a result, both can be enabled without conflict. -std = ["secp256k1/std", "hashes/std", "bech32/std", "internals/std", "hex/std"] -no-std = ["core2", "hashes/alloc", "hashes/core2", "bech32/alloc", "secp256k1/alloc", "hex/alloc", "hex/core2"] +std = ["secp256k1/std", "bitcoin-io/std", "hashes/std", "bech32/std", "internals/std", "hex/std"] +no-std = ["hashes/alloc", "hashes/core2", "bitcoin-io/core2", "bitcoin-io/alloc", "bech32/alloc", "secp256k1/alloc", "hex/alloc", "hex/core2"] [package.metadata.docs.rs] all-features = true @@ -41,12 +41,12 @@ hashes = { package = "bitcoin_hashes", version = "0.13.0", default-features = fa secp256k1 = { version = "0.28.0", default-features = false, features = ["hashes"] } hex_lit = "0.1.1" +bitcoin-io = { version = "0.1", default-features = false } + base64 = { version = "0.21.3", optional = true } # Only use this feature for no-std builds, otherwise use bitcoinconsensus-std. bitcoinconsensus = { version = "0.20.2-0.5.0", default-features = false, optional = true } -# There is no reason to use this dependency directly, it is activated by the "no-std" feature. -core2 = { version = "0.3.2", default-features = false, features = ["alloc"], optional = true } # Do NOT use this as a feature! Use the `serde` feature instead. actual-serde = { package = "serde", version = "1.0.103", default-features = false, features = [ "derive", "alloc" ], optional = true } diff --git a/bitcoin/embedded/Cargo.toml b/bitcoin/embedded/Cargo.toml index f68d7f3f..9ea41c17 100644 --- a/bitcoin/embedded/Cargo.toml +++ b/bitcoin/embedded/Cargo.toml @@ -17,7 +17,6 @@ panic-halt = "0.2.0" alloc-cortex-m = "0.4.1" bitcoin = { path="../", default-features = false, features = ["no-std", "secp-lowmemory"] } - [[bin]] name = "embedded" test = false @@ -33,3 +32,6 @@ path = "../../hashes" [patch.crates-io.bitcoin-internals] path = "../../internals" + +[patch.crates-io.bitcoin-io] +path = "../../io" diff --git a/bitcoin/src/lib.rs b/bitcoin/src/lib.rs index 68b7d246..03d516ed 100644 --- a/bitcoin/src/lib.rs +++ b/bitcoin/src/lib.rs @@ -23,7 +23,7 @@ //! deserialization. //! * `secp-lowmemory` - optimizations for low-memory devices. //! * `no-std` - enables additional features required for this crate to be usable -//! without std. Does **not** disable `std`. Depends on `core2`. +//! without std. Does **not** disable `std`. //! * `bitcoinconsensus-std` - enables `std` in `bitcoinconsensus` and communicates it //! to this crate so it knows how to implement //! `std::error::Error`. At this time there's a hack to @@ -68,9 +68,6 @@ pub extern crate bech32; /// Bitcoin's libbitcoinconsensus with Rust binding. pub extern crate bitcoinconsensus; -#[cfg(not(feature = "std"))] -extern crate core2; - /// Rust implementation of cryptographic hash function algorithems. pub extern crate hashes; @@ -118,16 +115,8 @@ pub mod taproot; // May depend on crate features and we don't want to bother with it #[allow(unused)] -#[cfg(feature = "std")] -use std::error::Error as StdError; -#[cfg(feature = "std")] -use std::io; - -#[allow(unused)] -#[cfg(not(feature = "std"))] -use core2::error::Error as StdError; -#[cfg(not(feature = "std"))] -use core2::io; +use bitcoin_io::error::Error as StdError; +use bitcoin_io::io; pub use crate::address::{Address, AddressType}; pub use crate::amount::{Amount, Denomination, SignedAmount}; @@ -163,6 +152,8 @@ pub use crate::taproot::{ #[cfg(not(feature = "std"))] mod io_extras { + use crate::io; + /// A writer which will move data into the void. pub struct Sink { _priv: (), @@ -171,12 +162,12 @@ mod io_extras { /// Creates an instance of a writer which will successfully consume all data. pub const fn sink() -> Sink { Sink { _priv: () } } - impl core2::io::Write for Sink { + impl io::Write for Sink { #[inline] - fn write(&mut self, buf: &[u8]) -> core2::io::Result { Ok(buf.len()) } + fn write(&mut self, buf: &[u8]) -> io::Result { Ok(buf.len()) } #[inline] - fn flush(&mut self) -> core2::io::Result<()> { Ok(()) } + fn flush(&mut self) -> io::Result<()> { Ok(()) } } } diff --git a/hashes/Cargo.toml b/hashes/Cargo.toml index 6f91a020..276fb3de 100644 --- a/hashes/Cargo.toml +++ b/hashes/Cargo.toml @@ -14,11 +14,11 @@ exclude = ["tests", "contrib"] [features] default = ["std"] -std = ["alloc", "hex/std"] +std = ["alloc", "hex/std", "bitcoin-io/std"] alloc = ["hex/alloc"] serde-std = ["serde/std"] # If you want I/O you must enable either "std" or "core2". -core2 = ["actual-core2", "hex/core2"] +core2 = ["bitcoin-io/core2", "hex/core2"] # Smaller (but slower) implementation of sha256, sha512 and ripemd160 small-hash = [] @@ -29,13 +29,12 @@ rustdoc-args = ["--cfg", "docsrs"] [dependencies] hex = { package = "hex-conservative", version = "0.1.1", default-features = false } +bitcoin-io = { version = "0.1", default-features = false, optional = true } + schemars = { version = "0.8.3", optional = true } # Only enable this if you explicitly do not want to use "std", otherwise enable "serde-std". serde = { version = "1.0", default-features = false, optional = true } -# Do NOT use this feature! Use the "core2" feature instead. -actual-core2 = { package = "core2", version = "0.3.2", default-features = false, optional = true } - [dev-dependencies] serde_test = "1.0" serde_json = "1.0" diff --git a/hashes/embedded/Cargo.toml b/hashes/embedded/Cargo.toml index e44e0344..5bb9af26 100644 --- a/hashes/embedded/Cargo.toml +++ b/hashes/embedded/Cargo.toml @@ -19,7 +19,7 @@ cortex-m-semihosting = "0.3.3" panic-halt = "0.2.0" alloc-cortex-m = { version = "0.4.1", optional = true } bitcoin_hashes = { path="../", default-features = false, features = ["core2"] } -core2 = { version = "0.3.0", default_features = false } +bitcoin-io = { path = "../../io", default_features = false } [[bin]] name = "embedded" @@ -33,3 +33,6 @@ lto = true # better optimizations [patch.crates-io.bitcoin-internals] path = "../../internals" + +[patch.crates-io.bitcoin-io] +path = "../../io" diff --git a/hashes/embedded/src/main.rs b/hashes/embedded/src/main.rs index a43e7106..bba22ff8 100644 --- a/hashes/embedded/src/main.rs +++ b/hashes/embedded/src/main.rs @@ -10,7 +10,7 @@ extern crate bitcoin_hashes; #[cfg(feature = "alloc")] use alloc::string::ToString; use bitcoin_hashes::{sha256, Hash, HashEngine}; -use core2::io::Write; +use bitcoin_io::io::Write; use core::str::FromStr; use cortex_m_rt::entry; use cortex_m_semihosting::{debug, hprintln}; diff --git a/hashes/extended_tests/schemars/Cargo.toml b/hashes/extended_tests/schemars/Cargo.toml index 7c523c0f..8a8943d2 100644 --- a/hashes/extended_tests/schemars/Cargo.toml +++ b/hashes/extended_tests/schemars/Cargo.toml @@ -21,3 +21,6 @@ serde_json = "1.0" [patch.crates-io.bitcoin-internals] path = "../../../internals" + +[patch.crates-io.bitcoin-io] +path = "../../../io" diff --git a/hashes/src/lib.rs b/hashes/src/lib.rs index aa1c6ffc..91740f4e 100644 --- a/hashes/src/lib.rs +++ b/hashes/src/lib.rs @@ -81,8 +81,6 @@ // Exclude clippy lints we don't think are valuable #![allow(clippy::needless_question_mark)] // https://github.com/rust-bitcoin/rust-bitcoin/pull/2134 -#[cfg(all(not(test), not(feature = "std"), feature = "core2"))] -extern crate actual_core2 as core2; #[cfg(all(feature = "alloc", not(feature = "std")))] extern crate alloc; #[cfg(any(test, feature = "std"))] @@ -119,7 +117,7 @@ pub mod serde_macros; pub mod cmp; pub mod hash160; pub mod hmac; -#[cfg(any(test, feature = "std", feature = "core2"))] +#[cfg(feature = "bitcoin-io")] mod impls; pub mod ripemd160; pub mod sha1; @@ -131,12 +129,10 @@ pub mod sha512_256; pub mod siphash24; use core::{borrow, fmt, hash, ops}; -// You get I/O if you enable "std" or "core2" (as well as during testing). -#[cfg(any(test, feature = "std"))] -use std::io; -#[cfg(all(not(test), not(feature = "std"), feature = "core2"))] -use core2::io; +// You get I/O if you enable "std" or "core2" (as well as during testing). +#[cfg(feature = "bitcoin-io")] +use bitcoin_io::io; pub use hmac::{Hmac, HmacEngine}; /// A hashing engine which bytes can be serialized into. diff --git a/io/Cargo.toml b/io/Cargo.toml new file mode 100644 index 00000000..ea5bc43a --- /dev/null +++ b/io/Cargo.toml @@ -0,0 +1,25 @@ +[package] +name = "bitcoin-io" +version = "0.1.0" +authors = ["Matt Corallo "] +license = "CC0-1.0" +repository = "https://github.com/rust-bitcoin/rust-bitcoin" +documentation = "https://docs.rs/bitcoin-io/" +description = "Simple I/O traits for no-std (and std) environments" +categories = ["no-std"] +keywords = [ "io", "no-std" ] +readme = "README.md" +edition = "2018" +exclude = ["tests", "contrib"] + +[features] +default = ["std"] +std = [] +alloc = ["core2/alloc"] + +[package.metadata.docs.rs] +all-features = true +rustdoc-args = ["--cfg", "docsrs"] + +[dependencies] +core2 = { version = "0.3", default-features = false, optional = true } diff --git a/io/src/lib.rs b/io/src/lib.rs new file mode 100644 index 00000000..766dce62 --- /dev/null +++ b/io/src/lib.rs @@ -0,0 +1,32 @@ +//! Rust-Bitcoin IO Library +//! +//! Because the core `std::io` module is not yet exposed in `no-std` Rust, building `no-std` +//! applications which require reading and writing objects via standard traits is not generally +//! possible. While there is ongoing work to improve this situation, this module is not likely to +//! be available for applications with broad rustc version support for some time. +//! +//! Thus, this library exists to export a minmal version of `std::io`'s traits which `no-std` +//! applications may need. With the `std` feature, these traits are also implemented for the +//! `std::io` traits, allowing standard objects to be used wherever the traits from this crate are +//! required. +//! +//! This traits are not one-for-one drop-ins, but are as close as possible while still implementing +//! `std::io`'s traits without unnecessary complexity. + +// Experimental features we need. +#![cfg_attr(docsrs, feature(doc_auto_cfg))] + +#![cfg_attr(not(feature = "std"), no_std)] + +#[cfg(all(not(feature = "std"), not(feature = "core2")))] +compile_error!("At least one of std or core2 must be enabled"); + +#[cfg(feature = "std")] +pub use std::io; +#[cfg(feature = "std")] +pub use std::error; + +#[cfg(not(feature = "std"))] +pub use core2::io; +#[cfg(not(feature = "std"))] +pub use core2::error; From a0ade883b69ab0f7bbc8b6198b6795855d8e00f7 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sat, 9 Sep 2023 23:31:48 +0000 Subject: [PATCH 02/16] [IO] Move io module into selected re-exports In the coming commits we'll move our `io` module to having its own implementations of the usual I/O traits and structs. Here we first move to re-exporting only exactly what we need, allowing us to whittle the list down from a fixed set. --- io/src/lib.rs | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/io/src/lib.rs b/io/src/lib.rs index 766dce62..e09557a1 100644 --- a/io/src/lib.rs +++ b/io/src/lib.rs @@ -21,12 +21,25 @@ #[cfg(all(not(feature = "std"), not(feature = "core2")))] compile_error!("At least one of std or core2 must be enabled"); -#[cfg(feature = "std")] -pub use std::io; #[cfg(feature = "std")] pub use std::error; - -#[cfg(not(feature = "std"))] -pub use core2::io; #[cfg(not(feature = "std"))] pub use core2::error; + +#[cfg(any(feature = "alloc", feature = "std"))] +extern crate alloc; + +/// Standard I/O stream definitions which are API-equivalent to `std`'s `io` module. See +/// [`std::io`] for more info. +pub mod io { + #[cfg(all(not(feature = "std"), not(feature = "core2")))] + compile_error!("At least one of std or core2 must be enabled"); + + #[cfg(feature = "std")] + pub use std::io::{Read, Write, Cursor, Take, IoSlice, Error, ErrorKind, Result}; + + #[cfg(not(feature = "std"))] + pub use core2::io::{Read, Write, Cursor, Take, Error, ErrorKind, Result}; + + +} From 5e0209569c1eb4005c08fb6da9387a95203480b7 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 12 Sep 2023 17:45:08 +0000 Subject: [PATCH 03/16] Use `io::sink` rather than our custom `EmptyWrite` utility --- bitcoin/src/blockdata/block.rs | 4 ++-- bitcoin/src/blockdata/transaction.rs | 4 ++-- bitcoin/src/lib.rs | 24 ------------------------ bitcoin/src/p2p/message_bloom.rs | 3 +-- io/src/lib.rs | 2 +- 5 files changed, 6 insertions(+), 31 deletions(-) diff --git a/bitcoin/src/blockdata/block.rs b/bitcoin/src/blockdata/block.rs index dfa2381a..be5427f4 100644 --- a/bitcoin/src/blockdata/block.rs +++ b/bitcoin/src/blockdata/block.rs @@ -637,7 +637,7 @@ mod benches { use super::Block; use crate::consensus::{deserialize, Decodable, Encodable}; - use crate::EmptyWrite; + use crate::io::sink; #[bench] pub fn bench_stream_reader(bh: &mut Bencher) { @@ -674,7 +674,7 @@ mod benches { let block: Block = deserialize(&raw_block[..]).unwrap(); bh.iter(|| { - let size = block.consensus_encode(&mut EmptyWrite); + let size = block.consensus_encode(&mut sink()); black_box(&size); }); } diff --git a/bitcoin/src/blockdata/transaction.rs b/bitcoin/src/blockdata/transaction.rs index 22c4b3b0..21ee67b7 100644 --- a/bitcoin/src/blockdata/transaction.rs +++ b/bitcoin/src/blockdata/transaction.rs @@ -2112,7 +2112,7 @@ mod benches { use super::Transaction; use crate::consensus::{deserialize, Encodable}; - use crate::EmptyWrite; + use crate::io::sink; const SOME_TX: &str = "0100000001a15d57094aa7a21a28cb20b59aab8fc7d1149a3bdbcddba9c622e4f5f6a99ece010000006c493046022100f93bb0e7d8db7bd46e40132d1f8242026e045f03a0efe71bbb8e3f475e970d790221009337cd7f1f929f00cc6ff01f03729b069a7c21b59b1736ddfee5db5946c5da8c0121033b9b137ee87d5a812d6f506efdd37f0affa7ffc310711c06c7f3e097c9447c52ffffffff0100e1f505000000001976a9140389035a9225b3839e2bbf32d826a1e222031fd888ac00000000"; @@ -2147,7 +2147,7 @@ mod benches { let tx: Transaction = deserialize(&raw_tx).unwrap(); bh.iter(|| { - let size = tx.consensus_encode(&mut EmptyWrite); + let size = tx.consensus_encode(&mut sink()); black_box(&size); }); } diff --git a/bitcoin/src/lib.rs b/bitcoin/src/lib.rs index 03d516ed..65984cbf 100644 --- a/bitcoin/src/lib.rs +++ b/bitcoin/src/lib.rs @@ -196,27 +196,3 @@ mod prelude { pub use hex::DisplayHex; } - -#[cfg(bench)] -use bench::EmptyWrite; - -#[cfg(bench)] -mod bench { - use core::fmt::Arguments; - - use crate::io::{IoSlice, Result, Write}; - - #[derive(Default, Clone, Debug, PartialEq, Eq)] - pub struct EmptyWrite; - - impl Write for EmptyWrite { - fn write(&mut self, buf: &[u8]) -> Result { Ok(buf.len()) } - fn write_vectored(&mut self, bufs: &[IoSlice]) -> Result { - Ok(bufs.iter().map(|s| s.len()).sum()) - } - fn flush(&mut self) -> Result<()> { Ok(()) } - - fn write_all(&mut self, _: &[u8]) -> Result<()> { Ok(()) } - fn write_fmt(&mut self, _: Arguments) -> Result<()> { Ok(()) } - } -} diff --git a/bitcoin/src/p2p/message_bloom.rs b/bitcoin/src/p2p/message_bloom.rs index e3dcbe18..812ce2f5 100644 --- a/bitcoin/src/p2p/message_bloom.rs +++ b/bitcoin/src/p2p/message_bloom.rs @@ -5,10 +5,9 @@ //! This module describes BIP37 Connection Bloom filtering network messages. //! -use std::io; - use crate::consensus::{encode, Decodable, Encodable, ReadExt}; use crate::internal_macros::impl_consensus_encoding; +use crate::io; /// `filterload` message sets the current bloom filter #[derive(Clone, PartialEq, Eq, Debug)] diff --git a/io/src/lib.rs b/io/src/lib.rs index e09557a1..e4a4a79e 100644 --- a/io/src/lib.rs +++ b/io/src/lib.rs @@ -36,7 +36,7 @@ pub mod io { compile_error!("At least one of std or core2 must be enabled"); #[cfg(feature = "std")] - pub use std::io::{Read, Write, Cursor, Take, IoSlice, Error, ErrorKind, Result}; + pub use std::io::{Read, Write, sink, Cursor, Take, Error, ErrorKind, Result}; #[cfg(not(feature = "std"))] pub use core2::io::{Read, Write, Cursor, Take, Error, ErrorKind, Result}; From 2348449d2a39db6212eff16861c4bff58c015ede Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sun, 10 Sep 2023 01:04:34 +0000 Subject: [PATCH 04/16] Stop relying on `std::io::Write`'s `&mut Write` blanket impl `std::io::Write` is implemented for all `&mut std::io::Write`. This makes it easy to have APIs that mix and match owned `Write`s with mutable references to `Write`s. However, in the next commit we add our own `Write` trait which we intend to implement for all `std::io::Write`. Sadly, this is mutually exclusive with a blanket implementation on our own `&mut Write`, as that would conflict with an `std::io::Write` blanket impl. Thus, in order to use the `Write for all &mut Write` blanket impl in rust-bitcoin, we'd have to bound all `Write`s by `std::io::Write`, as we're unable to provide a blanket `Write for &mut Write` impl. Here we stop relying on that blanket impl in order to introduce the new trait in the next commit. --- bitcoin/src/bip158.rs | 2 +- bitcoin/src/consensus/encode.rs | 6 +-- bitcoin/src/crypto/key.rs | 2 +- bitcoin/src/crypto/sighash.rs | 76 ++++++++++++++++----------------- bitcoin/src/taproot.rs | 6 +-- 5 files changed, 46 insertions(+), 46 deletions(-) diff --git a/bitcoin/src/bip158.rs b/bitcoin/src/bip158.rs index 2028070d..61b3c6e2 100644 --- a/bitcoin/src/bip158.rs +++ b/bitcoin/src/bip158.rs @@ -393,7 +393,7 @@ impl<'a, W: io::Write> GcsFilterWriter<'a, W> { mapped.sort_unstable(); // write number of elements as varint - let mut wrote = VarInt::from(mapped.len()).consensus_encode(&mut self.writer)?; + let mut wrote = VarInt::from(mapped.len()).consensus_encode(self.writer)?; // write out deltas of sorted values into a Golonb-Rice coded bit stream let mut writer = BitStreamWriter::new(self.writer); diff --git a/bitcoin/src/consensus/encode.rs b/bitcoin/src/consensus/encode.rs index b6ed05b5..42d11a1e 100644 --- a/bitcoin/src/consensus/encode.rs +++ b/bitcoin/src/consensus/encode.rs @@ -643,11 +643,11 @@ impl_vec!((u32, Address)); #[cfg(feature = "std")] impl_vec!(AddrV2Message); -pub(crate) fn consensus_encode_with_size( +pub(crate) fn consensus_encode_with_size( data: &[u8], - mut w: W, + w: &mut W, ) -> Result { - let vi_len = VarInt(data.len() as u64).consensus_encode(&mut w)?; + let vi_len = VarInt(data.len() as u64).consensus_encode(w)?; w.emit_slice(data)?; Ok(vi_len + data.len()) } diff --git a/bitcoin/src/crypto/key.rs b/bitcoin/src/crypto/key.rs index 2805f4e3..366369a4 100644 --- a/bitcoin/src/crypto/key.rs +++ b/bitcoin/src/crypto/key.rs @@ -68,7 +68,7 @@ impl PublicKey { } /// Write the public key into a writer - pub fn write_into(&self, mut writer: W) -> Result<(), io::Error> { + pub fn write_into(&self, writer: &mut W) -> Result<(), io::Error> { self.with_serialized(|bytes| writer.write_all(bytes)) } diff --git a/bitcoin/src/crypto/sighash.rs b/bitcoin/src/crypto/sighash.rs index 2b5cd835..859aa34d 100644 --- a/bitcoin/src/crypto/sighash.rs +++ b/bitcoin/src/crypto/sighash.rs @@ -675,7 +675,7 @@ impl> SighashCache { /// [`io::Write`] trait. pub fn taproot_encode_signing_data_to>( &mut self, - mut writer: Write, + writer: &mut Write, input_index: usize, prevouts: &Prevouts, annex: Option, @@ -687,18 +687,18 @@ impl> SighashCache { let (sighash, anyone_can_pay) = sighash_type.split_anyonecanpay_flag(); // epoch - 0u8.consensus_encode(&mut writer)?; + 0u8.consensus_encode(writer)?; // * Control: // hash_type (1). - (sighash_type as u8).consensus_encode(&mut writer)?; + (sighash_type as u8).consensus_encode(writer)?; // * Transaction Data: // nVersion (4): the nVersion of the transaction. - self.tx.borrow().version.consensus_encode(&mut writer)?; + self.tx.borrow().version.consensus_encode(writer)?; // nLockTime (4): the nLockTime of the transaction. - self.tx.borrow().lock_time.consensus_encode(&mut writer)?; + self.tx.borrow().lock_time.consensus_encode(writer)?; // If the hash_type & 0x80 does not equal SIGHASH_ANYONECANPAY: // sha_prevouts (32): the SHA256 of the serialization of all input outpoints. @@ -706,16 +706,16 @@ impl> SighashCache { // sha_scriptpubkeys (32): the SHA256 of the serialization of all spent output scriptPubKeys. // sha_sequences (32): the SHA256 of the serialization of all input nSequence. if !anyone_can_pay { - self.common_cache().prevouts.consensus_encode(&mut writer)?; - self.taproot_cache(prevouts.get_all()?).amounts.consensus_encode(&mut writer)?; - self.taproot_cache(prevouts.get_all()?).script_pubkeys.consensus_encode(&mut writer)?; - self.common_cache().sequences.consensus_encode(&mut writer)?; + self.common_cache().prevouts.consensus_encode(writer)?; + self.taproot_cache(prevouts.get_all()?).amounts.consensus_encode(writer)?; + self.taproot_cache(prevouts.get_all()?).script_pubkeys.consensus_encode(writer)?; + self.common_cache().sequences.consensus_encode(writer)?; } // If hash_type & 3 does not equal SIGHASH_NONE or SIGHASH_SINGLE: // sha_outputs (32): the SHA256 of the serialization of all outputs in CTxOut format. if sighash != TapSighashType::None && sighash != TapSighashType::Single { - self.common_cache().outputs.consensus_encode(&mut writer)?; + self.common_cache().outputs.consensus_encode(writer)?; } // * Data about this input: @@ -728,7 +728,7 @@ impl> SighashCache { if leaf_hash_code_separator.is_some() { spend_type |= 2u8; } - spend_type.consensus_encode(&mut writer)?; + spend_type.consensus_encode(writer)?; // If hash_type & 0x80 equals SIGHASH_ANYONECANPAY: // outpoint (36): the COutPoint of this input (32-byte hash + 4-byte little-endian). @@ -742,12 +742,12 @@ impl> SighashCache { inputs_size: self.tx.borrow().input.len(), })?; let previous_output = prevouts.get(input_index)?; - txin.previous_output.consensus_encode(&mut writer)?; - previous_output.value.consensus_encode(&mut writer)?; - previous_output.script_pubkey.consensus_encode(&mut writer)?; - txin.sequence.consensus_encode(&mut writer)?; + txin.previous_output.consensus_encode(writer)?; + previous_output.value.consensus_encode(writer)?; + previous_output.script_pubkey.consensus_encode(writer)?; + txin.sequence.consensus_encode(writer)?; } else { - (input_index as u32).consensus_encode(&mut writer)?; + (input_index as u32).consensus_encode(writer)?; } // If an annex is present (the lowest bit of spend_type is set): @@ -757,7 +757,7 @@ impl> SighashCache { let mut enc = sha256::Hash::engine(); annex.consensus_encode(&mut enc)?; let hash = sha256::Hash::from_engine(enc); - hash.consensus_encode(&mut writer)?; + hash.consensus_encode(writer)?; } // * Data about this output: @@ -775,7 +775,7 @@ impl> SighashCache { })? .consensus_encode(&mut enc)?; let hash = sha256::Hash::from_engine(enc); - hash.consensus_encode(&mut writer)?; + hash.consensus_encode(writer)?; } // if (scriptpath): @@ -783,9 +783,9 @@ impl> SighashCache { // ss += bytes([0]) // ss += struct.pack("> SighashCache { /// [`Self::p2wpkh_signature_hash`] and [`SighashCache::p2wsh_signature_hash`].) pub fn segwit_v0_encode_signing_data_to( &mut self, - mut writer: Write, + writer: &mut Write, input_index: usize, script_code: &Script, value: Amount, @@ -872,21 +872,21 @@ impl> SighashCache { let (sighash, anyone_can_pay) = sighash_type.split_anyonecanpay_flag(); - self.tx.borrow().version.consensus_encode(&mut writer)?; + self.tx.borrow().version.consensus_encode(writer)?; if !anyone_can_pay { - self.segwit_cache().prevouts.consensus_encode(&mut writer)?; + self.segwit_cache().prevouts.consensus_encode(writer)?; } else { - zero_hash.consensus_encode(&mut writer)?; + zero_hash.consensus_encode(writer)?; } if !anyone_can_pay && sighash != EcdsaSighashType::Single && sighash != EcdsaSighashType::None { - self.segwit_cache().sequences.consensus_encode(&mut writer)?; + self.segwit_cache().sequences.consensus_encode(writer)?; } else { - zero_hash.consensus_encode(&mut writer)?; + zero_hash.consensus_encode(writer)?; } { @@ -896,14 +896,14 @@ impl> SighashCache { inputs_size: self.tx.borrow().input.len(), })?; - txin.previous_output.consensus_encode(&mut writer)?; - script_code.consensus_encode(&mut writer)?; - value.consensus_encode(&mut writer)?; - txin.sequence.consensus_encode(&mut writer)?; + txin.previous_output.consensus_encode(writer)?; + script_code.consensus_encode(writer)?; + value.consensus_encode(writer)?; + txin.sequence.consensus_encode(writer)?; } if sighash != EcdsaSighashType::Single && sighash != EcdsaSighashType::None { - self.segwit_cache().outputs.consensus_encode(&mut writer)?; + self.segwit_cache().outputs.consensus_encode(writer)?; } else if sighash == EcdsaSighashType::Single && input_index < self.tx.borrow().output.len() { let mut single_enc = LegacySighash::engine(); @@ -914,8 +914,8 @@ impl> SighashCache { writer.write_all(&zero_hash[..])?; } - self.tx.borrow().lock_time.consensus_encode(&mut writer)?; - sighash_type.to_u32().consensus_encode(&mut writer)?; + self.tx.borrow().lock_time.consensus_encode(writer)?; + sighash_type.to_u32().consensus_encode(writer)?; Ok(()) } @@ -986,7 +986,7 @@ impl> SighashCache { /// that must be handled by the caller (see [`EncodeSigningDataResult::is_sighash_single_bug`]). pub fn legacy_encode_signing_data_to>( &self, - writer: Write, + writer: &mut Write, input_index: usize, script_pubkey: &Script, sighash_type: U, @@ -1013,7 +1013,7 @@ impl> SighashCache { fn encode_signing_data_to_inner( self_: &Transaction, - mut writer: Write, + writer: &mut Write, input_index: usize, script_pubkey: &Script, sighash_type: u32, @@ -1074,8 +1074,8 @@ impl> SighashCache { _ => unreachable!(), }; // hash the result - tx.consensus_encode(&mut writer)?; - sighash_type.to_le_bytes().consensus_encode(&mut writer)?; + tx.consensus_encode(writer)?; + sighash_type.to_le_bytes().consensus_encode(writer)?; Ok(()) } diff --git a/bitcoin/src/taproot.rs b/bitcoin/src/taproot.rs index 6be5e586..addf3de0 100644 --- a/bitcoin/src/taproot.rs +++ b/bitcoin/src/taproot.rs @@ -1109,7 +1109,7 @@ impl TaprootMerkleBranch { /// # Returns /// /// The number of bytes written to the writer. - pub fn encode(&self, mut writer: Write) -> io::Result { + pub fn encode(&self, writer: &mut Write) -> io::Result { for hash in self.0.iter() { writer.write_all(hash.as_ref())?; } @@ -1237,12 +1237,12 @@ impl ControlBlock { /// # Returns /// /// The number of bytes written to the writer. - pub fn encode(&self, mut writer: Write) -> io::Result { + pub fn encode(&self, writer: &mut Write) -> io::Result { let first_byte: u8 = i32::from(self.output_key_parity) as u8 | self.leaf_version.to_consensus(); writer.write_all(&[first_byte])?; writer.write_all(&self.internal_key.serialize())?; - self.merkle_branch.encode(&mut writer)?; + self.merkle_branch.encode(writer)?; Ok(self.size()) } From 5f2395ce5635d93ca27a445df58188b21e2bceda Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Sun, 5 Nov 2023 04:33:44 +0000 Subject: [PATCH 05/16] Add missing `?Sized` bounds to `io::Write` parameters Since we are no longer relying on the blanket `io::Write` impl for `&mut io::Write`, we should now ensure that we do not require `Sized` for our `io::Write` bounds, as its unnecessarily restrictive and can no longer be worked around by simply adding an `&mut`. --- bitcoin/src/crypto/key.rs | 2 +- bitcoin/src/crypto/sighash.rs | 8 ++++---- bitcoin/src/taproot.rs | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/bitcoin/src/crypto/key.rs b/bitcoin/src/crypto/key.rs index 366369a4..7ba63a89 100644 --- a/bitcoin/src/crypto/key.rs +++ b/bitcoin/src/crypto/key.rs @@ -68,7 +68,7 @@ impl PublicKey { } /// Write the public key into a writer - pub fn write_into(&self, writer: &mut W) -> Result<(), io::Error> { + pub fn write_into(&self, writer: &mut W) -> Result<(), io::Error> { self.with_serialized(|bytes| writer.write_all(bytes)) } diff --git a/bitcoin/src/crypto/sighash.rs b/bitcoin/src/crypto/sighash.rs index 859aa34d..6fb2ddbe 100644 --- a/bitcoin/src/crypto/sighash.rs +++ b/bitcoin/src/crypto/sighash.rs @@ -673,7 +673,7 @@ impl> SighashCache { /// Encodes the BIP341 signing data for any flag type into a given object implementing a /// [`io::Write`] trait. - pub fn taproot_encode_signing_data_to>( + pub fn taproot_encode_signing_data_to>( &mut self, writer: &mut Write, input_index: usize, @@ -860,7 +860,7 @@ impl> SighashCache { /// `script_code` is dependent on the type of the spend transaction. For p2wpkh use /// [`Script::p2wpkh_script_code`], for p2wsh just pass in the witness script. (Also see /// [`Self::p2wpkh_signature_hash`] and [`SighashCache::p2wsh_signature_hash`].) - pub fn segwit_v0_encode_signing_data_to( + pub fn segwit_v0_encode_signing_data_to( &mut self, writer: &mut Write, input_index: usize, @@ -984,7 +984,7 @@ impl> SighashCache { /// /// This function can't handle the SIGHASH_SINGLE bug internally, so it returns [`EncodeSigningDataResult`] /// that must be handled by the caller (see [`EncodeSigningDataResult::is_sighash_single_bug`]). - pub fn legacy_encode_signing_data_to>( + pub fn legacy_encode_signing_data_to>( &self, writer: &mut Write, input_index: usize, @@ -1011,7 +1011,7 @@ impl> SighashCache { return EncodeSigningDataResult::SighashSingleBug; } - fn encode_signing_data_to_inner( + fn encode_signing_data_to_inner( self_: &Transaction, writer: &mut Write, input_index: usize, diff --git a/bitcoin/src/taproot.rs b/bitcoin/src/taproot.rs index addf3de0..c6bd03c5 100644 --- a/bitcoin/src/taproot.rs +++ b/bitcoin/src/taproot.rs @@ -1109,7 +1109,7 @@ impl TaprootMerkleBranch { /// # Returns /// /// The number of bytes written to the writer. - pub fn encode(&self, writer: &mut Write) -> io::Result { + pub fn encode(&self, writer: &mut Write) -> io::Result { for hash in self.0.iter() { writer.write_all(hash.as_ref())?; } @@ -1237,7 +1237,7 @@ impl ControlBlock { /// # Returns /// /// The number of bytes written to the writer. - pub fn encode(&self, writer: &mut Write) -> io::Result { + pub fn encode(&self, writer: &mut Write) -> io::Result { let first_byte: u8 = i32::from(self.output_key_parity) as u8 | self.leaf_version.to_consensus(); writer.write_all(&[first_byte])?; From ac678bb4356cb081e2fa042c69477a9b9d9a0e29 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 12 Sep 2023 17:47:37 +0000 Subject: [PATCH 06/16] [IO] Move to custom `Write` trait mirroring `std::io::Write` In order to move towards our own I/O traits in the `rust-bitcoin` ecosystem, we have to slowly replace our use of the `std` and `core2` traits. Here we take the first real step, replacing `{std,core2}::io::Write` withour own `bitcoin_io::io::Write`. We provide a blanket impl for our trait for all `std::io::Write`, if the `std` feature is enabled, allowing users who use their own streams or `std` streams to call `rust-bitcoin` methods directly. --- io/src/lib.rs | 57 +++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 55 insertions(+), 2 deletions(-) diff --git a/io/src/lib.rs b/io/src/lib.rs index e4a4a79e..85b2afe9 100644 --- a/io/src/lib.rs +++ b/io/src/lib.rs @@ -36,10 +36,63 @@ pub mod io { compile_error!("At least one of std or core2 must be enabled"); #[cfg(feature = "std")] - pub use std::io::{Read, Write, sink, Cursor, Take, Error, ErrorKind, Result}; + pub use std::io::{Read, sink, Cursor, Take, Error, ErrorKind, Result}; #[cfg(not(feature = "std"))] - pub use core2::io::{Read, Write, Cursor, Take, Error, ErrorKind, Result}; + pub use core2::io::{Read, Cursor, Take, Error, ErrorKind, Result}; + /// A generic trait describing an output stream. See [`std::io::Write`] for more info. + pub trait Write { + fn write(&mut self, buf: &[u8]) -> Result; + fn flush(&mut self) -> Result<()>; + #[inline] + fn write_all(&mut self, mut buf: &[u8]) -> Result<()> { + while !buf.is_empty() { + match self.write(buf) { + Ok(0) => return Err(Error::new(ErrorKind::UnexpectedEof, "")), + Ok(len) => buf = &buf[len..], + Err(e) if e.kind() == ErrorKind::Interrupted => {} + Err(e) => return Err(e), + } + } + Ok(()) + } + } + + #[cfg(feature = "std")] + impl Write for W { + #[inline] + fn write(&mut self, buf: &[u8]) -> Result { + ::write(self, buf) + } + #[inline] + fn flush(&mut self) -> Result<()> { + ::flush(self) + } + } + + #[cfg(all(feature = "alloc", not(feature = "std")))] + impl Write for alloc::vec::Vec { + #[inline] + fn write(&mut self, buf: &[u8]) -> Result { + self.extend_from_slice(buf); + Ok(buf.len()) + } + #[inline] + fn flush(&mut self) -> Result<()> { Ok(()) } + } + + #[cfg(not(feature = "std"))] + impl<'a> Write for &'a mut [u8] { + #[inline] + fn write(&mut self, buf: &[u8]) -> Result { + let cnt = core::cmp::min(self.len(), buf.len()); + self[..cnt].copy_from_slice(&buf[..cnt]); + *self = &mut core::mem::take(self)[cnt..]; + Ok(cnt) + } + #[inline] + fn flush(&mut self) -> Result<()> { Ok(()) } + } } From 7eb5d65bda62230c50c7492b962efe9bd055aa6b Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 12 Sep 2023 19:32:16 +0000 Subject: [PATCH 07/16] [IO] Provide a macro which implements `io::Write` for types With the new `bitcoin_io` library, implementing `io::Write` manually is somewhat tricky - for `std` users we really want to provide an `std::io::Write` implementation, however for `no-std` users we want to implement against our internal trait. Sadly we cannot provide a blanket implementation of `std::io::Write` for all types whcih implement our `io::Write` trait as its an out-of-crate impl. Instead, we provide a macro which will either implement `std::io::Write` or our `io::Write` depending on the feature flags set on `bitcoin_io`. --- hashes/src/impls.rs | 90 +++++++++++++++++++++++---------------------- hashes/src/lib.rs | 3 -- io/src/lib.rs | 54 +++++++++++++++++++++++++++ 3 files changed, 101 insertions(+), 46 deletions(-) diff --git a/hashes/src/impls.rs b/hashes/src/impls.rs index 088a7f1f..57c9a723 100644 --- a/hashes/src/impls.rs +++ b/hashes/src/impls.rs @@ -5,65 +5,69 @@ //! Implementations of traits defined in `std` / `core2` and not in `core`. //! -use crate::{hmac, io, ripemd160, sha1, sha256, sha512, siphash24, HashEngine}; +use bitcoin_io::impl_write; -impl io::Write for sha1::HashEngine { - fn flush(&mut self) -> io::Result<()> { Ok(()) } +use crate::{hmac, ripemd160, sha1, sha256, sha512, siphash24, HashEngine}; - fn write(&mut self, buf: &[u8]) -> io::Result { - self.input(buf); +impl_write!( + sha1::HashEngine, + |us: &mut sha1::HashEngine, buf| { + us.input(buf); Ok(buf.len()) - } -} + }, + |_us| { Ok(()) } +); -impl io::Write for sha256::HashEngine { - fn flush(&mut self) -> io::Result<()> { Ok(()) } - - fn write(&mut self, buf: &[u8]) -> io::Result { - self.input(buf); +impl_write!( + sha256::HashEngine, + |us: &mut sha256::HashEngine, buf| { + us.input(buf); Ok(buf.len()) - } -} + }, + |_us| { Ok(()) } +); -impl io::Write for sha512::HashEngine { - fn flush(&mut self) -> io::Result<()> { Ok(()) } - - fn write(&mut self, buf: &[u8]) -> io::Result { - self.input(buf); +impl_write!( + sha512::HashEngine, + |us: &mut sha512::HashEngine, buf| { + us.input(buf); Ok(buf.len()) - } -} + }, + |_us| { Ok(()) } +); -impl io::Write for ripemd160::HashEngine { - fn flush(&mut self) -> io::Result<()> { Ok(()) } - - fn write(&mut self, buf: &[u8]) -> io::Result { - self.input(buf); +impl_write!( + ripemd160::HashEngine, + |us: &mut ripemd160::HashEngine, buf| { + us.input(buf); Ok(buf.len()) - } -} + }, + |_us| { Ok(()) } +); -impl io::Write for siphash24::HashEngine { - fn flush(&mut self) -> io::Result<()> { Ok(()) } - - fn write(&mut self, buf: &[u8]) -> io::Result { - self.input(buf); +impl_write!( + siphash24::HashEngine, + |us: &mut siphash24::HashEngine, buf| { + us.input(buf); Ok(buf.len()) - } -} + }, + |_us| { Ok(()) } +); -impl io::Write for hmac::HmacEngine { - fn flush(&mut self) -> io::Result<()> { Ok(()) } - - fn write(&mut self, buf: &[u8]) -> io::Result { - self.input(buf); +impl_write!( + hmac::HmacEngine, + |us: &mut hmac::HmacEngine, buf| { + us.input(buf); Ok(buf.len()) - } -} + }, + |_us| { Ok(()) }, + T: crate::Hash +); #[cfg(test)] mod tests { - use super::io::Write; + use bitcoin_io::io::Write; + use crate::{hash160, hmac, ripemd160, sha1, sha256, sha256d, sha512, siphash24, Hash}; macro_rules! write_test { diff --git a/hashes/src/lib.rs b/hashes/src/lib.rs index 91740f4e..10c47d75 100644 --- a/hashes/src/lib.rs +++ b/hashes/src/lib.rs @@ -130,9 +130,6 @@ pub mod siphash24; use core::{borrow, fmt, hash, ops}; -// You get I/O if you enable "std" or "core2" (as well as during testing). -#[cfg(feature = "bitcoin-io")] -use bitcoin_io::io; pub use hmac::{Hmac, HmacEngine}; /// A hashing engine which bytes can be serialized into. diff --git a/io/src/lib.rs b/io/src/lib.rs index 85b2afe9..93d09795 100644 --- a/io/src/lib.rs +++ b/io/src/lib.rs @@ -96,3 +96,57 @@ pub mod io { fn flush(&mut self) -> Result<()> { Ok(()) } } } + +#[doc(hidden)] +#[cfg(feature = "std")] +/// Re-export std for the below macro +pub use std as _std; + +#[macro_export] +/// Because we cannot provide a blanket implementation of [`std::io::Write`] for all implementers +/// of this crate's `io::Write` trait, we provide this macro instead. +/// +/// This macro will implement `Write` given a `write` and `flush` fn, either by implementing the +/// crate's native `io::Write` trait directly, or a more generic trait from `std` for users using +/// that feature. In any case, this crate's `io::Write` feature will be implemented for the given +/// type, even if indirectly. +#[cfg(not(feature = "std"))] +macro_rules! impl_write { + ($ty: ty, $write_fn: expr, $flush_fn: expr $(, $bounded_ty: ident : $bounds: path),*) => { + impl<$($bounded_ty: $bounds),*> $crate::io::Write for $ty { + #[inline] + fn write(&mut self, buf: &[u8]) -> $crate::io::Result { + $write_fn(self, buf) + } + #[inline] + fn flush(&mut self) -> $crate::io::Result<()> { + $flush_fn(self) + } + } + } +} + + +#[macro_export] +/// Because we cannot provide a blanket implementation of [`std::io::Write`] for all implementers +/// of this crate's `io::Write` trait, we provide this macro instead. +/// +/// This macro will implement `Write` given a `write` and `flush` fn, either by implementing the +/// crate's native `io::Write` trait directly, or a more generic trait from `std` for users using +/// that feature. In any case, this crate's `io::Write` feature will be implemented for the given +/// type, even if indirectly. +#[cfg(feature = "std")] +macro_rules! impl_write { + ($ty: ty, $write_fn: expr, $flush_fn: expr $(, $bounded_ty: ident : $bounds: path),*) => { + impl<$($bounded_ty: $bounds),*> $crate::_std::io::Write for $ty { + #[inline] + fn write(&mut self, buf: &[u8]) -> $crate::_std::io::Result { + $write_fn(self, buf) + } + #[inline] + fn flush(&mut self) -> $crate::_std::io::Result<()> { + $flush_fn(self) + } + } + } +} From 6aa7ccf84103dde36af241052ab4a5baa55a1743 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 12 Sep 2023 17:47:47 +0000 Subject: [PATCH 08/16] [IO] Replace `std::io::Sink` usage with our own trivial impl --- bitcoin/src/lib.rs | 2 +- io/src/lib.rs | 29 ++++++++++++++++++++++++++++- 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/bitcoin/src/lib.rs b/bitcoin/src/lib.rs index 65984cbf..8d1980dc 100644 --- a/bitcoin/src/lib.rs +++ b/bitcoin/src/lib.rs @@ -189,7 +189,7 @@ mod prelude { pub use std::collections::{BTreeMap, BTreeSet, btree_map, BinaryHeap}; #[cfg(feature = "std")] - pub use std::io::sink; + pub use crate::io::sink; #[cfg(not(feature = "std"))] pub use crate::io_extras::sink; diff --git a/io/src/lib.rs b/io/src/lib.rs index 93d09795..ee531fc4 100644 --- a/io/src/lib.rs +++ b/io/src/lib.rs @@ -36,7 +36,7 @@ pub mod io { compile_error!("At least one of std or core2 must be enabled"); #[cfg(feature = "std")] - pub use std::io::{Read, sink, Cursor, Take, Error, ErrorKind, Result}; + pub use std::io::{Read, Cursor, Take, Error, ErrorKind, Result}; #[cfg(not(feature = "std"))] pub use core2::io::{Read, Cursor, Take, Error, ErrorKind, Result}; @@ -95,6 +95,33 @@ pub mod io { #[inline] fn flush(&mut self) -> Result<()> { Ok(()) } } + + /// A sink to which all writes succeed. See [`std::io::Sink`] for more info. + pub struct Sink; + #[cfg(not(feature = "std"))] + impl Write for Sink { + #[inline] + fn write(&mut self, buf: &[u8]) -> Result { + Ok(buf.len()) + } + #[inline] + fn write_all(&mut self, _: &[u8]) -> Result<()> { Ok(()) } + #[inline] + fn flush(&mut self) -> Result<()> { Ok(()) } + } + #[cfg(feature = "std")] + impl std::io::Write for Sink { + #[inline] + fn write(&mut self, buf: &[u8]) -> std::io::Result { + Ok(buf.len()) + } + #[inline] + fn write_all(&mut self, _: &[u8]) -> std::io::Result<()> { Ok(()) } + #[inline] + fn flush(&mut self) -> std::io::Result<()> { Ok(()) } + } + /// Returns a sink to which all writes succeed. See [`std::io::sink`] for more info. + pub fn sink() -> Sink { Sink } } #[doc(hidden)] From 2364e1a877e43f9acb6d803cd5be7dc2f6d828b9 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 11 Sep 2023 17:56:39 +0000 Subject: [PATCH 09/16] Stop relying on blanket Read impl for all &mut Read --- bitcoin/src/bip158.rs | 14 +++++--------- bitcoin/src/consensus/encode.rs | 6 +++--- bitcoin/src/crypto/key.rs | 12 ++++++------ bitcoin/src/psbt/raw.rs | 2 +- 4 files changed, 15 insertions(+), 19 deletions(-) diff --git a/bitcoin/src/bip158.rs b/bitcoin/src/bip158.rs index 61b3c6e2..49347977 100644 --- a/bitcoin/src/bip158.rs +++ b/bitcoin/src/bip158.rs @@ -271,9 +271,7 @@ impl GcsFilterReader { I::Item: Borrow<[u8]>, R: io::Read + ?Sized, { - let mut decoder = reader; - let n_elements: VarInt = Decodable::consensus_decode(&mut decoder).unwrap_or(VarInt(0)); - let reader = &mut decoder; + let n_elements: VarInt = Decodable::consensus_decode(reader).unwrap_or(VarInt(0)); // map hashes to [0, n_elements << grp] let nm = n_elements.0 * self.m; let mut mapped = @@ -316,9 +314,7 @@ impl GcsFilterReader { I::Item: Borrow<[u8]>, R: io::Read + ?Sized, { - let mut decoder = reader; - let n_elements: VarInt = Decodable::consensus_decode(&mut decoder).unwrap_or(VarInt(0)); - let reader = &mut decoder; + let n_elements: VarInt = Decodable::consensus_decode(reader).unwrap_or(VarInt(0)); // map hashes to [0, n_elements << grp] let nm = n_elements.0 * self.m; let mut mapped = @@ -442,7 +438,7 @@ impl GcsFilter { /// Golomb-Rice decodes a number from a bit stream (parameter 2^k). fn golomb_rice_decode(&self, reader: &mut BitStreamReader) -> Result where - R: io::Read, + R: io::Read + ?Sized, { let mut q = 0u64; while reader.read(1)? == 1 { @@ -459,13 +455,13 @@ impl GcsFilter { } /// Bitwise stream reader. -pub struct BitStreamReader<'a, R> { +pub struct BitStreamReader<'a, R: ?Sized> { buffer: [u8; 1], offset: u8, reader: &'a mut R, } -impl<'a, R: io::Read> BitStreamReader<'a, R> { +impl<'a, R: io::Read + ?Sized> BitStreamReader<'a, R> { /// Creates a new [`BitStreamReader`] that reads bitwise from a given `reader`. pub fn new(reader: &'a mut R) -> BitStreamReader<'a, R> { BitStreamReader { buffer: [0u8], reader, offset: 8 } diff --git a/bitcoin/src/consensus/encode.rs b/bitcoin/src/consensus/encode.rs index 42d11a1e..6d0a3a7b 100644 --- a/bitcoin/src/consensus/encode.rs +++ b/bitcoin/src/consensus/encode.rs @@ -662,8 +662,8 @@ struct ReadBytesFromFiniteReaderOpts { /// This function relies on reader being bound in amount of data /// it returns for OOM protection. See [`Decodable::consensus_decode_from_finite_reader`]. #[inline] -fn read_bytes_from_finite_reader( - mut d: D, +fn read_bytes_from_finite_reader( + d: &mut D, mut opts: ReadBytesFromFiniteReaderOpts, ) -> Result, Error> { let mut ret = vec![]; @@ -1234,7 +1234,7 @@ mod tests { for chunk_size in 1..20 { assert_eq!( read_bytes_from_finite_reader( - io::Cursor::new(&data), + &mut io::Cursor::new(&data), ReadBytesFromFiniteReaderOpts { len: data.len(), chunk_size } ) .unwrap(), diff --git a/bitcoin/src/crypto/key.rs b/bitcoin/src/crypto/key.rs index 7ba63a89..01ff2b13 100644 --- a/bitcoin/src/crypto/key.rs +++ b/bitcoin/src/crypto/key.rs @@ -76,7 +76,7 @@ impl PublicKey { /// /// This internally reads the first byte before reading the rest, so /// use of a `BufReader` is recommended. - pub fn read_from(mut reader: R) -> Result { + pub fn read_from(reader: &mut R) -> Result { let mut bytes = [0; 65]; reader.read_exact(&mut bytes[0..1])?; @@ -914,11 +914,11 @@ mod tests { // sanity checks assert!(PublicKey::read_from(&mut cursor).is_err()); - assert!(PublicKey::read_from(io::Cursor::new(&[])).is_err()); - assert!(PublicKey::read_from(io::Cursor::new(&[0; 33][..])).is_err()); - assert!(PublicKey::read_from(io::Cursor::new(&[2; 32][..])).is_err()); - assert!(PublicKey::read_from(io::Cursor::new(&[0; 65][..])).is_err()); - assert!(PublicKey::read_from(io::Cursor::new(&[4; 64][..])).is_err()); + assert!(PublicKey::read_from(&mut io::Cursor::new(&[])).is_err()); + assert!(PublicKey::read_from(&mut io::Cursor::new(&[0; 33][..])).is_err()); + assert!(PublicKey::read_from(&mut io::Cursor::new(&[2; 32][..])).is_err()); + assert!(PublicKey::read_from(&mut io::Cursor::new(&[0; 65][..])).is_err()); + assert!(PublicKey::read_from(&mut io::Cursor::new(&[4; 64][..])).is_err()); } #[test] diff --git a/bitcoin/src/psbt/raw.rs b/bitcoin/src/psbt/raw.rs index effd805b..85275835 100644 --- a/bitcoin/src/psbt/raw.rs +++ b/bitcoin/src/psbt/raw.rs @@ -196,7 +196,7 @@ where } // core2 doesn't have read_to_end -pub(crate) fn read_to_end(mut d: D) -> Result, io::Error> { +pub(crate) fn read_to_end(d: &mut D) -> Result, io::Error> { let mut result = vec![]; let mut buf = [0u8; 64]; loop { From 7395093f946c5d0818962d6f1362d000221eed2d Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 11 Sep 2023 17:56:57 +0000 Subject: [PATCH 10/16] Stop relying on `Take`'s `by_ref` method --- bitcoin/src/consensus/encode.rs | 2 +- bitcoin/src/internal_macros.rs | 2 +- bitcoin/src/p2p/message.rs | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/bitcoin/src/consensus/encode.rs b/bitcoin/src/consensus/encode.rs index 6d0a3a7b..e618c925 100644 --- a/bitcoin/src/consensus/encode.rs +++ b/bitcoin/src/consensus/encode.rs @@ -320,7 +320,7 @@ pub trait Decodable: Sized { /// instead. #[inline] fn consensus_decode(reader: &mut R) -> Result { - Self::consensus_decode_from_finite_reader(reader.take(MAX_VEC_SIZE as u64).by_ref()) + Self::consensus_decode_from_finite_reader(&mut reader.take(MAX_VEC_SIZE as u64)) } } diff --git a/bitcoin/src/internal_macros.rs b/bitcoin/src/internal_macros.rs index cf95940f..940d1c1e 100644 --- a/bitcoin/src/internal_macros.rs +++ b/bitcoin/src/internal_macros.rs @@ -37,7 +37,7 @@ macro_rules! impl_consensus_encoding { use crate::io::Read as _; let mut r = r.take($crate::consensus::encode::MAX_VEC_SIZE as u64); Ok($thing { - $($field: $crate::consensus::Decodable::consensus_decode(r.by_ref())?),+ + $($field: $crate::consensus::Decodable::consensus_decode(&mut r)?),+ }) } } diff --git a/bitcoin/src/p2p/message.rs b/bitcoin/src/p2p/message.rs index 0b92e089..d4d0cb59 100644 --- a/bitcoin/src/p2p/message.rs +++ b/bitcoin/src/p2p/message.rs @@ -429,7 +429,7 @@ impl Decodable for HeaderDeserializationWrapper { #[inline] fn consensus_decode(r: &mut R) -> Result { - Self::consensus_decode_from_finite_reader(r.take(MAX_MSG_SIZE as u64).by_ref()) + Self::consensus_decode_from_finite_reader(&mut r.take(MAX_MSG_SIZE as u64)) } } @@ -534,7 +534,7 @@ impl Decodable for RawNetworkMessage { #[inline] fn consensus_decode(r: &mut R) -> Result { - Self::consensus_decode_from_finite_reader(r.take(MAX_MSG_SIZE as u64).by_ref()) + Self::consensus_decode_from_finite_reader(&mut r.take(MAX_MSG_SIZE as u64)) } } From 141343edb47ea11c3dc1663d8906307c211acd01 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 11 Sep 2023 17:57:07 +0000 Subject: [PATCH 11/16] [IO] Move to custom `Read` trait mirroring `std::io::Read` In order to move towards our own I/O traits in the `rust-bitcoin` ecosystem, we have to slowly replace our use of the `std` and `core2` traits. Here we take the second big step, replacing `{std,core2}::io::Read` with our own `bitcoin_io::io::Read`. We provide a blanket impl for our trait for all `std::io::Read`, if the `std` feature is enabled, allowing users who use their own streams or `std` streams to call `rust-bitcoin` methods directly. --- bitcoin/src/internal_macros.rs | 1 - bitcoin/src/p2p/message.rs | 1 - io/src/lib.rs | 57 ++++++++++++++++++++++++++++++++-- 3 files changed, 55 insertions(+), 4 deletions(-) diff --git a/bitcoin/src/internal_macros.rs b/bitcoin/src/internal_macros.rs index 940d1c1e..db46f944 100644 --- a/bitcoin/src/internal_macros.rs +++ b/bitcoin/src/internal_macros.rs @@ -34,7 +34,6 @@ macro_rules! impl_consensus_encoding { fn consensus_decode( r: &mut R, ) -> Result<$thing, $crate::consensus::encode::Error> { - use crate::io::Read as _; let mut r = r.take($crate::consensus::encode::MAX_VEC_SIZE as u64); Ok($thing { $($field: $crate::consensus::Decodable::consensus_decode(&mut r)?),+ diff --git a/bitcoin/src/p2p/message.rs b/bitcoin/src/p2p/message.rs index d4d0cb59..448e8a1c 100644 --- a/bitcoin/src/p2p/message.rs +++ b/bitcoin/src/p2p/message.rs @@ -10,7 +10,6 @@ use core::convert::TryFrom; use core::{fmt, iter}; use hashes::{sha256d, Hash}; -use io::Read as _; use crate::blockdata::{block, transaction}; use crate::consensus::encode::{self, CheckedData, Decodable, Encodable, VarInt}; diff --git a/io/src/lib.rs b/io/src/lib.rs index ee531fc4..c25bd3b9 100644 --- a/io/src/lib.rs +++ b/io/src/lib.rs @@ -32,14 +32,67 @@ extern crate alloc; /// Standard I/O stream definitions which are API-equivalent to `std`'s `io` module. See /// [`std::io`] for more info. pub mod io { + use core::convert::TryInto; + #[cfg(all(not(feature = "std"), not(feature = "core2")))] compile_error!("At least one of std or core2 must be enabled"); #[cfg(feature = "std")] - pub use std::io::{Read, Cursor, Take, Error, ErrorKind, Result}; + pub use std::io::{Cursor, Error, ErrorKind, Result}; #[cfg(not(feature = "std"))] - pub use core2::io::{Read, Cursor, Take, Error, ErrorKind, Result}; + pub use core2::io::{Cursor, Error, ErrorKind, Result}; + + /// A generic trait describing an input stream. See [`std::io::Read`] for more info. + pub trait Read { + fn read(&mut self, buf: &mut [u8]) -> Result; + #[inline] + fn read_exact(&mut self, mut buf: &mut [u8]) -> Result<()> { + while !buf.is_empty() { + match self.read(buf) { + Ok(0) => return Err(Error::new(ErrorKind::UnexpectedEof, "")), + Ok(len) => buf = &mut buf[len..], + Err(e) if e.kind() == ErrorKind::Interrupted => {} + Err(e) => return Err(e), + } + } + Ok(()) + } + #[inline] + fn take(&mut self, limit: u64) -> Take { + Take { reader: self, remaining: limit } + } + } + + pub struct Take<'a, R: Read + ?Sized> { + reader: &'a mut R, + remaining: u64, + } + impl<'a, R: Read + ?Sized> Read for Take<'a, R> { + #[inline] + fn read(&mut self, buf: &mut [u8]) -> Result { + let len = core::cmp::min(buf.len(), self.remaining.try_into().unwrap_or(buf.len())); + let read = self.reader.read(&mut buf[..len])?; + self.remaining -= read.try_into().unwrap_or(self.remaining); + Ok(read) + } + } + + #[cfg(feature = "std")] + impl Read for R { + #[inline] + fn read(&mut self, buf: &mut [u8]) -> Result { + ::read(self, buf) + } + } + + #[cfg(all(feature = "core2", not(feature = "std")))] + impl Read for R { + #[inline] + fn read(&mut self, buf: &mut [u8]) -> Result { + ::read(self, buf) + } + } /// A generic trait describing an output stream. See [`std::io::Write`] for more info. pub trait Write { From 3caaadf9bb20b181c84de6f1d8936d8b0b69b4fc Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 12 Sep 2023 05:21:50 +0000 Subject: [PATCH 12/16] [IO] Replace the `io::Cursor` re-export with our own `Cursor` --- bitcoin/src/crypto/key.rs | 1 - io/src/lib.rs | 43 ++++++++++++++++++++++++++++++++++----- 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/bitcoin/src/crypto/key.rs b/bitcoin/src/crypto/key.rs index 01ff2b13..5352a505 100644 --- a/bitcoin/src/crypto/key.rs +++ b/bitcoin/src/crypto/key.rs @@ -752,7 +752,6 @@ mod tests { use super::*; use crate::address::Address; - use crate::io; use crate::network::Network::{Bitcoin, Testnet}; #[test] diff --git a/io/src/lib.rs b/io/src/lib.rs index c25bd3b9..96d07280 100644 --- a/io/src/lib.rs +++ b/io/src/lib.rs @@ -38,10 +38,10 @@ pub mod io { compile_error!("At least one of std or core2 must be enabled"); #[cfg(feature = "std")] - pub use std::io::{Cursor, Error, ErrorKind, Result}; + pub use std::io::{Error, ErrorKind, Result}; #[cfg(not(feature = "std"))] - pub use core2::io::{Cursor, Error, ErrorKind, Result}; + pub use core2::io::{Error, ErrorKind, Result}; /// A generic trait describing an input stream. See [`std::io::Read`] for more info. pub trait Read { @@ -86,11 +86,44 @@ pub mod io { } } - #[cfg(all(feature = "core2", not(feature = "std")))] - impl Read for R { + #[cfg(not(feature = "std"))] + impl Read for &[u8] { #[inline] fn read(&mut self, buf: &mut [u8]) -> Result { - ::read(self, buf) + let cnt = core::cmp::min(self.len(), buf.len()); + buf[..cnt].copy_from_slice(&self[..cnt]); + *self = &self[cnt..]; + Ok(cnt) + } + } + + pub struct Cursor { + inner: T, + pos: u64, + } + impl> Cursor { + #[inline] + pub fn new(inner: T) -> Self { + Cursor { inner, pos: 0 } + } + #[inline] + pub fn position(&self) -> u64 { + self.pos + } + #[inline] + pub fn into_inner(self) -> T { + self.inner + } + } + impl> Read for Cursor { + #[inline] + fn read(&mut self, buf: &mut [u8]) -> Result { + let inner: &[u8] = self.inner.as_ref(); + let start_pos = self.pos.try_into().unwrap_or(inner.len()); + let read = core::cmp::min(inner.len().saturating_sub(start_pos), buf.len()); + buf[..read].copy_from_slice(&inner[start_pos..start_pos + read]); + self.pos = self.pos.saturating_add(read.try_into().unwrap_or(u64::max_value() /* unreachable */)); + Ok(read) } } From 9e1cd372cb88ca08ce49262a7f92731ec7508e8d Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 15 Sep 2023 03:46:27 +0000 Subject: [PATCH 13/16] Use `io::Error::get_ref()` over `std::error::Error::source()` --- bitcoin/src/consensus/serde.rs | 12 +++--------- bitcoin/src/lib.rs | 3 --- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/bitcoin/src/consensus/serde.rs b/bitcoin/src/consensus/serde.rs index 62c17469..1b201996 100644 --- a/bitcoin/src/consensus/serde.rs +++ b/bitcoin/src/consensus/serde.rs @@ -174,10 +174,8 @@ impl<'a, T: 'a + Encodable, E: ByteEncoder> fmt::Display for DisplayWrapper<'a, self.0.consensus_encode(&mut writer).map_err(|error| { #[cfg(debug_assertions)] { - use crate::StdError; - if error.kind() != io::ErrorKind::Other - || error.source().is_some() + || error.get_ref().is_some() || !writer.writer.was_error { panic!( @@ -429,8 +427,6 @@ impl>> IterReader { fn new(iterator: I) -> Self { IterReader { iterator: iterator.fuse(), error: None } } fn decode(mut self) -> Result> { - use crate::StdError; - let result = T::consensus_decode(&mut self); match (result, self.error) { (Ok(_), None) if self.iterator.next().is_some() => { @@ -438,7 +434,7 @@ impl>> IterReader { }, (Ok(value), None) => Ok(value), (Ok(_), Some(error)) => panic!("{} silently ate the error: {:?}", core::any::type_name::(), error), - (Err(ConsensusError::Io(io_error)), Some(de_error)) if io_error.kind() == io::ErrorKind::Other && io_error.source().is_none() => Err(DecodeError::Other(de_error)), + (Err(ConsensusError::Io(io_error)), Some(de_error)) if io_error.kind() == io::ErrorKind::Other && io_error.get_ref().is_none() => Err(DecodeError::Other(de_error)), (Err(consensus_error), None) => Err(DecodeError::Consensus(consensus_error)), (Err(ConsensusError::Io(io_error)), de_error) => panic!("Unexpected IO error {:?} returned from {}::consensus_decode(), deserialization error: {:?}", io_error, core::any::type_name::(), de_error), (Err(consensus_error), Some(de_error)) => panic!("{} should've returned `Other` IO error because of deserialization error {:?} but it returned consensus error {:?} instead", core::any::type_name::(), de_error, consensus_error), @@ -494,8 +490,6 @@ impl With { if serializer.is_human_readable() { serializer.collect_str(&DisplayWrapper::<'_, _, E>(value, Default::default())) } else { - use crate::StdError; - let serializer = serializer.serialize_seq(None)?; let mut writer = BinWriter { serializer, error: None }; @@ -505,7 +499,7 @@ impl With { (Ok(_), Some(error)) => panic!("{} silently ate an IO error: {:?}", core::any::type_name::(), error), (Err(io_error), Some(ser_error)) - if io_error.kind() == io::ErrorKind::Other && io_error.source().is_none() => + if io_error.kind() == io::ErrorKind::Other && io_error.get_ref().is_none() => Err(ser_error), (Err(io_error), ser_error) => panic!( "{} returned an unexpected IO error: {:?} serialization error: {:?}", diff --git a/bitcoin/src/lib.rs b/bitcoin/src/lib.rs index 8d1980dc..c86fbea6 100644 --- a/bitcoin/src/lib.rs +++ b/bitcoin/src/lib.rs @@ -113,9 +113,6 @@ pub mod sign_message; pub mod string; pub mod taproot; -// May depend on crate features and we don't want to bother with it -#[allow(unused)] -use bitcoin_io::error::Error as StdError; use bitcoin_io::io; pub use crate::address::{Address, AddressType}; From c95b59327a106817671d70da66abe43bd24c5925 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 15 Sep 2023 03:55:47 +0000 Subject: [PATCH 14/16] Explicitly use `std::io::Error` when implementing `std` traits In the next commit, `std::io::Error` will be replaced with a different type, and as a result `std::io::Error` must be referred to explicitly. --- bitcoin/src/p2p/address.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bitcoin/src/p2p/address.rs b/bitcoin/src/p2p/address.rs index 2ae9bba1..5e470551 100644 --- a/bitcoin/src/p2p/address.rs +++ b/bitcoin/src/p2p/address.rs @@ -116,7 +116,7 @@ impl fmt::Debug for Address { impl ToSocketAddrs for Address { type Iter = iter::Once; - fn to_socket_addrs(&self) -> Result { + fn to_socket_addrs(&self) -> Result { Ok(iter::once(self.socket_addr()?)) } } @@ -296,7 +296,7 @@ impl Decodable for AddrV2Message { impl ToSocketAddrs for AddrV2Message { type Iter = iter::Once; - fn to_socket_addrs(&self) -> Result { + fn to_socket_addrs(&self) -> Result { Ok(iter::once(self.socket_addr()?)) } } From b7dd16da99dd214c3e0569cdc0331b9a5e5c3953 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 15 Sep 2023 03:56:07 +0000 Subject: [PATCH 15/16] [IO] Use our own io::Error type In order to move towards our own I/O traits in the `rust-bitcoin` ecosystem, we have to slowly replace our use of the `std` and `core2` traits. This is the final step in removing the explicit `core2` dependency for I/O in no-std - replacing the `io::Error` type with our own. Sadly the `std::io::Error` type requires `std::error::Error` as a bound on the inner error, which is rather difficult to duplicate in a way that allows for mapping to `std` and back. To take a more general approach, we use bound on any `Debug` instead. --- io/src/lib.rs | 196 +++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 178 insertions(+), 18 deletions(-) diff --git a/io/src/lib.rs b/io/src/lib.rs index 96d07280..b21acc8a 100644 --- a/io/src/lib.rs +++ b/io/src/lib.rs @@ -18,14 +18,6 @@ #![cfg_attr(not(feature = "std"), no_std)] -#[cfg(all(not(feature = "std"), not(feature = "core2")))] -compile_error!("At least one of std or core2 must be enabled"); - -#[cfg(feature = "std")] -pub use std::error; -#[cfg(not(feature = "std"))] -pub use core2::error; - #[cfg(any(feature = "alloc", feature = "std"))] extern crate alloc; @@ -33,15 +25,183 @@ extern crate alloc; /// [`std::io`] for more info. pub mod io { use core::convert::TryInto; + use core::fmt::{Debug, Display, Formatter}; + #[cfg(any(feature = "alloc", feature = "std"))] + use alloc::boxed::Box; - #[cfg(all(not(feature = "std"), not(feature = "core2")))] - compile_error!("At least one of std or core2 must be enabled"); + #[cfg(all(feature = "alloc", not(feature = "std")))] + mod sealed { + use alloc::boxed::Box; + use alloc::string::String; + use core::fmt::Debug; + pub trait IntoBoxDynDebug { + fn into(self) -> Box; + } + impl IntoBoxDynDebug for &str { + fn into(self) -> Box { + Box::new(String::from(self)) + } + } + impl IntoBoxDynDebug for String { + fn into(self) -> Box { + Box::new(self) + } + } + } + + #[derive(Debug)] + pub struct Error { + kind: ErrorKind, + + #[cfg(feature = "std")] + error: Option>, + #[cfg(all(feature = "alloc", not(feature = "std")))] + error: Option>, + } + impl Error { + #[cfg(feature = "std")] + pub fn new(kind: ErrorKind, error: E) -> Error + where + E: Into>, + { + Self { kind, error: Some(error.into()) } + } + #[cfg(all(feature = "alloc", not(feature = "std")))] + pub fn new(kind: ErrorKind, error: E) -> Error { + Self { kind, error: Some(error.into()) } + } + + pub fn kind(&self) -> ErrorKind { + self.kind + } + } + + impl From for Error { + fn from(kind: ErrorKind) -> Error { + Self { + kind, + #[cfg(any(feature = "std", feature = "alloc"))] + error: None, + } + } + } + + impl Display for Error { + fn fmt(&self, fmt: &mut Formatter) -> core::result::Result<(), core::fmt::Error> { + fmt.write_fmt(format_args!("I/O Error: {}", self.kind.description()))?; + #[cfg(any(feature = "alloc", feature = "std"))] + if let Some(e) = &self.error { + fmt.write_fmt(format_args!(". {:?}", e))?; + } + Ok(()) + } + } #[cfg(feature = "std")] - pub use std::io::{Error, ErrorKind, Result}; + impl std::error::Error for Error { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + self.error.as_ref().and_then(|e| e.as_ref().source()) + } + #[allow(deprecated)] + fn description(&self) -> &str { + match self.error.as_ref() { + Some(e) => e.description(), + None => self.kind.description(), + } + } + #[allow(deprecated)] + fn cause(&self) -> Option<&dyn std::error::Error> { + self.error.as_ref().and_then(|e| e.as_ref().cause()) + } + } - #[cfg(not(feature = "std"))] - pub use core2::io::{Error, ErrorKind, Result}; + impl Error { + #[cfg(feature = "std")] + pub fn get_ref(&self) -> Option<&(dyn std::error::Error + Send + Sync + 'static)> { + self.error.as_deref() + } + #[cfg(all(feature = "alloc", not(feature = "std")))] + pub fn get_ref(&self) -> Option<&(dyn Debug + Send + Sync + 'static)> { + self.error.as_deref() + } + } + + #[cfg(feature = "std")] + impl From for Error { + fn from(o: std::io::Error) -> Error { + Self { kind: ErrorKind::from_std(o.kind()), error: o.into_inner() } + } + } + + #[cfg(feature = "std")] + impl From for std::io::Error { + fn from(o: Error) -> std::io::Error { + if let Some(err) = o.error { + std::io::Error::new(o.kind.to_std(), err) + } else { + o.kind.to_std().into() + } + } + } + + macro_rules! define_errorkind { + ($($kind: ident),*) => { + #[non_exhaustive] + #[derive(Clone, Copy, Debug, Hash, PartialEq, Eq)] + /// A minimal subset of [`std::io::ErrorKind`] which is used for [`Error`]. Note that, as with + /// [`std::io`], only [`Self::Interrupted`] has defined semantics in this crate, all other + /// variants are provided here only to provide higher-fidelity conversions to and from + /// [`std::io::Error`]. + pub enum ErrorKind { + $($kind),* + } + + impl ErrorKind { + fn description(&self) -> &'static str { + match self { + $(Self::$kind => stringify!($kind)),* + } + } + #[cfg(feature = "std")] + fn to_std(self) -> std::io::ErrorKind { + match self { + $(Self::$kind => std::io::ErrorKind::$kind),* + } + } + #[cfg(feature = "std")] + fn from_std(o: std::io::ErrorKind) -> ErrorKind { + match o { + $(std::io::ErrorKind::$kind => ErrorKind::$kind),*, + _ => ErrorKind::Other + } + } + } + } + } + + define_errorkind!( + NotFound, + PermissionDenied, + ConnectionRefused, + ConnectionReset, + ConnectionAborted, + NotConnected, + AddrInUse, + AddrNotAvailable, + BrokenPipe, + AlreadyExists, + WouldBlock, + InvalidInput, + InvalidData, + TimedOut, + WriteZero, + Interrupted, + UnexpectedEof, + // Note: Any time we bump the MSRV any new error kinds should be added here! + Other + ); + + pub type Result = core::result::Result; /// A generic trait describing an input stream. See [`std::io::Read`] for more info. pub trait Read { @@ -50,7 +210,7 @@ pub mod io { fn read_exact(&mut self, mut buf: &mut [u8]) -> Result<()> { while !buf.is_empty() { match self.read(buf) { - Ok(0) => return Err(Error::new(ErrorKind::UnexpectedEof, "")), + Ok(0) => return Err(ErrorKind::UnexpectedEof.into()), Ok(len) => buf = &mut buf[len..], Err(e) if e.kind() == ErrorKind::Interrupted => {} Err(e) => return Err(e), @@ -82,7 +242,7 @@ pub mod io { impl Read for R { #[inline] fn read(&mut self, buf: &mut [u8]) -> Result { - ::read(self, buf) + Ok(::read(self, buf)?) } } @@ -136,7 +296,7 @@ pub mod io { fn write_all(&mut self, mut buf: &[u8]) -> Result<()> { while !buf.is_empty() { match self.write(buf) { - Ok(0) => return Err(Error::new(ErrorKind::UnexpectedEof, "")), + Ok(0) => return Err(ErrorKind::UnexpectedEof.into()), Ok(len) => buf = &buf[len..], Err(e) if e.kind() == ErrorKind::Interrupted => {} Err(e) => return Err(e), @@ -150,11 +310,11 @@ pub mod io { impl Write for W { #[inline] fn write(&mut self, buf: &[u8]) -> Result { - ::write(self, buf) + Ok(::write(self, buf)?) } #[inline] fn flush(&mut self) -> Result<()> { - ::flush(self) + Ok(::flush(self)?) } } From add371d263e1d8d9205d80889ba6565af01b16b4 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 6 Nov 2023 19:22:43 +0000 Subject: [PATCH 16/16] Remove `core2` dependency entirely --- Cargo-minimal.lock | 21 --------------------- Cargo-recent.lock | 21 --------------------- bitcoin/Cargo.toml | 2 +- bitcoin/src/crypto/key.rs | 2 +- contrib/test.sh | 2 -- hashes/Cargo.toml | 4 ++-- hashes/contrib/test.sh | 2 +- hashes/embedded/Cargo.toml | 2 +- hashes/src/impls.rs | 4 ++-- io/Cargo.toml | 5 +---- 10 files changed, 9 insertions(+), 56 deletions(-) diff --git a/Cargo-minimal.lock b/Cargo-minimal.lock index 9f7011eb..52268334 100644 --- a/Cargo-minimal.lock +++ b/Cargo-minimal.lock @@ -72,9 +72,6 @@ dependencies = [ [[package]] name = "bitcoin-io" version = "0.1.0" -dependencies = [ - "core2", -] [[package]] name = "bitcoin_hashes" @@ -116,15 +113,6 @@ version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d4c819a1287eb618df47cc647173c5c4c66ba19d888a6e50d605672aed3140de" -[[package]] -name = "core2" -version = "0.3.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2cf12d2dad3ed124aa116f59561428478993d69ab81ae4d30e5349c9c5b5a5f6" -dependencies = [ - "memchr", -] - [[package]] name = "dyn-clone" version = "1.0.0" @@ -153,9 +141,6 @@ name = "hex-conservative" version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "30ed443af458ccb6d81c1e7e661545f94d3176752fb1df2f543b902a1e0f51e2" -dependencies = [ - "core2", -] [[package]] name = "hex_lit" @@ -198,12 +183,6 @@ version = "0.2.64" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "74dfca3d9957906e8d1e6a0b641dc9a59848e793f1da2165889fd4f62d10d79c" -[[package]] -name = "memchr" -version = "2.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e01e64d9017d18e7fc09d8e4fe0e28ff6931019e979fb8019319db7ca827f8a6" - [[package]] name = "memmap2" version = "0.5.10" diff --git a/Cargo-recent.lock b/Cargo-recent.lock index 932cd2b5..27677e11 100644 --- a/Cargo-recent.lock +++ b/Cargo-recent.lock @@ -71,9 +71,6 @@ dependencies = [ [[package]] name = "bitcoin-io" version = "0.1.0" -dependencies = [ - "core2", -] [[package]] name = "bitcoin_hashes" @@ -115,15 +112,6 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" -[[package]] -name = "core2" -version = "0.3.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "239fa3ae9b63c2dc74bd3fa852d4792b8b305ae64eeede946265b6af62f1fff3" -dependencies = [ - "memchr", -] - [[package]] name = "dyn-clone" version = "1.0.11" @@ -152,9 +140,6 @@ name = "hex-conservative" version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "30ed443af458ccb6d81c1e7e661545f94d3176752fb1df2f543b902a1e0f51e2" -dependencies = [ - "core2", -] [[package]] name = "hex_lit" @@ -197,12 +182,6 @@ version = "0.2.142" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6a987beff54b60ffa6d51982e1aa1146bc42f19bd26be28b0586f252fccf5317" -[[package]] -name = "memchr" -version = "2.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2dffe52ecf27772e601905b7522cb4ef790d2cc203488bbd0e2fe85fcb74566d" - [[package]] name = "memmap2" version = "0.5.10" diff --git a/bitcoin/Cargo.toml b/bitcoin/Cargo.toml index 5556e3e4..0b9933cc 100644 --- a/bitcoin/Cargo.toml +++ b/bitcoin/Cargo.toml @@ -27,7 +27,7 @@ bitcoinconsensus-std = ["bitcoinconsensus/std", "std"] # Instead no-std enables additional features required for this crate to be usable without std. # As a result, both can be enabled without conflict. std = ["secp256k1/std", "bitcoin-io/std", "hashes/std", "bech32/std", "internals/std", "hex/std"] -no-std = ["hashes/alloc", "hashes/core2", "bitcoin-io/core2", "bitcoin-io/alloc", "bech32/alloc", "secp256k1/alloc", "hex/alloc", "hex/core2"] +no-std = ["hashes/alloc", "hashes/io", "bitcoin-io/alloc", "bech32/alloc", "secp256k1/alloc", "hex/alloc"] [package.metadata.docs.rs] all-features = true diff --git a/bitcoin/src/crypto/key.rs b/bitcoin/src/crypto/key.rs index 5352a505..5fc95e6c 100644 --- a/bitcoin/src/crypto/key.rs +++ b/bitcoin/src/crypto/key.rs @@ -84,7 +84,7 @@ impl PublicKey { reader.read_exact(&mut bytes[1..])?; Self::from_slice(bytes).map_err(|e| { - // Need a static string for core2 + // Need a static string for no-std io #[cfg(feature = "std")] let reason = e; #[cfg(not(feature = "std"))] diff --git a/contrib/test.sh b/contrib/test.sh index 986f48f4..96ea2d70 100755 --- a/contrib/test.sh +++ b/contrib/test.sh @@ -17,8 +17,6 @@ if cargo --version | grep ${MSRV}; then cargo update -p schemars --precise 0.8.12 # schemars_derive 0.8.13 uses edition 2021 cargo update -p schemars_derive --precise 0.8.12 - # memchr 2.6.0 uses edition 2021 - cargo update -p memchr --precise 2.5.0 # byteorder 1.5.0 uses edition 2021 cargo update -p byteorder --precise 1.4.3 diff --git a/hashes/Cargo.toml b/hashes/Cargo.toml index 276fb3de..7c298aa3 100644 --- a/hashes/Cargo.toml +++ b/hashes/Cargo.toml @@ -17,8 +17,8 @@ default = ["std"] std = ["alloc", "hex/std", "bitcoin-io/std"] alloc = ["hex/alloc"] serde-std = ["serde/std"] -# If you want I/O you must enable either "std" or "core2". -core2 = ["bitcoin-io/core2", "hex/core2"] +# If you want I/O you must enable either "std" or "io". +io = ["bitcoin-io"] # Smaller (but slower) implementation of sha256, sha512 and ripemd160 small-hash = [] diff --git a/hashes/contrib/test.sh b/hashes/contrib/test.sh index 37160d0e..a0dc5e34 100755 --- a/hashes/contrib/test.sh +++ b/hashes/contrib/test.sh @@ -2,7 +2,7 @@ set -ex -FEATURES="serde serde-std std core2 alloc" +FEATURES="serde serde-std std io alloc" cargo --version rustc --version diff --git a/hashes/embedded/Cargo.toml b/hashes/embedded/Cargo.toml index 5bb9af26..5d9644c4 100644 --- a/hashes/embedded/Cargo.toml +++ b/hashes/embedded/Cargo.toml @@ -18,7 +18,7 @@ cortex-m-rt = "0.6.10" cortex-m-semihosting = "0.3.3" panic-halt = "0.2.0" alloc-cortex-m = { version = "0.4.1", optional = true } -bitcoin_hashes = { path="../", default-features = false, features = ["core2"] } +bitcoin_hashes = { path="../", default-features = false, features = ["io"] } bitcoin-io = { path = "../../io", default_features = false } [[bin]] diff --git a/hashes/src/impls.rs b/hashes/src/impls.rs index 57c9a723..0f7cc5a0 100644 --- a/hashes/src/impls.rs +++ b/hashes/src/impls.rs @@ -1,8 +1,8 @@ // SPDX-License-Identifier: CC0-1.0 -//! `std` / `core2` Impls. +//! `std` / `io` Impls. //! -//! Implementations of traits defined in `std` / `core2` and not in `core`. +//! Implementations of traits defined in `std` / `io` and not in `core`. //! use bitcoin_io::impl_write; diff --git a/io/Cargo.toml b/io/Cargo.toml index ea5bc43a..87101120 100644 --- a/io/Cargo.toml +++ b/io/Cargo.toml @@ -15,11 +15,8 @@ exclude = ["tests", "contrib"] [features] default = ["std"] std = [] -alloc = ["core2/alloc"] +alloc = [] [package.metadata.docs.rs] all-features = true rustdoc-args = ["--cfg", "docsrs"] - -[dependencies] -core2 = { version = "0.3", default-features = false, optional = true }