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

Corrections to V7 transaction format and using updated test vectors #66

Merged
merged 16 commits into from
Jul 24, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
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

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion zcash_primitives/src/transaction/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,11 @@ use crate::{
use crate::transaction::builder::Error::{IssuanceBuilder, IssuanceBundle};
use orchard::note::AssetBase;
use orchard::orchard_flavor::OrchardVanilla;
#[cfg(zcash_unstable = "nu6")] /* TODO nu7 */ use orchard::orchard_flavor::OrchardZSA;
#[cfg(zcash_unstable = "nu6")] /* TODO nu7 */
use orchard::{
issuance::{IssueBundle, IssueInfo},
keys::{IssuanceAuthorizingKey, IssuanceValidatingKey},
orchard_flavor::OrchardZSA,
};
#[cfg(zcash_unstable = "nu6")] /* TODO nu7 */ use rand_core::OsRng;

Expand Down
8 changes: 4 additions & 4 deletions zcash_primitives/src/transaction/components/issuance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,10 @@ fn read_authorization<R: Read>(mut reader: R) -> io::Result<Signed> {
}

fn read_action<R: Read>(mut reader: R) -> io::Result<IssueAction> {
let finalize = reader.read_u8()? != 0;
let notes = Vector::read(&mut reader, |r| read_note(r))?;
let asset_descr_bytes = Vector::read(&mut reader, |r| r.read_u8())?;
let asset_descr: String = String::from_utf8(asset_descr_bytes).unwrap();
let notes = Vector::read(&mut reader, |r| read_note(r))?;
let finalize = reader.read_u8()? != 0;
Ok(IssueAction::from_parts(asset_descr, notes, finalize))
}

Expand Down Expand Up @@ -131,11 +131,11 @@ pub fn write_v7_bundle<W: Write>(

fn write_action<W: Write>(action: &IssueAction, mut writer: W) -> io::Result<()> {
let is_finalized_u8: u8 = if action.is_finalized() { 1 } else { 0 };
writer.write_u8(is_finalized_u8)?;
Vector::write(&mut writer, action.notes(), |w, note| write_note(note, w))?;
Vector::write(&mut writer, action.asset_desc().as_bytes(), |w, b| {
w.write_u8(*b)
})?;
Vector::write(&mut writer, action.notes(), |w, note| write_note(note, w))?;
writer.write_u8(is_finalized_u8)?;
Ok(())
}

Expand Down
8 changes: 4 additions & 4 deletions zcash_primitives/src/transaction/components/orchard.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ pub fn read_v5_bundle<R: Read>(
}
}

/// Reads an [`orchard::Bundle`] from a v5 transaction format.
pub fn read_v6_bundle<R: Read>(
/// Reads an [`orchard::Bundle`] from a v7 transaction format.
pub fn read_v7_bundle<R: Read>(
mut reader: R,
) -> io::Result<Option<orchard::Bundle<Authorized, Amount, OrchardZSA>>> {
#[allow(clippy::redundant_closure)]
Expand Down Expand Up @@ -312,8 +312,8 @@ pub fn write_v5_bundle<W: Write>(
Ok(())
}

/// Writes an [`orchard::Bundle`] in the v6 transaction format.
pub fn write_v6_bundle<W: Write>(
/// Writes an [`orchard::Bundle`] in the v7 transaction format.
pub fn write_v7_bundle<W: Write>(
bundle: Option<&orchard::Bundle<Authorized, Amount, OrchardZSA>>,
mut writer: W,
) -> io::Result<()> {
Expand Down
8 changes: 4 additions & 4 deletions zcash_primitives/src/transaction/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ const V5_VERSION_GROUP_ID: u32 = 0x26A7270A;
#[cfg(zcash_unstable = "nu6")] /* TODO nu7 */
const V7_TX_VERSION: u32 = 7;
#[cfg(zcash_unstable = "nu6")] /* TODO nu7 */
const V7_VERSION_GROUP_ID: u32 = 0x26A7270C; // TODO ???
const V7_VERSION_GROUP_ID: u32 = 0x124A69F8; // TODO ???
PaulLaux marked this conversation as resolved.
Show resolved Hide resolved

/// These versions are used exclusively for in-development transaction
/// serialization, and will never be active under the consensus rules.
Expand Down Expand Up @@ -930,7 +930,7 @@ impl Transaction {
Self::read_v5_header_fragment(&mut reader)?;
let transparent_bundle = Self::read_transparent(&mut reader)?;
let sapling_bundle = sapling_serialization::read_v5_bundle(&mut reader)?;
let orchard_zsa_bundle = orchard_serialization::read_v6_bundle(&mut reader)?;
let orchard_zsa_bundle = orchard_serialization::read_v7_bundle(&mut reader)?;
let issue_bundle = issuance::read_v7_bundle(&mut reader)?;

#[cfg(zcash_unstable = "zfuture")]
Expand Down Expand Up @@ -1079,13 +1079,13 @@ impl Transaction {
if self.sprout_bundle.is_some() {
return Err(io::Error::new(
io::ErrorKind::InvalidInput,
"Sprout components cannot be present when serializing to the V5 transaction format.",
"Sprout components cannot be present when serializing to the V7 transaction format.",
));
}
self.write_v5_header(&mut writer)?;
self.write_transparent(&mut writer)?;
self.write_v5_sapling(&mut writer)?;
orchard_serialization::write_v6_bundle(self.orchard_zsa_bundle.as_ref(), &mut writer)?;
orchard_serialization::write_v7_bundle(self.orchard_zsa_bundle.as_ref(), &mut writer)?;
issuance::write_v7_bundle(self.issue_bundle.as_ref(), &mut writer)?;
#[cfg(zcash_unstable = "zfuture")]
self.write_tze(&mut writer)?;
Expand Down
7 changes: 6 additions & 1 deletion zcash_primitives/src/transaction/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,12 @@ fn zip_0244() {
fn to_test_txdata(
tv: &self::data::zip_0244::TestVector,
) -> (TransactionData<TestUnauthorized>, TxDigests<Blake2bHash>) {
let tx = Transaction::read(&tv.tx[..], BranchId::Nu5).unwrap();
let tx = Transaction::read(
&tv.tx[..],
#[cfg(not(zcash_unstable = "nu6"))] /* TODO nu7 */ BranchId::Nu5,
Copy link

@PaulLaux PaulLaux Jul 23, 2024

Choose a reason for hiding this comment

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

new line after /* TODO nu7 */ (all occurrences)

Copy link
Author

Choose a reason for hiding this comment

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

cargo fmt pushes it back to the above line (so making it new line would make it fail the Rustfmt CI check).

Copy link
Author

Choose a reason for hiding this comment

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

I confirmed that this is the case for all lines where the newline is not used.

#[cfg(zcash_unstable = "nu6")] /* TODO nu7 */ BranchId::Nu7,
)
.unwrap();

assert_eq!(tx.txid.as_ref(), &tv.txid);
assert_eq!(tx.auth_commitment().as_ref(), &tv.auth_digest);
Expand Down
Loading
Loading