DNM: WIP: Make remaining Orchard PCZT fields optional#2552
Closed
czarcas7ic wants to merge 1 commit into
Closed
Conversation
czarcas7ic
commented
Jul 7, 2026
|
|
||
| let bundle = orchard | ||
| .into_parsed_with_version( | ||
| global.tx_version, |
Collaborator
Author
There was a problem hiding this comment.
We pass the tx version since leaving out the anchor is only valid in v6 transactions.
czarcas7ic
commented
Jul 7, 2026
| Ok(Self { | ||
| /// omitting empty Transparent, Sapling, and Orchard bundles. Infallible: the v2 | ||
| /// encoding can represent every in-memory PCZT. | ||
| impl From<super::Pczt> for Pczt { |
Collaborator
Author
There was a problem hiding this comment.
This is From rather than TryFrom because v2 now represents missing Orchard derived fields as Optional, so encoding no longer needs to reject them.
czarcas7ic
commented
Jul 7, 2026
Comment on lines
+488
to
+500
| impl TryFrom<super::Action> for Action { | ||
| type Error = crate::EncodingError; | ||
|
|
||
| fn try_from(action: super::Action) -> Result<Self, Self::Error> { | ||
| Ok(Self { | ||
| // This encoding has no representation for an elided derived field; | ||
| // reject any action that has redacted one (see | ||
| // [`crate::EncodingError::RequiresV2`]). | ||
| cv_net: action.cv_net.ok_or(crate::EncodingError::RequiresV2)?, | ||
| spend: Spend::try_from(action.spend)?, | ||
| output: Output::try_from(action.output)?, | ||
| rcv: action.rcv, | ||
| } | ||
| }) |
Collaborator
Author
There was a problem hiding this comment.
This is TryFrom rather than From because v1 cannot represent missing Orchard derived fields (not Optional), so encoding must reject redacted actions.
czarcas7ic
commented
Jul 7, 2026
| /// the real anchor. | ||
| /// | ||
| /// On error the bundle may be left partially filled. | ||
| pub fn fill_derived_fields(&mut self, tx_version: u32) -> Result<(), FillError> { |
Collaborator
Author
There was a problem hiding this comment.
One of the primary functions to review here, which keystone call to fill in the respective elided fields.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
DNM/WIP. This branch isolates the remaining PCZT Orchard optional-field work on a review base that matches the local stack.
It makes the remaining recomputable Orchard-shaped fields optional in the v2 wire encoding, adds
MemoKindfor elidedenc_ciphertextreconstruction, fills derived fields before parsing/signing, and rejects anchor elision before v6.Validation:
cargo fmt --allcargo check -p pczt --all-featurescargo test -p pczt --test end_to_end --all-featuresgit diff --check