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

Add #![no_std] support #64

Merged
merged 46 commits into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from 45 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
6f1c9bb
feat: add `#![no_std]` support.
overcat Sep 1, 2024
4608d1f
improve ci
overcat Sep 1, 2024
74a4619
Merge branch 'refs/heads/main' into no-std
overcat Sep 3, 2024
0dd5a74
data-encoding
overcat Sep 4, 2024
69c57aa
Merge branch 'main' into no-std
overcat Sep 5, 2024
bbfd0db
dont use data-encoding/alloc
overcat Sep 5, 2024
8fccb55
WIP, no alloc
overcat Sep 5, 2024
722ee54
WIP, no alloc
overcat Sep 5, 2024
d17f046
WIP, no alloc
overcat Sep 6, 2024
66d69b0
WIP, no alloc
overcat Sep 6, 2024
883e531
WIP, no alloc
overcat Sep 6, 2024
5c7ccc7
WIP, no alloc
overcat Sep 6, 2024
3dde835
WIP, no alloc
overcat Sep 6, 2024
6177b46
WIP, no alloc
overcat Sep 6, 2024
ba16b92
fix test
overcat Sep 6, 2024
543cc55
update readme
overcat Sep 6, 2024
7cb6aa0
WIP
overcat Sep 6, 2024
dba717d
remove TODO mark
overcat Sep 6, 2024
3c17916
tiny refactor
overcat Sep 6, 2024
6b61cd0
Revert "tiny refactor"
overcat Sep 9, 2024
56a1769
Revert "remove TODO mark"
overcat Sep 9, 2024
38b5a73
Revert "WIP"
overcat Sep 9, 2024
9ee8b87
Revert "update readme"
overcat Sep 9, 2024
6b41645
Revert "fix test"
overcat Sep 9, 2024
01aa8c9
Revert "WIP, no alloc"
overcat Sep 9, 2024
73baaef
Revert "WIP, no alloc"
overcat Sep 9, 2024
bd61866
Revert "WIP, no alloc"
overcat Sep 9, 2024
beb6aa0
Revert "WIP, no alloc"
overcat Sep 9, 2024
4f62421
Revert "WIP, no alloc"
overcat Sep 9, 2024
91ae219
Revert "WIP, no alloc"
overcat Sep 9, 2024
79a3fae
Revert "WIP, no alloc"
overcat Sep 9, 2024
6669ed8
Revert "WIP, no alloc"
overcat Sep 9, 2024
f50b9c6
Revert "dont use data-encoding/alloc"
overcat Sep 9, 2024
8c8cd68
ci test features
overcat Sep 9, 2024
843261c
remove data-encoding/std
overcat Sep 9, 2024
9903961
dont need rustup update?
overcat Sep 9, 2024
9203271
fix ci
overcat Sep 9, 2024
30c62ff
fix ci
overcat Sep 9, 2024
4576351
add build
overcat Sep 9, 2024
e1a5f26
Merge branch 'refs/heads/main' into no-std
overcat Sep 10, 2024
84d4453
update
overcat Sep 11, 2024
6bbe8de
debug cargo version
overcat Sep 11, 2024
a1dade8
update rust
overcat Sep 11, 2024
1c57ff4
ci
overcat Sep 11, 2024
4173e3f
ci
overcat Sep 11, 2024
7c67328
review
overcat Sep 11, 2024
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
8 changes: 6 additions & 2 deletions .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,12 @@ jobs:
run: echo RUSTFLAGS='-Dwarnings' >> $GITHUB_ENV
- run: rustup update
- run: cargo version
- run: cargo build --target ${{ matrix.target }}
- run: cargo test --target ${{ matrix.target }}
- uses: stellar/binaries@v30
with:
name: cargo-hack
version: 0.5.28
- run: cargo-hack hack build --feature-powerset --target ${{ matrix.target }}
- run: cargo-hack hack test --feature-powerset --target ${{ matrix.target }}
leighmcculloch marked this conversation as resolved.
Show resolved Hide resolved

publish-dry-run:
if: startsWith(github.head_ref, 'release/')
Expand Down
4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ path = "src/bin/stellar-strkey/main.rs"
required-features = ["cli"]
doctest = false

[build_dependencies]
[build-dependencies]
overcat marked this conversation as resolved.
Show resolved Hide resolved
crate-git-revision = "0.0.6"

[dev-dependencies]
proptest = "1.0.0"

[dependencies]
data-encoding = "2.6.0"
data-encoding = { version = "2.6.0", default-features = false, features = ["alloc"] }
leighmcculloch marked this conversation as resolved.
Show resolved Hide resolved
clap = { version = "4.2.4", default-features = false, features = ["std", "derive", "usage", "help"], optional = true }
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ To use the library, include in your toml:
stellar-strkey = "..."
```

This crate can be used in `no_std` environments.
overcat marked this conversation as resolved.
Show resolved Hide resolved
However, please note that it relies on the [`alloc`](https://docs.rust-embedded.org/book/collections/#using-alloc) crate for certain types such as `Vec`.

#### CLI

To use the CLI:
Expand Down
3 changes: 3 additions & 0 deletions src/convert.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
// TODO: Could encode and decode, and the functions upstream that call them, be
// const fn's?

use alloc::string::String;
use alloc::vec::Vec;

use crate::{crc::checksum, error::DecodeError};

pub fn encode(ver: u8, payload: &[u8]) -> String {
Expand Down
1 change: 1 addition & 0 deletions src/crc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ pub fn checksum(data: &[u8]) -> [u8; 2] {
mod tests {
use super::checksum;
extern crate proptest;
use alloc::vec::Vec;
use proptest::prelude::*;

#[test]
Expand Down
27 changes: 14 additions & 13 deletions src/ed25519.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,20 @@
use std::{
fmt::{Debug, Display},
str::FromStr,
};

use crate::{
convert::{decode, encode},
error::DecodeError,
version,
};

use alloc::{format, string::String, vec, vec::Vec};
use core::{
fmt::{Debug, Display},
str::FromStr,
};

#[derive(Clone, Copy, Hash, PartialEq, Eq, PartialOrd, Ord)]
pub struct PrivateKey(pub [u8; 32]);

impl Debug for PrivateKey {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
write!(f, "PrivateKey(")?;
write!(
f,
Expand Down Expand Up @@ -51,7 +52,7 @@ impl PrivateKey {
}

impl Display for PrivateKey {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
write!(f, "{}", self.to_string())
}
}
Expand All @@ -68,7 +69,7 @@ impl FromStr for PrivateKey {
pub struct PublicKey(pub [u8; 32]);

impl Debug for PublicKey {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
write!(f, "PublicKey(")?;
write!(
f,
Expand Down Expand Up @@ -106,7 +107,7 @@ impl PublicKey {
}

impl Display for PublicKey {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
write!(f, "{}", self.to_string())
}
}
Expand All @@ -126,7 +127,7 @@ pub struct MuxedAccount {
}

impl Debug for MuxedAccount {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
write!(f, "MuxedAccount(")?;
write!(
f,
Expand Down Expand Up @@ -174,7 +175,7 @@ impl MuxedAccount {
}

impl Display for MuxedAccount {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
write!(f, "{}", self.to_string())
}
}
Expand All @@ -197,7 +198,7 @@ pub struct SignedPayload {
}

impl Debug for SignedPayload {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
write!(f, "MuxedAccount(")?;
write!(
f,
Expand Down Expand Up @@ -321,7 +322,7 @@ impl SignedPayload {
}

impl Display for SignedPayload {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
write!(f, "{}", self.to_string())
}
}
Expand Down
3 changes: 3 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
#![no_std]
extern crate alloc;

#[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Hash)]
pub struct Version<'a> {
pub pkg: &'a str,
Expand Down
20 changes: 12 additions & 8 deletions src/strkey.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
use std::{fmt::Debug, fmt::Display, str::FromStr};
use alloc::{format, string::String};
use core::{
fmt::{Debug, Display},
str::FromStr,
};

use crate::{
convert::{decode, encode},
Expand Down Expand Up @@ -55,7 +59,7 @@ impl Strkey {
}

impl Display for Strkey {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
write!(f, "{}", self.to_string())
}
}
Expand All @@ -72,7 +76,7 @@ impl FromStr for Strkey {
pub struct PreAuthTx(pub [u8; 32]);

impl Debug for PreAuthTx {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
write!(f, "PreAuthTx(")?;
write!(
f,
Expand Down Expand Up @@ -107,7 +111,7 @@ impl PreAuthTx {
}

impl Display for PreAuthTx {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
write!(f, "{}", self.to_string())
}
}
Expand All @@ -124,7 +128,7 @@ impl FromStr for PreAuthTx {
pub struct HashX(pub [u8; 32]);

impl Debug for HashX {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
write!(f, "HashX(")?;
write!(
f,
Expand Down Expand Up @@ -159,7 +163,7 @@ impl HashX {
}

impl Display for HashX {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
write!(f, "{}", self.to_string())
}
}
Expand All @@ -176,7 +180,7 @@ impl FromStr for HashX {
pub struct Contract(pub [u8; 32]);

impl Debug for Contract {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
write!(f, "Contract(")?;
write!(
f,
Expand Down Expand Up @@ -211,7 +215,7 @@ impl Contract {
}

impl Display for Contract {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
write!(f, "{}", self.to_string())
}
}
Expand Down
8 changes: 5 additions & 3 deletions tests/tests.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
use stellar_strkey::*;
#![no_std]

Copy link
Member

Choose a reason for hiding this comment

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

Tests always run with std imported anyway, because the test framework itself uses std, so I don't think we need the changes in this file.

Is there a reason using alloc explicitly here benefits?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you are right, I have removed it.

But I am curious if we should add a test to see if it works properly under no std? It seems that there are some testing frameworks that can achieve this: https://github.com/knurling-rs/defmt

Copy link
Member

Choose a reason for hiding this comment

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

We could add a crate to the repo that imports strkey and gets built as part of testing, that provides it's own panic handler, because the only way it'll build is if std isn't imported because there can't be two panic handler and std provides one.

https://doc.rust-lang.org/nomicon/panic-handler.html

Copy link
Member

@leighmcculloch leighmcculloch Sep 11, 2024

Choose a reason for hiding this comment

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

On the same note that's a lot of test machinery for checking something that is unlikely to be the case since the no_std attribute is in use.

The risk is that it depends on a crate that pulls in std, but we're unlikely to add new deps to this repo.

I wonder if something like https://github.com/cackle-rs/cackle could detect it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @leighmcculloch, I have integrated version 0.0.11 into Ledger, and it's working very well. Currently, I want to make rs-stellar-xdr support no std, so I will work on the tests here and consider adding no alloc support in my free time.

Copy link
Member

@leighmcculloch leighmcculloch Sep 11, 2024

Choose a reason for hiding this comment

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

There's nothing riding on no alloc, so if it's not of value to you, I suggest holding off as there's nothing that I know of needing it right now.

extern crate alloc;
extern crate proptest;

use proptest::prelude::*;
use alloc::{format, string::String, vec, vec::Vec};
use proptest::proptest;
use stellar_strkey::*;

#[test]
fn test_valid_public_keys() {
Expand Down
Loading