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

Block header parsing and transaction ID computation #66

Merged
merged 9 commits into from
Apr 5, 2019
Merged
Show file tree
Hide file tree
Changes from 3 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
7 changes: 7 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions zcash_primitives/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ authors = [
[dependencies]
byteorder = "1"
ff = { path = "../ff" }
hex = "0.3"
Copy link
Collaborator

Choose a reason for hiding this comment

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

lazy_static = "1"
pairing = { path = "../pairing" }
rand = "0.4"
Expand Down
42 changes: 42 additions & 0 deletions zcash_primitives/src/block.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
use hex;
use std::fmt;
use std::ops::Deref;

#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
pub struct BlockHash(pub [u8; 32]);

impl fmt::Display for BlockHash {
Copy link
Contributor

Choose a reason for hiding this comment

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

Aside from the name, BlockHash is the same as TxId and in this commit we write the same fn fmt twice. This makes me wonder if there should be a single type (named ByteArray32 or something like that) rather than two.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not convinced that the extra indirection to the underlying bytes is worth it yet. If we find the reverse-hex format is bleeding in too much from zcashd then we can revisit this.

fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
let mut data = self.0.to_vec();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead, you could initialize a local clone of the self.0: [u8; 32] and then [T]::reverse() on that, rather than allocating.

(Not blocking.)

data.reverse();
formatter.write_str(&hex::encode(data))
}
}

/// A Zcash block header.
pub struct BlockHeader(BlockHeaderData);

impl Deref for BlockHeader {
type Target = BlockHeaderData;

fn deref(&self) -> &BlockHeaderData {
&self.0
}
}

pub struct BlockHeaderData {
pub version: i32,
pub prev_block: BlockHash,
pub merkle_root: [u8; 32],
pub final_sapling_root: [u8; 32],
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if this is out of scope for the PR, but the BlockHash is a typed wrapper for a [u8; 32]; are the merkle_root and final_sapling_root comparable 32-byte arrays, or semantically-different types? Should they also be newtype wrappers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are all hash outputs, but of at least two different hash functions. In zcashd they are represented by the same 256-bit binary blob type, which has the "feature" that when rendered in hex in zcashd RPC output, the bytes are reversed (behaviour inherited from Bitcoin Core).

I use separate types for block hashes and transaction IDs because those are the only two that external code is likely to actually interact with, whereas other block header fields (thus far) will only be used internally. Once the refactor is further along and we get to the stage of cleaning up the crate APIs, we should revisit this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Opened #73.

pub time: u32,
pub bits: u32,
pub nonce: [u8; 32],
pub solution: Vec<u8>,
}

impl BlockHeaderData {
pub fn freeze(self) -> BlockHeader {
BlockHeader(self)
}
}
2 changes: 2 additions & 0 deletions zcash_primitives/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@ extern crate lazy_static;
extern crate blake2_rfc;
extern crate byteorder;
extern crate ff;
extern crate hex;
extern crate pairing;
extern crate rand;
extern crate sapling_crypto;

use sapling_crypto::jubjub::JubjubBls12;

pub mod block;
pub mod sapling;
mod serialize;
pub mod transaction;
Expand Down
13 changes: 13 additions & 0 deletions zcash_primitives/src/transaction/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt};
use hex;
use sapling_crypto::redjubjub::Signature;
use std::fmt;
use std::io::{self, Read, Write};
use std::ops::Deref;

Expand All @@ -20,6 +22,17 @@ const OVERWINTER_TX_VERSION: u32 = 3;
const SAPLING_VERSION_GROUP_ID: u32 = 0x892F2085;
const SAPLING_TX_VERSION: u32 = 4;

#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
pub struct TxId(pub [u8; 32]);

impl fmt::Display for TxId {
fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
let mut data = self.0.to_vec();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as the other review comment about something similar to this.

data.reverse();
formatter.write_str(&hex::encode(data))
}
}

/// A Zcash transaction.
#[derive(Debug)]
pub struct Transaction(TransactionData);
Expand Down