Skip to content

pczt: Add pay_manual command.#136

Merged
nuttycom merged 3 commits into
zcash:mainfrom
nuttycom:feature/pczt-pay_manual
Dec 22, 2025
Merged

pczt: Add pay_manual command.#136
nuttycom merged 3 commits into
zcash:mainfrom
nuttycom:feature/pczt-pay_manual

Conversation

@nuttycom
Copy link
Copy Markdown
Collaborator

@nuttycom nuttycom commented Dec 19, 2025

This builds atop #134 and depends upon zcash/librustzcash#2094

Fixes #102

@nuttycom nuttycom force-pushed the feature/pczt-pay_manual branch 2 times, most recently from ccf6582 to d2df284 Compare December 19, 2025 22:21
Copy link
Copy Markdown
Collaborator

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

Reviewed d2df284.

Comment on lines +106 to +109
// TODO: we should return an error if any of the UTXOs being spent are outputs of a
// coinbase transaction and the change address is transparent-only; however, we do not have
// the information necessary to make such a determination about the inputs here; we don't
// get that information from the RawTransaction data returned from the light client server.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yes we do. Once we have raw_tx resolved from the coin's prevout, we can determine whether it is a coinbase transaction via bundle.is_coinbase().

What we should do is augment the Coin helper JSON so that if the caller provides a transparent change_address, we can require them to specify an is_coinbase: Option<bool> field if they provided value and script_pubkey (i.e. in the current situation where a remote lookup would not be performed).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I don't understand the second paragraph here. When spending a coinbase output, we still have to perform a lookup of that output in order to get the txout; what is the is_coinbase field you're suggesting supposed to be used for?

Copy link
Copy Markdown
Collaborator

@str4d str4d Dec 20, 2025

Choose a reason for hiding this comment

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

What I mean is, if the caller has provided the txout data in their JSON (i.e. the Coin.value and Coin.script_pubkey optional fields), they should also be required to say if the coin is coinbase or not if the change address is transparent such that it matters (as otherwise there was no point in providing those two fields, as it wouldn't avoid the lookup).

Comment thread src/commands/pczt/pay_manual.rs
Comment thread src/commands/pczt/pay_manual.rs Outdated
Comment thread src/commands/pczt/pay_manual.rs Outdated
Comment thread src/commands/pczt/pay_manual.rs Outdated
@nuttycom nuttycom force-pushed the feature/pczt-pay_manual branch from d2df284 to eb015d9 Compare December 20, 2025 00:15
str4d
str4d previously approved these changes Dec 20, 2025
Copy link
Copy Markdown
Collaborator

@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 eb015d9 (that commit specifically, as I authored the first commit in #134) modulo clippy failures.

@nuttycom nuttycom merged commit 593f822 into zcash:main Dec 22, 2025
8 checks passed
@nuttycom nuttycom deleted the feature/pczt-pay_manual branch December 22, 2025 17: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.

Add command to create a shielding PCZT from provided transparent inputs

2 participants