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

Minimise FFI in the public API #507

Merged
merged 1 commit into from Nov 14, 2022

Conversation

tcharding
Copy link
Member

Normal users should never need to directly interact with the FFI layer.

Audit and reduce the use of ffi types in the public API of various types. Leave only the implementation of CPtr, and document this clearly as not required by normal users. Done for:

  • PublicKey
  • XOnlyPublicKey
  • KeyPair
  • ecdsa::Signature
  • ecdsa::RecoverableSignature

Normal users should never need to directly interact with the FFI layer.

Audit and reduce the use of `ffi` types in the public API of various
types. Leave only the implementation of `CPtr`, and document this
clearly as not required by normal users. Done for:

- PublicKey
- XOnlyPublicKey
- KeyPair
- ecdsa::Signature
- ecdsa::RecoverableSignature
@apoelstra
Copy link
Member

Looks good! I think the description could be changed to "make all raw pointer methods go through the CPtr trait" rather than "Minimise FFI in the public API" since this change is actually fairly narrow.

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 68c7385

@apoelstra
Copy link
Member

I'm gonna merge anyway, unless you ninja-edit the description/title in the next 5 minutes :).

@apoelstra apoelstra merged commit 432f293 into rust-bitcoin:master Nov 14, 2022
@tcharding tcharding deleted the 11-08-mimimise-ffi-in-api branch November 16, 2022 02:47
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.

None yet

2 participants