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 global context API #392

Merged
merged 4 commits into from
Feb 9, 2022
Merged

Conversation

tcharding
Copy link
Member

@tcharding tcharding commented Feb 8, 2022

Our API often involves a Secp256k1 parameter, when users enable the global-context feature they must then pass SECP256K1 into these functions. This is kind of clunky since the global is by definition available everywhere.

Make the API more ergonomic for global-context builds by adding various API functions/methods that use the global context implicitly.

The first 3 patches are clean up.

Resolves: #330

Copy link
Contributor

@thomaseizinger thomaseizinger left a comment

Choose a reason for hiding this comment

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

Yep, that is what I had in mind! Thanks for tackling this! :)

I am obviously biased though :D

When we removed the "global-context-less-secure" a duplicate feature
snuck in, remove it.
The global context is already in scope in tests since we use a glob
import. No clue why Clippy does not warn for this.

Remove unnecessary import statement in test function.
The `from_secret_key` method definition currently uses non-standard
indentation.

Improve uniformity by using 'standard' indentation.
Our API often involves a `Secp256k1` parameter, when users enable the
`global-context` feature they must then pass `SECP256K1` into these
functions. This is kind of clunky since the global is by definition
available everywhere.

Make the API more ergonomic for `global-context` builds by adding
various API functions/methods that use the global context implicitly.
@tcharding tcharding marked this pull request as ready for review February 9, 2022 09:24
Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK eb453b8

Strictly additive, ok to merge in a minor revision.

I'm a tiny bit hesitant about this because it reflects a shift in the library's philosophy toward encouraging use of the global context (since these methdos are more convenient than the existing ones) biut I think that this is the direction we're moving in, even upstreatm.

@apoelstra apoelstra merged commit 4f0f542 into rust-bitcoin:master Feb 9, 2022
@tcharding tcharding deleted the global-api branch February 11, 2022 08:36
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.

Provide more ergonomic APIs using global-context
3 participants