ZSA synced with the upstream v4.2.0#114
Conversation
…action/arbitrary.rs
… explicit imlps as const generic are stabilized
…ssuance module as they are not used (librutzcash serialization is used)
….toml, which add PartialEq and Eq derives to orchard flavor marker types
…ZSA marker types from orchard, rename OrchardFlavorExt to ShieldedDataFlavor and rest of the code according those changes
There was a problem hiding this comment.
Overall good, added comments.
In addition:
Issuance/burn validation lives entirely in zebra-state (validate_and_get_changes()), not in zebra-consensus.
All other consensus rules (proofs, signatures, scripts) are checked in zebra-consensus . verify_v6_transaction() is a passthrough to verify_v5_transaction (as commented). This means the consensus crate doesn't independently enforce any ZIP-226/227 rules. Need to discuss if this is the best design decision.
The best improvment I can see is to split check_issue_bundle_without_sighash() in Orchard into two functions: one that requires get_global_records and the other that does not and checks everything that does not require state. Move the second function into zebra-consensus. Let's keep this issue for last/later. It is a non-blocking issue for this PR.
Also,
For RPC, getrawtransaction (verbose) and getblock does not expose anything related to v6. The minimum is fee, zip233_amount, burn_exist:bool and issuance_exist:bool. Later we will add full details. Also, enableZSA should be exposed next to enableSpends and enableOutputs.
| run: | | ||
| df -h | ||
| free -h | ||
| lscpu | egrep 'Model name|Socket|Thread|Core|CPU\(s\)' |
There was a problem hiding this comment.
do we need to submit this file or is it just for us?
There was a problem hiding this comment.
Yes, this file is only for our internal use. I’ll remove it before preparing the upstream PR.
| .into_iter() | ||
| .flat_map(orchard::ShieldedData::nullifiers) | ||
| #[cfg(all(zcash_unstable = "nu7", feature = "tx_v6"))] | ||
| pub fn orchard_issue_data(&self) -> &Option<orchard_zsa::IssueData> { |
There was a problem hiding this comment.
Should not return &Option<_> but Option<&_> or Option<_>
There was a problem hiding this comment.
Fixed. Also renamed orchard_issue_data() method to orchard_zsa_issue_data() to match Transaction::V6 struct field name.
clippy::manual_option_zip lint in address UTXO reading code. Our fork is intentionally pinned to v4.2.0, so we need this small local patch instead of waiting for a later upstream merge.
This reverts commit 9fe1f13.
As discussed, we decided not to do that in this PR.
Done. I added:
|
…ds and add snapshots for new get_asset_state test cases
3c412b6 to
c2b3485
Compare
…ike test networks
The setup-rust-toolchain action exports RUSTFLAGS=-D warnings, which replaces (rather than merges with) .cargo/config.toml rustflags, so --cfg zcash_unstable="nu7" was silently dropped in CI. Pass it via the action's rustflags input instead. Also drop the cfg gate on the Nu7 consensus branch ID so chain history works in builds without zcash_unstable="nu7"; Nu7-specific transaction support remains cfg-gated elsewhere.
PaulLaux
left a comment
There was a problem hiding this comment.
I pushed some comment fixes / todos.
Let's merge it.
(After, please see the follow-up comment)
| OrchardWorkflowBlock { | ||
| height: 5, | ||
| bytes: ORCHARD_ZSA_WORKFLOW_BLOCK_5_BYTES.as_slice(), | ||
| is_valid: false |
There was a problem hiding this comment.
is_valid: bool is not good enough. Any malformed block will fail here, and the test will pass. Replace with the specific expected error/error text so we are sure the correct rejection flow was triggered (follow-up in the next PR).
ZSA synced with the upstream v4.2.0