fixfees on grin-wallet (#526)

* change fee to fee_fields in various structs and fix tests for new fee calculation

* set up accept_fee_base param in grin-wallet.toml

* fix bug and adopt new tx_fee interface

* Fix doctests

* Remove left over debug statement

* revert field name feef to fee to remain slate compatible

* use into() for some literal fee_fields

* add height arguments to FeeFields methods where necessary

* Fix tests

* Rename fee_fields -> fee on Context

* Rename fee_fields -> fee on TxLogEntry

* Update to grin master

* replace fee(0) by fee(2*YEAR_HEIGHT) to get fee masking

* Fix compilation error and revert Cargo lock

* fix wrong height passed into next_difficulty

Co-authored-by: Jasper van der Maarel <j@sper.dev>

libwallet/src/api_impl/foreign.rs

@@ -17,6 +17,7 @@ use strum::IntoEnumIterator;
 
 use crate::api_impl::owner::finalize_tx as owner_finalize;
 use crate::api_impl::owner::{check_ttl, post_tx};
+use crate::grin_core::core::FeeFields;
 use crate::grin_keychain::Keychain;
 use crate::grin_util::secp::key::SecretKey;
 use crate::internal::{selection, tx, updater};
@@ -122,7 +123,7 @@ where
 	}
 
 	ret_slate.amount = 0;
-	ret_slate.fee = 0;
+	ret_slate.fee_fields = FeeFields::zero();
 	ret_slate.remove_other_sigdata(&keychain, &context.sec_nonce, &context.sec_key)?;
 	ret_slate.state = SlateState::Standard2;
 

libwallet/src/api_impl/owner.rs

@@ -16,6 +16,7 @@
 
 use uuid::Uuid;
 
+use crate::grin_core::consensus::YEAR_HEIGHT;
 use crate::grin_core::core::hash::Hashed;
 use crate::grin_core::core::Transaction;
 use crate::grin_util::secp::key::SecretKey;
@@ -37,7 +38,7 @@ use ed25519_dalek::PublicKey as DalekPublicKey;
 use ed25519_dalek::SecretKey as DalekSecretKey;
 use ed25519_dalek::Verifier;
 
-use std::convert::TryFrom;
+use std::convert::{TryFrom, TryInto};
 use std::sync::mpsc::Sender;
 use std::sync::Arc;
 
@@ -403,7 +404,7 @@ where
 		tx.amount_credited - tx.amount_debited
 	} else {
 		let fee = match tx.fee {
-			Some(f) => f,
+			Some(f) => f.fee(2 * YEAR_HEIGHT), // apply fee mask past HF4
 			None => 0,
 		};
 		tx.amount_debited - tx.amount_credited - fee
@@ -495,7 +496,7 @@ where
 			&parent_key_id,
 		)?;
 		slate.amount = total;
-		slate.fee = fee;
+		slate.fee_fields = fee.try_into().unwrap();
 		return Ok(slate);
 	}
 
@@ -799,7 +800,7 @@ where
 			args.max_outputs as usize,
 			args.num_change_outputs as usize,
 			args.selection_strategy_is_use_all,
-			Some(context.fee),
+			Some(context.fee.map(|f| f.fee(current_height)).unwrap_or(0)),
 			parent_key_id.clone(),
 			false,
 			true,
@@ -869,6 +870,7 @@ where
 }
 
 /// get stored tx
+/// crashes if stored tx has total fees exceeding 2^40 nanogrin
 pub fn get_stored_tx<'a, T: ?Sized, C, K>(
 	w: &T,
 	tx_id: Option<u32>,
@@ -905,7 +907,7 @@ where
 		Some(tx) => {
 			let mut slate = Slate::blank(2, false);
 			slate.tx = Some(tx.clone());
-			slate.fee = tx.fee();
+			slate.fee_fields = tx.aggregate_fee_fields(0).unwrap();
 			slate.id = id.clone();
 			slate.offset = tx.offset;
 			slate.state = SlateState::Standard3;

libwallet/src/internal/selection.rs

@@ -30,6 +30,7 @@ use crate::slate::Slate;
 use crate::types::*;
 use crate::util::OnionV3Address;
 use std::collections::HashMap;
+use std::convert::TryInto;
 
 /// Initialize a transaction on the sender side, returns a corresponding
 /// libwallet transaction slate with the appropriate inputs selected,
@@ -74,7 +75,7 @@ where
 	}
 
 	// Update the fee on the slate so we account for this when building the tx.
-	slate.fee = fee;
+	slate.fee_fields = fee.try_into().unwrap();
 	slate.add_transaction_elements(keychain, &ProofBuilder::new(keychain), elems)?;
 
 	// Create our own private context
@@ -85,7 +86,7 @@ where
 		is_initiator,
 	);
 
-	context.fee = fee;
+	context.fee = Some(slate.fee_fields);
 	context.amount = slate.amount;
 
 	// Store our private identifiers for each input
@@ -151,7 +152,7 @@ where
 		t.tx_slate_id = Some(slate_id);
 		let filename = format!("{}.grintx", slate_id);
 		t.stored_tx = Some(filename);
-		t.fee = Some(context.fee);
+		t.fee = context.fee;
 		t.ttl_cutoff_height = match slate.ttl_cutoff_height {
 			0 => None,
 			n => Some(n),
@@ -268,7 +269,7 @@ where
 
 	context.add_output(&key_id, &None, amount);
 	context.amount = amount;
-	context.fee = slate.fee;
+	context.fee = slate.fee_fields.as_opt();
 	let commit = wallet.calc_commit_for_cache(keychain_mask, amount, &key_id_inner)?;
 	let mut batch = wallet.batch(keychain_mask)?;
 	let log_id = batch.next_tx_log_id(&parent_key_id)?;
@@ -398,7 +399,7 @@ where
 	// sender
 
 	// First attempt to spend without change
-	let mut fee = tx_fee(coins.len(), 1, 1, None);
+	let mut fee = tx_fee(coins.len(), 1, 1);
 	let mut total: u64 = coins.iter().map(|c| c.value).sum();
 	let mut amount_with_fee = amount + fee;
 
@@ -427,7 +428,7 @@ where
 
 	// We need to add a change address or amount with fee is more than total
 	if total != amount_with_fee {
-		fee = tx_fee(coins.len(), num_outputs, 1, None);
+		fee = tx_fee(coins.len(), num_outputs, 1);
 		amount_with_fee = amount + fee;
 
 		// Here check if we have enough outputs for the amount including fee otherwise
@@ -455,7 +456,7 @@ where
 				parent_key_id,
 			)
 			.1;
-			fee = tx_fee(coins.len(), num_outputs, 1, None);
+			fee = tx_fee(coins.len(), num_outputs, 1);
 			total = coins.iter().map(|c| c.value).sum();
 			amount_with_fee = amount + fee;
 		}
@@ -655,7 +656,9 @@ where
 	// restore the original amount, fee
 	slate.amount = context.amount;
 	if update_fee {
-		slate.fee = context.fee;
+		slate.fee_fields = context
+			.fee
+			.ok_or_else(|| ErrorKind::Fee("Missing fee fields".into()))?;
 	}
 
 	let keychain = wallet.keychain(keychain_mask)?;

libwallet/src/internal/tx.rs

@@ -35,6 +35,7 @@ use ed25519_dalek::PublicKey as DalekPublicKey;
 use ed25519_dalek::SecretKey as DalekSecretKey;
 use ed25519_dalek::Signature as DalekSignature;
 use ed25519_dalek::{Signer, Verifier};
+use grin_wallet_util::grin_core::core::FeeFields;
 
 // static for incrementing test UUIDs
 lazy_static! {
@@ -276,13 +277,13 @@ where
 		init_tx_args.selection_strategy_is_use_all,
 		&parent_key_id,
 	)?;
-	slate.fee = fee;
+	slate.fee_fields = FeeFields::new(0, fee)?;
 
 	let keychain = wallet.keychain(keychain_mask)?;
 
 	// Create our own private context
 	let mut context = Context::new(keychain.secp(), &parent_key_id, use_test_rng, true);
-	context.fee = slate.fee;
+	context.fee = Some(slate.fee_fields);
 	context.amount = slate.amount;
 	context.late_lock_args = Some(init_tx_args.clone());
 
@@ -585,7 +586,7 @@ mod test {
 	use super::*;
 	use rand::rngs::mock::StepRng;
 
-	use crate::grin_core::core::KernelFeatures;
+	use crate::grin_core::core::{FeeFields, KernelFeatures};
 	use crate::grin_core::libtx::{build, ProofBuilder};
 	use crate::grin_keychain::{
 		BlindSum, BlindingFactor, ExtKeychain, ExtKeychainPath, Keychain, SwitchCommitmentType,
@@ -601,14 +602,18 @@ mod test {
 		let key_id1 = ExtKeychainPath::new(1, 1, 0, 0, 0).to_identifier();
 
 		let tx1 = build::transaction(
-			KernelFeatures::Plain { fee: 0 },
+			KernelFeatures::Plain {
+				fee: FeeFields::zero(),
+			},
 			&[build::output(105, key_id1.clone())],
 			&keychain,
 			&builder,
 		)
 		.unwrap();
 		let tx2 = build::transaction(
-			KernelFeatures::Plain { fee: 0 },
+			KernelFeatures::Plain {
+				fee: FeeFields::zero(),
+			},
 			&[build::input(105, key_id1.clone())],
 			&keychain,
 			&builder,

libwallet/src/slate.rs

@@ -16,10 +16,11 @@
 //! around during an interactive wallet exchange
 
 use crate::error::{Error, ErrorKind};
+use crate::grin_core::consensus::YEAR_HEIGHT;
 use crate::grin_core::core::amount_to_hr_string;
 use crate::grin_core::core::transaction::{
-	Input, Inputs, KernelFeatures, NRDRelativeHeight, Output, OutputFeatures, Transaction,
-	TxKernel, Weighting,
+	FeeFields, Input, Inputs, KernelFeatures, NRDRelativeHeight, Output, OutputFeatures,
+	Transaction, TxKernel, Weighting,
 };
 use crate::grin_core::core::verifier_cache::LruVerifierCache;
 use crate::grin_core::libtx::{aggsig, build, proof::ProofBuild, tx_fee};
@@ -105,8 +106,8 @@ pub struct Slate {
 	pub tx: Option<Transaction>,
 	/// base amount (excluding fee)
 	pub amount: u64,
-	/// fee amount
-	pub fee: u64,
+	/// fee amount and shift
+	pub fee_fields: FeeFields,
 	/// TTL, the block height at which wallets
 	/// should refuse to process the transaction and unlock all
 	/// associated outputs
@@ -265,7 +266,7 @@ impl Slate {
 			state,
 			tx: Some(Slate::empty_transaction()),
 			amount: 0,
-			fee: 0,
+			fee_fields: FeeFields::zero(),
 			ttl_cutoff_height: 0,
 			kernel_features: 0,
 			offset: BlindingFactor::zero(),
@@ -351,10 +352,12 @@ impl Slate {
 	// Any other value is invalid.
 	fn kernel_features(&self) -> Result<KernelFeatures, Error> {
 		match self.kernel_features {
-			0 => Ok(KernelFeatures::Plain { fee: self.fee }),
+			0 => Ok(KernelFeatures::Plain {
+				fee: self.fee_fields,
+			}),
 			1 => Err(ErrorKind::InvalidKernelFeatures(1).into()),
 			2 => Ok(KernelFeatures::HeightLocked {
-				fee: self.fee,
+				fee: self.fee_fields,
 				lock_height: match &self.kernel_features_args {
 					Some(a) => a.lock_height,
 					None => {
@@ -363,7 +366,7 @@ impl Slate {
 				},
 			}),
 			3 => Ok(KernelFeatures::NoRecentDuplicate {
-				fee: self.fee,
+				fee: self.fee_fields,
 				relative_height: match &self.kernel_features_args {
 					Some(a) => NRDRelativeHeight::new(a.lock_height)?,
 					None => {
@@ -549,24 +552,23 @@ impl Slate {
 		// double check the fee amount included in the partial tx
 		// we don't necessarily want to just trust the sender
 		// we could just overwrite the fee here (but we won't) due to the sig
-		let fee = tx_fee(
-			tx.inputs().len(),
-			tx.outputs().len(),
-			tx.kernels().len(),
-			None,
-		);
+		let fee = tx_fee(tx.inputs().len(), tx.outputs().len(), tx.kernels().len());
 
-		if fee > tx.fee() {
-			return Err(
-				ErrorKind::Fee(format!("Fee Dispute Error: {}, {}", tx.fee(), fee,)).into(),
-			);
+		if fee > tx.fee(2 * YEAR_HEIGHT) {
+			// apply fee mask past HF4
+			return Err(ErrorKind::Fee(format!(
+				"Fee Dispute Error: {}, {}",
+				tx.fee(2 * YEAR_HEIGHT),
+				fee,
+			))
+			.into());
 		}
 
-		if fee > self.amount + self.fee {
+		if fee > self.amount + self.fee_fields.fee(2 * YEAR_HEIGHT) {
 			let reason = format!(
 				"Rejected the transfer because transaction fee ({}) exceeds received amount ({}).",
 				amount_to_hr_string(fee, false),
-				amount_to_hr_string(self.amount + self.fee, false)
+				amount_to_hr_string(self.amount + self.fee_fields.fee(2 * YEAR_HEIGHT), false)
 			);
 			info!("{}", reason);
 			return Err(ErrorKind::Fee(reason).into());
@@ -676,7 +678,7 @@ impl Slate {
 		// confirm the overall transaction is valid (including the updated kernel)
 		// accounting for tx weight limits
 		let verifier_cache = Arc::new(RwLock::new(LruVerifierCache::new()));
-		if let Err(e) = final_tx.validate(Weighting::AsTransaction, verifier_cache) {
+		if let Err(e) = final_tx.validate(Weighting::AsTransaction, verifier_cache, 0) {
 			error!("Error with final tx validation: {}", e);
 			Err(e.into())
 		} else {
@@ -708,7 +710,7 @@ impl From<Slate> for SlateV4 {
 			state,
 			tx: _,
 			amount,
-			fee,
+			fee_fields,
 			kernel_features,
 			ttl_cutoff_height: ttl,
 			offset: off,
@@ -734,7 +736,7 @@ impl From<Slate> for SlateV4 {
 			sta,
 			coms: (&slate).into(),
 			amt: amount,
-			fee,
+			fee: fee_fields,
 			feat: kernel_features,
 			ttl,
 			off,
@@ -754,7 +756,7 @@ impl From<&Slate> for SlateV4 {
 			state,
 			tx: _,
 			amount,
-			fee,
+			fee_fields,
 			kernel_features,
 			ttl_cutoff_height: ttl,
 			offset,
@@ -766,7 +768,7 @@ impl From<&Slate> for SlateV4 {
 		let num_parts = *num_parts;
 		let id = *id;
 		let amount = *amount;
-		let fee = *fee;
+		let fee_fields = *fee_fields;
 		let feat = *kernel_features;
 		let ttl = *ttl;
 		let off = offset.clone();
@@ -787,7 +789,7 @@ impl From<&Slate> for SlateV4 {
 			sta,
 			coms: slate.into(),
 			amt: amount,
-			fee,
+			fee: fee_fields,
 			feat,
 			ttl,
 			off,
@@ -914,7 +916,7 @@ impl From<SlateV4> for Slate {
 			sta,
 			coms: _,
 			amt: amount,
-			fee,
+			fee: fee_fields,
 			feat: kernel_features,
 			ttl: ttl_cutoff_height,
 			off: offset,
@@ -940,7 +942,7 @@ impl From<SlateV4> for Slate {
 			state,
 			tx: (&slate).into(),
 			amount,
-			fee,
+			fee_fields,
 			kernel_features,
 			ttl_cutoff_height,
 			offset,
@@ -960,7 +962,7 @@ pub fn tx_from_slate_v4(slate: &SlateV4) -> Option<Transaction> {
 	let secp = static_secp_instance();
 	let secp = secp.lock();
 	let mut calc_slate = Slate::blank(2, false);
-	calc_slate.fee = slate.fee;
+	calc_slate.fee_fields = slate.fee;
 	for d in slate.sigs.iter() {
 		calc_slate.participant_data.push(ParticipantData {
 			public_blind_excess: d.xs,

libwallet/src/slate_versions/v4.rs

@@ -53,6 +53,7 @@
 //! *  The `receiver_signature` field is renamed to `rsig`
 //! * `rsig` may be omitted if it has not yet been filled out
 
+use crate::grin_core::core::FeeFields;
 use crate::grin_core::core::{Input, Output, TxKernel};
 use crate::grin_core::libtx::secp_ser;
 use crate::grin_keychain::{BlindingFactor, Identifier};
@@ -95,11 +96,10 @@ pub struct SlateV4 {
 	#[serde(skip_serializing_if = "u64_is_blank")]
 	#[serde(default = "default_u64_0")]
 	pub amt: u64,
-	/// fee amount
-	#[serde(with = "secp_ser::string_or_u64")]
-	#[serde(default = "default_u64")]
-	#[serde(skip_serializing_if = "u64_is_blank")]
-	pub fee: u64,
+	/// fee
+	#[serde(skip_serializing_if = "fee_is_zero")]
+	#[serde(default = "default_fee")]
+	pub fee: FeeFields,
 	/// kernel features, if any
 	#[serde(skip_serializing_if = "u8_is_blank")]
 	#[serde(default = "default_u8_0")]
@@ -297,10 +297,6 @@ fn default_range_proof() -> Option<RangeProof> {
 	None
 }
 
-fn default_u64() -> u64 {
-	0
-}
-
 fn u64_is_blank(u: &u64) -> bool {
 	*u == 0
 }
@@ -312,6 +308,15 @@ fn default_u8_0() -> u8 {
 fn u8_is_blank(u: &u8) -> bool {
 	*u == 0
 }
+
+fn fee_is_zero(f: &FeeFields) -> bool {
+	f.is_zero()
+}
+
+fn default_fee() -> FeeFields {
+	FeeFields::zero()
+}
+
 /// A mining node requests new coinbase via the foreign api every time a new candidate block is built.
 #[derive(Serialize, Deserialize, Debug, Clone)]
 pub struct CoinbaseV4 {

libwallet/src/slate_versions/v4_bin.rs

@@ -14,7 +14,8 @@
 
 //! Wraps a V4 Slate into a V4 Binary slate
 
-use crate::grin_core::core::transaction::OutputFeatures;
+use crate::grin_core::consensus::YEAR_HEIGHT;
+use crate::grin_core::core::transaction::{FeeFields, OutputFeatures};
 use crate::grin_core::ser as grin_ser;
 use crate::grin_core::ser::{Readable, Reader, Writeable, Writer};
 use crate::grin_keychain::BlindingFactor;
@@ -87,8 +88,8 @@ struct SlateOptFields {
 	pub num_parts: u8,
 	/// amt, default 0
 	pub amt: u64,
-	/// fee, default 0
-	pub fee: u64,
+	/// fee_fields, default FeeFields::zero()
+	pub fee: FeeFields,
 	/// kernel features, default 0
 	pub feat: u8,
 	/// ttl, default 0
@@ -107,7 +108,8 @@ impl Writeable for SlateOptFields {
 		if self.amt > 0 {
 			status |= 0x02;
 		}
-		if self.fee > 0 {
+		if self.fee.fee(2 * YEAR_HEIGHT) > 0 {
+			// apply fee mask past HF4
 			status |= 0x04;
 		}
 		if self.feat > 0 {
@@ -124,7 +126,7 @@ impl Writeable for SlateOptFields {
 			writer.write_u64(self.amt)?;
 		}
 		if status & 0x04 > 0 {
-			writer.write_u64(self.fee)?;
+			self.fee.write(writer)?;
 		}
 		if status & 0x08 > 0 {
 			writer.write_u8(self.feat)?;
@@ -150,9 +152,9 @@ impl Readable for SlateOptFields {
 			0
 		};
 		let fee = if status & 0x04 > 0 {
-			reader.read_u64()?
+			FeeFields::read(reader)?
 		} else {
-			0
+			FeeFields::zero()
 		};
 		let feat = if status & 0x08 > 0 {
 			reader.read_u8()?
@@ -488,7 +490,9 @@ fn slate_v4_serialize_deserialize() {
 	use crate::grin_util::from_hex;
 	use crate::grin_util::secp::key::PublicKey;
 	use crate::Slate;
+	use grin_wallet_util::grin_core::global::{set_local_chain_type, ChainTypes};
 	use grin_wallet_util::grin_keychain::{ExtKeychain, Keychain, SwitchCommitmentType};
+	set_local_chain_type(ChainTypes::Mainnet);
 	let slate = Slate::blank(1, false);
 	let mut v4 = SlateV4::from(slate);
 

libwallet/src/types.rs

@@ -18,6 +18,7 @@
 use crate::config::{TorConfig, WalletConfig};
 use crate::error::{Error, ErrorKind};
 use crate::grin_core::core::hash::Hash;
+use crate::grin_core::core::FeeFields;
 use crate::grin_core::core::{Output, Transaction, TxKernel};
 use crate::grin_core::libtx::{aggsig, secp_ser};
 use crate::grin_core::{global, ser};
@@ -554,7 +555,7 @@ pub struct Context {
 	/// needed by the other party
 	pub amount: u64,
 	/// store the calculated fee
-	pub fee: u64,
+	pub fee: Option<FeeFields>,
 	/// Payment proof sender address derivation path, if needed
 	pub payment_proof_derivation_index: Option<u32>,
 	/// If late-locking, store my tranasction creation prefs
@@ -608,7 +609,7 @@ impl Context {
 			input_ids: vec![],
 			output_ids: vec![],
 			amount: 0,
-			fee: 0,
+			fee: None,
 			payment_proof_derivation_index: None,
 			late_lock_args: None,
 			calculated_excess: None,
@@ -818,8 +819,7 @@ pub struct TxLogEntry {
 	#[serde(with = "secp_ser::string_or_u64")]
 	pub amount_debited: u64,
 	/// Fee
-	#[serde(with = "secp_ser::opt_string_or_u64")]
-	pub fee: Option<u64>,
+	pub fee: Option<FeeFields>,
 	/// Cutoff block height
 	#[serde(with = "secp_ser::opt_string_or_u64")]
 	#[serde(default)]

libwallet/tests/libwallet.rs

@@ -14,6 +14,7 @@
 //! core::libtx specific tests
 use grin_wallet_libwallet::Context;
 use grin_wallet_util::grin_core::core::transaction;
+use grin_wallet_util::grin_core::core::FeeFields;
 use grin_wallet_util::grin_core::libtx::{aggsig, proof};
 use grin_wallet_util::grin_keychain::{
 	BlindSum, BlindingFactor, ExtKeychain, ExtKeychainPath, Keychain, SwitchCommitmentType,
@@ -23,9 +24,11 @@ use grin_wallet_util::grin_util::secp::key::{PublicKey, SecretKey};
 use rand::thread_rng;
 
 fn kernel_sig_msg() -> secp::Message {
-	transaction::KernelFeatures::Plain { fee: 0 }
-		.kernel_sig_msg()
-		.unwrap()
+	transaction::KernelFeatures::Plain {
+		fee: FeeFields::zero(),
+	}
+	.kernel_sig_msg()
+	.unwrap()
 }
 
 #[test]