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.

Sign up for GitHub

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

Jump to bottom

Error handling and init no-copy in JNI #158

Merged
merged 3 commits into from
Mar 18, 2024
Merged

Error handling and init no-copy in JNI #158

merged 3 commits into from
Mar 18, 2024

Conversation

thomas-quadratic
Copy link
Contributor

This PR has two purposes:

  1. Error handling: as much as possible, pub extern fn should not panic. We catch errors and throw exceptions in the JVM.
  2. No-copy: as much as possible, we should not have to copy data before passing to the FFI.

This is still a draft PR for the moment, as I am making sure tests are in sync.

@thomas-quadratic
feat: no copy and error checks for JNI
0ea260d 
Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>
@thomas-quadratic
fix: tests
93d969d 
Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>
github-advanced-security[bot]
github-advanced-security bot found potential problems Mar 8, 2024
View reviewed changes
ipa-multipoint/src/main/java/org/hyperledger/besu/nativelib/ipamultipoint/LibIpaMultipoint.java Fixed Show fixed Hide fixed
ipa-multipoint/src/main/java/org/hyperledger/besu/nativelib/ipamultipoint/LibIpaMultipoint.java Fixed Show fixed Hide fixed
ipa-multipoint/src/main/java/org/hyperledger/besu/nativelib/ipamultipoint/LibIpaMultipoint.java Fixed Show fixed Hide fixed
ipa-multipoint/src/main/java/org/hyperledger/besu/nativelib/ipamultipoint/LibIpaMultipoint.java Fixed Show fixed Hide fixed
ipa-multipoint/src/main/java/org/hyperledger/besu/nativelib/ipamultipoint/LibIpaMultipoint.java Fixed Show fixed Hide fixed
ipa-multipoint/src/main/java/org/hyperledger/besu/nativelib/ipamultipoint/LibIpaMultipoint.java Fixed Show fixed Hide fixed
ipa-multipoint/src/main/java/org/hyperledger/besu/nativelib/ipamultipoint/LibIpaMultipoint.java Fixed Show fixed Hide fixed
ipa-multipoint/src/main/java/org/hyperledger/besu/nativelib/ipamultipoint/LibIpaMultipoint.java Fixed Show fixed Hide fixed
ipa-multipoint/src/main/java/org/hyperledger/besu/nativelib/ipamultipoint/LibIpaMultipoint.java Fixed Show fixed Hide fixed
@thomas-quadratic thomas-quadratic marked this pull request as ready for review March 8, 2024 16:55
This was referenced Mar 12, 2024
Closed
Closed
matkt
matkt reviewed Mar 13, 2024
View reviewed changes
Copy link
Contributor

@matkt matkt left a comment

Choose a reason for hiding this comment

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

LGTM, if we can just add a specific commit for rust verkle and fix javadoc issues before merging

It will be nice to add some description for each method in order to simplify the readability

ipa-multipoint/ipa_multipoint_jni/Cargo.toml Outdated Show resolved Hide resolved
@thomas-quadratic
FIX: javadocs and rust-verkle revision
1dbf51f 
Signed-off-by: Thomas Zamojski <thomas.zamojski@quadratic-labs.com>
@matkt matkt merged commit 007c808 into hyperledger:main Mar 18, 2024
11 checks passed
@thomas-quadratic thomas-quadratic mentioned this pull request Jun 20, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matkt matkt matkt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@thomas-quadratic @matkt