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

zcash_client_backend: Make scan range bounds required in data_api::chain::scan_cached_blocks #867

Merged
merged 3 commits into from
Jul 6, 2023
Merged

zcash_client_backend: Make scan range bounds required in data_api::chain::scan_cached_blocks #867

merged 3 commits into from
Jul 6, 2023

Conversation

nuttycom
Copy link
Contributor

@nuttycom nuttycom commented Jul 3, 2023

Builds on #861

@nuttycom nuttycom force-pushed the feature/pre_dag_sync-require_scan_range branch 2 times, most recently from f6ab555 to b8b4be9 Compare July 3, 2023 22:20
@codecov
Copy link

codecov bot commented Jul 3, 2023 •
edited
Loading

Codecov Report

Patch coverage: 59.48% and project coverage change: -0.18 ⚠️

Comparison is base (1e5b23a) 70.20% compared to head (f06ab68) 70.02%.

❗ Current head f06ab68 differs from pull request most recent head 81a32f2. Consider uploading reports for the commit 81a32f2 to get more accurate results

Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #867      +/-   ##
==========================================
- Coverage   70.20%   70.02%   -0.18%     
==========================================
  Files         127      127              
  Lines       11918    11968      +50     
==========================================
+ Hits         8367     8381      +14     
- Misses       3551     3587      +36
Impacted Files Coverage Δ
zcash_client_backend/src/data_api.rs 40.67% <ø> (ø)
zcash_client_sqlite/src/chain.rs 52.52% <ø> (ø)
zcash_client_sqlite/src/lib.rs 57.10% <0.00%> (-0.92%) ⬇️
zcash_client_sqlite/src/wallet/sapling.rs 77.86% <ø> (ø)
zcash_client_backend/src/scanning.rs 45.25% <39.39%> (+3.01%) ⬆️
..._sqlite/src/wallet/init/migrations/ufvk_support.rs 71.42% <50.00%> (-0.69%) ⬇️
zcash_client_backend/src/data_api/chain.rs 59.49% <62.50%> (-5.42%) ⬇️
zcash_client_backend/src/data_api/wallet.rs 86.97% <87.50%> (-0.21%) ⬇️
zcash_client_sqlite/src/wallet.rs 70.72% <100.00%> (-2.63%) ⬇️
...e/src/wallet/init/migrations/v_transactions_net.rs 73.33% <100.00%> (ø)

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@nuttycom nuttycom force-pushed the feature/pre_dag_sync-require_scan_range branch 2 times, most recently from 3420f4b to 8300c4a Compare July 4, 2023 19:55
str4d
str4d reviewed Jul 5, 2023
View reviewed changes
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 8300c4a with suggestions. Blocked on #861 merging.

zcash_client_backend/src/scanning.rs Outdated Show resolved Hide resolved
zcash_client_backend/src/scanning.rs Outdated Show resolved Hide resolved
zcash_client_backend/src/scanning.rs Outdated Show resolved Hide resolved
zcash_client_backend/src/data_api/chain.rs Show resolved Hide resolved
zcash_client_backend/src/data_api/chain.rs Outdated Show resolved Hide resolved
@nuttycom
Split PoolType enum into PoolType and ShieldedProtocol
faccf56 
There are cases where we wish to return informaiton that is relevant to
a specific shielded protocol and `Transparent` is an invalid case. This
is a minor preparatory refactoring that makes this distinction
expressible.
@nuttycom nuttycom force-pushed the feature/pre_dag_sync-require_scan_range branch from 8300c4a to 9ba6fa0 Compare July 6, 2023 00:25
@nuttycom nuttycom mentioned this pull request Jul 6, 2023
Closed
@nuttycom nuttycom force-pushed the feature/pre_dag_sync-require_scan_range branch from 9ba6fa0 to 98147e0 Compare July 6, 2023 00:56
str4d
str4d requested changes Jul 6, 2023
View reviewed changes
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 98147e0 after one fix.

zcash_client_backend/src/data_api/chain.rs Show resolved Hide resolved
zcash_client_backend/src/scanning.rs Outdated Show resolved Hide resolved
@nuttycom nuttycom force-pushed the feature/pre_dag_sync-require_scan_range branch from 2d90c4a to f06ab68 Compare July 6, 2023 17:21
@nuttycom nuttycom force-pushed the feature/pre_dag_sync-require_scan_range branch from f06ab68 to 81a32f2 Compare July 6, 2023 17:24
str4d
str4d approved these changes Jul 6, 2023
View reviewed changes
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 81a32f2

@str4d str4d merged commit 82705a4 into zcash:main Jul 6, 2023
9 of 10 checks passed
@nuttycom nuttycom deleted the feature/pre_dag_sync-require_scan_range branch July 6, 2023 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@str4d str4d str4d 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.
2 participants
@nuttycom @str4d