Skip to content

[ZIP-233] Network Sustainability Mechanism: Burning#1879

Merged
nuttycom merged 22 commits into
zcash:mainfrom
nuttycom:nsm
Aug 6, 2025
Merged

[ZIP-233] Network Sustainability Mechanism: Burning#1879
nuttycom merged 22 commits into
zcash:mainfrom
nuttycom:nsm

Conversation

@nuttycom
Copy link
Copy Markdown
Collaborator

This builds atop #1567 to bring it up to date with the current state of main and address comments from code review.

Copy link
Copy Markdown
Collaborator Author

@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 304c71b - this is left as a comment because I opened this PR to subsume #1567

Copy link
Copy Markdown
Collaborator Author

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

re-utACK d47531c with lint fixes.

@daira
Copy link
Copy Markdown
Contributor

daira commented Jul 22, 2025

It looks like the build is failing in CI. I will wait until that's fixed to review.

Comment thread zcash_primitives/src/transaction/builder.rs Outdated
Comment thread zcash_primitives/src/transaction/builder.rs Outdated
Comment thread zcash_primitives/src/transaction/builder.rs
Comment thread zcash_primitives/src/transaction/components/orchard.rs
Comment thread zcash_primitives/src/transaction/mod.rs
Comment thread zcash_primitives/src/transaction/mod.rs Outdated
Comment thread zcash_primitives/src/transaction/mod.rs
Copy link
Copy Markdown
Contributor

@arya2 arya2 left a comment

Choose a reason for hiding this comment

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

This is looking good. I still need to review the test data.

Comment thread components/zcash_protocol/src/value.rs
Comment thread zcash_primitives/src/transaction/builder.rs Outdated
Comment thread zcash_primitives/src/transaction/mod.rs
Comment thread zcash_primitives/src/transaction/mod.rs Outdated
Comment thread zcash_primitives/src/transaction/mod.rs Outdated
Comment thread zcash_primitives/src/transaction/mod.rs
Comment thread zcash_primitives/src/transaction/mod.rs
Comment thread zcash_primitives/src/transaction/mod.rs
Comment thread zcash_primitives/src/transaction/mod.rs
assert_eq!(&data[..], &encoded[..]);
}

#[cfg(test)]
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.

It would be nice to export transaction::tests::data from lib directly instead of adding everything in transaction::tests into compilations with the test-dependencies feature enabled and hiding most of it individually.

@nuttycom nuttycom requested review from arya2 and daira August 1, 2025 22:37
@nuttycom nuttycom force-pushed the nsm branch 2 times, most recently from 49724d5 to 2db87d3 Compare August 1, 2025 22:47
Copy link
Copy Markdown
Collaborator Author

@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 3c946e7

@zcash zcash deleted a comment from codecov Bot Aug 1, 2025
Comment thread zcash_primitives/src/transaction/mod.rs Outdated
Comment thread zcash_primitives/src/transaction/mod.rs Outdated
Comment thread zcash_primitives/src/transaction/mod.rs Outdated
daira
daira previously approved these changes Aug 2, 2025
Copy link
Copy Markdown
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.

One blocking comment, otherwise utACK.

Co-authored-by: Daira-Emma Hopwood <daira@jacaranda.org>
@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 2, 2025

Codecov Report

❌ Patch coverage is 20.18349% with 87 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.61%. Comparing base (9dcd4a9) to head (f3045af).
⚠️ Report is 23 commits behind head on main.

Files with missing lines Patch % Lines
zcash_primitives/src/transaction/mod.rs 11.11% 64 Missing ⚠️
zcash_primitives/src/transaction/builder.rs 11.11% 8 Missing ⚠️
pczt/src/roles/tx_extractor/mod.rs 0.00% 4 Missing ⚠️
zcash_client_sqlite/src/wallet.rs 0.00% 4 Missing ⚠️
components/zcash_protocol/src/value.rs 0.00% 2 Missing ⚠️
...h_primitives/src/transaction/components/orchard.rs 0.00% 2 Missing ⚠️
zcash_primitives/src/transaction/sighash.rs 0.00% 2 Missing ⚠️
zcash_primitives/src/transaction/sighash_v6.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1879      +/-   ##
==========================================
- Coverage   57.79%   57.61%   -0.18%     
==========================================
  Files         184      185       +1     
  Lines       22386    22478      +92     
==========================================
+ Hits        12938    12951      +13     
- Misses       9448     9527      +79     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

daira
daira previously approved these changes Aug 4, 2025
Copy link
Copy Markdown
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.

re-utACK

Copy link
Copy Markdown
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 nuttycom merged commit 3f03a4a into zcash:main Aug 6, 2025
41 of 43 checks passed
@aphelionz
Copy link
Copy Markdown
Member

Whooo!!! 🥳

@nuttycom nuttycom deleted the nsm branch August 6, 2025 16:26
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.

6 participants