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

No_std support for floresta-common #254

Merged
merged 1 commit into from
Oct 15, 2024

Conversation

JoseSK999
Copy link
Contributor

I have tried to make floresta-common be able to compile without the standard library.

  1. Added the missing no-std feature in Cargo.toml, together with a default std feature.
  2. Added the missing core2 and hashbrown dependencies (used in the no-std prelude).
  3. sha2, miniscript and bitcoin default features disabled to avoid importing std-lib, although our new std feature enables it.
  4. Also added a new descriptors-no-std feature to use them without the std-lib.

It should be able to compile without std-lib by running cargo build --no-default-features --features no-std, but we get an error with the current fork of the bitcoin crate. Concretely at bitcoin/src/consensus/encode.rs:

impl_vec!(UtreexoBlock);
impl_vec!(CompactLeafData);

UtreexoBlock and CompactLeafData are not found in the scope. The first one is because the import requires the std-lib, the second one perhaps because of the impl_consensus_encoding! macro used for CompactLeafData.

The rest of floresta crates use the std version of floresta-common, just as before, so this change shouldn't break current things.

@Davidson-Souza
Copy link
Collaborator

UtreexoBlock and CompactLeafData are not found in the scope. The first one is because the import requires the std-lib, the second one perhaps because of the impl_consensus_encoding! macro used for CompactLeafData.

Wait, I should've removed this. We have this implemented in floresta-wire. What do you think about removing it from our rust-bitcoin fork?

@JoseSK999
Copy link
Contributor Author

In floresta-chain no? If we have the types and implementations duplicated sure, it is a good idea! Why else do we need the bitcoin fork?

@jaoleal
Copy link
Contributor

jaoleal commented Oct 11, 2024

Dealing with messages, somethings we would have to reimplement...

That actually might be easy to do since davidson already did it in the fork.

@Davidson-Souza
Copy link
Collaborator

@JoseSK999 #258 removes all this unused code. Does this PR compile no-std after that?

@JoseSK999
Copy link
Contributor Author

Yes @Davidson-Souza, if I apply this commit on top of yours it compiles with no-std.

@Davidson-Souza
Copy link
Collaborator

Yes @Davidson-Souza, if I apply this commit on top of yours it compiles with no-std.

Nice! I'll merge my pr and you rebase on it

@JoseSK999
Copy link
Contributor Author

Done!

@Davidson-Souza Davidson-Souza merged commit 76122a9 into vinteumorg:master Oct 15, 2024
6 checks passed
@JoseSK999 JoseSK999 deleted the features branch November 11, 2024 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants