Serialize/Deserialize sapling data in V5 #1860
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -66,6 +66,65 @@ impl<P: ZkSnarkProof> ZcashDeserialize for Option<JoinSplitData<P>> { | |||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| struct SigShieldedData<'a> { | ||||||||
| sig: bool, | ||||||||
| shielded_data: &'a Option<ShieldedData>, | ||||||||
| } | ||||||||
|
Comment on lines
+69
to
+72
There was a problem hiding this comment.
There was a problem hiding this comment. It is needed for the order of the fields inside a transaction. In V4 transactions we have the spends and outputs for sapling then the joinsplits and then the binding sig. This is explained in https://github.com/ZcashFoundation/zebra/blob/main/zebra-chain/src/transaction/serialize.rs#L147-L153 and can be verified by checking the fields of the V4 transaction in the spec. In the other hand in the V5 transactions the binding sig is intermediately after the sapling spends and outputs so it can be serialized right there as there is nothing in the middle. There was a problem hiding this comment. Please add a doc comment for this type, that explains why it's needed, and how it should be used. There was a problem hiding this comment. Actually, it looks like we can't use this type, because |
||||||||
|
|
||||||||
| impl ZcashSerialize for SigShieldedData<'_> { | ||||||||
| fn zcash_serialize<W: io::Write>(&self, mut writer: W) -> Result<(), io::Error> { | ||||||||
| let sig = self.sig; | ||||||||
| let shielded_data = self.shielded_data; | ||||||||
| match shielded_data { | ||||||||
| None => { | ||||||||
| // Signal no shielded spends and no shielded outputs. | ||||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||||
| writer.write_compactsize(0)?; | ||||||||
| writer.write_compactsize(0)?; | ||||||||
| } | ||||||||
| Some(shielded_data) => { | ||||||||
| writer.write_compactsize(shielded_data.spends().count() as u64)?; | ||||||||
| for spend in shielded_data.spends() { | ||||||||
| spend.zcash_serialize(&mut writer)?; | ||||||||
| } | ||||||||
| writer.write_compactsize(shielded_data.outputs().count() as u64)?; | ||||||||
| for output in shielded_data.outputs() { | ||||||||
| output.zcash_serialize(&mut writer)?; | ||||||||
| } | ||||||||
| if sig { | ||||||||
| writer.write_all(&<[u8; 64]>::from(shielded_data.binding_sig)[..])? | ||||||||
| } | ||||||||
| } | ||||||||
| } | ||||||||
| Ok(()) | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| fn deserialize_shielded_data<R: io::Read>( | ||||||||
| mut reader: R, | ||||||||
| mut shielded_spends: Vec<sapling::Spend>, | ||||||||
| mut shielded_outputs: Vec<sapling::Output>, | ||||||||
| ) -> Result<Option<ShieldedData>, SerializationError> { | ||||||||
| use futures::future::Either::*; | ||||||||
|
|
||||||||
| if !shielded_spends.is_empty() { | ||||||||
| Ok(Some(ShieldedData { | ||||||||
| first: Left(shielded_spends.remove(0)), | ||||||||
| rest_spends: shielded_spends, | ||||||||
| rest_outputs: shielded_outputs, | ||||||||
| binding_sig: reader.read_64_bytes()?.into(), | ||||||||
| })) | ||||||||
| } else if !shielded_outputs.is_empty() { | ||||||||
| Ok(Some(ShieldedData { | ||||||||
| first: Right(shielded_outputs.remove(0)), | ||||||||
| rest_spends: shielded_spends, | ||||||||
| rest_outputs: shielded_outputs, | ||||||||
| binding_sig: reader.read_64_bytes()?.into(), | ||||||||
| })) | ||||||||
| } else { | ||||||||
| Ok(None) | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| impl ZcashSerialize for Transaction { | ||||||||
| fn zcash_serialize<W: io::Write>(&self, mut writer: W) -> Result<(), io::Error> { | ||||||||
| // Post-Sapling, transaction size is limited to MAX_BLOCK_BYTES. | ||||||||
|
|
@@ -131,7 +190,7 @@ impl ZcashSerialize for Transaction { | |||||||
| outputs, | ||||||||
| lock_time, | ||||||||
| expiry_height, | ||||||||
| sapling_value_balance, | ||||||||
| shielded_data, | ||||||||
| joinsplit_data, | ||||||||
| } => { | ||||||||
|
|
@@ -142,7 +201,7 @@ impl ZcashSerialize for Transaction { | |||||||
| outputs.zcash_serialize(&mut writer)?; | ||||||||
| lock_time.zcash_serialize(&mut writer)?; | ||||||||
| writer.write_u32::<LittleEndian>(expiry_height.0)?; | ||||||||
| sapling_value_balance.zcash_serialize(&mut writer)?; | ||||||||
|
|
||||||||
| // The previous match arms serialize in one go, because the | ||||||||
| // internal structure happens to nicely line up with the | ||||||||
|
|
@@ -152,29 +211,19 @@ impl ZcashSerialize for Transaction { | |||||||
| // instead we have to interleave serialization of the | ||||||||
| // ShieldedData and the JoinSplitData. | ||||||||
|
|
||||||||
| // Serialize ShieldedData without doing the binding_sig. | ||||||||
| let sig_shielded_data = SigShieldedData { | ||||||||
| sig: false, | ||||||||
| shielded_data, | ||||||||
| }; | ||||||||
| sig_shielded_data.zcash_serialize(&mut writer)?; | ||||||||
|
|
||||||||
| match joinsplit_data { | ||||||||
| None => writer.write_compactsize(0)?, | ||||||||
| Some(jsd) => jsd.zcash_serialize(&mut writer)?, | ||||||||
| } | ||||||||
|
|
||||||||
| // Manually write the binding_sig after the JoinSplitData. | ||||||||
| match shielded_data { | ||||||||
| Some(sd) => writer.write_all(&<[u8; 64]>::from(sd.binding_sig)[..])?, | ||||||||
| None => {} | ||||||||
|
|
@@ -185,6 +234,8 @@ impl ZcashSerialize for Transaction { | |||||||
| expiry_height, | ||||||||
| inputs, | ||||||||
| outputs, | ||||||||
| shielded_data, | ||||||||
| sapling_value_balance, | ||||||||
| rest, | ||||||||
| } => { | ||||||||
| // Write version 5 and set the fOverwintered bit. | ||||||||
|
|
@@ -194,6 +245,20 @@ impl ZcashSerialize for Transaction { | |||||||
| writer.write_u32::<LittleEndian>(expiry_height.0)?; | ||||||||
| inputs.zcash_serialize(&mut writer)?; | ||||||||
| outputs.zcash_serialize(&mut writer)?; | ||||||||
|
|
||||||||
| // Version 5 internal structure is different from version 4. | ||||||||
| // Here we can do the binding signature without interleave | ||||||||
| // the serialization. | ||||||||
|
|
||||||||
| // Serialize the ShieldedData including the binding_sig. | ||||||||
| let sig_shielded_data = SigShieldedData { | ||||||||
| sig: true, | ||||||||
| shielded_data, | ||||||||
| }; | ||||||||
| sig_shielded_data.zcash_serialize(&mut writer)?; | ||||||||
|
|
||||||||
| sapling_value_balance.zcash_serialize(&mut writer)?; | ||||||||
|
|
||||||||
| // write the rest | ||||||||
| writer.write_all(rest)?; | ||||||||
| } | ||||||||
|
|
@@ -266,36 +331,19 @@ impl ZcashDeserialize for Transaction { | |||||||
| let outputs = Vec::zcash_deserialize(&mut reader)?; | ||||||||
| let lock_time = LockTime::zcash_deserialize(&mut reader)?; | ||||||||
| let expiry_height = block::Height(reader.read_u32::<LittleEndian>()?); | ||||||||
| let sapling_value_balance = (&mut reader).zcash_deserialize_into()?; | ||||||||
| let shielded_spends = Vec::zcash_deserialize(&mut reader)?; | ||||||||
| let shielded_outputs = Vec::zcash_deserialize(&mut reader)?; | ||||||||
| let joinsplit_data = OptV4Jsd::zcash_deserialize(&mut reader)?; | ||||||||
| let shielded_data = | ||||||||
|
There was a problem hiding this comment.
There was a problem hiding this comment. The idea is to use the same code in V4 and V5 as they are doing almost the same for sapling stuff(except for the order of the fields). There was a problem hiding this comment. Even though they have the same names, the underlying types of these fields are different, so it might be best to just duplicate some code. |
||||||||
| deserialize_shielded_data(&mut reader, shielded_spends, shielded_outputs)?; | ||||||||
|
|
||||||||
| Ok(Transaction::V4 { | ||||||||
| inputs, | ||||||||
| outputs, | ||||||||
| lock_time, | ||||||||
| expiry_height, | ||||||||
| sapling_value_balance, | ||||||||
| shielded_data, | ||||||||
| joinsplit_data, | ||||||||
| }) | ||||||||
|
|
@@ -309,6 +357,12 @@ impl ZcashDeserialize for Transaction { | |||||||
| let expiry_height = block::Height(reader.read_u32::<LittleEndian>()?); | ||||||||
| let inputs = Vec::zcash_deserialize(&mut reader)?; | ||||||||
| let outputs = Vec::zcash_deserialize(&mut reader)?; | ||||||||
| let shielded_spends = Vec::zcash_deserialize(&mut reader)?; | ||||||||
| let shielded_outputs = Vec::zcash_deserialize(&mut reader)?; | ||||||||
| let shielded_data = | ||||||||
| deserialize_shielded_data(&mut reader, shielded_spends, shielded_outputs)?; | ||||||||
| let sapling_value_balance = (&mut reader).zcash_deserialize_into()?; | ||||||||
|
Comment on lines
+360
to
+364
There was a problem hiding this comment. If inputs and outputs are I will add a test for this bug to the design in #1863. After #1863 is completed, please revise all the code in this PR, based on the latest spec.
|
||||||||
|
|
||||||||
| let mut rest = Vec::new(); | ||||||||
| reader.read_to_end(&mut rest)?; | ||||||||
|
|
||||||||
|
|
@@ -317,6 +371,8 @@ impl ZcashDeserialize for Transaction { | |||||||
| expiry_height, | ||||||||
| inputs, | ||||||||
| outputs, | ||||||||
| shielded_data, | ||||||||
| sapling_value_balance, | ||||||||
| rest, | ||||||||
| }) | ||||||||
| } | ||||||||
|
|
||||||||
This file contains 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
This file contains 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
This file contains 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
This file contains 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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍