Skip to content

pczt: Support appending external signatures to inputs#2047

Merged
str4d merged 2 commits into
mainfrom
pczt-append-transparent-sigs
Dec 5, 2025
Merged

pczt: Support appending external signatures to inputs#2047
str4d merged 2 commits into
mainfrom
pczt-append-transparent-sigs

Conversation

@str4d
Copy link
Copy Markdown
Contributor

@str4d str4d commented Nov 20, 2025

No description provided.

@str4d str4d force-pushed the pczt-append-transparent-sigs branch from d4395c1 to a87b7d3 Compare November 20, 2025 17:25
nuttycom
nuttycom previously approved these changes Nov 20, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@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 a87b7d3 with nits

Comment thread zcash_transparent/src/pczt/signer.rs Outdated
Comment thread zcash_transparent/src/pczt/signer.rs Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 20, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 0% with 44 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.67%. Comparing base (31f37ed) to head (feefa60).
⚠️ Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
zcash_transparent/src/pczt/signer.rs 0.00% 41 Missing ⚠️
pczt/src/roles/signer/mod.rs 0.00% 3 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2047      +/-   ##
==========================================
- Coverage   67.88%   67.67%   -0.21%     
==========================================
  Files         214      215       +1     
  Lines       28780    28869      +89     
==========================================
  Hits        19536    19536              
- Misses       9244     9333      +89

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

@str4d str4d force-pushed the pczt-append-transparent-sigs branch from a87b7d3 to 5798789 Compare November 21, 2025 12:32
@str4d str4d changed the title pczt: Support appending external sigs to transparent inputs pczt: Support appending external signatures to inputs Nov 21, 2025
@str4d
Copy link
Copy Markdown
Contributor Author

str4d commented Nov 21, 2025

Force-pushed to address review comments, and add support for Sapling and Orchard external signatures.

@str4d str4d force-pushed the pczt-append-transparent-sigs branch from 5798789 to 370bf25 Compare November 21, 2025 12:35
@str4d
Copy link
Copy Markdown
Contributor Author

str4d commented Nov 21, 2025

Force-pushed to fix dependency patch error.

nuttycom
nuttycom reviewed Nov 21, 2025
View reviewed changes
Comment thread pczt/src/roles/signer/mod.rs Outdated
nuttycom
nuttycom requested changes Nov 21, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

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

It looks like there's some patch dependency confusion happening with this PR. Were there some changes that weren't pushed to sapling-crypto, or was this PR not updated after the dependency revision changed?

@str4d str4d force-pushed the pczt-append-transparent-sigs branch from 370bf25 to 038a681 Compare November 25, 2025 11:26
@str4d
Copy link
Copy Markdown
Contributor Author

str4d commented Nov 25, 2025

Force-pushed to fix typos and simplify the sighash APIs.

@gstohl
Copy link
Copy Markdown

gstohl commented Nov 25, 2025

@str4d was my naming in my pull request that bad, also concise > verbose ? I will close it and remember for next time.

@str4d
Copy link
Copy Markdown
Contributor Author

str4d commented Nov 25, 2025

@gstohl I generally prefer to fix my own PRs in-situ; in this case a dependency bug hid the actual compilation failure prior to me heading off for the weekend, so I didn't get back to it until today. As for the sighash refactor, the approach I've taken encapsulates the SignableInput logic into a single place, rather than spreading it across the pczt and zcash_transparent crates.

@gstohl
Copy link
Copy Markdown

gstohl commented Nov 25, 2025

@str4d can totally understand that, I needed this to be fixed and the helper function exposed for the zypherpunk hackathon track, thus I quickly applied the fixes on my fork made the PR and went on building the FFI bridges. I'm going to replace my fork with this branch and test it in my bridges tonight. Thank you.

@str4d str4d force-pushed the pczt-append-transparent-sigs branch from 2d18d03 to feefa60 Compare December 5, 2025 22:32
@str4d
Copy link
Copy Markdown
Contributor Author

str4d commented Dec 5, 2025

Rebased on main to fix changelog merge conflicts.

nuttycom
nuttycom approved these changes Dec 5, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

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

@str4d str4d merged commit fe3a0fc into main Dec 5, 2025
46 checks passed
@str4d str4d deleted the pczt-append-transparent-sigs branch December 5, 2025 23:04
@gstohl gstohl mentioned this pull request Dec 7, 2025
Merged
daira
daira reviewed Dec 8, 2025
View reviewed changes
Comment thread pczt/src/roles/signer/mod.rs
daira
daira reviewed Dec 8, 2025
View reviewed changes
Comment thread zcash_transparent/src/pczt/signer.rs
daira
daira reviewed Dec 8, 2025
View reviewed changes
Comment thread zcash_transparent/src/pczt/signer.rs
daira
daira reviewed Dec 8, 2025
View reviewed changes
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.

Post-hoc reviewed with comments.

p0mvn added a commit to valargroup/librustzcash that referenced this pull request Apr 10, 2026
@p0mvn
pczt: Add public getter for shielded sighash
8bc4f72 
Backports the `Signer::shielded_sighash` accessor from main (added in
zcash#2047) to the 0.5.x series, enabling downstream
consumers to obtain the sighash for producing signatures externally
(e.g. via a hardware wallet).

Made-with: Cursor
@p0mvn p0mvn mentioned this pull request Apr 10, 2026
Closed
This was referenced Apr 10, 2026
Closed
Closed
nuttycom pushed a commit to valargroup/librustzcash that referenced this pull request Apr 23, 2026
@p0mvn @nuttycom
pczt: Add public getter for shielded sighash
16cac0a 
Backports the `Signer::shielded_sighash` accessor from main (added in
zcash#2047) to the 0.5.x series, enabling downstream
consumers to obtain the sighash for producing signatures externally
(e.g. via a hardware wallet).

Made-with: Cursor
nuttycom pushed a commit to valargroup/librustzcash that referenced this pull request Apr 23, 2026
@p0mvn @nuttycom
pczt: Add public getter for shielded sighash
a65f752 
Backports the `Signer::shielded_sighash` accessor from main (added in
zcash#2047) to the 0.5.x series, enabling downstream
consumers to obtain the sighash for producing signatures externally
(e.g. via a hardware wallet).

Made-with: Cursor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@daira daira daira left review comments

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

4 participants

@str4d @gstohl @nuttycom @daira