[Floonet] add feature for height locked kernels (#2168)

* add feature for height locked kernels

* add function to compute kernel features appropriate for lock height, and use it

* only sign kernel-features relevant fields; refactor Features

* simplify invalid kernel logic

* remove unused height arg to reward::output and run some rustfmt

* replace nested if/else by match

core/src/core/block.rs

@@ -28,7 +28,7 @@ use crate::core::compact_block::{CompactBlock, CompactBlockBody};
 use crate::core::hash::{Hash, Hashed, ZERO_HASH};
 use crate::core::verifier_cache::VerifierCache;
 use crate::core::{
-	transaction, Commitment, Input, KernelFeatures, Output, OutputFeatures, Transaction,
+	transaction, Commitment, Input, Output, Transaction,
 	TransactionBody, TxKernel,
 };
 use crate::global;
@@ -663,14 +663,14 @@ impl Block {
 			.body
 			.outputs
 			.iter()
-			.filter(|out| out.features.contains(OutputFeatures::COINBASE_OUTPUT))
+			.filter(|out| out.is_coinbase())
 			.collect::<Vec<&Output>>();
 
 		let cb_kerns = self
 			.body
 			.kernels
 			.iter()
-			.filter(|kernel| kernel.features.contains(KernelFeatures::COINBASE_KERNEL))
+			.filter(|kernel| kernel.is_coinbase())
 			.collect::<Vec<&TxKernel>>();
 
 		{

core/src/core/compact_block.rs

@@ -19,7 +19,7 @@ use rand::{thread_rng, Rng};
 use crate::core::block::{Block, BlockHeader, Error};
 use crate::core::hash::Hashed;
 use crate::core::id::ShortIdentifiable;
-use crate::core::{KernelFeatures, Output, OutputFeatures, ShortId, TxKernel};
+use crate::core::{Output, ShortId, TxKernel};
 use crate::ser::{self, read_multi, Readable, Reader, VerifySortedAndUnique, Writeable, Writer};
 
 /// Container for full (full) outputs and kernels and kern_ids for a compact block.
@@ -168,7 +168,7 @@ impl From<Block> for CompactBlock {
 		let out_full = block
 			.outputs()
 			.iter()
-			.filter(|x| x.features.contains(OutputFeatures::COINBASE_OUTPUT))
+			.filter(|x| x.is_coinbase())
 			.cloned()
 			.collect::<Vec<_>>();
 
@@ -176,7 +176,7 @@ impl From<Block> for CompactBlock {
 		let mut kern_ids = vec![];
 
 		for k in block.kernels() {
-			if k.features.contains(KernelFeatures::COINBASE_KERNEL) {
+			if k.is_coinbase() {
 				kern_full.push(k.clone());
 			} else {
 				kern_ids.push(k.short_id(&header.hash(), nonce));

core/src/core/transaction.rs

@@ -40,10 +40,12 @@ bitflags! {
 	/// Options for a kernel's structure or use
 	#[derive(Serialize, Deserialize)]
 	pub struct KernelFeatures: u8 {
-		/// No flags
-		const DEFAULT_KERNEL = 0b00000000;
-		/// Kernel matching a coinbase output
-		const COINBASE_KERNEL = 0b00000001;
+		/// 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;
 	}
 }
 
@@ -202,7 +204,39 @@ impl PMMRable for TxKernel {
 	}
 }
 
+impl KernelFeatures {
+	/// Is this a coinbase kernel?
+	pub fn is_coinbase(&self) -> bool {
+		self.contains(KernelFeatures::COINBASE)
+	}
+
+	/// Is this a plain kernel?
+	pub fn is_plain(&self) -> bool {
+		!self.is_coinbase() && !self.is_height_locked()
+	}
+
+	/// Is this a height locked kernel?
+	pub fn is_height_locked(&self) -> bool {
+		self.contains(KernelFeatures::HEIGHT_LOCKED)
+	}
+}
+
 impl TxKernel {
+	/// Is this a coinbase kernel?
+	pub fn is_coinbase(&self) -> bool {
+		self.features.is_coinbase()
+	}
+
+	/// Is this a plain kernel?
+	pub fn is_plain(&self) -> bool {
+		self.features.is_plain()
+	}
+
+	/// Is this a height locked kernel?
+	pub fn is_height_locked(&self) -> bool {
+		self.features.is_height_locked()
+	}
+
 	/// Return the excess commitment for this tx_kernel.
 	pub fn excess(&self) -> Commitment {
 		self.excess
@@ -219,6 +253,10 @@ impl TxKernel {
 	/// as a public key and checking the signature verifies with the fee as
 	/// message.
 	pub fn verify(&self) -> Result<(), Error> {
+		if self.is_coinbase() && self.fee != 0 || !self.is_height_locked() && self.lock_height != 0
+		{
+			return Err(Error::InvalidKernelFeatures);
+		}
 		let secp = static_secp_instance();
 		let secp = secp.lock();
 		let sig = &self.excess_sig;
@@ -242,7 +280,7 @@ impl TxKernel {
 	/// Build an empty tx kernel with zero values.
 	pub fn empty() -> TxKernel {
 		TxKernel {
-			features: KernelFeatures::DEFAULT_KERNEL,
+			features: KernelFeatures::PLAIN,
 			fee: 0,
 			lock_height: 0,
 			excess: Commitment::from_vec(vec![0; 33]),
@@ -258,6 +296,7 @@ impl TxKernel {
 	/// Builds a new tx kernel with the provided lock_height.
 	pub fn with_lock_height(self, lock_height: u64) -> TxKernel {
 		TxKernel {
+			features: kernel_features(lock_height),
 			lock_height,
 			..self
 		}
@@ -583,25 +622,17 @@ impl TransactionBody {
 		Ok(())
 	}
 
-	// Verify we have no outputs tagged as COINBASE_OUTPUT.
+	// Verify we have no outputs tagged as COINBASE.
 	fn verify_output_features(&self) -> Result<(), Error> {
-		if self
-			.outputs
-			.iter()
-			.any(|x| x.features.contains(OutputFeatures::COINBASE_OUTPUT))
-		{
+		if self.outputs.iter().any(|x| x.is_coinbase()) {
 			return Err(Error::InvalidOutputFeatures);
 		}
 		Ok(())
 	}
 
-	// Verify we have no kernels tagged as COINBASE_KERNEL.
+	// Verify we have no kernels tagged as COINBASE.
 	fn verify_kernel_features(&self) -> Result<(), Error> {
-		if self
-			.kernels
-			.iter()
-			.any(|x| x.features.contains(KernelFeatures::COINBASE_KERNEL))
-		{
+		if self.kernels.iter().any(|x| x.is_coinbase()) {
 			return Err(Error::InvalidKernelFeatures);
 		}
 		Ok(())
@@ -1078,6 +1109,16 @@ impl Input {
 	pub fn commitment(&self) -> Commitment {
 		self.commit
 	}
+
+	/// Is this a coinbase input?
+	pub fn is_coinbase(&self) -> bool {
+		self.features.is_coinbase()
+	}
+
+	/// Is this a plain input?
+	pub fn is_plain(&self) -> bool {
+		self.features.is_plain()
+	}
 }
 
 bitflags! {
@@ -1085,9 +1126,9 @@ bitflags! {
 	#[derive(Serialize, Deserialize)]
 	pub struct OutputFeatures: u8 {
 		/// No flags
-		const DEFAULT_OUTPUT = 0b00000000;
+		const PLAIN    = 0;
 		/// Output is a coinbase output, must not be spent until maturity
-		const COINBASE_OUTPUT = 0b00000001;
+		const COINBASE = 1;
 	}
 }
 
@@ -1157,12 +1198,34 @@ impl PMMRable for Output {
 	}
 }
 
+impl OutputFeatures {
+	/// Is this a coinbase output?
+	pub fn is_coinbase(&self) -> bool {
+		self.contains(OutputFeatures::COINBASE)
+	}
+
+	/// Is this a plain output?
+	pub fn is_plain(&self) -> bool {
+		!self.contains(OutputFeatures::COINBASE)
+	}
+}
+
 impl Output {
 	/// Commitment for the output
 	pub fn commitment(&self) -> Commitment {
 		self.commit
 	}
 
+	/// Is this a coinbase kernel?
+	pub fn is_coinbase(&self) -> bool {
+		self.features.is_coinbase()
+	}
+
+	/// Is this a plain kernel?
+	pub fn is_plain(&self) -> bool {
+		self.features.is_plain()
+	}
+
 	/// Range proof for the output
 	pub fn proof(&self) -> RangeProof {
 		self.proof
@@ -1288,25 +1351,43 @@ impl From<Output> for OutputIdentifier {
 /// to produce a 32 byte message to sign.
 ///
 /// testnet4: msg = (fee || lock_height)
-/// mainnet:  msg = hash(fee || lock_height || features)
+/// mainnet:  msg = hash(features)                       for coinbase kernels
+///                 hash(features || fee)                for plain kernels
+///                 hash(features || fee || lock_height) for height locked kernels
 ///
 pub fn kernel_sig_msg(
 	fee: u64,
 	lock_height: u64,
 	features: KernelFeatures,
 ) -> Result<secp::Message, Error> {
+	if features.is_coinbase() && fee != 0 || !features.is_height_locked() && lock_height != 0 {
+		return Err(Error::InvalidKernelFeatures);
+	}
 	let msg = if global::is_testnet() {
 		let mut bytes = [0; 32];
 		BigEndian::write_u64(&mut bytes[16..24], fee);
 		BigEndian::write_u64(&mut bytes[24..], lock_height);
 		secp::Message::from_slice(&bytes)?
 	} else {
-		let hash = (fee, lock_height, features).hash();
+		let hash = match features {
+			KernelFeatures::COINBASE => (features).hash(),
+			KernelFeatures::PLAIN => (features, fee).hash(),
+			_ => (features, fee, lock_height).hash(),
+		};
 		secp::Message::from_slice(&hash.as_bytes())?
 	};
 	Ok(msg)
 }
 
+/// kernel features as determined by lock height
+pub fn kernel_features(lock_height: u64) -> KernelFeatures {
+	if lock_height > 0 {
+		KernelFeatures::HEIGHT_LOCKED
+	} else {
+		KernelFeatures::PLAIN
+	}
+}
+
 #[cfg(test)]
 mod test {
 	use super::*;
@@ -1325,7 +1406,7 @@ mod test {
 		let sig = secp::Signature::from_raw_data(&[0; 64]).unwrap();
 
 		let kernel = TxKernel {
-			features: KernelFeatures::DEFAULT_KERNEL,
+			features: KernelFeatures::PLAIN,
 			lock_height: 0,
 			excess: commit,
 			excess_sig: sig.clone(),
@@ -1335,7 +1416,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::DEFAULT_KERNEL);
+		assert_eq!(kernel2.features, KernelFeatures::PLAIN);
 		assert_eq!(kernel2.lock_height, 0);
 		assert_eq!(kernel2.excess, commit);
 		assert_eq!(kernel2.excess_sig, sig.clone());
@@ -1343,7 +1424,7 @@ mod test {
 
 		// now check a kernel with lock_height serialize/deserialize correctly
 		let kernel = TxKernel {
-			features: KernelFeatures::DEFAULT_KERNEL,
+			features: KernelFeatures::HEIGHT_LOCKED,
 			lock_height: 100,
 			excess: commit,
 			excess_sig: sig.clone(),
@@ -1353,7 +1434,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::DEFAULT_KERNEL);
+		assert_eq!(kernel2.features, KernelFeatures::HEIGHT_LOCKED);
 		assert_eq!(kernel2.lock_height, 100);
 		assert_eq!(kernel2.excess, commit);
 		assert_eq!(kernel2.excess_sig, sig.clone());
@@ -1380,7 +1461,7 @@ mod test {
 		let commit = keychain.commit(5, &key_id).unwrap();
 
 		let input = Input {
-			features: OutputFeatures::DEFAULT_OUTPUT,
+			features: OutputFeatures::PLAIN,
 			commit: commit,
 		};
 
@@ -1396,7 +1477,7 @@ 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_OUTPUT,
+			features: OutputFeatures::COINBASE,
 			commit: commit,
 		};
 

core/src/libtx/aggsig.rs

@@ -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_OUTPUT,
+///		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::DEFAULT_KERNEL).unwrap();
+/// let msg = kernel_sig_msg(0, height, KernelFeatures::HEIGHT_LOCKED).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();
@@ -315,14 +315,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_OUTPUT,
+///		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::DEFAULT_KERNEL).unwrap();
+/// let msg = kernel_sig_msg(0, height, KernelFeatures::HEIGHT_LOCKED).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();

core/src/libtx/build.rs

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

core/src/libtx/reward.rs

@@ -27,7 +27,6 @@ pub fn output<K>(
 	keychain: &K,
 	key_id: &Identifier,
 	fees: u64,
-	height: u64,
 ) -> Result<(Output, TxKernel), Error>
 where
 	K: Keychain,
@@ -40,7 +39,7 @@ where
 	let rproof = proof::create(keychain, value, key_id, commit, None)?;
 
 	let output = Output {
-		features: OutputFeatures::COINBASE_OUTPUT,
+		features: OutputFeatures::COINBASE,
 		commit: commit,
 		proof: rproof,
 	};
@@ -53,22 +52,18 @@ where
 	let pubkey = excess.to_pubkey(&secp)?;
 
 	// NOTE: Remember we sign the fee *and* the lock_height.
-	// For a coinbase output the fee is 0 and the lock_height is
-	// the lock_height of the coinbase output itself,
-	// not the lock_height of the tx (there is no tx for a coinbase output).
-	// This output will not be spendable earlier than lock_height (and we sign this
-	// here).
-	let msg = kernel_sig_msg(0, height, KernelFeatures::COINBASE_KERNEL)?;
+	// For a coinbase output the fee is 0 and the lock_height is 0
+	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_KERNEL,
+		features: KernelFeatures::COINBASE,
 		excess: excess,
 		excess_sig: sig,
 		fee: 0,
-		// lock_height here is the height of the block (tx should be valid immediately)
-		// *not* the lock_height of the coinbase output (only spendable 1,000 blocks later)
-		lock_height: height,
+		// lock_height here is 0
+		// *not* the maturity of the coinbase output (only spendable 1,440 blocks later)
+		lock_height: 0,
 	};
 	Ok((output, proof))
 }

core/src/libtx/slate.rs

@@ -16,10 +16,10 @@
 //! around during an interactive wallet exchange
 
 use crate::blake2::blake2b::blake2b;
-use crate::core::committed::Committed;
-use crate::core::transaction::{kernel_sig_msg, KernelFeatures, Transaction};
-use crate::core::verifier_cache::LruVerifierCache;
 use crate::core::amount_to_hr_string;
+use crate::core::committed::Committed;
+use crate::core::transaction::{kernel_features, kernel_sig_msg, Transaction};
+use crate::core::verifier_cache::LruVerifierCache;
 use crate::keychain::{BlindSum, BlindingFactor, Keychain};
 use crate::libtx::error::{Error, ErrorKind};
 use crate::libtx::{aggsig, build, tx_fee};
@@ -160,8 +160,7 @@ impl Slate {
 	// Currently includes the fee and the lock_height.
 	fn msg_to_sign(&self) -> Result<secp::Message, Error> {
 		// Currently we only support interactively creating a tx with a "default" kernel.
-		let features = KernelFeatures::DEFAULT_KERNEL;
-
+		let features = kernel_features(self.lock_height);
 		let msg = kernel_sig_msg(self.fee, self.lock_height, features)?;
 		Ok(msg)
 	}