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

Add support for transparent-source-only (TEX) addresses #1257

Merged
merged 56 commits into from
Jul 17, 2024

Conversation

daira
Copy link
Contributor

@daira daira commented Mar 12, 2024

fixes #1167

@daira daira added the A-wallet Area: light wallet backend. label Mar 12, 2024
@daira daira added this to the Librustzcash Zashi 1.0 milestone Mar 12, 2024
@daira daira force-pushed the implement-transparent-source-only branch from 4f54370 to 24b8147 Compare March 12, 2024 00:21
Copy link

codecov bot commented Mar 12, 2024

Codecov Report

Attention: Patch coverage is 79.79798% with 180 lines in your changes missing coverage. Please review.

Project coverage is 64.19%. Comparing base (5bd911f) to head (f8bedd8).
Report is 13 commits behind head on main.

Files Patch % Lines
zcash_client_backend/src/data_api/wallet.rs 71.72% 54 Missing ⚠️
zcash_client_backend/src/data_api.rs 4.00% 24 Missing ⚠️
zcash_client_backend/src/wallet.rs 34.61% 17 Missing ⚠️
zcash_client_backend/src/proto.rs 18.75% 13 Missing ⚠️
zcash_client_backend/src/fees/common.rs 91.66% 11 Missing ⚠️
zcash_client_backend/src/proposal.rs 0.00% 10 Missing ⚠️
zcash_client_sqlite/src/wallet/transparent.rs 86.11% 10 Missing ⚠️
...ent_backend/src/data_api/wallet/input_selection.rs 93.04% 8 Missing ⚠️
zcash_client_sqlite/src/lib.rs 87.09% 8 Missing ⚠️
..._client_sqlite/src/wallet/transparent/ephemeral.rs 95.45% 7 Missing ⚠️
... and 7 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1257      +/-   ##
==========================================
+ Coverage   63.20%   64.19%   +0.99%     
==========================================
  Files         128      130       +2     
  Lines       14869    15423     +554     
==========================================
+ Hits         9398     9901     +503     
- Misses       5471     5522      +51     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@daira daira force-pushed the implement-transparent-source-only branch 6 times, most recently from 0fc932c to 62298aa Compare March 19, 2024 01:50
@nuttycom nuttycom removed this from the Librustzcash Zashi 1.0 milestone Mar 26, 2024
@daira daira force-pushed the implement-transparent-source-only branch from a8e279e to 1af90e3 Compare May 18, 2024 03:45
@daira daira force-pushed the implement-transparent-source-only branch 4 times, most recently from 85dd964 to 7c8e775 Compare May 30, 2024 17:14
@daira daira force-pushed the implement-transparent-source-only branch 8 times, most recently from ba99ab5 to 964ef81 Compare June 4, 2024 09:54
nuttycom
nuttycom previously approved these changes Jul 16, 2024
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 with a few minor notes.

Comment on lines -1242 to -1246
if d_tx
.tx()
.transparent_bundle()
.iter()
.any(|b| !b.vout.is_empty())
Copy link
Contributor

Choose a reason for hiding this comment

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

As a nit, this condition did allow us to skip the get_funding_accounts query; it's a trivial optimization, but it avoided a database roundtrip by just inspecting in-memory data.

zcash_client_sqlite/src/lib.rs Show resolved Hide resolved
zcash_client_sqlite/src/wallet/db.rs Outdated Show resolved Hide resolved
zcash_client_sqlite/src/wallet/transparent/ephemeral.rs Outdated Show resolved Hide resolved
@nuttycom nuttycom force-pushed the implement-transparent-source-only branch 3 times, most recently from e097829 to ba32fba Compare July 16, 2024 23:28
@nuttycom nuttycom force-pushed the implement-transparent-source-only branch from f01a878 to 5bfd5e6 Compare July 17, 2024 13:50
@nuttycom nuttycom force-pushed the implement-transparent-source-only branch from 5bfd5e6 to 24b6d50 Compare July 17, 2024 13:52
nuttycom
nuttycom previously approved these changes Jul 17, 2024
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 ab7f1b9

@nuttycom nuttycom requested a review from str4d July 17, 2024 14:11
This also provides additional documentation for why it's necessary
to store ephemeral_addresses table entries at indicies that do not
correspond to valid addresses.
@nuttycom nuttycom force-pushed the implement-transparent-source-only branch from ab7f1b9 to f8bedd8 Compare July 17, 2024 17:16
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 f8bedd8

Copy link
Contributor

@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 f8bedd8

@str4d str4d merged commit eaa43b4 into zcash:main Jul 17, 2024
27 checks passed
pub(super) const INDEX_EPHEMERAL_ADDRESSES_ADDRESS: &str = r#"
CREATE INDEX ephemeral_addresses_address ON ephemeral_addresses (
address ASC
)"#;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For reference, the ephemeral_addr_uniq constraint has essentially the same effect as a unique index.

Copy link
Contributor Author

@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.

Post-hoc utACK for the last 4 commits, i.e. up to and including f8bedd8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-wallet Area: light wallet backend. C-zip-impl Category: An implementation of a ZIP, or a request to do so
Projects
None yet
Development

Successfully merging this pull request may close these issues.

zcash_client_backend: Implement ZIP 320 transaction generation
4 participants