Skip to content

zcash_client_sqlite: Fix incomplete truncation due to bad reorg handling.#1549

Merged
nuttycom merged 7 commits into
zcash:mainfrom
nuttycom:debug/commitment_tree_conflict
Sep 27, 2024
Merged

zcash_client_sqlite: Fix incomplete truncation due to bad reorg handling.#1549
nuttycom merged 7 commits into
zcash:mainfrom
nuttycom:debug/commitment_tree_conflict

Conversation

@nuttycom

Copy link
Copy Markdown
Collaborator

No description provided.

@nuttycom nuttycom force-pushed the debug/commitment_tree_conflict branch from 1fc8848 to b367c65 Compare September 26, 2024 22:12
@nuttycom nuttycom marked this pull request as ready for review September 26, 2024 22:12
@nuttycom nuttycom force-pushed the debug/commitment_tree_conflict branch 5 times, most recently from 700ca4e to 54695c2 Compare September 26, 2024 23:13
@codecov

codecov Bot commented Sep 26, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 62.30769% with 98 lines in your changes missing coverage. Please review.

Project coverage is 61.62%. Comparing base (d8db481) to head (11f5a15).
Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
zcash_client_sqlite/src/wallet.rs 50.00% 31 Missing ⚠️
zcash_client_backend/src/data_api/wallet.rs 12.12% 29 Missing ⚠️
zcash_client_backend/src/data_api/testing/pool.rs 86.66% 10 Missing ⚠️
...let/init/migrations/fix_broken_commitment_trees.rs 60.00% 10 Missing ⚠️
zcash_client_sqlite/src/wallet/commitment_tree.rs 75.00% 6 Missing ⚠️
zcash_client_sqlite/src/error.rs 0.00% 4 Missing ⚠️
zcash_client_backend/src/data_api/testing.rs 83.33% 2 Missing ⚠️
zcash_client_backend/src/proposal.rs 0.00% 2 Missing ⚠️
zcash_client_sqlite/src/lib.rs 85.71% 2 Missing ⚠️
zcash_client_sqlite/src/testing.rs 71.42% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1549      +/-   ##
==========================================
+ Coverage   61.32%   61.62%   +0.30%     
==========================================
  Files         147      148       +1     
  Lines       18667    18808     +141     
==========================================
+ Hits        11447    11590     +143     
+ Misses       7220     7218       -2     

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

@nuttycom nuttycom force-pushed the debug/commitment_tree_conflict branch 2 times, most recently from abe88b4 to a4217d6 Compare September 27, 2024 14:55
@nuttycom

Copy link
Copy Markdown
Collaborator Author

This branch has been tested with an affected wallet, and resolves the problems that blocked additional scanning.

@str4d str4d left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Flushing comments as of 5f4aef5.

Comment thread zcash_client_backend/src/data_api/testing/pool.rs Outdated
Comment thread zcash_client_backend/src/data_api/testing/pool.rs
Comment thread zcash_client_backend/src/data_api/testing/pool.rs Outdated
Comment thread zcash_client_backend/src/data_api/testing/pool.rs Outdated
Comment thread zcash_client_backend/src/data_api/testing/pool.rs Outdated
Comment thread zcash_client_backend/src/data_api.rs Outdated
Comment thread zcash_client_sqlite/src/wallet.rs
Comment thread zcash_client_sqlite/src/wallet.rs Outdated
Comment thread Cargo.toml Outdated
Comment thread zcash_client_backend/src/proposal.rs

@str4d str4d left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed a4217d6.

Comment thread zcash_client_sqlite/src/wallet.rs Outdated
Comment thread zcash_client_sqlite/src/wallet/commitment_tree.rs
Comment thread zcash_client_sqlite/src/wallet.rs
@nuttycom nuttycom force-pushed the debug/commitment_tree_conflict branch from a4217d6 to 4dd90b7 Compare September 27, 2024 17:12
@nuttycom

Copy link
Copy Markdown
Collaborator Author

force-pushed to address review comments.

Comment thread zcash_client_backend/src/data_api/testing/pool.rs Outdated
@nuttycom nuttycom force-pushed the debug/commitment_tree_conflict branch from 4dd90b7 to 11f5a15 Compare September 27, 2024 17:41
@nuttycom

Copy link
Copy Markdown
Collaborator Author

force-pushed to correctly uncomment tests that were inadvertently left commented out.

@str4d str4d left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

utACK 11f5a15

st.truncate_to_height_retaining_cache(reorg_height);

// The following error-prone tree state is generated by the current truncate implementation:
// The following error-prone tree state is generated by the a previous (buggy) truncate

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

5a599f1:

Suggested change
// The following error-prone tree state is generated by the a previous (buggy) truncate
// The following error-prone tree state is generated by a previous (buggy) truncate

Comment thread Cargo.toml
orchard = { git = "https://github.com/zcash/orchard", rev = "55fb089a335bbbc1cda186c706bc037073df8eb7" }
incrementalmerkletree = { git = "https://github.com/zcash/incrementalmerkletree", rev = "ffe4234788fd22662b937ba7c6ea01535fcc1293" }
incrementalmerkletree-testing = { git = "https://github.com/zcash/incrementalmerkletree", rev = "ffe4234788fd22662b937ba7c6ea01535fcc1293" }
shardtree = { git = "https://github.com/zcash/incrementalmerkletree", rev = "ffe4234788fd22662b937ba7c6ea01535fcc1293" }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I checked these commits are what I expected.

@nuttycom nuttycom merged commit 467552e into zcash:main Sep 27, 2024
@nuttycom nuttycom deleted the debug/commitment_tree_conflict branch September 27, 2024 18:22
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.

2 participants