Repository: rust-bitcoin/rust-bitcoin
Pull request #2480 (2024-02-15 03:38:23)

Add a new base58 crate
Add a new `base58check` crate to the workspace and move the `bitcoin::base58` module to it.

Done as part of crate smashing, specifically so that we can make `bip32` into a separate crate.

tcharding: Note, `base58` and `bitcoin-base58` are both taken on crates.io - we could reach out to the `bitcoin-base58` fella and see if he'd give it up. Lets get it totally ready for release first but eh.
coveralls: ## Pull Request Test Coverage Report for [Build 7910781048](https://coveralls.io/builds/65710022)


### Details

* **0** of **0**   changed or added relevant lines in **0** files are covered.
* No unchanged relevant lines lost coverage.
* Overall coverage remained the same at **84.038%**

---



|  Totals | [![Coverage Status](https://coveralls.io/builds/65710022/badge)](https://coveralls.io/builds/65710022) |
| :-- | --: |
| Change from base [Build 7892264908](https://coveralls.io/builds/65677470): |  0.0% |
| Covered Lines: | 19433 |
| Relevant Lines: | 23124 |

---
##### 💛  - [Coveralls](https://coveralls.io)

apoelstra: I asked for the name here https://github.com/debris/base58/issues/10
apoelstra: If we can't get the name I think we should use `base58check` or `base58ck`. Since the existing `base58` crate doesn't actually implement the base58check algorithm, it just does base58 without any checksumming support.
tcharding: The two separate pathecs is a bit klunky, it was intended to assist review but I don't think it is working - can squash.
Kixunil: The file move has an important role of explaining to git that the stuff moved so I think either we should keep the module or check if squashing shows as move with change in git (sometimes these things work) and if so, we can squash.
tcharding: Man, I gotta have more faith in `git` - single commit shows file move.
tcharding: CI fails look unrelated to this PR.
apoelstra: There's been no activity on my "may we please have the `base58` name" request, so I think we should change our name to `base58check`.

In our README.md we should also add a short blurb comparing this crate to `base58` (which does not support base58check and therefore can't validate address checksums, and also appears to be unmaintained) and `bitcoin-base58` (which is part of a much larger project to convert C++ Bitcoin code to Rust, vs our code which is a whole new implementation.)
apoelstra: utACK  64e73a923c1144ba1725f49910fe4ee321dd03cf

Struggling to test this until the issues with nightly are resolved.
tcharding: Needs #2448 to get past CI.
tcharding: I added a section to the readme, please review my wording especially the stuff about perf.
apoelstra: I'd suggest

```
There are two other crates on crates.io that implement base58 encoding and decoding. This
crate differs from them because:

1. [bitcoin-base58](https://crates.io/crates/bitcoin-base58) is transpiled from the C++ code in Bitcoin Core as part of a large long-term transpilation project; this crate is a pure Rust implementation intended to be production-ready and to provide a rustic API.
2. [base58](https://crates.io/crates/base58) is incomplete and appears unmaintained as of February 2024. It does not validate checksums and will therefore accept invalid Bitcoin addresses. It may be appropriate in cases where performance is more important than safety.
```

I'm not sure what to make of your "should offer similar functionality" sentence.
tcharding: Nice, thanks. Used as suggested (added line breaks).
tcharding: Rebased and fixed the manifest patch in `bitcoin/embedded`.
Kixunil: Is "rustic API" really an appropriate shorthand for "Rust-idiomatic API"?
Kixunil: Regarding name: I prefer `check` rather than `ck` since it's more readable (although it's quite possible that everyone who knows what base58 check means will know what the ck is and who doesn't know what the check is will not understand it right away anyway).

I don't know if people in the wild tend to write "base58check" or "base58 check" if the latter then I want the crate name to be `base58-check`.
apoelstra: I've only ever seen base58check written without the space. I'm ambivalent about `check` vs `ck`, though slightly prefer the longer one. Also ambivalent about "rustic API" vs "Rust-idiomatic API" :).
tcharding: I added "checksum" to the keywords list, which is now
```rust
keywords = ["bitcoin", "base58", "encode", "decode", "checksum"]
``` 
tcharding: We only get five, are these the best five we can find?

tcharding: Changes in force push:
- Left the crate name as it is `base58check`
- Remove "rustic" because I usual do definition lookups on any complex English word, or slang, because I know others will and this is not useful https://duckduckgo.com/?q=rustic+def&t=canonical&ia=web
- Added additional keyword "checksum"
- Used "ecosystem" instead of "network"
tcharding: Needs#2499

Patch:
diff --git a/Cargo-minimal.lock b/Cargo-minimal.lock
index 01f89427..d7692bf9 100644
--- a/Cargo-minimal.lock
+++ b/Cargo-minimal.lock
@@ -8,6 +8,14 @@ version = "1.0.57"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "08f9b8508dccb7687a1d6c4ce66b2b0ecef467c94667de27d8d7fe1f8d2a9cdc"
 
+[[package]]
+name = "base58check"
+version = "0.1.0"
+dependencies = [
+ "bitcoin_hashes",
+ "hex-conservative",
+]
+
 [[package]]
 name = "base64"
 version = "0.21.3"
@@ -34,6 +42,7 @@ dependencies = [
 name = "bitcoin"
 version = "0.31.0"
 dependencies = [
+ "base58check",
  "base64",
  "bech32",
  "bincode",
diff --git a/Cargo-recent.lock b/Cargo-recent.lock
index 79b6aee4..452d925e 100644
--- a/Cargo-recent.lock
+++ b/Cargo-recent.lock
@@ -8,6 +8,14 @@ version = "1.0.71"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "9c7d0618f0e0b7e8ff11427422b64564d5fb0be1940354bfe2e0529b18a9d9b8"
 
+[[package]]
+name = "base58check"
+version = "0.1.0"
+dependencies = [
+ "bitcoin_hashes",
+ "hex-conservative",
+]
+
 [[package]]
 name = "base64"
 version = "0.21.3"
@@ -33,6 +41,7 @@ dependencies = [
 name = "bitcoin"
 version = "0.31.0"
 dependencies = [
+ "base58check",
  "base64",
  "bech32",
  "bincode",
diff --git a/Cargo.toml b/Cargo.toml
index 77469170..e076d996 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,7 +1,10 @@
 [workspace]
-members = ["bitcoin", "hashes", "internals", "fuzz", "io", "units"]
+members = ["bitcoin", "hashes", "internals", "fuzz", "io", "units", "base58"]
 resolver = "2"
 
+[patch.crates-io.base58check]
+path = "base58"
+
 [patch.crates-io.bitcoin]
 path = "bitcoin"
 
diff --git a/base58/Cargo.toml b/base58/Cargo.toml
new file mode 100644
index 00000000..d51fb5bd
--- /dev/null
+++ b/base58/Cargo.toml
@@ -0,0 +1,27 @@
+[package]
+name = "base58check"
+version = "0.1.0"
+authors = ["Andrew Poelstra <apoelstra@wpsoftware.net>"]
+license = "CC0-1.0"
+repository = "https://github.com/rust-bitcoin/rust-bitcoin/"
+description = "Bitcoin base58 encoding with checksum."
+categories = ["cryptography::cryptocurrencies", "encoding"]
+keywords = ["bitcoin", "base58", "encode", "decode", "checksum"]
+readme = "README.md"
+edition = "2021"
+rust-version = "1.56.1"
+exclude = ["tests", "contrib"]
+
+[features]
+default = ["std"]
+std = ["hashes/std"]
+
+[package.metadata.docs.rs]
+all-features = true
+rustdoc-args = ["--cfg", "docsrs"]
+
+[dependencies]
+hashes = { package = "bitcoin_hashes", version = "0.13.0", default-features = false, features = ["alloc"] }
+
+[dev-dependencies]
+hex = { package = "hex-conservative", version = "0.1.1", default-features = false, features = ["alloc"] }
diff --git a/base58/README.md b/base58/README.md
new file mode 100644
index 00000000..a8571737
--- /dev/null
+++ b/base58/README.md
@@ -0,0 +1,27 @@
+Bitcoin base58 encoding
+=======================
+
+This crate provides encoding and decoding of base58 strings as defined by the Bitcoin ecosystem
+including the checksum.
+
+There are two other crates on crates.io that implement base58 encoding and decoding. This
+crate differs from them because:
+
+1. [bitcoin-base58](https://crates.io/crates/bitcoin-base58) is transpiled from the C++ code in
+   Bitcoin Core as part of a large long-term transpilation project; this crate is a pure Rust
+   implementation intended to be production-ready and to provide an Rust-idiomatic API.
+
+2. [base58](https://crates.io/crates/base58) is incomplete and appears unmaintained as of
+   February 2024. It does not validate checksums and will therefore accept invalid Bitcoin
+   addresses. It may be appropriate in cases where performance is more important than safety.
+
+
+## Minimum Supported Rust Version (MSRV)
+
+This library should always compile with any combination of features on **Rust 1.56.1**.
+
+
+## Licensing
+
+The code in this project is licensed under the [Creative Commons CC0 1.0 Universal license](LICENSE).
+We use the [SPDX license list](https://spdx.org/licenses/) and [SPDX IDs](https://spdx.dev/ids/).
diff --git a/bitcoin/src/base58.rs b/base58/src/lib.rs
similarity index 92%
rename from bitcoin/src/base58.rs
rename to base58/src/lib.rs
index a68e8131..a0aa04ca 100644
--- a/bitcoin/src/base58.rs
+++ b/base58/src/lib.rs
@@ -1,18 +1,36 @@
 // SPDX-License-Identifier: CC0-1.0
 
-//! Base58 encoder and decoder.
-//!
-//! This module provides functions for encoding and decoding base58 slices and
-//! strings respectively.
+//! Bitcoin base58 encoding and decoding.
 //!
+//! This crate can be used in a no-std environment but requires an allocator.
+
+#![no_std]
+// Experimental features we need.
+#![cfg_attr(docsrs, feature(doc_auto_cfg))]
+#![cfg_attr(bench, feature(test))]
+// Coding conventions.
+#![warn(missing_docs)]
+// Instead of littering the codebase for non-fuzzing code just globally allow.
+#![cfg_attr(fuzzing, allow(dead_code, unused_imports))]
+// Exclude lints we don't think are valuable.
+#![allow(clippy::needless_question_mark)] // https://github.com/rust-bitcoin/rust-bitcoin/pull/2134
+#![allow(clippy::manual_range_contains)] // More readable than clippy's format.
+
+#[macro_use]
+extern crate alloc;
 
-use core::{fmt, iter, slice, str};
+#[cfg(feature = "std")]
+extern crate std;
 
-use hashes::{sha256d, Hash};
+static BASE58_CHARS: &[u8] = b"123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz";
 
-use crate::prelude::*;
+use core::{fmt, iter, slice, str};
+#[cfg(not(feature = "std"))]
+pub use alloc::{string::String, vec::Vec};
+#[cfg(feature = "std")]
+pub use std::{string::String, vec::Vec};
 
-static BASE58_CHARS: &[u8] = b"123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz";
+use hashes::{sha256d, Hash};
 
 #[rustfmt::skip]
 static BASE58_DIGITS: [Option<u8>; 128] = [
diff --git a/bitcoin/Cargo.toml b/bitcoin/Cargo.toml
index 823e9aa0..91e3c6fb 100644
--- a/bitcoin/Cargo.toml
+++ b/bitcoin/Cargo.toml
@@ -15,7 +15,7 @@ exclude = ["tests", "contrib"]
 
 [features]
 default = [ "std", "secp-recovery" ]
-std = ["bech32/std", "hashes/std", "hex/std", "internals/std", "io/std", "secp256k1/std", "units/std"]
+std = ["base58/std", "bech32/std", "hashes/std", "hex/std", "internals/std", "io/std", "secp256k1/std", "units/std"]
 rand-std = ["secp256k1/rand-std", "std"]
 rand = ["secp256k1/rand"]
 serde = ["actual-serde", "hashes/serde", "secp256k1/serde", "internals/serde", "units/serde"]
@@ -28,6 +28,7 @@ all-features = true
 rustdoc-args = ["--cfg", "docsrs"]
 
 [dependencies]
+base58 = { package = "base58check", version = "0.1.0", default-features = false }
 bech32 = { version = "0.10.0-beta", default-features = false, features = ["alloc"] }
 hashes = { package = "bitcoin_hashes", version = "0.13.0", default-features = false, features = ["alloc", "io"] }
 hex = { package = "hex-conservative", version = "0.1.1", default-features = false, features = ["alloc"] }
diff --git a/bitcoin/embedded/Cargo.toml b/bitcoin/embedded/Cargo.toml
index 3f26fce5..ccd22f96 100644
--- a/bitcoin/embedded/Cargo.toml
+++ b/bitcoin/embedded/Cargo.toml
@@ -27,6 +27,9 @@ codegen-units = 1 # better optimizations
 debug = true # symbols are nice and they don't increase the size on Flash
 lto = true # better optimizations
 
+[patch.crates-io.base58check]
+path = "../../base58"
+
 [patch.crates-io.bitcoin_hashes]
 path = "../../hashes"
 
diff --git a/bitcoin/src/lib.rs b/bitcoin/src/lib.rs
index b2d51c63..c9f92e6b 100644
--- a/bitcoin/src/lib.rs
+++ b/bitcoin/src/lib.rs
@@ -93,7 +93,6 @@
 #[macro_use]
 pub mod p2p;
 pub mod address;
-pub mod base58;
 pub mod bip152;
 pub mod bip158;
 pub mod bip32;
@@ -196,3 +195,8 @@ fn consensus_encode<W: Write + ?Sized>(&self, w: &mut W) -> Result<usize, io::Er
         }
     }
 }
+
+pub mod base58 {
+    //! Bitcoin base58 encoding and decoding.
+    pub use base58::{decode, decode_check, encode, encode_check, encode_check_to_fmt, Error};
+}


Test patch:
diff --git a/base58/contrib/test_vars.sh b/base58/contrib/test_vars.sh
new file mode 100644
index 00000000..7701af20
--- /dev/null
+++ b/base58/contrib/test_vars.sh
@@ -0,0 +1,10 @@
+#!/usr/bin/env bash
+
+# Test all these features with "std" enabled.
+FEATURES_WITH_STD=""
+
+# Test all these features without "std" enabled.
+FEATURES_WITHOUT_STD=""
+
+# Run and lint these examples.
+EXAMPLES=""