Skip to content

Commit

Permalink
fix(diagnostics): Hex-encode debug format of commitments, nonces, and…
Browse files Browse the repository at this point in the history
… nullifiers (#5960)

* Format commitments and nonces as hex

* Format Sprout and Sapling nullifiers as hex when debugging

* Format Sprout commitments as hex when debugging

* Format redpallas keys as hex when debugging

* Update code that we're going to delete in the next PR anyway
  • Loading branch information
teor2345 authored Jan 17, 2023
1 parent 1bb8a9c commit 8c45196
Show file tree
Hide file tree
Showing 27 changed files with 200 additions and 78 deletions.
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

0 comments on commit 8c45196

Please sign in to comment.