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

Feature branch for 0.7.0 #123

Merged
merged 10 commits into from
Mar 25, 2024
Merged

Feature branch for 0.7.0 #123

merged 10 commits into from
Mar 25, 2024

Conversation

str4d
Copy link
Contributor

@str4d str4d commented Mar 14, 2024

  • Migrated to new accounts backend.
  • Seed relevance API.
  • Orchard enabled.
  • Payments to TEX addresses supported.

@str4d str4d added this to the iOS Zashi 1.0 milestone Mar 14, 2024
@str4d str4d marked this pull request as draft March 14, 2024 16:37
Copy link
Contributor

@nuttycom nuttycom left a comment

Choose a reason for hiding this comment

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

One blocking bug.

rust/src/lib.rs Show resolved Hide resolved
rust/src/lib.rs Outdated Show resolved Hide resolved
rust/src/lib.rs Outdated Show resolved Hide resolved
rust/src/lib.rs Outdated Show resolved Hide resolved
@str4d str4d force-pushed the ffi-0.7.0 branch 2 times, most recently from d5d609c to 7860e5e Compare March 14, 2024 18:40
Copy link
Contributor

@nuttycom nuttycom left a comment

Choose a reason for hiding this comment

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

utACK 7860e5e

str4d added 3 commits March 15, 2024 15:02
Includes changes to how accounts are stored and referenced. We now
need to remember and provide the seed fingerprint; for now, given
that we know we only create derived accounts from a single seed, we
search for an account with a matching ZIP 32 account index.
Copy link
Contributor

@nuttycom nuttycom left a comment

Choose a reason for hiding this comment

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

utACK 70cce12

/// Returns:
/// - `1` for `Ok(true)`.
/// - `0` for `Ok(false)`.
/// - `-1` for `Err(_)`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Add safety documentation of arguments.

rust/src/lib.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@daira daira left a comment

Choose a reason for hiding this comment

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

utACK

nuttycom and others added 3 commits March 18, 2024 08:29
This fixes a bug in the `v_transactions` view related to changes in how
primary key columns are named in the database.
Includes some renames, and a built-in seed relevancy API that we now
use.
This enables generation of Orchard receivers for unified addresses by default.
Updates to:
- zcash_primitives 0.15.0
- zcash_proofs 0.15.0
- zcash_client_backend 0.12.0
- zcash_client_sqlite 0.10.0
@str4d str4d marked this pull request as ready for review March 25, 2024 21:35
Copy link
Contributor Author

@str4d str4d left a comment

Choose a reason for hiding this comment

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

utACK c918662

*
* Returns:
* - `1` for `Ok(true)`.
* - `0` for `Ok(false)`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Non-blocking: Ideally, documentation at this level would not depend on knowing the semantics of the corresponding Rust function (which isn't linked here).

@@ -1013,6 +1093,8 @@ struct FfiScanSummary *zcashlc_scan_blocks(const uint8_t *fs_block_cache_root,
const uint8_t *db_data,
uintptr_t db_data_len,
int32_t from_height,
const uint8_t *from_state,
uintptr_t from_state_len,
Copy link
Contributor

Choose a reason for hiding this comment

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

These new parameters have not been documented in the safety section. Non-blocking but please file an issue if it isn't fixed now.

Copy link
Contributor

Choose a reason for hiding this comment

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

Binary file not reviewed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Binary file not reviewed.

Copy link
Contributor

@daira daira left a comment

Choose a reason for hiding this comment

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

utACK. Comments are non-blocking, but please file an issue for the safety documentation comments if you merge as-is. [Done in #124.]

@str4d str4d merged commit d73ee9a into main Mar 25, 2024
@str4d str4d deleted the ffi-0.7.0 branch March 25, 2024 22:01
@str4d str4d restored the ffi-0.7.0 branch March 25, 2024 22:02
@str4d str4d deleted the ffi-0.7.0 branch March 25, 2024 22:02
@nuttycom nuttycom mentioned this pull request Mar 26, 2024
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