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

Make static-context and lazy-static-context addictive #82

Merged
merged 2 commits into from
Sep 16, 2021

Conversation

sorpaas
Copy link
Member

@sorpaas sorpaas commented Sep 15, 2021

closes #77

@athei
Copy link
Member

athei commented Sep 15, 2021

I hate to be this guy but we need a README update regarding this feature 😅

Copy link
Contributor

@dvdplm dvdplm left a comment

Choose a reason for hiding this comment

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

Definitely would be good to document the features offered in the readme.

Comment on lines 50 to 51
/// A static ECMult context.
// Correct `pre_g` values are fed into `ECMultContext::new_from_raw`, generated by build script.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// A static ECMult context.
// Correct `pre_g` values are fed into `ECMultContext::new_from_raw`, generated by build script.
/// A static ECMult context. Requires that the `static-context` to be enabled and that `lazy-static-context` is **not** enabled.
/// Correct `pre_g` values are fed into `ECMultContext::new_from_raw`, generated by build script.

Copy link
Member Author

Choose a reason for hiding this comment

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

ECMULT_CONTEXT is defined either here or through lazy_static. It is always available either with static-context or lazy-static-context.

The second line is related to implementation details of how this is generated, so it doesn't really fit the docs.

/// A static ECMult context.
// Correct `pre_g` values are fed into `ECMultContext::new_from_raw`, generated by build script.
pub static ECMULT_CONTEXT: ECMultContext =
unsafe { ECMultContext::new_from_raw(include!(concat!(env!("OUT_DIR"), "/const.rs"))) };

#[cfg(feature = "static-context")]
#[cfg(all(feature = "static-context", not(feature = "lazy-static-context")))]
/// A static ECMultGen context.
Copy link
Contributor

Choose a reason for hiding this comment

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

Same note in the docs here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Same as above.

@sorpaas sorpaas merged commit b30fdab into master Sep 16, 2021
@athei athei deleted the sp-feature-set branch September 16, 2021 10:03
trevor-crypto pushed a commit to monacohq/libsecp256k1 that referenced this pull request May 31, 2022
* Make static-context and lazy-static-context addictive

* Fix README docs regarding `lazy-static-context` overwrites `static-context`
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.

Poor usage of Cargo features for static-context vs lazy-static-context
3 participants