-
Notifications
You must be signed in to change notification settings - Fork 112
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
Adding global context secp256k1 #45
Conversation
@elichai Is the serde_json necessary? what is his purpose? |
It's used in the tests so I moved it from the dependencies to the dev-dependencies |
got it, thanks. |
CONTEXT = Some(Secp256k1::verification_only()); | ||
}); | ||
unsafe { CONTEXT.as_ref().unwrap() } | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Drive by comment: Did you consider using lazy_static
for this?
e.g. https://github.com/comit-network/comit-rs/blob/master/vendor/secp256k1_support/src/lib.rs#L15
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I have. that's basically what's lazy_static is doing and this doesn't require a new dependency
I am approving this PR given the following pre-condition:
|
This resolves #44
Before this for every multiplication you created the context. which is a very hungry operation (cpu, memory etc).
Instead we're creating a context only once and keep using that.
this context is thread safe. you can pass it around however you want.
the only constraint are:
There is no promise about the exact memory size and representation, meaning that you cannot save it to disk and reload later.
no one should try and modify it's memory(not a problem in rust)