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 `secp256k1` and possibly wrap its types. #264

Closed
dconnolly opened this issue Feb 24, 2020 · 5 comments
Closed

Add `secp256k1` and possibly wrap its types. #264

dconnolly opened this issue Feb 24, 2020 · 5 comments

Comments

@dconnolly
Copy link
Member

@dconnolly dconnolly commented Feb 24, 2020

The secp256k1 crate is a Rust wrapper around the well-vetted and constant-time C library that is actively maintained.

While including a non-trivial C dependency into our (so far) all-Rust project isn't the ideal answer, this library is a better choice in terms of maintainability (and offloading that to someone else) for us than some of the all-Rust alternatives out there, which are either less-great from a constant-time, implementation perspective or are just less mature than bitcoin-core/secp256k1.

@dconnolly dconnolly added this to the Validate transactions. milestone Feb 24, 2020
@dconnolly dconnolly added this to To Do in 🦓 via automation Feb 24, 2020
@dconnolly dconnolly self-assigned this Feb 24, 2020
@hdevalence hdevalence changed the title Use `secp256k1` to represent and do transparent key stuff Add `secp256k1` and possibly wrap its types. Feb 25, 2020
@hdevalence

This comment has been minimized.

Copy link
Member

@hdevalence hdevalence commented Feb 25, 2020

Changed title so that we have one issue for integrating the dependency and one issue for adding transparent keys and addresses.

@dconnolly

This comment has been minimized.

Copy link
Member Author

@dconnolly dconnolly commented Feb 25, 2020

Pulling this in exposes the signing operations needed to verify script ops for transparent transactions without joinsplits.

@dconnolly dconnolly moved this from To Do to In progress in 🦓 Feb 25, 2020
@hdevalence

This comment has been minimized.

Copy link
Member

@hdevalence hdevalence commented Feb 25, 2020

The library requires building a context object and using the context, which is sort of awkward. Maybe the easiest thing to do would be to use lazy_static to build the context and store it as a "constant" somewhere in zebra-chain.

@saleemrashid

This comment has been minimized.

Copy link

@saleemrashid saleemrashid commented Feb 25, 2020

Not to co-opt this issue, but it could be worth looking into sandboxing. As far as I know, libsecp256k1 makes no syscalls, so you could use the strictest sandboxing policy. I think Google has some excellent open source, cross-platform sandboxing libraries, but they're for C++ (could be useful for seeing which APIs to use on each platform).

@hdevalence

This comment has been minimized.

Copy link
Member

@hdevalence hdevalence commented Feb 25, 2020

Sandboxing seems great, but unfortunately I don't think that we have resources to do it in the medium term. I don't feel great about C libraries in general, but I feel that of all C libraries that a Rust project might use, this one is relatively less risk than others.

🦓 automation moved this from In progress to Done Feb 28, 2020
@dconnolly dconnolly reopened this Feb 28, 2020
🦓 automation moved this from Done to In progress Feb 28, 2020
@dconnolly dconnolly closed this Feb 28, 2020
🦓 automation moved this from In progress to Done Feb 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
🦓
  
Done
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants
You can’t perform that action at this time.