Implement kernel and output features as enums (#2312)

* use enums for kernel and output features

* rustfmt

* add test coverage around deserializing kernel features
This commit is contained in:
Antioch Peverell 2019-01-08 16:07:38 +00:00 committed by GitHub
parent c388af6603
commit 27801f6a93
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 120 additions and 87 deletions

2
Cargo.lock generated
View file

@ -771,11 +771,11 @@ dependencies = [
name = "grin_core"
version = "0.5.1"
dependencies = [
"bitflags 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)",
"blake2-rfc 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)",
"byteorder 1.2.7 (registry+https://github.com/rust-lang/crates.io-index)",
"chrono 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)",
"croaring 0.3.7 (registry+https://github.com/rust-lang/crates.io-index)",
"enum_primitive 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
"failure 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)",
"failure_derive 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)",
"grin_keychain 0.5.1",

View file

@ -44,8 +44,8 @@ pub fn get_output(
// For now we can just try both (but this probably needs to be part of the api
// params)
let outputs = [
OutputIdentifier::new(OutputFeatures::PLAIN, &commit),
OutputIdentifier::new(OutputFeatures::COINBASE, &commit),
OutputIdentifier::new(OutputFeatures::Plain, &commit),
OutputIdentifier::new(OutputFeatures::Coinbase, &commit),
];
for x in outputs.iter() {

View file

@ -10,10 +10,10 @@ workspace = ".."
edition = "2018"
[dependencies]
bitflags = "1"
blake2-rfc = "0.2"
byteorder = "1"
croaring = "=0.3"
enum_primitive = "0.1"
failure = "0.1"
failure_derive = "0.1"
lazy_static = "1"

View file

@ -28,32 +28,43 @@ use crate::util::secp;
use crate::util::secp::pedersen::{Commitment, RangeProof};
use crate::util::static_secp_instance;
use crate::util::RwLock;
use enum_primitive::FromPrimitive;
use std::cmp::max;
use std::cmp::Ordering;
use std::collections::HashSet;
use std::sync::Arc;
use std::{error, fmt};
bitflags! {
/// Options for a kernel's structure or use
#[derive(Serialize, Deserialize)]
pub struct KernelFeatures: u8 {
/// plain kernel has fee, but no lock_height
const PLAIN = 0;
/// coinbase kernel has neither fee nor lock_height (both zero)
const COINBASE = 1;
/// absolute height locked kernel; has fee and lock_height
const HEIGHT_LOCKED = 2;
/// Enum of various supported kernel "features".
enum_from_primitive! {
/// Various flavors of tx kernel.
#[derive(Debug, Clone, Copy, PartialEq, Serialize, Deserialize)]
#[repr(u8)]
pub enum KernelFeatures {
/// Plain kernel (the default for Grin txs).
Plain = 0,
/// A coinbase kernel.
Coinbase = 1,
/// A kernel with an expicit lock height.
HeightLocked = 2,
}
}
impl Writeable for KernelFeatures {
fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ser::Error> {
writer.write_u8(self.bits())?;
writer.write_u8(*self as u8)?;
Ok(())
}
}
impl Readable for KernelFeatures {
fn read(reader: &mut dyn Reader) -> Result<KernelFeatures, ser::Error> {
let features =
KernelFeatures::from_u8(reader.read_u8()?).ok_or(ser::Error::CorruptedData)?;
Ok(features)
}
}
/// Errors thrown by Transaction validation
#[derive(Clone, Eq, Debug, PartialEq)]
pub enum Error {
@ -181,10 +192,8 @@ impl Writeable for TxKernel {
impl Readable for TxKernel {
fn read(reader: &mut dyn Reader) -> Result<TxKernel, ser::Error> {
let features =
KernelFeatures::from_bits(reader.read_u8()?).ok_or(ser::Error::CorruptedData)?;
Ok(TxKernel {
features,
features: KernelFeatures::read(reader)?,
fee: reader.read_u64()?,
lock_height: reader.read_u64()?,
excess: Commitment::read(reader)?,
@ -205,17 +214,17 @@ impl PMMRable for TxKernel {
impl KernelFeatures {
/// Is this a coinbase kernel?
pub fn is_coinbase(&self) -> bool {
self.contains(KernelFeatures::COINBASE)
*self == KernelFeatures::Coinbase
}
/// Is this a plain kernel?
pub fn is_plain(&self) -> bool {
!self.is_coinbase() && !self.is_height_locked()
*self == KernelFeatures::Plain
}
/// Is this a height locked kernel?
pub fn is_height_locked(&self) -> bool {
self.contains(KernelFeatures::HEIGHT_LOCKED)
*self == KernelFeatures::HeightLocked
}
}
@ -278,7 +287,7 @@ impl TxKernel {
/// Build an empty tx kernel with zero values.
pub fn empty() -> TxKernel {
TxKernel {
features: KernelFeatures::PLAIN,
features: KernelFeatures::Plain,
fee: 0,
lock_height: 0,
excess: Commitment::from_vec(vec![0; 33]),
@ -1073,7 +1082,7 @@ impl ::std::hash::Hash for Input {
/// an Input as binary.
impl Writeable for Input {
fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ser::Error> {
writer.write_u8(self.features.bits())?;
self.features.write(writer)?;
self.commit.write(writer)?;
Ok(())
}
@ -1083,11 +1092,8 @@ impl Writeable for Input {
/// an Input from a binary stream.
impl Readable for Input {
fn read(reader: &mut dyn Reader) -> Result<Input, ser::Error> {
let features =
OutputFeatures::from_bits(reader.read_u8()?).ok_or(ser::Error::CorruptedData)?;
let features = OutputFeatures::read(reader)?;
let commit = Commitment::read(reader)?;
Ok(Input::new(features, commit))
}
}
@ -1122,14 +1128,31 @@ impl Input {
}
}
bitflags! {
/// Options for block validation
#[derive(Serialize, Deserialize)]
pub struct OutputFeatures: u8 {
/// No flags
const PLAIN = 0;
/// Output is a coinbase output, must not be spent until maturity
const COINBASE = 1;
/// Enum of various supported kernel "features".
enum_from_primitive! {
/// Various flavors of tx kernel.
#[derive(Debug, Clone, Copy, PartialEq, Serialize, Deserialize)]
#[repr(u8)]
pub enum OutputFeatures {
/// Plain output (the default for Grin txs).
Plain = 0,
/// A coinbase output.
Coinbase = 1,
}
}
impl Writeable for OutputFeatures {
fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ser::Error> {
writer.write_u8(*self as u8)?;
Ok(())
}
}
impl Readable for OutputFeatures {
fn read(reader: &mut dyn Reader) -> Result<OutputFeatures, ser::Error> {
let features =
OutputFeatures::from_u8(reader.read_u8()?).ok_or(ser::Error::CorruptedData)?;
Ok(features)
}
}
@ -1164,7 +1187,7 @@ impl ::std::hash::Hash for Output {
/// an Output as binary.
impl Writeable for Output {
fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ser::Error> {
writer.write_u8(self.features.bits())?;
self.features.write(writer)?;
self.commit.write(writer)?;
// The hash of an output doesn't include the range proof, which
// is committed to separately
@ -1179,11 +1202,8 @@ impl Writeable for Output {
/// an Output from a binary stream.
impl Readable for Output {
fn read(reader: &mut dyn Reader) -> Result<Output, ser::Error> {
let features =
OutputFeatures::from_bits(reader.read_u8()?).ok_or(ser::Error::CorruptedData)?;
Ok(Output {
features,
features: OutputFeatures::read(reader)?,
commit: Commitment::read(reader)?,
proof: RangeProof::read(reader)?,
})
@ -1202,12 +1222,12 @@ impl PMMRable for Output {
impl OutputFeatures {
/// Is this a coinbase output?
pub fn is_coinbase(&self) -> bool {
self.contains(OutputFeatures::COINBASE)
*self == OutputFeatures::Coinbase
}
/// Is this a plain output?
pub fn is_plain(&self) -> bool {
!self.contains(OutputFeatures::COINBASE)
*self == OutputFeatures::Plain
}
}
@ -1308,7 +1328,7 @@ impl OutputIdentifier {
pub fn to_hex(&self) -> String {
format!(
"{:b}{}",
self.features.bits(),
self.features as u8,
util::to_hex(self.commit.0.to_vec()),
)
}
@ -1320,7 +1340,7 @@ impl FixedLength for OutputIdentifier {
impl Writeable for OutputIdentifier {
fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ser::Error> {
writer.write_u8(self.features.bits())?;
self.features.write(writer)?;
self.commit.write(writer)?;
Ok(())
}
@ -1328,10 +1348,8 @@ impl Writeable for OutputIdentifier {
impl Readable for OutputIdentifier {
fn read(reader: &mut dyn Reader) -> Result<OutputIdentifier, ser::Error> {
let features =
OutputFeatures::from_bits(reader.read_u8()?).ok_or(ser::Error::CorruptedData)?;
Ok(OutputIdentifier {
features,
features: OutputFeatures::read(reader)?,
commit: Commitment::read(reader)?,
})
}
@ -1358,18 +1376,17 @@ pub fn kernel_sig_msg(
features: KernelFeatures,
) -> Result<secp::Message, Error> {
let valid_features = match features {
KernelFeatures::COINBASE => fee == 0 && lock_height == 0,
KernelFeatures::PLAIN => lock_height == 0,
KernelFeatures::HEIGHT_LOCKED => true,
_ => false,
KernelFeatures::Coinbase => fee == 0 && lock_height == 0,
KernelFeatures::Plain => lock_height == 0,
KernelFeatures::HeightLocked => true,
};
if !valid_features {
return Err(Error::InvalidKernelFeatures);
}
let hash = match features {
KernelFeatures::COINBASE => (features).hash(),
KernelFeatures::PLAIN => (features, fee).hash(),
_ => (features, fee, lock_height).hash(),
KernelFeatures::Coinbase => (features).hash(),
KernelFeatures::Plain => (features, fee).hash(),
KernelFeatures::HeightLocked => (features, fee, lock_height).hash(),
};
Ok(secp::Message::from_slice(&hash.as_bytes())?)
}
@ -1377,9 +1394,9 @@ pub fn kernel_sig_msg(
/// kernel features as determined by lock height
pub fn kernel_features(lock_height: u64) -> KernelFeatures {
if lock_height > 0 {
KernelFeatures::HEIGHT_LOCKED
KernelFeatures::HeightLocked
} else {
KernelFeatures::PLAIN
KernelFeatures::Plain
}
}
@ -1401,7 +1418,7 @@ mod test {
let sig = secp::Signature::from_raw_data(&[0; 64]).unwrap();
let kernel = TxKernel {
features: KernelFeatures::PLAIN,
features: KernelFeatures::Plain,
lock_height: 0,
excess: commit,
excess_sig: sig.clone(),
@ -1411,7 +1428,7 @@ mod test {
let mut vec = vec![];
ser::serialize(&mut vec, &kernel).expect("serialized failed");
let kernel2: TxKernel = ser::deserialize(&mut &vec[..]).unwrap();
assert_eq!(kernel2.features, KernelFeatures::PLAIN);
assert_eq!(kernel2.features, KernelFeatures::Plain);
assert_eq!(kernel2.lock_height, 0);
assert_eq!(kernel2.excess, commit);
assert_eq!(kernel2.excess_sig, sig.clone());
@ -1419,7 +1436,7 @@ mod test {
// now check a kernel with lock_height serialize/deserialize correctly
let kernel = TxKernel {
features: KernelFeatures::HEIGHT_LOCKED,
features: KernelFeatures::HeightLocked,
lock_height: 100,
excess: commit,
excess_sig: sig.clone(),
@ -1429,7 +1446,7 @@ mod test {
let mut vec = vec![];
ser::serialize(&mut vec, &kernel).expect("serialized failed");
let kernel2: TxKernel = ser::deserialize(&mut &vec[..]).unwrap();
assert_eq!(kernel2.features, KernelFeatures::HEIGHT_LOCKED);
assert_eq!(kernel2.features, KernelFeatures::HeightLocked);
assert_eq!(kernel2.lock_height, 100);
assert_eq!(kernel2.excess, commit);
assert_eq!(kernel2.excess_sig, sig.clone());
@ -1456,7 +1473,7 @@ mod test {
let commit = keychain.commit(5, &key_id).unwrap();
let input = Input {
features: OutputFeatures::PLAIN,
features: OutputFeatures::Plain,
commit: commit,
};
@ -1472,11 +1489,27 @@ mod test {
// now generate the short_id for a *very* similar output (single feature flag
// different) and check it generates a different short_id
let input = Input {
features: OutputFeatures::COINBASE,
features: OutputFeatures::Coinbase,
commit: commit,
};
let short_id = input.short_id(&block_hash, nonce);
assert_eq!(short_id, ShortId::from_hex("3f0377c624e9").unwrap());
}
#[test]
fn kernel_features_serialization() {
let features = KernelFeatures::from_u8(0).unwrap();
assert_eq!(features, KernelFeatures::Plain);
let features = KernelFeatures::from_u8(1).unwrap();
assert_eq!(features, KernelFeatures::Coinbase);
let features = KernelFeatures::from_u8(2).unwrap();
assert_eq!(features, KernelFeatures::HeightLocked);
// Verify we cannot deserialize an unexpected kernel feature
let features = KernelFeatures::from_u8(3);
assert_eq!(features, None);
}
}

View file

@ -93,7 +93,7 @@ pub fn genesis_floo() -> core::Block {
..Default::default()
});
let kernel = core::TxKernel {
features: core::KernelFeatures::COINBASE,
features: core::KernelFeatures::Coinbase,
fee: 0,
lock_height: 0,
excess: Commitment::from_vec(
@ -111,7 +111,7 @@ pub fn genesis_floo() -> core::Block {
.unwrap(),
};
let output = core::Output {
features: core::OutputFeatures::COINBASE,
features: core::OutputFeatures::Coinbase,
commit: Commitment::from_vec(
util::from_hex(
"08c12007af16d1ee55fffe92cef808c77e318dae70c3bc70cb6361f49d517f1b68".to_string(),
@ -190,14 +190,14 @@ pub fn genesis_main() -> core::Block {
..Default::default()
});
let kernel = core::TxKernel {
features: core::KernelFeatures::COINBASE,
features: core::KernelFeatures::Coinbase,
fee: 0,
lock_height: 0,
excess: Commitment::from_vec(vec![]), // REPLACE
excess_sig: Signature::from_raw_data(&[0; 64]).unwrap(), //REPLACE
};
let output = core::Output {
features: core::OutputFeatures::COINBASE,
features: core::OutputFeatures::Coinbase,
commit: Commitment::from_vec(vec![]), // REPLACE
proof: RangeProof {
plen: SINGLE_BULLET_PROOF_SIZE,

View file

@ -21,9 +21,9 @@
#![deny(unused_mut)]
#![warn(missing_docs)]
#[macro_use]
extern crate bitflags;
use blake2_rfc as blake2;
#[macro_use]
extern crate enum_primitive;
use grin_keychain as keychain;
use grin_util as util;
#[macro_use]

View file

@ -241,14 +241,14 @@ pub fn verify_partial_sig(
/// let commit = keychain.commit(value, &key_id).unwrap();
/// let rproof = proof::create(&keychain, value, &key_id, commit, None).unwrap();
/// let output = Output {
/// features: OutputFeatures::COINBASE,
/// features: OutputFeatures::Coinbase,
/// commit: commit,
/// proof: rproof,
/// };
/// let height = 20;
/// let over_commit = secp.commit_value(reward(fees)).unwrap();
/// let out_commit = output.commitment();
/// let msg = kernel_sig_msg(0, height, KernelFeatures::HEIGHT_LOCKED).unwrap();
/// let msg = kernel_sig_msg(0, height, KernelFeatures::HeightLocked).unwrap();
/// let excess = secp.commit_sum(vec![out_commit], vec![over_commit]).unwrap();
/// let pubkey = excess.to_pubkey(&secp).unwrap();
/// let sig = aggsig::sign_from_key_id(&secp, &keychain, &msg, value, &key_id, Some(&pubkey)).unwrap();
@ -306,14 +306,14 @@ where
/// let commit = keychain.commit(value, &key_id).unwrap();
/// let rproof = proof::create(&keychain, value, &key_id, commit, None).unwrap();
/// let output = Output {
/// features: OutputFeatures::COINBASE,
/// features: OutputFeatures::Coinbase,
/// commit: commit,
/// proof: rproof,
/// };
/// let height = 20;
/// let over_commit = secp.commit_value(reward(fees)).unwrap();
/// let out_commit = output.commitment();
/// let msg = kernel_sig_msg(0, height, KernelFeatures::HEIGHT_LOCKED).unwrap();
/// let msg = kernel_sig_msg(0, height, KernelFeatures::HeightLocked).unwrap();
/// let excess = secp.commit_sum(vec![out_commit], vec![over_commit]).unwrap();
/// let pubkey = excess.to_pubkey(&secp).unwrap();
/// let sig = aggsig::sign_from_key_id(&secp, &keychain, &msg, value, &key_id, Some(&pubkey)).unwrap();

View file

@ -73,7 +73,7 @@ where
"Building input (spending regular output): {}, {}",
value, key_id
);
build_input(value, OutputFeatures::PLAIN, key_id)
build_input(value, OutputFeatures::Plain, key_id)
}
/// Adds a coinbase input spending a coinbase output.
@ -82,7 +82,7 @@ where
K: Keychain,
{
debug!("Building input (spending coinbase): {}, {}", value, key_id);
build_input(value, OutputFeatures::COINBASE, key_id)
build_input(value, OutputFeatures::Coinbase, key_id)
}
/// Adds an output with the provided value and key identifier from the
@ -101,7 +101,7 @@ where
(
tx.with_output(Output {
features: OutputFeatures::PLAIN,
features: OutputFeatures::Plain,
commit: commit,
proof: rproof,
}),

View file

@ -35,7 +35,7 @@ where
let rproof = proof::create(keychain, value, key_id, commit, None)?;
let output = Output {
features: OutputFeatures::COINBASE,
features: OutputFeatures::Coinbase,
commit: commit,
proof: rproof,
};
@ -49,11 +49,11 @@ where
// NOTE: Remember we sign the fee *and* the lock_height.
// For a coinbase output the fee is 0 and the lock_height is 0
let msg = kernel_sig_msg(0, 0, KernelFeatures::COINBASE)?;
let msg = kernel_sig_msg(0, 0, KernelFeatures::Coinbase)?;
let sig = aggsig::sign_from_key_id(&secp, keychain, &msg, value, &key_id, Some(&pubkey))?;
let proof = TxKernel {
features: KernelFeatures::COINBASE,
features: KernelFeatures::Coinbase,
excess: excess,
excess_sig: sig,
fee: 0,

View file

@ -162,7 +162,7 @@ fn remove_coinbase_output_flag() {
let mut b = new_block(vec![], &keychain, &prev, &key_id);
assert!(b.outputs()[0].is_coinbase());
b.outputs_mut()[0].features = OutputFeatures::PLAIN;
b.outputs_mut()[0].features = OutputFeatures::Plain;
assert_eq!(b.verify_coinbase(), Err(Error::CoinbaseSumMismatch));
assert!(b
@ -184,7 +184,7 @@ fn remove_coinbase_kernel_flag() {
let mut b = new_block(vec![], &keychain, &prev, &key_id);
assert!(b.kernels()[0].is_coinbase());
b.kernels_mut()[0].features = KernelFeatures::PLAIN;
b.kernels_mut()[0].features = KernelFeatures::Plain;
// Flipping the coinbase flag results in kernels not summing correctly.
assert_eq!(

View file

@ -122,7 +122,7 @@ fn build_tx_kernel() {
let kern = &tx.kernels()[0];
kern.verify().unwrap();
assert_eq!(kern.features, KernelFeatures::PLAIN);
assert_eq!(kern.features, KernelFeatures::Plain);
assert_eq!(kern.fee, tx.fee());
}

View file

@ -31,7 +31,7 @@ fn test_output_ser_deser() {
let proof = proof::create(&keychain, 5, &key_id, commit, None).unwrap();
let out = Output {
features: OutputFeatures::PLAIN,
features: OutputFeatures::Plain,
commit: commit,
proof: proof,
};
@ -40,7 +40,7 @@ fn test_output_ser_deser() {
ser::serialize(&mut vec, &out).expect("serialized failed");
let dout: Output = ser::deserialize(&mut &vec[..]).unwrap();
assert_eq!(dout.features, OutputFeatures::PLAIN);
assert_eq!(dout.features, OutputFeatures::Plain);
assert_eq!(dout.commit, out.commit);
assert_eq!(dout.proof, out.proof);
}

View file

@ -38,7 +38,7 @@ fn test_verifier_cache_rangeproofs() {
let proof = proof::create(&keychain, 5, &key_id, commit, None).unwrap();
let out = Output {
features: OutputFeatures::PLAIN,
features: OutputFeatures::Plain,
commit: commit,
proof: proof,
};

View file

@ -45,8 +45,8 @@ pub use self::{testclient::LocalWalletClient, testclient::WalletProxy};
/// Get an output from the chain locally and present it back as an API output
fn get_output_local(chain: &chain::Chain, commit: &pedersen::Commitment) -> Option<api::Output> {
let outputs = [
OutputIdentifier::new(OutputFeatures::PLAIN, commit),
OutputIdentifier::new(OutputFeatures::COINBASE, commit),
OutputIdentifier::new(OutputFeatures::Plain, commit),
OutputIdentifier::new(OutputFeatures::Coinbase, commit),
];
for x in outputs.iter() {

View file

@ -26,7 +26,7 @@ use grin_wallet as wallet;
use rand::thread_rng;
fn kernel_sig_msg() -> secp::Message {
transaction::kernel_sig_msg(0, 0, transaction::KernelFeatures::PLAIN).unwrap()
transaction::kernel_sig_msg(0, 0, transaction::KernelFeatures::Plain).unwrap()
}
#[test]