Rename shielded_data to sapling_shielded_data as needed (#2072)

This change help avoid bugs that confuse sapling with orchard (or sprout).

```sh
fastmod shielded_data sapling_shielded_data
```
This commit is contained in:
teor 2021-04-27 23:37:53 +10:00 committed by GitHub
parent 1f40498fcf
commit 247620320e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 48 additions and 40 deletions

View File

@ -108,7 +108,7 @@ proptest! {
/// Serialize and deserialize `PerSpendAnchor` shielded data by including it /// Serialize and deserialize `PerSpendAnchor` shielded data by including it
/// in a V4 transaction /// in a V4 transaction
#[test] #[test]
fn shielded_data_v4_roundtrip( fn sapling_shielded_data_v4_roundtrip(
shielded_v4 in any::<sapling::ShieldedData<PerSpendAnchor>>(), shielded_v4 in any::<sapling::ShieldedData<PerSpendAnchor>>(),
) { ) {
zebra_test::init(); zebra_test::init();
@ -137,7 +137,7 @@ proptest! {
/// Serialize and deserialize `SharedAnchor` shielded data /// Serialize and deserialize `SharedAnchor` shielded data
#[test] #[test]
fn shielded_data_v5_roundtrip( fn sapling_shielded_data_v5_roundtrip(
shielded_v5 in any::<sapling::ShieldedData<SharedAnchor>>(), shielded_v5 in any::<sapling::ShieldedData<SharedAnchor>>(),
) { ) {
zebra_test::init(); zebra_test::init();
@ -160,7 +160,7 @@ proptest! {
/// Test v4 with empty spends, but some outputs /// Test v4 with empty spends, but some outputs
#[test] #[test]
fn shielded_data_v4_outputs_only( fn sapling_shielded_data_v4_outputs_only(
shielded_v4 in any::<sapling::ShieldedData<PerSpendAnchor>>(), shielded_v4 in any::<sapling::ShieldedData<PerSpendAnchor>>(),
) { ) {
zebra_test::init(); zebra_test::init();
@ -199,7 +199,7 @@ proptest! {
/// Test the v5 shared anchor serialization condition: empty spends, but some outputs /// Test the v5 shared anchor serialization condition: empty spends, but some outputs
#[test] #[test]
fn shielded_data_v5_outputs_only( fn sapling_shielded_data_v5_outputs_only(
shielded_v5 in any::<sapling::ShieldedData<SharedAnchor>>(), shielded_v5 in any::<sapling::ShieldedData<SharedAnchor>>(),
) { ) {
zebra_test::init(); zebra_test::init();

View File

@ -87,8 +87,8 @@ impl ZcashSerialize for Option<sapling::ShieldedData<SharedAnchor>> {
// nOutputsSapling // nOutputsSapling
writer.write_compactsize(0)?; writer.write_compactsize(0)?;
} }
Some(shielded_data) => { Some(sapling_shielded_data) => {
shielded_data.zcash_serialize(&mut writer)?; sapling_shielded_data.zcash_serialize(&mut writer)?;
} }
} }
Ok(()) Ok(())
@ -312,14 +312,16 @@ impl ZcashSerialize for Transaction {
writer.write_compactsize(0)?; writer.write_compactsize(0)?;
writer.write_compactsize(0)?; writer.write_compactsize(0)?;
} }
Some(shielded_data) => { Some(sapling_shielded_data) => {
shielded_data.value_balance.zcash_serialize(&mut writer)?; sapling_shielded_data
writer.write_compactsize(shielded_data.spends().count() as u64)?; .value_balance
for spend in shielded_data.spends() { .zcash_serialize(&mut writer)?;
writer.write_compactsize(sapling_shielded_data.spends().count() as u64)?;
for spend in sapling_shielded_data.spends() {
spend.zcash_serialize(&mut writer)?; spend.zcash_serialize(&mut writer)?;
} }
writer.write_compactsize(shielded_data.outputs().count() as u64)?; writer.write_compactsize(sapling_shielded_data.outputs().count() as u64)?;
for output in shielded_data for output in sapling_shielded_data
.outputs() .outputs()
.cloned() .cloned()
.map(sapling::OutputInTransactionV4) .map(sapling::OutputInTransactionV4)

View File

@ -420,11 +420,11 @@ impl<'a> SigHasher<'a> {
fn hash_shielded_spends<W: io::Write>(&self, mut writer: W) -> Result<(), io::Error> { fn hash_shielded_spends<W: io::Write>(&self, mut writer: W) -> Result<(), io::Error> {
use Transaction::*; use Transaction::*;
let shielded_data = match self.trans { let sapling_shielded_data = match self.trans {
V4 { V4 {
sapling_shielded_data: Some(shielded_data), sapling_shielded_data: Some(sapling_shielded_data),
.. ..
} => shielded_data, } => sapling_shielded_data,
V4 { V4 {
sapling_shielded_data: None, sapling_shielded_data: None,
.. ..
@ -433,7 +433,7 @@ impl<'a> SigHasher<'a> {
V1 { .. } | V2 { .. } | V3 { .. } => unreachable!(ZIP243_EXPLANATION), V1 { .. } | V2 { .. } | V3 { .. } => unreachable!(ZIP243_EXPLANATION),
}; };
if shielded_data.spends().next().is_none() { if sapling_shielded_data.spends().next().is_none() {
return writer.write_all(&[0; 32]); return writer.write_all(&[0; 32]);
} }
@ -443,7 +443,7 @@ impl<'a> SigHasher<'a> {
.to_state(); .to_state();
// TODO: make a generic wrapper in `spends.rs` that does this serialization // TODO: make a generic wrapper in `spends.rs` that does this serialization
for spend in shielded_data.spends() { for spend in sapling_shielded_data.spends() {
// This is the canonical transaction serialization, minus the `spendAuthSig`. // This is the canonical transaction serialization, minus the `spendAuthSig`.
spend.cv.zcash_serialize(&mut hash)?; spend.cv.zcash_serialize(&mut hash)?;
// TODO: ZIP-243 Sapling to Canopy only // TODO: ZIP-243 Sapling to Canopy only
@ -459,11 +459,11 @@ impl<'a> SigHasher<'a> {
fn hash_shielded_outputs<W: io::Write>(&self, mut writer: W) -> Result<(), io::Error> { fn hash_shielded_outputs<W: io::Write>(&self, mut writer: W) -> Result<(), io::Error> {
use Transaction::*; use Transaction::*;
let shielded_data = match self.trans { let sapling_shielded_data = match self.trans {
V4 { V4 {
sapling_shielded_data: Some(shielded_data), sapling_shielded_data: Some(sapling_shielded_data),
.. ..
} => shielded_data, } => sapling_shielded_data,
V4 { V4 {
sapling_shielded_data: None, sapling_shielded_data: None,
.. ..
@ -472,7 +472,7 @@ impl<'a> SigHasher<'a> {
V1 { .. } | V2 { .. } | V3 { .. } => unreachable!(ZIP243_EXPLANATION), V1 { .. } | V2 { .. } | V3 { .. } => unreachable!(ZIP243_EXPLANATION),
}; };
if shielded_data.outputs().next().is_none() { if sapling_shielded_data.outputs().next().is_none() {
return writer.write_all(&[0; 32]); return writer.write_all(&[0; 32]);
} }
@ -482,7 +482,7 @@ impl<'a> SigHasher<'a> {
.to_state(); .to_state();
// Correctness: checked for V4 transaction above // Correctness: checked for V4 transaction above
for output in shielded_data for output in sapling_shielded_data
.outputs() .outputs()
.cloned() .cloned()
.map(sapling::OutputInTransactionV4) .map(sapling::OutputInTransactionV4)

View File

@ -34,8 +34,8 @@ where
sapling_shielded_data, sapling_shielded_data,
.. ..
} => { } => {
if let Some(shielded_data) = sapling_shielded_data { if let Some(sapling_shielded_data) = sapling_shielded_data {
for spend in shielded_data.spends_per_anchor() { for spend in sapling_shielded_data.spends_per_anchor() {
tracing::trace!(?spend); tracing::trace!(?spend);
let spend_rsp = spend_verifier let spend_rsp = spend_verifier
@ -46,7 +46,7 @@ where
async_checks.push(spend_rsp); async_checks.push(spend_rsp);
} }
for output in shielded_data.outputs() { for output in sapling_shielded_data.outputs() {
tracing::trace!(?output); tracing::trace!(?output);
let output_rsp = output_verifier let output_rsp = output_verifier
@ -117,8 +117,8 @@ where
sapling_shielded_data, sapling_shielded_data,
.. ..
} => { } => {
if let Some(shielded_data) = sapling_shielded_data { if let Some(sapling_shielded_data) = sapling_shielded_data {
for output in shielded_data.outputs() { for output in sapling_shielded_data.outputs() {
// This changes the primary inputs to the proof // This changes the primary inputs to the proof
// verification, causing it to fail for this proof. // verification, causing it to fail for this proof.
let mut modified_output = output.clone(); let mut modified_output = output.clone();

View File

@ -212,10 +212,10 @@ where
async_checks.push(rsp.boxed()); async_checks.push(rsp.boxed());
} }
if let Some(shielded_data) = sapling_shielded_data { if let Some(sapling_shielded_data) = sapling_shielded_data {
check::shielded_balances_match(&shielded_data)?; check::shielded_balances_match(&sapling_shielded_data)?;
for spend in shielded_data.spends_per_anchor() { for spend in sapling_shielded_data.spends_per_anchor() {
// Consensus rule: cv and rk MUST NOT be of small // Consensus rule: cv and rk MUST NOT be of small
// order, i.e. [h_J]cv MUST NOT be 𝒪_J and [h_J]rk // order, i.e. [h_J]cv MUST NOT be 𝒪_J and [h_J]rk
// MUST NOT be 𝒪_J. // MUST NOT be 𝒪_J.
@ -256,7 +256,7 @@ where
async_checks.push(rsp.boxed()); async_checks.push(rsp.boxed());
} }
for output in shielded_data.outputs() { for output in sapling_shielded_data.outputs() {
// Consensus rule: cv and wpk MUST NOT be of small // Consensus rule: cv and wpk MUST NOT be of small
// order, i.e. [h_J]cv MUST NOT be 𝒪_J and [h_J]wpk // order, i.e. [h_J]cv MUST NOT be 𝒪_J and [h_J]wpk
// MUST NOT be 𝒪_J. // MUST NOT be 𝒪_J.
@ -281,11 +281,11 @@ where
async_checks.push(output_rsp.boxed()); async_checks.push(output_rsp.boxed());
} }
let bvk = shielded_data.binding_verification_key(); let bvk = sapling_shielded_data.binding_verification_key();
// TODO: enable async verification and remove this block - #1939 // TODO: enable async verification and remove this block - #1939
{ {
let item: zebra_chain::primitives::redjubjub::batch::Item = (bvk, shielded_data.binding_sig, &shielded_sighash).into(); let item: zebra_chain::primitives::redjubjub::batch::Item = (bvk, sapling_shielded_data.binding_sig, &shielded_sighash).into();
item.verify_single().unwrap_or_else(|binding_sig_error| { item.verify_single().unwrap_or_else(|binding_sig_error| {
let binding_sig_error = binding_sig_error.to_string(); let binding_sig_error = binding_sig_error.to_string();
tracing::warn!(%binding_sig_error, "ignoring"); tracing::warn!(%binding_sig_error, "ignoring");
@ -300,7 +300,7 @@ where
let _rsp = redjubjub_verifier let _rsp = redjubjub_verifier
.ready_and() .ready_and()
.await? .await?
.call((bvk, shielded_data.binding_sig, &shielded_sighash).into()) .call((bvk, sapling_shielded_data.binding_sig, &shielded_sighash).into())
.boxed(); .boxed();
// TODO: stop ignoring binding signature errors - #1939 // TODO: stop ignoring binding signature errors - #1939

View File

@ -340,17 +340,23 @@ impl<AnchorV> UpdateWith<Option<sapling::ShieldedData<AnchorV>>> for Chain
where where
AnchorV: sapling::AnchorVariant + Clone, AnchorV: sapling::AnchorVariant + Clone,
{ {
fn update_chain_state_with(&mut self, shielded_data: &Option<sapling::ShieldedData<AnchorV>>) { fn update_chain_state_with(
if let Some(shielded_data) = shielded_data { &mut self,
for nullifier in shielded_data.nullifiers() { sapling_shielded_data: &Option<sapling::ShieldedData<AnchorV>>,
) {
if let Some(sapling_shielded_data) = sapling_shielded_data {
for nullifier in sapling_shielded_data.nullifiers() {
self.sapling_nullifiers.insert(*nullifier); self.sapling_nullifiers.insert(*nullifier);
} }
} }
} }
fn revert_chain_state_with(&mut self, shielded_data: &Option<sapling::ShieldedData<AnchorV>>) { fn revert_chain_state_with(
if let Some(shielded_data) = shielded_data { &mut self,
for nullifier in shielded_data.nullifiers() { sapling_shielded_data: &Option<sapling::ShieldedData<AnchorV>>,
) {
if let Some(sapling_shielded_data) = sapling_shielded_data {
for nullifier in sapling_shielded_data.nullifiers() {
assert!( assert!(
self.sapling_nullifiers.remove(nullifier), self.sapling_nullifiers.remove(nullifier),
"nullifier must be present if block was" "nullifier must be present if block was"