Merge pull request #67 from mwcproject/v5.3.2/fee_fix

Separate fee related weights from block weight. Remove tx fee change …

chain/src/pipe.rs

@@ -26,6 +26,7 @@ use crate::error::Error;
 use crate::store;
 use crate::txhashset;
 use crate::types::{CommitPos, Options, Tip};
+use grin_core::core::Transaction;
 use grin_util::RwLock;
 use std::collections::HashSet;
 use std::iter::FromIterator;
@@ -524,7 +525,7 @@ fn validate_header(header: &BlockHeader, ctx: &mut BlockContext<'_>) -> Result<(
 	}
 
 	// Block header is invalid (and block is invalid) if this lower bound is too heavy for a full block.
-	let weight = TransactionBody::weight_by_iok(0, num_outputs, num_kernels);
+	let weight = Transaction::weight_for_size(0, num_outputs, num_kernels);
 	if weight > global::max_block_weight() {
 		return Err(Error::Block(block::Error::TooHeavy));
 	}

core/src/consensus.rs

@@ -25,13 +25,13 @@ use crate::pow::Difficulty;
 use std::cmp::{max, min};
 
 /// A grin is divisible to 10^9, following the SI prefixes
-pub const GRIN_BASE: u64 = 1_000_000_000;
+pub const MWC_BASE: u64 = 1_000_000_000;
 /// Milligrin, a thousand of a grin
-pub const MILLI_GRIN: u64 = GRIN_BASE / 1_000;
+pub const MILLI_MWC: u64 = MWC_BASE / 1_000;
 /// Microgrin, a thousand of a milligrin
-pub const MICRO_GRIN: u64 = MILLI_GRIN / 1_000;
+pub const MICRO_MWC: u64 = MILLI_MWC / 1_000;
 /// Nanogrin, smallest unit, takes a billion to make a grin
-pub const NANO_GRIN: u64 = 1;
+pub const NANO_MWC: u64 = 1;
 
 /// Block interval, in seconds, the network will tune its next_target for. Note
 /// that we may reduce this value in the future as we get more data on mining
@@ -112,14 +112,23 @@ pub const CUT_THROUGH_HORIZON: u32 = WEEK_HEIGHT as u32;
 /// easier to reason about.
 pub const STATE_SYNC_THRESHOLD: u32 = 2 * DAY_HEIGHT as u32;
 
-/// Weight of an input when counted against the max block weight capacity
-pub const INPUT_WEIGHT: u64 = 1;
+/// Size Weight of an input when counted against the max block weight capacity
+pub const BLOCK_INPUT_WEIGHT: u64 = 1;
 
-/// Weight of an output when counted against the max block weight capacity
-pub const OUTPUT_WEIGHT: u64 = 21;
+/// Size Weight of an output when counted against the max block weight capacity
+pub const BLOCK_OUTPUT_WEIGHT: u64 = 21;
 
-/// Weight of a kernel when counted against the max block weight capacity
-pub const KERNEL_WEIGHT: u64 = 3;
+/// Size Weight of a kernel when counted against the max block weight capacity
+pub const BLOCK_KERNEL_WEIGHT: u64 = 3;
+
+/// Transaction fee weight of input
+pub const TXFEE_INPUT_WEIGHT: u64 = 1;
+
+/// Transaction fee weight of output
+pub const TXFEE_OUTPUT_WEIGHT: u64 = 4;
+
+/// Transaction fee weight of kernel
+pub const TXFEE_KERNEL_WEIGHT: u64 = 1;
 
 /// Total maximum block weight. At current sizes, this means a maximum
 /// theoretical size of:

core/src/core.rs

@@ -24,7 +24,7 @@ pub mod merkle_proof;
 pub mod pmmr;
 pub mod transaction;
 
-use crate::consensus::GRIN_BASE;
+use crate::consensus::MWC_BASE;
 use util::secp::pedersen::Commitment;
 
 pub use self::block::*;
@@ -60,7 +60,7 @@ pub fn amount_from_hr_string(amount: &str) -> Result<u64, Error> {
 			(parse_grins(gs)?, parse_ngrins(&tail[1..])?)
 		}
 	};
-	Ok(grins * GRIN_BASE + ngrins)
+	Ok(grins * MWC_BASE + ngrins)
 }
 
 fn parse_grins(amount: &str) -> Result<u64, Error> {
@@ -74,7 +74,7 @@ fn parse_grins(amount: &str) -> Result<u64, Error> {
 }
 
 lazy_static! {
-	static ref WIDTH: usize = (GRIN_BASE as f64).log(10.0) as usize + 1;
+	static ref WIDTH: usize = (MWC_BASE as f64).log(10.0) as usize + 1;
 }
 
 fn parse_ngrins(amount: &str) -> Result<u64, Error> {
@@ -93,7 +93,7 @@ fn parse_ngrins(amount: &str) -> Result<u64, Error> {
 /// Common method for converting an amount to a human-readable string
 
 pub fn amount_to_hr_string(amount: u64, truncate: bool) -> String {
-	let amount = (amount as f64 / GRIN_BASE as f64) as f64;
+	let amount = (amount as f64 / MWC_BASE as f64) as f64;
 	let hr = format!("{:.*}", WIDTH, amount);
 	if truncate {
 		let nzeros = hr.chars().rev().take_while(|x| x == &'0').count();

core/src/core/block.rs

@@ -504,7 +504,7 @@ impl Readable for UntrustedBlockHeader {
 
 		// Validate global output and kernel MMR sizes against upper bounds based on block height.
 		let global_weight =
-			TransactionBody::weight_by_iok(0, header.output_mmr_count(), header.kernel_mmr_count());
+			Transaction::weight_for_size(0, header.output_mmr_count(), header.kernel_mmr_count());
 		if global_weight > global::max_block_weight() * (header.height + 1) {
 			return Err(ser::Error::CorruptedData(
 				"Tx global weight is exceed the limit".to_string(),

core/src/core/transaction.rs

@@ -17,6 +17,7 @@
 use crate::core::block::HeaderVersion;
 use crate::core::hash::{DefaultHashable, Hashed};
 use crate::core::{committed, Committed};
+use crate::global::get_accept_fee_base;
 use crate::libtx::{aggsig, secp_ser};
 use crate::ser::{
 	self, read_multi, PMMRable, ProtocolVersion, Readable, Reader, VerifySortedAndUnique,
@@ -908,7 +909,7 @@ impl Readable for TransactionBody {
 
 		// Quick block weight check before proceeding.
 		// Note: We use weight_as_block here (inputs have weight).
-		let tx_block_weight = TransactionBody::weight_by_iok(num_inputs, num_outputs, num_kernels);
+		let tx_block_weight = Transaction::weight_for_size(num_inputs, num_outputs, num_kernels);
 
 		if num_inputs > ser::READ_VEC_SIZE_LIMIT
 			|| num_outputs > ser::READ_VEC_SIZE_LIMIT
@@ -1141,23 +1142,14 @@ impl TransactionBody {
 	}
 
 	/// Calculate weight of transaction using block weighing
-	pub fn weight(&self) -> u64 {
-		TransactionBody::weight_by_iok(
+	pub fn weight_size(&self) -> u64 {
+		Transaction::weight_for_size(
 			self.inputs.len() as u64,
 			self.outputs.len() as u64,
 			self.kernels.len() as u64,
 		)
 	}
 
-	/// Calculate transaction weight using block weighing from transaction
-	/// details. Consensus critical and uses consensus weight values.
-	pub fn weight_by_iok(num_inputs: u64, num_outputs: u64, num_kernels: u64) -> u64 {
-		num_inputs
-			.saturating_mul(consensus::INPUT_WEIGHT as u64)
-			.saturating_add(num_outputs.saturating_mul(consensus::OUTPUT_WEIGHT as u64))
-			.saturating_add(num_kernels.saturating_mul(consensus::KERNEL_WEIGHT as u64))
-	}
-
 	/// Lock height of a body is the max lock height of the kernels.
 	pub fn lock_height(&self) -> u64 {
 		self.kernels
@@ -1175,7 +1167,7 @@ impl TransactionBody {
 	fn verify_weight(&self, weighting: Weighting) -> Result<(), Error> {
 		// A coinbase reward is a single output and a single kernel (for now).
 		// We need to account for this when verifying max tx weights.
-		let coinbase_weight = consensus::OUTPUT_WEIGHT + consensus::KERNEL_WEIGHT;
+		let coinbase_weight = consensus::BLOCK_OUTPUT_WEIGHT + consensus::BLOCK_KERNEL_WEIGHT;
 
 		// If "tx" body then remember to reduce the max_block_weight by the weight of a kernel.
 		// If "limited tx" then compare against the provided max_weight.
@@ -1197,7 +1189,7 @@ impl TransactionBody {
 			}
 		};
 
-		if self.weight() > max_weight {
+		if self.weight_size() > max_weight {
 			return Err(Error::TooHeavy);
 		}
 		Ok(())
@@ -1527,41 +1519,46 @@ impl Transaction {
 	/// Can be used to compare txs by their fee/weight ratio, aka feerate.
 	/// Don't use these values for anything else though due to precision multiplier.
 	pub fn fee_rate(&self, height: u64) -> u64 {
-		self.fee(height) / self.weight() as u64
+		self.fee(height) / self.weight_size() as u64
 	}
 
 	/// Calculate transaction weight
-	pub fn weight(&self) -> u64 {
-		self.body.weight()
+	pub fn weight_size(&self) -> u64 {
+		self.body.weight_size()
 	}
 
 	/// Transaction minimum acceptable fee
-	pub fn accept_fee(&self, height: u64) -> u64 {
-		// Note MWC. Header Version 3  is future versions for the mainnet,
-		// This feature is related to miners only, there is no consensus breaking.
-		if consensus::header_version(height) < HeaderVersion(3) {
-			Transaction::old_weight_by_iok(
-				self.body.inputs.len() as u64,
-				self.body.outputs.len() as u64,
-				self.body.kernels.len() as u64,
-			) * consensus::MILLI_GRIN
-		} else {
-			self.weight() * global::get_accept_fee_base()
-		}
-	}
-
-	/// Old weight definition for pool acceptance
-	pub fn old_weight_by_iok(num_inputs: u64, num_outputs: u64, num_kernels: u64) -> u64 {
+	/// _height is kept for possible fee formula change that will require hardfork
+	pub fn accept_fee(&self, _height: u64) -> u64 {
+		// Note, this code is different from grin. Grin is using the same formula to calculate the transaction/block size and the
+		// fees. Migration was done with hardfork.
+		// _height
+		Transaction::weight_for_fee(
+			self.body.inputs.len() as u64,
+			self.body.outputs.len() as u64,
+			self.body.kernels.len() as u64,
+		) * get_accept_fee_base()
+	}
+
+	/// Transaction weight for fee
+	/// Consensus related, if transaction fee below expected values, it will be rejected by mining node
+	pub fn weight_for_fee(num_inputs: u64, num_outputs: u64, num_kernels: u64) -> u64 {
+		// Outputs*4 + kernels*1 - inputs*1
 		let body_weight = num_outputs
-			.saturating_mul(4)
-			.saturating_add(num_kernels)
-			.saturating_sub(num_inputs);
+			.saturating_mul(consensus::TXFEE_OUTPUT_WEIGHT as u64)
+			.saturating_add(num_kernels.saturating_mul(consensus::TXFEE_KERNEL_WEIGHT as u64))
+			.saturating_sub(num_inputs.saturating_mul(consensus::TXFEE_INPUT_WEIGHT as u64));
+
 		max(body_weight, 1)
 	}
 
-	/// Calculate transaction weight from transaction details
-	pub fn weight_by_iok(num_inputs: u64, num_outputs: u64, num_kernels: u64) -> u64 {
-		TransactionBody::weight_by_iok(num_inputs, num_outputs, num_kernels)
+	/// Calculate transaction weight by size, for block weight.
+	/// Consensus critical and uses consensus weight values.
+	pub fn weight_for_size(num_inputs: u64, num_outputs: u64, num_kernels: u64) -> u64 {
+		num_inputs
+			.saturating_mul(consensus::BLOCK_INPUT_WEIGHT as u64)
+			.saturating_add(num_outputs.saturating_mul(consensus::BLOCK_OUTPUT_WEIGHT as u64))
+			.saturating_add(num_kernels.saturating_mul(consensus::BLOCK_KERNEL_WEIGHT as u64))
 	}
 }
 

core/src/global.rs

@@ -18,9 +18,9 @@
 
 use crate::consensus;
 use crate::consensus::{
-	graph_weight, HeaderDifficultyInfo, BASE_EDGE_BITS, BLOCK_TIME_SEC, COINBASE_MATURITY,
-	CUT_THROUGH_HORIZON, DAY_HEIGHT, DEFAULT_MIN_EDGE_BITS, DIFFICULTY_ADJUST_WINDOW,
-	INITIAL_DIFFICULTY, KERNEL_WEIGHT, MAX_BLOCK_WEIGHT, OUTPUT_WEIGHT, PROOFSIZE,
+	graph_weight, HeaderDifficultyInfo, BASE_EDGE_BITS, BLOCK_KERNEL_WEIGHT, BLOCK_OUTPUT_WEIGHT,
+	BLOCK_TIME_SEC, COINBASE_MATURITY, CUT_THROUGH_HORIZON, DAY_HEIGHT, DEFAULT_MIN_EDGE_BITS,
+	DIFFICULTY_ADJUST_WINDOW, INITIAL_DIFFICULTY, MAX_BLOCK_WEIGHT, PROOFSIZE,
 	SECOND_POW_EDGE_BITS, STATE_SYNC_THRESHOLD,
 };
 use crate::core::block::Block;
@@ -83,7 +83,7 @@ pub const TESTING_INITIAL_DIFFICULTY: u64 = 1;
 pub const TESTING_MAX_BLOCK_WEIGHT: u64 = 250;
 
 /// Default unit of fee per tx weight, making each output cost about a Grincent
-pub const DEFAULT_ACCEPT_FEE_BASE: u64 = consensus::MILLI_GRIN; // Keeping default base is same, no changes for MWC     GRIN_BASE / 100 / 20; // 500_000
+pub const DEFAULT_ACCEPT_FEE_BASE: u64 = consensus::MILLI_MWC; // Keeping default base is same, no changes for MWC     GRIN_BASE / 100 / 20; // 500_000
 
 /// If a peer's last updated difficulty is 2 hours ago and its difficulty's lower than ours,
 /// we're sure this peer is a stuck node, and we will kick out such kind of stuck peers.
@@ -398,7 +398,7 @@ pub fn max_block_weight() -> u64 {
 
 /// Maximum allowed transaction weight (1 weight unit ~= 32 bytes)
 pub fn max_tx_weight() -> u64 {
-	let coinbase_weight = OUTPUT_WEIGHT + KERNEL_WEIGHT;
+	let coinbase_weight = BLOCK_OUTPUT_WEIGHT + BLOCK_KERNEL_WEIGHT;
 	max_block_weight().saturating_sub(coinbase_weight) as u64
 }
 

core/src/libtx/mod.rs

@@ -36,7 +36,7 @@ pub use crate::libtx::error::Error;
 
 /// Transaction fee calculation given numbers of inputs, outputs, and kernels
 pub fn tx_fee(input_len: usize, output_len: usize, kernel_len: usize) -> u64 {
-	Transaction::weight_by_iok(input_len as u64, output_len as u64, kernel_len as u64)
+	Transaction::weight_for_fee(input_len as u64, output_len as u64, kernel_len as u64)
 		* get_accept_fee_base()
 }
 

core/tests/block.rs

@@ -14,7 +14,7 @@
 
 mod common;
 use crate::common::{new_block, tx1i2o, tx2i1o, txspend1i1o};
-use crate::core::consensus::{self, OUTPUT_WEIGHT};
+use crate::core::consensus::{self, BLOCK_OUTPUT_WEIGHT};
 use crate::core::core::block::{Block, BlockHeader, Error, HeaderVersion, UntrustedBlockHeader};
 use crate::core::core::hash::Hashed;
 use crate::core::core::id::ShortIdentifiable;
@@ -42,7 +42,7 @@ fn too_large_block() {
 	test_setup();
 	let keychain = ExtKeychain::from_random_seed(false).unwrap();
 	let builder = ProofBuilder::new(&keychain);
-	let max_out = global::max_block_weight() / OUTPUT_WEIGHT;
+	let max_out = global::max_block_weight() / BLOCK_OUTPUT_WEIGHT;
 
 	let mut pks = vec![];
 	for n in 0..(max_out + 1) {

core/tests/transaction.rs

@@ -214,7 +214,8 @@ fn test_verify_cut_through_coinbase() -> Result<(), Error> {
 #[test]
 fn test_fee_fields() -> Result<(), Error> {
 	global::set_local_chain_type(global::ChainTypes::UserTesting);
-	global::set_local_accept_fee_base(500_000);
+	let local_base_fee = 500_000;
+	global::set_local_accept_fee_base(local_base_fee);
 
 	let keychain = ExtKeychain::from_random_seed(false)?;
 
@@ -236,16 +237,12 @@ fn test_fee_fields() -> Result<(), Error> {
 	.expect("valid tx");
 
 	let hf2_height = 2 * consensus::TESTING_HARD_FORK_INTERVAL;
-	assert_eq!(
-		tx.accept_fee(hf2_height),
-		(1 * 1 + 1 * 21 + 1 * 3) * 500_000
-	);
-	assert_eq!(tx.fee(hf2_height), 42);
+	assert_eq!(tx.accept_fee(hf2_height), (1 * 4 + 1 * 1 - 1 * 1) * 500_000);
 	assert_eq!(tx.fee(hf2_height), 42);
 	assert_eq!(tx.shifted_fee(hf2_height), 21);
 	assert_eq!(
 		tx.accept_fee(hf2_height - 1),
-		(1 * 4 + 1 * 1 - 1 * 1) * 1_000_000
+		(1 * 4 + 1 * 1 - 1 * 1) * 500_000
 	);
 	assert_eq!(tx.fee(hf2_height - 1), 42 + (1u64 << 40));
 	assert_eq!(tx.shifted_fee(hf2_height - 1), 42 + (1u64 << 40));
@@ -260,7 +257,7 @@ fn test_fee_fields() -> Result<(), Error> {
 	assert_eq!(tx.fee(hf2_height), 147);
 	assert_eq!(tx.shifted_fee(hf2_height), 36);
 	assert_eq!(tx.aggregate_fee_fields(hf2_height), FeeFields::new(2, 147));
-	assert_eq!(tx_fee(1, 1, 3), 15_500_000);
+	assert_eq!(tx_fee(1, 1, 3), (1 * 4 + 3 - 1) * local_base_fee);
 
 	Ok(())
 }

p2p/src/msg.rs

@@ -93,7 +93,7 @@ enum_from_primitive! {
 
 /// Max theoretical size of a block filled with outputs.
 fn max_block_size() -> u64 {
-	(global::max_block_weight() / consensus::OUTPUT_WEIGHT * 708) as u64
+	(global::max_block_weight() / consensus::BLOCK_OUTPUT_WEIGHT * 708) as u64
 }
 
 // Max msg size when msg type is unknown.

pool/tests/block_max_weight.rs

@@ -82,7 +82,7 @@ fn test_block_building_max_weight() {
 		[2_500_000, 90_000, 80_000, 30_000, 70_000, 60_000]
 	);
 	assert_eq!(
-		txs.iter().map(|x| x.weight()).collect::<Vec<_>>(),
+		txs.iter().map(|x| x.weight_size()).collect::<Vec<_>>(),
 		[88, 46, 46, 25, 46, 46]
 	);
 	assert_eq!(
@@ -110,7 +110,7 @@ fn test_block_building_max_weight() {
 		[2_500_000, 90_000, 80_000, 70_000]
 	);
 	assert_eq!(
-		txs.iter().map(|x| x.weight()).collect::<Vec<_>>(),
+		txs.iter().map(|x| x.weight_size()).collect::<Vec<_>>(),
 		[88, 46, 46, 46]
 	);
 	assert_eq!(

pool/tests/nrd_kernels_enabled.rs

@@ -51,7 +51,7 @@ fn test_nrd_kernels_enabled() {
 
 	// Spend the initial coinbase.
 	let header_1 = chain.get_header_by_height(1).unwrap();
-	let mg = consensus::MILLI_GRIN;
+	let mg = consensus::MILLI_MWC;
 	let tx = test_transaction_spending_coinbase(
 		&keychain,
 		&header_1,