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

fix(diagnostics): Hex-encode debug format of commitments, nonces, and nullifiers #5960

Merged
merged 5 commits into from
Jan 17, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion zebra-chain/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ impl Block {
None => Err(CommitmentError::MissingBlockHeight {
block_hash: self.hash(),
}),
Some(height) => Commitment::from_bytes(self.header.commitment_bytes, network, height),
Some(height) => Commitment::from_bytes(*self.header.commitment_bytes, network, height),
}
}

Expand Down
20 changes: 11 additions & 9 deletions zebra-chain/src/block/arbitrary.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
//! Randomised property testing for [`Block`]s.

use std::{collections::HashMap, sync::Arc};

use proptest::{
arbitrary::{any, Arbitrary},
prelude::*,
};

use std::{collections::HashMap, sync::Arc};

use crate::{
amount::NonNegative,
block,
fmt::SummaryDebug,
fmt::{HexDebug, SummaryDebug},
history_tree::HistoryTree,
parameters::{
Network,
Expand Down Expand Up @@ -482,13 +482,13 @@ impl Block {
// needs to be well-formed, i.e. smaller than 𝑞_J, so we
// arbitrarily set it to 1.
let block_header = Arc::make_mut(&mut block.header);
block_header.commitment_bytes = [0u8; 32];
block_header.commitment_bytes = [0u8; 32].into();
block_header.commitment_bytes[0] = 1;
}
std::cmp::Ordering::Equal => {
// The Heartwood activation block has a hardcoded all-zeroes commitment.
let block_header = Arc::make_mut(&mut block.header);
block_header.commitment_bytes = [0u8; 32];
block_header.commitment_bytes = [0u8; 32].into();
}
std::cmp::Ordering::Greater => {
// Set the correct commitment bytes according to the network upgrade.
Expand All @@ -505,10 +505,12 @@ impl Block {
&auth_data_root,
);
let block_header = Arc::make_mut(&mut block.header);
block_header.commitment_bytes = hash_block_commitments.into();
block_header.commitment_bytes =
hash_block_commitments.bytes_in_serialized_order().into();
} else {
let block_header = Arc::make_mut(&mut block.header);
block_header.commitment_bytes = history_tree_root.into();
block_header.commitment_bytes =
history_tree_root.bytes_in_serialized_order().into();
}
}
}
Expand Down Expand Up @@ -723,10 +725,10 @@ impl Arbitrary for Header {
(4u32..(i32::MAX as u32)),
any::<Hash>(),
any::<merkle::Root>(),
any::<[u8; 32]>(),
any::<HexDebug<[u8; 32]>>(),
serialization::arbitrary::datetime_u32(),
any::<CompactDifficulty>(),
any::<[u8; 32]>(),
any::<HexDebug<[u8; 32]>>(),
any::<equihash::Solution>(),
)
.prop_map(
Expand Down
48 changes: 46 additions & 2 deletions zebra-chain/src/block/commitment.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
//! The Commitment enum, used for the corresponding block header field.

use std::fmt;

use hex::{FromHex, ToHex};
use thiserror::Error;

Expand Down Expand Up @@ -97,6 +99,8 @@ pub(crate) const CHAIN_HISTORY_ACTIVATION_RESERVED: [u8; 32] = [0; 32];

impl Commitment {
/// Returns `bytes` as the Commitment variant for `network` and `height`.
//
// TODO: rename as from_bytes_in_serialized_order()
pub(super) fn from_bytes(
bytes: [u8; 32],
network: Network,
Expand Down Expand Up @@ -126,6 +130,8 @@ impl Commitment {
}

/// Returns the serialized bytes for this Commitment.
//
// TODO: refactor as bytes_in_serialized_order(&self)
#[cfg(test)]
pub(super) fn to_bytes(self) -> [u8; 32] {
use Commitment::*;
Expand All @@ -145,9 +151,23 @@ impl Commitment {
// - add methods for maintaining the MMR peaks, and calculating the root
// hash from the current set of peaks
// - move to a separate file
#[derive(Clone, Copy, Debug, Eq, PartialEq, Serialize, Deserialize)]
#[derive(Clone, Copy, Eq, PartialEq, Serialize, Deserialize)]
pub struct ChainHistoryMmrRootHash([u8; 32]);

impl fmt::Display for ChainHistoryMmrRootHash {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
f.write_str(&self.encode_hex::<String>())
}
}

impl fmt::Debug for ChainHistoryMmrRootHash {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
f.debug_tuple("ChainHistoryMmrRootHash")
.field(&self.encode_hex::<String>())
.finish()
}
}

impl From<[u8; 32]> for ChainHistoryMmrRootHash {
fn from(hash: [u8; 32]) -> Self {
ChainHistoryMmrRootHash(hash)
Expand Down Expand Up @@ -183,6 +203,11 @@ impl ChainHistoryMmrRootHash {

ChainHistoryMmrRootHash(internal_byte_order)
}

/// Returns the serialized bytes for this Commitment.
pub fn bytes_in_serialized_order(&self) -> [u8; 32] {
self.0
}
}

impl ToHex for &ChainHistoryMmrRootHash {
Expand Down Expand Up @@ -222,9 +247,23 @@ impl FromHex for ChainHistoryMmrRootHash {
/// - the transaction authorising data in this block.
///
/// Introduced in NU5.
#[derive(Clone, Copy, Debug, Eq, PartialEq, Serialize, Deserialize)]
#[derive(Clone, Copy, Eq, PartialEq, Serialize, Deserialize)]
pub struct ChainHistoryBlockTxAuthCommitmentHash([u8; 32]);

impl fmt::Display for ChainHistoryBlockTxAuthCommitmentHash {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
f.write_str(&self.encode_hex::<String>())
}
}

impl fmt::Debug for ChainHistoryBlockTxAuthCommitmentHash {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
f.debug_tuple("ChainHistoryBlockTxAuthCommitmentHash")
.field(&self.encode_hex::<String>())
.finish()
}
}

impl From<[u8; 32]> for ChainHistoryBlockTxAuthCommitmentHash {
fn from(hash: [u8; 32]) -> Self {
ChainHistoryBlockTxAuthCommitmentHash(hash)
Expand Down Expand Up @@ -292,6 +331,11 @@ impl ChainHistoryBlockTxAuthCommitmentHash {

ChainHistoryBlockTxAuthCommitmentHash(internal_byte_order)
}

/// Returns the serialized bytes for this Commitment.
pub fn bytes_in_serialized_order(&self) -> [u8; 32] {
self.0
}
}

impl ToHex for &ChainHistoryBlockTxAuthCommitmentHash {
Expand Down
5 changes: 3 additions & 2 deletions zebra-chain/src/block/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use chrono::{DateTime, Duration, Utc};
use thiserror::Error;

use crate::{
fmt::HexDebug,
serialization::{TrustedPreallocate, MAX_PROTOCOL_MESSAGE_LEN},
work::{difficulty::CompactDifficulty, equihash::Solution},
};
Expand Down Expand Up @@ -58,7 +59,7 @@ pub struct Header {
/// this field cannot be parsed without the network and height. Use
/// [`Block::commitment`](super::Block::commitment) to get the parsed
/// [`Commitment`](super::Commitment).
pub commitment_bytes: [u8; 32],
pub commitment_bytes: HexDebug<[u8; 32]>,

/// The block timestamp is a Unix epoch time (UTC) when the miner
/// started hashing the header (according to the miner).
Expand All @@ -77,7 +78,7 @@ pub struct Header {
/// An arbitrary field that miners can change to modify the header
/// hash in order to produce a hash less than or equal to the
/// target threshold.
pub nonce: [u8; 32],
pub nonce: HexDebug<[u8; 32]>,

/// The Equihash solution.
pub solution: Solution,
Expand Down
7 changes: 3 additions & 4 deletions zebra-chain/src/block/serialize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,14 @@ use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt};
use chrono::{TimeZone, Utc};

use crate::{
block::{header::ZCASH_BLOCK_VERSION, merkle, Block, CountedHeader, Hash, Header},
serialization::{
CompactSizeMessage, ReadZcashExt, SerializationError, ZcashDeserialize,
ZcashDeserializeInto, ZcashSerialize,
},
work::{difficulty::CompactDifficulty, equihash},
};

use super::{header::ZCASH_BLOCK_VERSION, merkle, Block, CountedHeader, Hash, Header};

/// The maximum size of a Zcash block, in bytes.
///
/// Post-Sapling, this is also the maximum size of a transaction
Expand Down Expand Up @@ -85,7 +84,7 @@ impl ZcashDeserialize for Header {
version,
previous_block_hash: Hash::zcash_deserialize(&mut reader)?,
merkle_root: merkle::Root(reader.read_32_bytes()?),
commitment_bytes: reader.read_32_bytes()?,
commitment_bytes: reader.read_32_bytes()?.into(),
// This can't panic, because all u32 values are valid `Utc.timestamp`s
time: Utc
.timestamp_opt(reader.read_u32::<LittleEndian>()?.into(), 0)
Expand All @@ -94,7 +93,7 @@ impl ZcashDeserialize for Header {
"out-of-range number of seconds and/or invalid nanosecond",
))?,
difficulty_threshold: CompactDifficulty(reader.read_u32::<LittleEndian>()?),
nonce: reader.read_32_bytes()?,
nonce: reader.read_32_bytes()?.into(),
solution: equihash::Solution::zcash_deserialize(reader)?,
})
}
Expand Down
2 changes: 1 addition & 1 deletion zebra-chain/src/block/tests/prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ proptest! {
let commitment = block.commitment(network);
if let Ok(commitment) = commitment {
let commitment_bytes = commitment.to_bytes();
prop_assert_eq![block.header.commitment_bytes, commitment_bytes];
prop_assert_eq![block.header.commitment_bytes.0, commitment_bytes];
}

// Check the block size limit
Expand Down
35 changes: 35 additions & 0 deletions zebra-chain/src/fmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,3 +160,38 @@ where

type Strategy = BoxedStrategy<Self>;
}

/// Wrapper to override `Debug`, redirecting it to hex-encode the type.
/// The type must be hex-encodable.
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)]
#[cfg_attr(any(test, feature = "proptest-impl"), derive(Arbitrary))]
#[serde(transparent)]
pub struct HexDebug<T: AsRef<[u8]>>(pub T);

impl<T: AsRef<[u8]>> fmt::Debug for HexDebug<T> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_tuple(std::any::type_name::<T>())
.field(&hex::encode(self.as_ref()))
.finish()
}
}

impl<T: AsRef<[u8]>> ops::Deref for HexDebug<T> {
type Target = T;

fn deref(&self) -> &Self::Target {
&self.0
}
}

impl<T: AsRef<[u8]>> ops::DerefMut for HexDebug<T> {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.0
}
}

impl<T: AsRef<[u8]>> From<T> for HexDebug<T> {
fn from(t: T) -> Self {
Self(t)
}
}
4 changes: 2 additions & 2 deletions zebra-chain/src/orchard/arbitrary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ impl Arbitrary for Signature<SpendAuth> {
fn arbitrary_with(_args: Self::Parameters) -> Self::Strategy {
(array::uniform32(any::<u8>()), array::uniform32(any::<u8>()))
.prop_map(|(r_bytes, s_bytes)| Self {
r_bytes,
s_bytes,
r_bytes: r_bytes.into(),
s_bytes: s_bytes.into(),
_marker: PhantomData,
})
.boxed()
Expand Down
2 changes: 2 additions & 0 deletions zebra-chain/src/primitives/proofs/bctv14.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
//! BCTV14 proofs for Zebra.

use std::{fmt, io};

use serde::{Deserialize, Serialize};
Expand Down
2 changes: 2 additions & 0 deletions zebra-chain/src/primitives/proofs/groth16.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
//! Groth16 proofs for Zebra.

use std::{fmt, io};

use serde::{Deserialize, Serialize};
Expand Down
6 changes: 3 additions & 3 deletions zebra-chain/src/primitives/redpallas/batch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ impl Verifier {

let s = {
// XXX-pallas: should not use CtOption here
let maybe_scalar = pallas::Scalar::from_repr(s_bytes);
let maybe_scalar = pallas::Scalar::from_repr(*s_bytes);
if maybe_scalar.is_some().into() {
maybe_scalar.unwrap()
} else {
Expand Down Expand Up @@ -258,10 +258,10 @@ impl Verifier {
//
// This validates the `rk` element, whose type is
// SpendAuthSig^{Orchard}.Public, i.e. ℙ.
VerificationKey::<SpendAuth>::try_from(vk_bytes.bytes)?.point
VerificationKey::<SpendAuth>::try_from(*vk_bytes.bytes)?.point
}
Inner::Binding { vk_bytes, .. } => {
VerificationKey::<Binding>::try_from(vk_bytes.bytes)?.point
VerificationKey::<Binding>::try_from(*vk_bytes.bytes)?.point
}
};

Expand Down
13 changes: 8 additions & 5 deletions zebra-chain/src/primitives/redpallas/signature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,16 @@ use std::{io, marker::PhantomData};

use super::SigType;

use crate::serialization::{ReadZcashExt, SerializationError, ZcashDeserialize, ZcashSerialize};
use crate::{
fmt::HexDebug,
serialization::{ReadZcashExt, SerializationError, ZcashDeserialize, ZcashSerialize},
};

/// A RedPallas signature.
#[derive(Copy, Clone, Debug, Eq, PartialEq, Deserialize, Serialize)]
pub struct Signature<T: SigType> {
pub(crate) r_bytes: [u8; 32],
pub(crate) s_bytes: [u8; 32],
pub(crate) r_bytes: HexDebug<[u8; 32]>,
pub(crate) s_bytes: HexDebug<[u8; 32]>,
pub(crate) _marker: PhantomData<T>,
}

Expand All @@ -29,8 +32,8 @@ impl<T: SigType> From<[u8; 64]> for Signature<T> {
let mut s_bytes = [0; 32];
s_bytes.copy_from_slice(&bytes[32..64]);
Signature {
r_bytes,
s_bytes,
r_bytes: r_bytes.into(),
s_bytes: s_bytes.into(),
_marker: PhantomData,
}
}
Expand Down
7 changes: 4 additions & 3 deletions zebra-chain/src/primitives/redpallas/signing_key.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::convert::{TryFrom, TryInto};
//! Redpallas signing keys for Zebra.

use std::marker::PhantomData;

use group::{ff::PrimeField, GroupEncoding};
Expand Down Expand Up @@ -117,8 +118,8 @@ impl<T: SigType> SigningKey<T> {
let s_bytes = (nonce + (c * self.sk)).to_repr();

Signature {
r_bytes,
s_bytes,
r_bytes: r_bytes.into(),
s_bytes: s_bytes.into(),
_marker: PhantomData,
}
}
Expand Down
Loading