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

fix: resolved design flaw in wallet_ffi library #3285

Merged
merged 2 commits into from
Sep 3, 2021

Conversation

StriderDM
Copy link
Contributor

Description
Exposed TariTransactionKernel instead of TariExcess, TariExcessSignature and TariExcessPublicNonce. This was done as they are all referring to fields from the same object and also to reduce the amount of wrapper classes needs on the other side of the FFI boundary.

Added accessor methods to retrieve data from the opaque pointer for TariTransactionKernel and return the data as a type (in this instance a cstring) which can safely cross the FFI boundary.

Updated library header.
Updated rust tests.
Updated cucumber tests.
Updated comments.
Incremented library version.

Merge #3275 first.

Motivation and Context

How Has This Been Tested?
cargo test --all --all-features
nvm use v12.22.6 && ./node_modules/.bin/cucumber-js features/WalletFFI.feature

Screen Shot 2021-09-02 at 5 08 56 PM

Exposed TariTransactionKernel instead of TariExcess, TariExcessSignature and TariExcessPublicNonce. This was done as they are all referring to fields from the same object and also to reduce the amount of wrapper classes needs on the other side of the FFI boundary.

Added accessor methods to retrieve data from the opaque pointer for TariTransactionKernel and return the data as a type (in this instance a cstring) which can safely cross the FFI boundary.

Updated library header.

Updated rust tests.

Updated cucumber tests.

Updated comments.

Incremented library version.

Fix typo

Fix duplicate line
@aviator-app aviator-app bot merged commit 2e6638c into tari-project:development Sep 3, 2021
Cifko pushed a commit to Cifko/tari that referenced this pull request Sep 10, 2021
Description
Exposed TariTransactionKernel instead of TariExcess, TariExcessSignature and TariExcessPublicNonce. This was done as they are all referring to fields from the same object and also to reduce the amount of wrapper classes needs on the other side of the FFI boundary.

Added accessor methods to retrieve data from the opaque pointer for TariTransactionKernel and return the data as a type (in this instance a cstring) which can safely cross the FFI boundary.

Updated library header.
Updated rust tests.
Updated cucumber tests.
Updated comments.
Incremented library version.

Merge tari-project#3275 first.

Motivation and Context
---

How Has This Been Tested?
cargo test --all --all-features
nvm use v12.22.6 && ./node_modules/.bin/cucumber-js features/WalletFFI.feature

![Screen Shot 2021-09-02 at 5 08 56 PM](https://user-images.githubusercontent.com/51991544/131870883-4b0175ee-f734-49f5-aeb0-b95842062fa2.png)
aviator-app bot pushed a commit that referenced this pull request Sep 22, 2021
Description
---
Include vendored openssl library when not building ffi wallet library.
Exclude vendored openssl library when building ffi wallet library as it needs to be cross compiled for mobile which is done in mobile_build.sh.

Merge #3285 first.

We had to do the same for sqlite here:
#3073

Motivation and Context
---
Current:
![Screen Shot 2021-09-06 at 6 30 17 PM](https://user-images.githubusercontent.com/51991544/132251475-cce3d7e4-7237-4c70-8f40-c92b12d0091a.png)

This PR:
![Screen Shot 2021-09-06 at 8 06 48 PM](https://user-images.githubusercontent.com/51991544/132251455-bbdfbc5c-864f-416a-953f-9f8f6504666b.png)


[mobile_build_logs.zip](https://github.com/tari-project/tari/files/7104686/mobile_build_logs.zip)

How Has This Been Tested?
---
cargo test --all --all-features
nvm use v12.22.6 && ./node_modules/.bin/cucumber-js features/WalletFFI.feature
sh  mobile_build.sh
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.

3 participants