fix kernel features

libwallet/src/contract/actions/README.md

@@ -41,9 +41,9 @@ Separating side effects until the 'save_step' part would make these functions mu
    races a transaction already in the mempool.
  - Add a Grin node API for kernel lookup by MMR index, then use the witness index instead
    of searching by commitment.
+ - Expose the kernel feature constants from `grin_core` and reuse them here.
  - Move payment-proof creation and verification onto `Slate` so it can also be used
    outside contracts and is versioned with the slate format.
- - Preserve kernel features and their arguments across V4 and V5; see #793.
  - Choose the sender derivation path explicitly and keep encrypted outgoing files
    identifiable locally.
  - Add contract history, lookup and transport.

libwallet/src/lib.rs

@@ -58,7 +58,7 @@ pub mod slatepack;
 mod types;
 
 pub use crate::error::Error;
-pub use crate::slate::{ParticipantData, Slate, SlateState};
+pub use crate::slate::{KernelFeaturesArgs, ParticipantData, Slate, SlateState};
 pub use crate::slate_versions::v4::sig_is_blank;
 pub use crate::slate_versions::{
 	SlateVersion, VersionedBinSlate, VersionedCoinbase, VersionedSlate, CURRENT_SLATE_VERSION,

libwallet/src/slate.rs

@@ -399,6 +399,37 @@ impl Slate {
 		}
 	}
 
+	/// Create a new slate with the provided kernel features
+	pub fn blank_with_kernel_features(
+		num_participants: u8,
+		is_invoice: bool,
+		kernel_features: KernelFeatures,
+	) -> Result<Slate, Error> {
+		let feature = kernel_features.as_u8();
+		let (fee_fields, kernel_features_args) = match kernel_features {
+			KernelFeatures::Plain { fee } => (fee, None),
+			KernelFeatures::Coinbase => return Err(Error::InvalidKernelFeatures(feature)),
+			KernelFeatures::HeightLocked { fee, lock_height } => {
+				(fee, Some(KernelFeaturesArgs { lock_height }))
+			}
+			KernelFeatures::NoRecentDuplicate {
+				fee,
+				relative_height,
+			} => (
+				fee,
+				Some(KernelFeaturesArgs {
+					lock_height: relative_height.into(),
+				}),
+			),
+		};
+		let mut slate = Slate::blank(num_participants, is_invoice);
+		slate.fee_fields = fee_fields;
+		slate.kernel_features = feature;
+		slate.kernel_features_args = kernel_features_args;
+		slate.update_kernel()?;
+		Ok(slate)
+	}
+
 	/// Removes any signature data that isn't mine, for compacting
 	/// slates for a return journey
 	// TODO: Check if this is a noop when we have only 2 parties. The first sig appears at
@@ -1121,6 +1152,8 @@ pub fn tx_from_slate_v5(slate: &SlateV5) -> Option<Transaction> {
 	let secp = secp.lock();
 	let mut calc_slate = Slate::blank(2, false);
 	calc_slate.fee_fields = slate.fee;
+	calc_slate.kernel_features = slate.feat;
+	calc_slate.kernel_features_args = slate.feat_args.as_ref().map(KernelFeaturesArgs::from);
 	for d in slate.sigs.iter() {
 		calc_slate.participant_data.push(ParticipantData {
 			public_blind_excess: d.xs,
@@ -1137,17 +1170,7 @@ pub fn tx_from_slate_v5(slate: &SlateV5) -> Option<Transaction> {
 		Err(_) => Signature::from_raw_data(&[0; 64]).unwrap(),
 	};
 	let kernel = TxKernel {
-		features: match slate.feat {
-			0 => KernelFeatures::Plain { fee: slate.fee },
-			1 => KernelFeatures::HeightLocked {
-				fee: slate.fee,
-				lock_height: match slate.feat_args.as_ref() {
-					Some(a) => a.lock_hgt,
-					None => 0,
-				},
-			},
-			_ => KernelFeatures::Plain { fee: slate.fee },
-		},
+		features: calc_slate.kernel_features().ok()?,
 		excess,
 		excess_sig,
 	};
@@ -1508,6 +1531,8 @@ pub fn tx_from_slate_v4(slate: &SlateV4) -> Option<Transaction> {
 	let secp = secp.lock();
 	let mut calc_slate = Slate::blank(2, false);
 	calc_slate.fee_fields = slate.fee;
+	calc_slate.kernel_features = slate.feat;
+	calc_slate.kernel_features_args = slate.feat_args.as_ref().map(KernelFeaturesArgs::from);
 	for d in slate.sigs.iter() {
 		calc_slate.participant_data.push(ParticipantData {
 			public_blind_excess: d.xs,
@@ -1524,17 +1549,7 @@ pub fn tx_from_slate_v4(slate: &SlateV4) -> Option<Transaction> {
 		Err(_) => Signature::from_raw_data(&[0; 64]).unwrap(),
 	};
 	let kernel = TxKernel {
-		features: match slate.feat {
-			0 => KernelFeatures::Plain { fee: slate.fee },
-			1 => KernelFeatures::HeightLocked {
-				fee: slate.fee,
-				lock_height: match slate.feat_args.as_ref() {
-					Some(a) => a.lock_hgt,
-					None => 0,
-				},
-			},
-			_ => KernelFeatures::Plain { fee: slate.fee },
-		},
+		features: calc_slate.kernel_features().ok()?,
 		excess,
 		excess_sig,
 	};

libwallet/src/slate_versions/mod.rs

@@ -43,6 +43,13 @@ pub const CURRENT_SLATE_VERSION: u16 = 5;
 /// The grin block header this slate is intended to be compatible with
 pub const GRIN_BLOCK_HEADER_VERSION: u16 = 3;
 
+const HEIGHT_LOCKED_FEATURE: u8 = 2;
+const NRD_FEATURE: u8 = 3;
+
+fn kernel_has_height_arg(feature: u8) -> bool {
+	matches!(feature, HEIGHT_LOCKED_FEATURE | NRD_FEATURE)
+}
+
 /// Existing versions of the slate
 #[derive(EnumIter, Serialize, Deserialize, Clone, Debug, PartialEq, PartialOrd, Eq, Ord)]
 pub enum SlateVersion {
@@ -177,7 +184,8 @@ impl VersionedCoinbase {
 /// Shared slate fixtures, used by the version conversion tests
 #[cfg(test)]
 pub mod tests {
-	use crate::grin_core::core::transaction::OutputFeatures;
+	use crate::grin_core::core::transaction::{KernelFeatures, NRDRelativeHeight, OutputFeatures};
+	use crate::grin_core::core::FeeFields;
 	use crate::grin_util::from_hex;
 	use crate::grin_util::secp::key::PublicKey;
 	use crate::grin_util::secp::pedersen::{Commitment, RangeProof};
@@ -472,6 +480,53 @@ pub mod tests {
 		Ok(())
 	}
 
+	#[test]
+	fn kernel_features_round_trip() -> Result<(), Error> {
+		set_local_chain_type(ChainTypes::Mainnet);
+		let fee = FeeFields::new(0, 42).unwrap();
+
+		for version in [SlateVersion::V4, SlateVersion::V5] {
+			let features = [
+				KernelFeatures::Plain { fee },
+				KernelFeatures::HeightLocked {
+					fee,
+					lock_height: 500_000,
+				},
+				KernelFeatures::NoRecentDuplicate {
+					fee,
+					relative_height: NRDRelativeHeight::new(10).unwrap(),
+				},
+			];
+
+			for expected in features {
+				let mut slate = Slate::blank_with_kernel_features(2, false, expected)?;
+				assert_eq!(slate.tx.as_ref().unwrap().kernels()[0].features, expected);
+				slate.payment_proof = None;
+				slate.tx = populate_test_slate()?.tx;
+
+				let versioned = VersionedSlate::into_version(slate.clone(), version.clone())?;
+				let json = serde_json::to_string(&versioned).unwrap();
+				let recovered: Slate = serde_json::from_str::<VersionedSlate>(&json)
+					.unwrap()
+					.into();
+				assert_eq!(recovered.tx.unwrap().kernels()[0].features, expected);
+
+				let bin: VersionedBinSlate = versioned.try_into()?;
+				let bytes = byte_ser::to_bytes(&bin).unwrap();
+				let recovered: Slate = VersionedSlate::from(
+					byte_ser::from_bytes::<VersionedBinSlate>(&bytes).unwrap(),
+				)
+				.into();
+
+				assert_eq!(recovered.tx.unwrap().kernels()[0].features, expected);
+			}
+		}
+
+		assert!(Slate::blank_with_kernel_features(2, false, KernelFeatures::Coinbase).is_err());
+
+		Ok(())
+	}
+
 	#[test]
 	fn slatepack_version_v4_v5_bin() -> Result<(), Error> {
 		set_local_chain_type(ChainTypes::Mainnet);

libwallet/src/slate_versions/v4_bin.rs

@@ -449,8 +449,8 @@ impl Writeable for SlateV4Bin {
 			proof: &v4.proof,
 		}
 		.write(writer)?;
-		// Write lock height for height locked kernels
-		if v4.feat == 2 {
+		// HeightLocked and NRD both carry a height argument
+		if super::kernel_has_height_arg(v4.feat) {
 			let lock_hgt = match &v4.feat_args {
 				Some(l) => l.lock_hgt,
 				None => 0,
@@ -484,7 +484,7 @@ impl Readable for SlateV4Bin {
 		let sigs = SigsWrap::read(reader)?.0;
 		let opt_structs = SlateOptStructs::read(reader)?;
 
-		let feat_args = if opts.feat == 2 {
+		let feat_args = if super::kernel_has_height_arg(opts.feat) {
 			Some(KernelFeaturesArgsV4 {
 				lock_hgt: reader.read_u64()?,
 			})

libwallet/src/slate_versions/v5_bin.rs

@@ -422,8 +422,8 @@ impl Writeable for SlateV5Bin {
 			proof: &v5.proof,
 		}
 		.write(writer)?;
-		// Write lock height for height locked kernels
-		if v5.feat == 2 {
+		// HeightLocked and NRD both carry a height argument
+		if super::kernel_has_height_arg(v5.feat) {
 			let lock_hgt = match &v5.feat_args {
 				Some(l) => l.lock_hgt,
 				None => 0,
@@ -457,7 +457,7 @@ impl Readable for SlateV5Bin {
 		let sigs = SigsWrap::read(reader)?.0;
 		let opt_structs = SlateOptStructs::read(reader)?;
 
-		let feat_args = if opts.feat == 2 {
+		let feat_args = if super::kernel_has_height_arg(opts.feat) {
 			Some(KernelFeaturesArgsV5 {
 				lock_hgt: reader.read_u64()?,
 			})