Skip to content

Fix rapid-gossip-sync no-std and properly test no-std in CI #1756

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

Merged

Conversation

TheBlueMatt
Copy link
Collaborator

While rapid-gossip-sync recently gained a no-std feature, it
didn't actually work, as there were still dangling references to
std and prelude assumptions. This makes rapid-gossip-sync
build (and test) properly in no-std.

Ultimately, Rust is incredibly forgiving in attempts to access std, making it
rather difficult to test no-std properly. In practice, the only
decent way to do so is to actually build for a platform that does
not have std, which we do here by building the no-std-check
crate for arm-thumbv7m-none-eabi.

@codecov-commenter
Copy link

codecov-commenter commented Oct 6, 2022

Codecov Report

Base: 90.73% // Head: 90.71% // Decreases project coverage by -0.01% ⚠️

Coverage data is based on head (7143599) compared to base (7544030).
Patch coverage: 85.71% of modified lines in pull request are covered.

❗ Current head 7143599 differs from pull request most recent head ada0df2. Consider uploading reports for the commit ada0df2 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1756      +/-   ##
==========================================
- Coverage   90.73%   90.71%   -0.02%     
==========================================
  Files          87       87              
  Lines       46713    46713              
  Branches    46713    46713              
==========================================
- Hits        42383    42374       -9     
- Misses       4330     4339       +9     
Impacted Files Coverage Δ
lightning-rapid-gossip-sync/src/lib.rs 91.35% <ø> (ø)
lightning-rapid-gossip-sync/src/error.rs 62.50% <50.00%> (ø)
lightning-rapid-gossip-sync/src/processing.rs 91.24% <100.00%> (ø)
lightning/src/ln/peer_handler.rs 55.96% <100.00%> (ø)
lightning/src/ln/functional_tests.rs 96.93% <0.00%> (-0.12%) ⬇️
lightning/src/ln/monitor_tests.rs 99.44% <0.00%> (-0.12%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

While `rapid-gossip-sync` recently gained a `no-std` feature, it
didn't actually work, as there were still dangling references to
`std` and prelude assumptions. This makes `rapid-gossip-sync`
build (and test) properly in `no-std`.
@TheBlueMatt TheBlueMatt force-pushed the 2022-10-rgs-no-std branch 2 times, most recently from c7fbeae to a59fc4d Compare October 6, 2022 19:39
Given there is only one instance in our code of `AtomicU64` its
simplest to just remove it rather than try to add some kind of
wrapper.
@TheBlueMatt TheBlueMatt added this to the 0.0.112 milestone Oct 6, 2022
Copy link
Contributor

@tnull tnull left a comment

Choose a reason for hiding this comment

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

Grrr. Excuse the oversight. Looks good mod the failing CI.

Rust is incredibly forgiving in attempts to access `std`, making it
rather difficult to test `no-std` properly. In practice, the only
decent way to do so is to actually build for a platform that does
not have `std`, which we do here by building the `no-std-check`
crate for `arm-thumbv7m-none-eabi`.
@valentinewallace valentinewallace merged commit 559ed20 into lightningdevkit:main Oct 7, 2022
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.

5 participants