Enhance and cleanup ivk-to-bytes-visibility-downgrade branch#81
Enhance and cleanup ivk-to-bytes-visibility-downgrade branch#81
Conversation
…t_partial_ord_impl_on_ord_type clippy error
…undle-in-v5-vector-serialization-with-burn-valid branch
PaulLaux
left a comment
PaulLaux
left a comment
There was a problem hiding this comment.
Good overall, added comments and questions.
Cargo.toml
Outdated
| dashmap = ">= 5.4, <5.5" | ||
| inferno = ">= 0.11, < 0.11.15" | ||
| pprof = { version = "0.9", features = ["criterion", "flamegraph"] } # MSRV 1.56 | ||
| backtrace = "=0.3.67" # Last version required rust 1.65 |
There was a problem hiding this comment.
Let's update to rust 1.65 as discussed with the zcash team. Remove this after update.
There was a problem hiding this comment.
Done, but I had to add two new pins because even Rust 1.65 can't compile the latest version:
1. Pinned blake2b_simd to "1.0.1" as its latest version requires Rust 1.66.
2. Pinned clap to "4.2.0" as its latest version requires Rust 1.70.
Additionally, I've added a couple of comments with "FIXME" to draw your attention to the lines that were updated in Cargo.toml in the latest version of Orchard from the Zcash team.
There was a problem hiding this comment.
Also, I've updated the dependencies in Cargo.toml to align with the latest changes from the Zcash team's upstream Cargo.toml.
src/bundle/burn_validation.rs
Outdated
| @@ -0,0 +1,146 @@ | |||
| //! Validating burn operations on asset bundles. | |||
| //! | |||
| //! The module provides a function `validate_bundle_burn` that can be used to validate a burn for a bundle. | |||
There was a problem hiding this comment.
"...to validate the burn values for the bundle."
src/issuance.rs
Outdated
| let bounds = ( | ||
| self.actions.dynamic_usage_bounds(), | ||
| self.ik.dynamic_usage_bounds(), | ||
| self.authorization.dynamic_usage_bounds(), |
There was a problem hiding this comment.
Same, replace tuple with short names for the variables.
src/issuance/batch.rs
Outdated
| // FIXME: take/save finalization set from/to the global state | ||
| let finalized = HashSet::<AssetBase>::new(); | ||
|
|
||
| // FIXME: process resulting supply_info |
There was a problem hiding this comment.
Same, Add a way to extract the resulting supply_info param.
src/issuance/batch.rs
Outdated
| // FIXME: take/save finalization set from/to the global state | ||
| let finalized = HashSet::<AssetBase>::new(); |
There was a problem hiding this comment.
We need a way to pass the finilized HashSet by the caller. Please add a way to transfer the parameter.
We can call it with an empty Set while global state is not implemented.
There was a problem hiding this comment.
Based on our latest discussion, I've reverted the changes related to BatchValidators as they are not used in Zebra: I have removed the modifications I made to the Orchard batch validator and completely removed Issuance batch validator I created before.
This reverts commit 9b0839b.
…e) as BatchValidators are not used in Zebra
… BatchValidator but also in Zebra too
PaulLaux
left a comment
PaulLaux
left a comment
There was a problem hiding this comment.
Very nice, one suggestion about number of lines. If this is not possible, let's merge the way it is.
dmidem
changed the title
src/note.rs
Outdated
| impl DynamicUsage for Note { | ||
| #[inline(always)] | ||
| fn dynamic_usage(&self) -> usize { | ||
| 0 | ||
| } | ||
|
|
||
| #[inline(always)] | ||
| fn dynamic_usage_bounds(&self) -> (usize, Option<usize>) { | ||
| (0, Some(0)) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
remove DynamicUsage across the entire PR if not needed in zebra
…micUsage is not used in Zebra)
… this impl was implemented not in this branch
…branch (after PR #81 was merged)
3 participants
This pull request includes the changes made in the
ivk-to-bytes-visibility-downgradebranch, which serves as the parent branch for thisivk-to-bytes-visibility-downgrade-with-burn-validbranch. Additionally, it incorporates some extra enhancements and cleanups. Also, it relocates theburn_validation.rsmodule from thelibrustzcashrepository to thisorchardrepository. While theburn_validation.rsmodule is not currently in use, it may potentially be valuable for invocation fromZebraproject code.