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

make lib komodefi compatible #8

Merged
merged 43 commits into from
Nov 7, 2023
Merged

make lib komodefi compatible #8

merged 43 commits into from
Nov 7, 2023

Conversation

borngraced
Copy link
Member

@borngraced borngraced commented Aug 22, 2023 •
edited
Loading

Changes/Modifications in this PR

In this PR, several changes and improvements have been introduced to enhance compatibility with Komodefi for the librustzcash library:

  • Created new crate to share common interfaces with WASM
  • Asynchronous initialisation functions.
  • The default WalletDb wrapper, which is based on the Sqlite Connection and does not support asynchronous environments, prompted the creation of an alternative solution. To address this limitation, I introduced an async-ready version of the wrapper named WalletDbAsync.
  • Re-export and create async versions of WalletWrite and WalletRead traits.
  • Previously, DataConnStmtCache was primarily used to prepare statements before executing various wallet actions, relying on references to Statement. To enhance compatibility in asynchronous environments, refactor the process by utilizing prepared statements when invoking (I/O) functions.
  • Refactor and use async_blocking for some sqlite I/O functions

@borngraced
save dev state — added mm2_ext modules to sqlite_client and backend
07ebc40
@borngraced
save dev state — create async fn version of data_api functions
d1158fe
@borngraced
save dev state — modified async BlockSource trait
8819ec2
@borngraced
use aync-ware mutex
c3ab476
@borngraced
save dev state — ported wallet_Write and Read
924c418
@borngraced
save dev state — impl inner fn for WalletDbAsync
3b1122b
@borngraced
save dev state — impl for_path fn for WalletDbAsync
34799ce
@borngraced
save dev state — make BlockSource Send
9d2c681
@borngraced
save dev state — finish modification
329e249
@borngraced
save dev state — finish modification
ecb0295
@borngraced
save dev state — minor changes
624ed96
@borngraced
save dev state — zcash extras
e473d79
@borngraced
Merge remote-tracking branch 'origin/mm2_extras' into mm2_extras
7fa620c
@borngraced
make NoteRef not Copy
bafac3a
@borngraced
move ShieldedOutput
3c21128
@borngraced
fix review notes and add doc comments
5d5d822
@borngraced
Revert "move ShieldedOutput"
d3d81c9 
This reverts commit 3c21128.
@borngraced
async init
8c12241
@borngraced
move WalletShieldedOutput
e89819f
@borngraced
make fake_compact_block and fake_compact_block_spending public
0c20210
@borngraced
move fake_compact_block and fake_compact_block_spending public
3d07af9
@borngraced
minor changes
9837e10
@borngraced
copy decrypt_and_store_transaction and create_spend_to_address as async
6ab22ea
@borngraced
make db initializations async
8f4b5e1
@shamardy shamardy mentioned this pull request Sep 18, 2023
Merged
8 tasks
@borngraced borngraced changed the title create and export async versions of data_api fn for use in mm2 make lib komodefi compatible Oct 2, 2023
onur-ozkan
onur-ozkan reviewed Oct 3, 2023
View reviewed changes
Copy link
Member

@onur-ozkan onur-ozkan left a comment

Choose a reason for hiding this comment

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

First review iteration:

zcash_client_backend/Cargo.toml Outdated Show resolved Hide resolved
zcash_client_backend/src/data_api.rs Outdated Show resolved Hide resolved
zcash_client_sqlite/Cargo.toml Outdated Show resolved Hide resolved
zcash_client_sqlite/Cargo.toml Outdated Show resolved Hide resolved
zcash_client_sqlite/src/for_async/init.rs Show resolved Hide resolved
zcash_extras/Cargo.toml Outdated Show resolved Hide resolved
shamardy
shamardy reviewed Oct 7, 2023
View reviewed changes
Copy link

@shamardy shamardy left a comment

Choose a reason for hiding this comment

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

Thanks you for the PR. First review iteration!

zcash_client_sqlite/src/for_async/init.rs Outdated Show resolved Hide resolved
zcash_client_sqlite/src/for_async/init.rs Outdated Show resolved Hide resolved
zcash_client_sqlite/src/for_async/init.rs Outdated Show resolved Hide resolved
zcash_client_sqlite/src/for_async/mod.rs Outdated Show resolved Hide resolved
zcash_client_sqlite/src/for_async/mod.rs Show resolved Hide resolved
onur-ozkan
onur-ozkan reviewed Oct 9, 2023
View reviewed changes
Copy link
Member

@onur-ozkan onur-ozkan left a comment

Choose a reason for hiding this comment

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

Can we somehow write tests for this?

One minor note from last review iteration from my side:

zcash_client_sqlite/Cargo.toml Outdated Show resolved Hide resolved
artemii235
artemii235 requested changes Oct 31, 2023
View reviewed changes
Copy link
Member

@artemii235 artemii235 left a comment

Choose a reason for hiding this comment

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

Thanks for the big work! I have few questions and changes requests.

zcash_client_sqlite/src/for_async/mod.rs Outdated Show resolved Hide resolved
zcash_client_sqlite/src/for_async/mod.rs Outdated Show resolved Hide resolved
zcash_client_sqlite/src/for_async/mod.rs Outdated Show resolved Hide resolved
zcash_client_sqlite/src/for_async/mod.rs Outdated Show resolved Hide resolved
zcash_client_sqlite/Cargo.toml Outdated Show resolved Hide resolved
zcash_extras/src/wallet.rs Outdated Show resolved Hide resolved
zcash_extras/src/wallet.rs Show resolved Hide resolved
zcash_proofs/src/prover.rs Outdated Show resolved Hide resolved
zcash_primitives/Cargo.toml Show resolved Hide resolved
zcash_primitives/Cargo.toml Show resolved Hide resolved
artemii235
artemii235 requested changes Nov 1, 2023
View reviewed changes
Copy link
Member

@artemii235 artemii235 left a comment

Choose a reason for hiding this comment

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

Next review iteration.

zcash_client_sqlite/src/for_async/mod.rs Outdated Show resolved Hide resolved
zcash_client_sqlite/src/for_async/mod.rs Outdated Show resolved Hide resolved
zcash_client_sqlite/src/for_async/mod.rs Outdated Show resolved Hide resolved
zcash_client_sqlite/src/for_async/mod.rs Outdated Show resolved Hide resolved
zcash_client_sqlite/src/lib.rs Outdated Show resolved Hide resolved
zcash_extras/src/lib.rs Outdated Show resolved Hide resolved
zcash_extras/Cargo.toml Show resolved Hide resolved
zcash_client_backend/src/data_api.rs Outdated Show resolved Hide resolved
artemii235
artemii235 requested changes Nov 2, 2023
View reviewed changes
Copy link
Member

@artemii235 artemii235 left a comment

Choose a reason for hiding this comment

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

Next iteration

zcash_client_sqlite/src/for_async/init.rs Outdated Show resolved Hide resolved
zcash_client_sqlite/src/for_async/init.rs Outdated Show resolved Hide resolved
zcash_client_sqlite/src/for_async/init.rs Outdated Show resolved Hide resolved
zcash_client_sqlite/src/for_async/init.rs Outdated Show resolved Hide resolved
zcash_extras/src/lib.rs Show resolved Hide resolved
zcash_extras/src/lib.rs Show resolved Hide resolved
artemii235
artemii235 previously approved these changes Nov 3, 2023
View reviewed changes
Copy link
Member

@artemii235 artemii235 left a comment

Choose a reason for hiding this comment

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

Thanks for all fixes 🙂

shamardy
shamardy reviewed Nov 7, 2023
View reviewed changes
Copy link

@shamardy shamardy left a comment

Choose a reason for hiding this comment

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

Thank you for the fixes! Only minor comments for this last review!

zcash_client_sqlite/src/wallet/transact.rs Outdated Show resolved Hide resolved
zcash_client_sqlite/src/wallet/transact.rs Outdated Show resolved Hide resolved
zcash_client_sqlite/src/lib.rs Outdated Show resolved Hide resolved
zcash_client_sqlite/src/lib.rs Outdated Show resolved Hide resolved
shamardy
shamardy approved these changes Nov 7, 2023
View reviewed changes
Copy link

@shamardy shamardy left a comment

Choose a reason for hiding this comment

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

🔥

@shamardy shamardy merged commit debae77 into master Nov 7, 2023
3 of 24 checks passed
@shamardy shamardy deleted the mm2_extras branch November 7, 2023 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@onur-ozkan onur-ozkan onur-ozkan left review comments

@shamardy shamardy shamardy approved these changes

@artemii235 artemii235 artemii235 left review comments

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

Successfully merging this pull request may close these issues.
4 participants
@borngraced @artemii235 @shamardy @onur-ozkan