Sign kernel features (#2104)

* include kernel features in msg being signed
hash the msg before signing it (for consistent 32 bytes)

* rustfmt

* fix various tests

* no HF for this (mainnet only)

.gitignore

@@ -11,3 +11,4 @@ grin.log
 wallet.seed
 test_output
 wallet_data
+wallet/db

core/src/core/hash.rs

@@ -34,7 +34,7 @@ pub const ZERO_HASH: Hash = Hash([0; 32]);
 /// A hash to uniquely (or close enough) identify one of the main blockchain
 /// constructs. Used pervasively for blocks, transactions and outputs.
 #[derive(Copy, Clone, PartialEq, PartialOrd, Eq, Ord, Hash, Serialize, Deserialize)]
-pub struct Hash(pub [u8; 32]);
+pub struct Hash([u8; 32]);
 
 impl fmt::Debug for Hash {
 	fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
@@ -71,6 +71,11 @@ impl Hash {
 		self.0.to_vec()
 	}
 
+	/// Returns a byte slice of the hash contents.
+	pub fn as_bytes(&self) -> &[u8] {
+		&self.0
+	}
+
 	/// Convert a hash to hex string format.
 	pub fn to_hex(&self) -> String {
 		util::to_hex(self.to_vec())

core/src/core/id.rs

@@ -51,8 +51,8 @@ impl<H: Hashed> ShortIdentifiable for H {
 		use std::hash::Hasher;
 
 		// extract k0/k1 from the block_hash
-		let k0 = LittleEndian::read_u64(&hash_with_nonce.0[0..8]);
-		let k1 = LittleEndian::read_u64(&hash_with_nonce.0[8..16]);
+		let k0 = LittleEndian::read_u64(&hash_with_nonce.as_bytes()[0..8]);
+		let k1 = LittleEndian::read_u64(&hash_with_nonce.as_bytes()[8..16]);
 
 		// initialize a siphasher24 with k0/k1
 		let mut sip_hasher = SipHasher24::new_with_keys(k0, k1);

core/src/core/transaction.rs

@@ -18,6 +18,7 @@ use crate::consensus;
 use crate::core::hash::Hashed;
 use crate::core::verifier_cache::VerifierCache;
 use crate::core::{committed, Committed};
+use crate::global;
 use crate::keychain::{self, BlindingFactor};
 use crate::ser::{
 	self, read_multi, FixedLength, PMMRable, Readable, Reader, VerifySortedAndUnique, Writeable,
@@ -46,6 +47,13 @@ bitflags! {
 	}
 }
 
+impl Writeable for KernelFeatures {
+	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ser::Error> {
+		writer.write_u8(self.bits())?;
+		Ok(())
+	}
+}
+
 /// Errors thrown by Transaction validation
 #[derive(Clone, Eq, Debug, PartialEq)]
 pub enum Error {
@@ -163,13 +171,9 @@ impl ::std::hash::Hash for TxKernel {
 
 impl Writeable for TxKernel {
 	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ser::Error> {
-		ser_multiwrite!(
-			writer,
-			[write_u8, self.features.bits()],
-			[write_u64, self.fee],
-			[write_u64, self.lock_height],
-			[write_fixed_bytes, &self.excess]
-		);
+		self.features.write(writer)?;
+		ser_multiwrite!(writer, [write_u64, self.fee], [write_u64, self.lock_height]);
+		self.excess.write(writer)?;
 		self.excess_sig.write(writer)?;
 		Ok(())
 	}
@@ -207,7 +211,7 @@ impl TxKernel {
 	/// The msg signed as part of the tx kernel.
 	/// Consists of the fee and the lock_height.
 	pub fn msg_to_sign(&self) -> Result<secp::Message, Error> {
-		let msg = kernel_sig_msg(self.fee, self.lock_height)?;
+		let msg = kernel_sig_msg(self.fee, self.lock_height, self.features)?;
 		Ok(msg)
 	}
 
@@ -272,29 +276,14 @@ pub struct TxKernelEntry {
 
 impl Writeable for TxKernelEntry {
 	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ser::Error> {
-		ser_multiwrite!(
-			writer,
-			[write_u8, self.kernel.features.bits()],
-			[write_u64, self.kernel.fee],
-			[write_u64, self.kernel.lock_height],
-			[write_fixed_bytes, &self.kernel.excess]
-		);
-		self.kernel.excess_sig.write(writer)?;
+		self.kernel.write(writer)?;
 		Ok(())
 	}
 }
 
 impl Readable for TxKernelEntry {
-	fn read(reader: &mut dyn Reader) -> Result<TxKernelEntry, ser::Error> {
-		let features =
-			KernelFeatures::from_bits(reader.read_u8()?).ok_or(ser::Error::CorruptedData)?;
-		let kernel = TxKernel {
-			features: features,
-			fee: reader.read_u64()?,
-			lock_height: reader.read_u64()?,
-			excess: Commitment::read(reader)?,
-			excess_sig: secp::Signature::read(reader)?,
-		};
+	fn read(reader: &mut Reader) -> Result<TxKernelEntry, ser::Error> {
+		let kernel = TxKernel::read(reader)?;
 		Ok(TxKernelEntry { kernel })
 	}
 }
@@ -1293,12 +1282,28 @@ impl From<Output> for OutputIdentifier {
 	}
 }
 
-/// Construct msg from tx fee and lock_height.
-pub fn kernel_sig_msg(fee: u64, lock_height: u64) -> Result<secp::Message, Error> {
-	let mut bytes = [0; 32];
-	BigEndian::write_u64(&mut bytes[16..24], fee);
-	BigEndian::write_u64(&mut bytes[24..], lock_height);
-	let msg = secp::Message::from_slice(&bytes)?;
+/// Construct msg from tx fee, lock_height and kernel features.
+/// In testnet4 we did not include the kernel features in the message being signed.
+/// In mainnet we changed this to include features and we hash (fee || lock_height || features)
+/// to produce a 32 byte message to sign.
+///
+/// testnet4: msg = (fee || lock_height)
+/// mainnet:  msg = hash(fee || lock_height || features)
+///
+pub fn kernel_sig_msg(
+	fee: u64,
+	lock_height: u64,
+	features: KernelFeatures,
+) -> Result<secp::Message, Error> {
+	let msg = if global::is_mainnet() {
+		let hash = (fee, lock_height, features).hash();
+		secp::Message::from_slice(&hash.as_bytes())?
+	} else {
+		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)?
+	};
 	Ok(msg)
 }
 

core/src/libtx/aggsig.rs

@@ -229,7 +229,7 @@ pub fn verify_partial_sig(
 /// use util::secp::key::{PublicKey, SecretKey};
 /// use util::secp::{ContextFlag, Secp256k1};
 /// use core::libtx::{aggsig, proof};
-/// use core::core::transaction::kernel_sig_msg;
+/// use core::core::transaction::{kernel_sig_msg, KernelFeatures};
 /// use core::core::{Output, OutputFeatures};
 /// use keychain::{Keychain, ExtKeychain};
 ///
@@ -248,7 +248,7 @@ pub fn verify_partial_sig(
 /// let height = 20;
 /// let over_commit = secp.commit_value(reward(fees)).unwrap();
 /// let out_commit = output.commitment();
-/// let msg = kernel_sig_msg(0, height).unwrap();
+/// let msg = kernel_sig_msg(0, height, KernelFeatures::DEFAULT_KERNEL).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, &key_id, Some(&pubkey)).unwrap();
@@ -301,7 +301,7 @@ where
 /// use core::libtx::{aggsig, proof};
 /// use util::secp::key::{PublicKey, SecretKey};
 /// use util::secp::{ContextFlag, Secp256k1};
-/// use core::core::transaction::kernel_sig_msg;
+/// use core::core::transaction::{kernel_sig_msg, KernelFeatures};
 /// use core::core::{Output, OutputFeatures};
 /// use keychain::{Keychain, ExtKeychain};
 ///
@@ -321,7 +321,7 @@ where
 /// let height = 20;
 /// let over_commit = secp.commit_value(reward(fees)).unwrap();
 /// let out_commit = output.commitment();
-/// let msg = kernel_sig_msg(0, height).unwrap();
+/// let msg = kernel_sig_msg(0, height, KernelFeatures::DEFAULT_KERNEL).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, &key_id, Some(&pubkey)).unwrap();

core/src/libtx/reward.rs

@@ -58,7 +58,7 @@ where
 	// 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)?;
+	let msg = kernel_sig_msg(0, height, KernelFeatures::COINBASE_KERNEL)?;
 	let sig = aggsig::sign_from_key_id(&secp, keychain, &msg, &key_id, Some(&pubkey))?;
 
 	let proof = TxKernel {

core/src/libtx/slate.rs

@@ -17,9 +17,9 @@
 
 use crate::blake2::blake2b::blake2b;
 use crate::core::committed::Committed;
-use crate::core::transaction::kernel_sig_msg;
+use crate::core::transaction::{kernel_sig_msg, KernelFeatures, Transaction};
 use crate::core::verifier_cache::LruVerifierCache;
-use crate::core::{amount_to_hr_string, Transaction};
+use crate::core::amount_to_hr_string;
 use crate::keychain::{BlindSum, BlindingFactor, Keychain};
 use crate::libtx::error::{Error, ErrorKind};
 use crate::libtx::{aggsig, build, tx_fee};
@@ -159,7 +159,10 @@ impl Slate {
 	// This is the msg that we will sign as part of the tx kernel.
 	// Currently includes the fee and the lock_height.
 	fn msg_to_sign(&self) -> Result<secp::Message, Error> {
-		let msg = kernel_sig_msg(self.fee, self.lock_height)?;
+		// Currently we only support interactively creating a tx with a "default" kernel.
+		let features = KernelFeatures::DEFAULT_KERNEL;
+
+		let msg = kernel_sig_msg(self.fee, self.lock_height, features)?;
 		Ok(msg)
 	}
 

core/tests/block.rs

@@ -18,6 +18,7 @@ use crate::core::consensus::{BLOCK_OUTPUT_WEIGHT, MAX_BLOCK_WEIGHT};
 use crate::core::core::block::Error;
 use crate::core::core::hash::Hashed;
 use crate::core::core::id::ShortIdentifiable;
+use crate::core::core::transaction;
 use crate::core::core::verifier_cache::{LruVerifierCache, VerifierCache};
 use crate::core::core::Committed;
 use crate::core::core::{Block, BlockHeader, CompactBlock, KernelFeatures, OutputFeatures};
@@ -193,14 +194,17 @@ fn remove_coinbase_kernel_flag() {
 		.features
 		.remove(KernelFeatures::COINBASE_KERNEL);
 
+	// Flipping the coinbase flag results in kernels not summing correctly.
 	assert_eq!(
 		b.verify_coinbase(),
 		Err(Error::Secp(secp::Error::IncorrectCommitSum))
 	);
 
+	// Also results in the block no longer validating correctly
+	// because the message being signed on each tx kernel includes the kernel features.
 	assert_eq!(
 		b.validate(&BlindingFactor::zero(), verifier_cache()),
-		Err(Error::Secp(secp::Error::IncorrectCommitSum))
+		Err(Error::Transaction(transaction::Error::IncorrectSignature))
 	);
 }
 

wallet/tests/libwallet.rs

@@ -12,7 +12,7 @@
 // limitations under the License.
 
 //! core::libtx specific tests
-use self::core::core::transaction::kernel_sig_msg;
+use self::core::core::transaction;
 use self::core::libtx::{aggsig, proof};
 use self::keychain::{BlindSum, BlindingFactor, ExtKeychain, Keychain};
 use self::util::secp;
@@ -25,6 +25,10 @@ use grin_util as util;
 use grin_wallet as wallet;
 use rand::thread_rng;
 
+fn kernel_sig_msg() -> secp::Message {
+	transaction::kernel_sig_msg(0, 0, transaction::KernelFeatures::DEFAULT_KERNEL).unwrap()
+}
+
 #[test]
 fn aggsig_sender_receiver_interaction() {
 	let sender_keychain = ExtKeychain::from_random_seed().unwrap();
@@ -105,7 +109,7 @@ fn aggsig_sender_receiver_interaction() {
 		)
 		.unwrap();
 
-		let msg = kernel_sig_msg(0, 0).unwrap();
+		let msg = kernel_sig_msg();
 		let sig_part = aggsig::calculate_partial_sig(
 			&keychain.secp(),
 			&rx_cx.sec_key,
@@ -122,7 +126,7 @@ fn aggsig_sender_receiver_interaction() {
 	// received in the response back from the receiver
 	{
 		let keychain = sender_keychain.clone();
-		let msg = kernel_sig_msg(0, 0).unwrap();
+		let msg = kernel_sig_msg();
 		let sig_verifies = aggsig::verify_partial_sig(
 			&keychain.secp(),
 			&rx_sig_part,
@@ -137,7 +141,7 @@ fn aggsig_sender_receiver_interaction() {
 	// now sender signs with their key
 	let sender_sig_part = {
 		let keychain = sender_keychain.clone();
-		let msg = kernel_sig_msg(0, 0).unwrap();
+		let msg = kernel_sig_msg();
 		let sig_part = aggsig::calculate_partial_sig(
 			&keychain.secp(),
 			&s_cx.sec_key,
@@ -154,7 +158,7 @@ fn aggsig_sender_receiver_interaction() {
 	// received by the sender
 	{
 		let keychain = receiver_keychain.clone();
-		let msg = kernel_sig_msg(0, 0).unwrap();
+		let msg = kernel_sig_msg();
 		let sig_verifies = aggsig::verify_partial_sig(
 			&keychain.secp(),
 			&sender_sig_part,
@@ -170,7 +174,7 @@ fn aggsig_sender_receiver_interaction() {
 	let (final_sig, final_pubkey) = {
 		let keychain = receiver_keychain.clone();
 
-		let msg = kernel_sig_msg(0, 0).unwrap();
+		let msg = kernel_sig_msg();
 		let our_sig_part = aggsig::calculate_partial_sig(
 			&keychain.secp(),
 			&rx_cx.sec_key,
@@ -205,7 +209,7 @@ fn aggsig_sender_receiver_interaction() {
 	// Receiver checks the final signature verifies
 	{
 		let keychain = receiver_keychain.clone();
-		let msg = kernel_sig_msg(0, 0).unwrap();
+		let msg = kernel_sig_msg();
 
 		// Receiver check the final signature verifies
 		let sig_verifies = aggsig::verify_completed_sig(
@@ -221,7 +225,7 @@ fn aggsig_sender_receiver_interaction() {
 	// Check we can verify the sig using the kernel excess
 	{
 		let keychain = ExtKeychain::from_random_seed().unwrap();
-		let msg = kernel_sig_msg(0, 0).unwrap();
+		let msg = kernel_sig_msg();
 		let sig_verifies =
 			aggsig::verify_single_from_commit(&keychain.secp(), &final_sig, &msg, &kernel_excess);
 
@@ -321,7 +325,7 @@ fn aggsig_sender_receiver_interaction_offset() {
 		)
 		.unwrap();
 
-		let msg = kernel_sig_msg(0, 0).unwrap();
+		let msg = kernel_sig_msg();
 		let sig_part = aggsig::calculate_partial_sig(
 			&keychain.secp(),
 			&rx_cx.sec_key,
@@ -338,7 +342,7 @@ fn aggsig_sender_receiver_interaction_offset() {
 	// received in the response back from the receiver
 	{
 		let keychain = sender_keychain.clone();
-		let msg = kernel_sig_msg(0, 0).unwrap();
+		let msg = kernel_sig_msg();
 		let sig_verifies = aggsig::verify_partial_sig(
 			&keychain.secp(),
 			&sig_part,
@@ -353,7 +357,7 @@ fn aggsig_sender_receiver_interaction_offset() {
 	// now sender signs with their key
 	let sender_sig_part = {
 		let keychain = sender_keychain.clone();
-		let msg = kernel_sig_msg(0, 0).unwrap();
+		let msg = kernel_sig_msg();
 		let sig_part = aggsig::calculate_partial_sig(
 			&keychain.secp(),
 			&s_cx.sec_key,
@@ -370,7 +374,7 @@ fn aggsig_sender_receiver_interaction_offset() {
 	// received by the sender
 	{
 		let keychain = receiver_keychain.clone();
-		let msg = kernel_sig_msg(0, 0).unwrap();
+		let msg = kernel_sig_msg();
 		let sig_verifies = aggsig::verify_partial_sig(
 			&keychain.secp(),
 			&sender_sig_part,
@@ -385,7 +389,7 @@ fn aggsig_sender_receiver_interaction_offset() {
 	// Receiver now builds final signature from sender and receiver parts
 	let (final_sig, final_pubkey) = {
 		let keychain = receiver_keychain.clone();
-		let msg = kernel_sig_msg(0, 0).unwrap();
+		let msg = kernel_sig_msg();
 		let our_sig_part = aggsig::calculate_partial_sig(
 			&keychain.secp(),
 			&rx_cx.sec_key,
@@ -420,7 +424,7 @@ fn aggsig_sender_receiver_interaction_offset() {
 	// Receiver checks the final signature verifies
 	{
 		let keychain = receiver_keychain.clone();
-		let msg = kernel_sig_msg(0, 0).unwrap();
+		let msg = kernel_sig_msg();
 
 		// Receiver check the final signature verifies
 		let sig_verifies = aggsig::verify_completed_sig(
@@ -436,7 +440,7 @@ fn aggsig_sender_receiver_interaction_offset() {
 	// Check we can verify the sig using the kernel excess
 	{
 		let keychain = ExtKeychain::from_random_seed().unwrap();
-		let msg = kernel_sig_msg(0, 0).unwrap();
+		let msg = kernel_sig_msg();
 		let sig_verifies =
 			aggsig::verify_single_from_commit(&keychain.secp(), &final_sig, &msg, &kernel_excess);