Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OrchardZSA backward compatability #100

Closed
wants to merge 45 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
8bee528
draft2
PaulLaux Jan 4, 2024
5f78e72
draft3
PaulLaux Jan 4, 2024
5221c29
draft4
PaulLaux Jan 4, 2024
e07fab9
draft5
PaulLaux Jan 4, 2024
ee0a07d
draft6
PaulLaux Jan 4, 2024
8d2d130
Generalize orchard_zsa for backward bompatibility with non-ZSA functi…
dmidem Jan 17, 2024
b177325
Minor changes in the doc comments
dmidem Jan 17, 2024
4900519
Add tests for note_encryption_v2
dmidem Jan 22, 2024
0a3d231
Rename V2 name suffixes to Vanilla, and V3 - to ZSA
dmidem Jan 24, 2024
f3ae948
Make the Circuit struct generic to support different Orchard variants…
dmidem Jan 29, 2024
fc64461
Continue Circuit generalization
dmidem Feb 6, 2024
c6f8936
Split circuit implementation into circuit_vanilla and circuit_zsa
dmidem Feb 12, 2024
ce46de0
Fix to support modified halo2
dmidem Feb 27, 2024
e5ab5d0
Add missed fields to fn print_action_circuit
dmidem Feb 27, 2024
a8dadc9
Pin half crate to 1.8.2 (to resolve MSRV conflict)
dmidem Feb 27, 2024
a8a72c2
Pin half crate to 1.8.2 for dev deps too (to resolve MSRV conflict)
dmidem Feb 27, 2024
120b090
Pin half crate to 2.2.1
dmidem Feb 27, 2024
5d5bbf1
Fix cargo clippy errors
dmidem Feb 27, 2024
97f55f9
Convert arb_... testing functions to methods of dummy generict struct…
dmidem Mar 4, 2024
90512c8
Make hash_bundle_txid_data function backwards compatible
dmidem Mar 4, 2024
416b496
Refactor note_encryption.rs:
dmidem Mar 12, 2024
a1e6cd6
Fix cargo clippy errors
dmidem Mar 12, 2024
8b28a0e
Minor fix (make action module pub)
dmidem Mar 12, 2024
fc20a5a
Make add_chip.rs a shared module between circuit_vanilla and circuit_…
dmidem Mar 15, 2024
1d68e0b
Introduce OrcharcCircuit trait to eliminate repetitive 'where crate::…
dmidem Mar 18, 2024
c820dae
Introdice NoteByteReader and NoteByteWriter traiots to use with NoteB…
dmidem Mar 19, 2024
088494a
note_encryption: extract Domain impl, OrchardDoimain and NoteBytes de…
dmidem Mar 19, 2024
46a222e
Use vec with a proper length for the concrete OrchardDomain to genera…
dmidem Mar 25, 2024
367c701
Use try_fold instead of fold when cargo clippy suggests it
dmidem Mar 25, 2024
dc97b7a
Rename domain_impl.rs in note_encryption to domain.rs
dmidem Mar 25, 2024
cc99400
Intriduce orchard_flavor module with OrchardVanilla and OrchardZSA st…
dmidem Mar 25, 2024
e2306e3
Fix naming (OrchardDomainContex to OrchardDomainBase, Curcuit to Orch…
dmidem Mar 28, 2024
d721463
Fix 'half' dep duplication in Cargo.toml
dmidem Apr 9, 2024
6ba0a38
Remove rng from AssetBase::random call
dmidem Apr 9, 2024
19dbd0a
Minor change (improive use::crate::...)
dmidem Apr 15, 2024
b6d577e
Add hashing of burn field to hash_bundle_txid_data
dmidem Apr 15, 2024
8795ca4
Revert "Add hashing of burn field to hash_bundle_txid_data"
dmidem Apr 15, 2024
58e05b1
Adapt Orchard to changes in zcash_note_encryption dependency, which n…
dmidem Apr 17, 2024
e67c792
Revert "Revert "Add hashing of burn field to hash_bundle_txid_data""
dmidem Apr 17, 2024
a9af64c
Updating test vectors to correspond to those generated by the zcash-t…
vivek-arte Apr 30, 2024
69dfd4a
Address PR #100 review notes (some issues remain unresolved)
dmidem May 20, 2024
c374534
Fix files missed in the previous commit
dmidem May 20, 2024
95d53e3
Minoir fix after cargo fmt
dmidem May 20, 2024
5ca986f
Remove unnecessary comment
dmidem May 20, 2024
465a6aa
Move BuilderArb struct declaration down to try to reduce git diff
dmidem Jun 3, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,5 @@ Cargo.lock
.vscode
.idea
action-circuit-layout.png
*.[0-9]
*.[0-9][0-9]
11 changes: 5 additions & 6 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,12 @@ rustdoc-args = ["--cfg", "docsrs", "--html-in-header", "katex-header.html"]
aes = "0.8"
bitvec = "1"
blake2b_simd = "=1.0.1" # Last version required rust 1.66
half = "=2.2.1" # Last version requires Rust 1.70
ff = "0.13"
fpe = "0.6"
group = { version = "0.13", features = ["wnaf-memuse"] }
halo2_gadgets = { git = "https://github.com/QED-it/halo2", branch = "zsa1" }
halo2_proofs = { git = "https://github.com/QED-it/halo2", branch = "zsa1", default-features = false, features = ["batch", "floor-planner-v1-legacy-pdqsort"] }
halo2_gadgets = { git = "https://github.com/QED-it/halo2", branch = "orchardzsa-backward-compatability" }
halo2_proofs = { git = "https://github.com/QED-it/halo2", branch = "orchardzsa-backward-compatability", default-features = false, features = ["batch", "floor-planner-v1-legacy-pdqsort"] }
hex = "0.4"
k256 = { version = "0.13.0", features = ["arithmetic", "schnorr"] }
lazy_static = "1"
Expand All @@ -54,14 +55,12 @@ plotters = { version = "0.3.0", optional = true }

[dev-dependencies]
bridgetree = "0.4"
criterion = "0.4" #Pinned: 0.5 depends on clap 4 which has MSRV 1.70
halo2_gadgets = { git = "https://github.com/QED-it/halo2", branch = "zsa1", features = ["test-dependencies"] }
hex = "0.4"
criterion = "0.4" # 0.5 depends on clap 4 which has MSRV 1.70
halo2_gadgets = { git = "https://github.com/QED-it/halo2", branch = "orchardzsa-backward-compatability", features = ["test-dependencies"] }
Copy link
Author

Choose a reason for hiding this comment

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

From @PaulLaux (possibly outdated as the file was changed):
are you sure we need to override it twice?
here and lines 29,33

Copy link
Author

Choose a reason for hiding this comment

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

Done. Fixed the hex and half dependencies by removing them from "dev-dependencies". Regarding hash - in the upstream (zcash/orchard 0.8.0), they replaced hash with ahash, which is only present in "dev-dependencies". As for other duplications (including halo2_gadgets), they cannot be removed from "dev-dependencies" because they have different features enabled.

proptest = "1.0.0"
zcash_note_encryption_zsa = { package = "zcash_note_encryption", version = "0.4", git = "https://github.com/QED-it/zcash_note_encryption", branch = "zsa1", features = ["pre-zip-212"] }
incrementalmerkletree = { version = "0.5", features = ["test-dependencies"] }
ahash = "=0.8.6" #Pinned: 0.8.7 depends on Rust 1.72
half = "=2.2.1" #Pinned: 2.3.1 requires Rust 1.70

[target.'cfg(unix)'.dev-dependencies]
inferno = "0.11" #Pinned
Expand Down
8 changes: 5 additions & 3 deletions benches/circuit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use orchard::{
bundle::Flags,
circuit::{ProvingKey, VerifyingKey},
keys::{FullViewingKey, Scope, SpendingKey},
orchard_flavors::OrchardZSA,
value::NoteValue,
Anchor, Bundle,
};
Expand All @@ -23,8 +24,9 @@ fn criterion_benchmark(c: &mut Criterion) {
let sk = SpendingKey::from_bytes([7; 32]).unwrap();
let recipient = FullViewingKey::from(&sk).address_at(0u32, Scope::External);

let vk = VerifyingKey::build();
let pk = ProvingKey::build();
// FIXME: consider adding test for OrchardVanilla as well
Copy link
Author

Choose a reason for hiding this comment

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

From @PaulLaux:
yes, we need both
lets have this as a separate task

Copy link
Author

Choose a reason for hiding this comment

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

Done (we have created a new task).

let vk = VerifyingKey::build::<OrchardZSA>();
let pk = ProvingKey::build::<OrchardZSA>();

let create_bundle = |num_recipients| {
let mut builder = Builder::new(
Expand All @@ -42,7 +44,7 @@ fn criterion_benchmark(c: &mut Criterion) {
)
.unwrap();
}
let bundle: Bundle<_, i64> = builder.build(rng).unwrap();
let bundle: Bundle<_, i64, OrchardZSA> = builder.build(rng).unwrap();

let instances: Vec<_> = bundle
.actions()
Expand Down
16 changes: 10 additions & 6 deletions benches/note_decryption.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ use orchard::{
circuit::ProvingKey,
keys::{FullViewingKey, PreparedIncomingViewingKey, Scope, SpendingKey},
note::AssetBase,
note_encryption_v3::{CompactAction, OrchardDomainV3},
note_encryption::{action::CompactAction, OrchardDomainBase},
orchard_flavors::OrchardZSA,
value::NoteValue,
Anchor, Bundle,
};
Expand All @@ -15,9 +16,12 @@ use zcash_note_encryption_zsa::{batch, try_compact_note_decryption, try_note_dec
#[cfg(unix)]
use pprof::criterion::{Output, PProfProfiler};

type OrchardDomainZSA = OrchardDomainBase<OrchardZSA>;

fn bench_note_decryption(c: &mut Criterion) {
let rng = OsRng;
let pk = ProvingKey::build();
// FIXME: consider adding test for OrchardVanilla as well
let pk = ProvingKey::build::<OrchardZSA>();

let fvk = FullViewingKey::from(&SpendingKey::from_bytes([7; 32]).unwrap());
let valid_ivk = fvk.to_ivk(Scope::External);
Expand Down Expand Up @@ -70,7 +74,7 @@ fn bench_note_decryption(c: &mut Criterion) {
None,
)
.unwrap();
let bundle: Bundle<_, i64> = builder.build(rng).unwrap();
let bundle: Bundle<_, i64, OrchardZSA> = builder.build(rng).unwrap();
bundle
.create_proof(&pk, rng)
.unwrap()
Expand All @@ -79,7 +83,7 @@ fn bench_note_decryption(c: &mut Criterion) {
};
let action = bundle.actions().first();

let domain = OrchardDomainV3::for_action(action);
let domain = OrchardDomainZSA::for_action(action);

let compact = {
let mut group = c.benchmark_group("note-decryption");
Expand Down Expand Up @@ -120,12 +124,12 @@ fn bench_note_decryption(c: &mut Criterion) {
let ivks = 2;
let valid_ivks = vec![valid_ivk; ivks];
let actions: Vec<_> = (0..100)
.map(|_| (OrchardDomainV3::for_action(action), action.clone()))
.map(|_| (OrchardDomainZSA::for_action(action), action.clone()))
.collect();
let compact: Vec<_> = (0..100)
.map(|_| {
(
OrchardDomainV3::for_action(action),
OrchardDomainZSA::for_action(action),
CompactAction::from(action),
)
})
Expand Down
156 changes: 85 additions & 71 deletions src/action.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use memuse::DynamicUsage;

use crate::{
note::{ExtractedNoteCommitment, Nullifier, TransmittedNoteCiphertext},
note_encryption::OrchardDomain,
primitives::redpallas::{self, SpendAuth},
value::ValueCommitment,
};
Expand All @@ -15,30 +16,30 @@ use crate::{
/// Internally, this may both consume a note and create a note, or it may do only one of
/// the two. TODO: Determine which is more efficient (circuit size vs bundle size).
#[derive(Debug, Clone)]
pub struct Action<A> {
pub struct Action<A, D: OrchardDomain> {
/// The nullifier of the note being spent.
nf: Nullifier,
/// The randomized verification key for the note being spent.
rk: redpallas::VerificationKey<SpendAuth>,
/// A commitment to the new note being created.
cmx: ExtractedNoteCommitment,
/// The transmitted note ciphertext.
encrypted_note: TransmittedNoteCiphertext,
encrypted_note: TransmittedNoteCiphertext<D>,
/// A commitment to the net value created or consumed by this action.
cv_net: ValueCommitment,
/// The authorization for this action.
authorization: A,
}

impl<T> Action<T> {
impl<A, D: OrchardDomain> Action<A, D> {
/// Constructs an `Action` from its constituent parts.
pub fn from_parts(
nf: Nullifier,
rk: redpallas::VerificationKey<SpendAuth>,
cmx: ExtractedNoteCommitment,
encrypted_note: TransmittedNoteCiphertext,
encrypted_note: TransmittedNoteCiphertext<D>,
cv_net: ValueCommitment,
authorization: T,
authorization: A,
) -> Self {
Action {
nf,
Expand Down Expand Up @@ -66,7 +67,7 @@ impl<T> Action<T> {
}

/// Returns the encrypted note ciphertext.
pub fn encrypted_note(&self) -> &TransmittedNoteCiphertext {
pub fn encrypted_note(&self) -> &TransmittedNoteCiphertext<D> {
&self.encrypted_note
}

Expand All @@ -76,12 +77,12 @@ impl<T> Action<T> {
}

/// Returns the authorization for this action.
pub fn authorization(&self) -> &T {
pub fn authorization(&self) -> &A {
&self.authorization
}

/// Transitions this action from one authorization state to another.
pub fn map<U>(self, step: impl FnOnce(T) -> U) -> Action<U> {
pub fn map<U>(self, step: impl FnOnce(A) -> U) -> Action<U, D> {
Action {
nf: self.nf,
rk: self.rk,
Expand All @@ -93,7 +94,7 @@ impl<T> Action<T> {
}

/// Transitions this action from one authorization state to another.
pub fn try_map<U, E>(self, step: impl FnOnce(T) -> Result<U, E>) -> Result<Action<U>, E> {
pub fn try_map<U, E>(self, step: impl FnOnce(A) -> Result<U, E>) -> Result<Action<U, D>, E> {
Ok(Action {
nf: self.nf,
rk: self.rk,
Expand All @@ -105,7 +106,7 @@ impl<T> Action<T> {
}
}

impl DynamicUsage for Action<redpallas::Signature<SpendAuth>> {
impl<D: OrchardDomain> DynamicUsage for Action<redpallas::Signature<SpendAuth>, D> {
#[inline(always)]
fn dynamic_usage(&self) -> usize {
0
Expand All @@ -132,6 +133,7 @@ pub(crate) mod testing {
commitment::ExtractedNoteCommitment, nullifier::testing::arb_nullifier,
testing::arb_note, TransmittedNoteCiphertext,
},
note_encryption::OrchardDomain,
primitives::redpallas::{
self,
testing::{arb_spendauth_signing_key, arb_spendauth_verification_key},
Expand All @@ -141,70 +143,82 @@ pub(crate) mod testing {

use super::Action;

prop_compose! {
/// Generate an action without authorization data.
pub fn arb_unauthorized_action(spend_value: NoteValue, output_value: NoteValue)(
nf in arb_nullifier(),
rk in arb_spendauth_verification_key(),
note in arb_note(output_value),
asset in arb_asset_base()
) -> Action<()> {
let cmx = ExtractedNoteCommitment::from(note.commitment());
let cv_net = ValueCommitment::derive(
spend_value - output_value,
ValueCommitTrapdoor::zero(),
asset
);
// FIXME: make a real one from the note.
let encrypted_note = TransmittedNoteCiphertext {
epk_bytes: [0u8; 32],
enc_ciphertext: [0u8; 612],
out_ciphertext: [0u8; 80]
};
Action {
nf,
rk,
cmx,
encrypted_note,
cv_net,
authorization: ()
/// `ActionArb` serves as a utility structure in property-based testing, designed specifically to adapt
/// `arb_...` functions for compatibility with both variations of the Orchard protocol: Vanilla and ZSA.
/// This adaptation is necessary due to the proptest crate's limitation, which prevents the direct
/// transformation of `arb_...` functions into generic forms suitable for testing different protocol
/// flavors.
#[derive(Debug)]
pub struct ActionArb<D: OrchardDomain> {
phantom: std::marker::PhantomData<D>,
}

impl<D: OrchardDomain> ActionArb<D> {
prop_compose! {
/// Generate an action without authorization data.
pub fn arb_unauthorized_action(spend_value: NoteValue, output_value: NoteValue)(
nf in arb_nullifier(),
rk in arb_spendauth_verification_key(),
note in arb_note(output_value),
asset in arb_asset_base()
) -> Action<(), D> {
let cmx = ExtractedNoteCommitment::from(note.commitment());
let cv_net = ValueCommitment::derive(
spend_value - output_value,
ValueCommitTrapdoor::zero(),
asset
);
// FIXME: make a real one from the note.
Copy link
Author

Choose a reason for hiding this comment

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

From @PaulLaux:
Let's do this
call arb_native_note() to create a note

Copy link
Author

Choose a reason for hiding this comment

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

Done.

let encrypted_note = TransmittedNoteCiphertext::<D> {
epk_bytes: [0u8; 32],
enc_ciphertext: D::NoteCiphertextBytes::from(vec![0u8; D::ENC_CIPHERTEXT_SIZE].as_ref()),
out_ciphertext: [0u8; 80]
};
Action {
nf,
rk,
cmx,
encrypted_note,
cv_net,
authorization: ()
}
}
}
}

prop_compose! {
/// Generate an action with invalid (random) authorization data.
pub fn arb_action(spend_value: NoteValue, output_value: NoteValue)(
nf in arb_nullifier(),
sk in arb_spendauth_signing_key(),
note in arb_note(output_value),
rng_seed in prop::array::uniform32(prop::num::u8::ANY),
fake_sighash in prop::array::uniform32(prop::num::u8::ANY),
asset in arb_asset_base()
) -> Action<redpallas::Signature<SpendAuth>> {
let cmx = ExtractedNoteCommitment::from(note.commitment());
let cv_net = ValueCommitment::derive(
spend_value - output_value,
ValueCommitTrapdoor::zero(),
asset
);

// FIXME: make a real one from the note.
let encrypted_note = TransmittedNoteCiphertext {
epk_bytes: [0u8; 32],
enc_ciphertext: [0u8; 612],
out_ciphertext: [0u8; 80]
};

let rng = StdRng::from_seed(rng_seed);

Action {
nf,
rk: redpallas::VerificationKey::from(&sk),
cmx,
encrypted_note,
cv_net,
authorization: sk.sign(rng, &fake_sighash),
prop_compose! {
/// Generate an action with invalid (random) authorization data.
pub fn arb_action(spend_value: NoteValue, output_value: NoteValue)(
nf in arb_nullifier(),
sk in arb_spendauth_signing_key(),
note in arb_note(output_value),
rng_seed in prop::array::uniform32(prop::num::u8::ANY),
fake_sighash in prop::array::uniform32(prop::num::u8::ANY),
asset in arb_asset_base()
) -> Action<redpallas::Signature<SpendAuth>, D> {
let cmx = ExtractedNoteCommitment::from(note.commitment());
let cv_net = ValueCommitment::derive(
spend_value - output_value,
ValueCommitTrapdoor::zero(),
asset
);

// FIXME: make a real one from the note.
let encrypted_note = TransmittedNoteCiphertext::<D> {
epk_bytes: [0u8; 32],
enc_ciphertext: D::NoteCiphertextBytes::from(vec![0u8; D::ENC_CIPHERTEXT_SIZE].as_ref()),
out_ciphertext: [0u8; 80]
};

let rng = StdRng::from_seed(rng_seed);

Action {
nf,
rk: redpallas::VerificationKey::from(&sk),
cmx,
encrypted_note,
cv_net,
authorization: sk.sign(rng, &fake_sighash),
}
}
}
}
Expand Down
Loading
Loading