Skip to content

Commit

Permalink
grin v5.3 (0022) Fixfees (mimblewimble#3481)
Browse files Browse the repository at this point in the history
See explanation: https://github.com/mimblewimble/grin-rfcs/blob/master/text/0017-fix-fees.md

                 * add FeeFields type

                 * use FeeFields with ::zero and try_into().unwrap()

                 * fixed tests

                 * avoid 0 accept_base_fee

                 * add aggregate_fee_fields method for transaction

                 * implement std::fmt::Display trait for FeeFields

                 * make base_fee argument non-optional in libtx::mod::tx_fee

                 * add global and thread local accept_fee_base; use to simplify tests

                 * set unusually high fee base for a change

                 * revert to optional base fee argument; default coming from either grin-{server,wallet}.toml

                 * remove optional base fee argument; can be set with global::set_local_accept_fee_base instead

                 * define constant global::DEFAULT_ACCEPT_FEE_BASE and set global value

                 * add Transaction::accept_fee() method and use

                 * Manual (de)ser impl on FeeFields

                 * fix comment bug

                 * Serialize FeeFields as int in tx

                 * allow feefields: u32:into() for tests

                 * try adding height args everywhere

                 * make FeeFields shift/fee methods height dependent

                 * prior to hf4 feefield testing

                 * rename selected fee_fields back to fee for serialization compatibility

                 * fix test_fee_fields test, merge conflict, and doctest use of obsolete fee_fields

                 * make accept_fee height dependent

                 * Accept any u64 in FeeFields deser
  • Loading branch information
bayk committed Jun 10, 2024
1 parent dc793c5 commit 566151f
Show file tree
Hide file tree
Showing 36 changed files with 876 additions and 373 deletions.
11 changes: 8 additions & 3 deletions api/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
use crate::chain;
use crate::core::core::hash::Hashed;
use crate::core::core::merkle_proof::MerkleProof;
use crate::core::core::{KernelFeatures, TxKernel};
use crate::core::core::{FeeFields, KernelFeatures, TxKernel};
use crate::core::{core, ser};
use crate::p2p;
use crate::util::secp::pedersen;
Expand Down Expand Up @@ -528,6 +528,7 @@ impl<'de> serde::de::Deserialize<'de> for OutputPrintable {
#[derive(Debug, Serialize, Deserialize, Clone)]
pub struct TxKernelPrintable {
pub features: String,
pub fee_shift: u8,
pub fee: u64,
pub lock_height: u64,
pub excess: String,
Expand All @@ -537,17 +538,21 @@ pub struct TxKernelPrintable {
impl TxKernelPrintable {
pub fn from_txkernel(k: &core::TxKernel) -> TxKernelPrintable {
let features = k.features.as_string();
let (fee, lock_height) = match k.features {
let (fee_fields, lock_height) = match k.features {
KernelFeatures::Plain { fee } => (fee, 0),
KernelFeatures::Coinbase => (0, 0),
KernelFeatures::Coinbase => (FeeFields::zero(), 0),
KernelFeatures::HeightLocked { fee, lock_height } => (fee, lock_height),
KernelFeatures::NoRecentDuplicate {
fee,
relative_height,
} => (fee, relative_height.into()),
};
// Printing for the most advanced version that we have. In prev versions the shift is 0, we should be good
let fee = fee_fields.fee(u64::MAX);
let fee_shift: u8 = fee_fields.fee_shift(u64::MAX);
TxKernelPrintable {
features,
fee_shift,
fee,
lock_height,
excess: k.excess.to_hex(),
Expand Down
2 changes: 1 addition & 1 deletion chain/src/pipe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,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_as_block(0, num_outputs, num_kernels);
let weight = TransactionBody::weight_by_iok(0, num_outputs, num_kernels);
if weight > global::max_block_weight() {
return Err(ErrorKind::Block(block::Error::TooHeavy).into());
}
Expand Down
6 changes: 3 additions & 3 deletions chain/tests/mine_nrd_kernel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ where
{
let prev = chain.head_header().unwrap();
let next_header_info = consensus::next_difficulty(1, chain.difficulty_iter().unwrap());
let fee = txs.iter().map(|x| x.fee()).sum();
let fee = txs.iter().map(|x| x.fee(prev.height + 1)).sum();
let reward = reward::output(
keychain,
&ProofBuilder::new(keychain),
Expand Down Expand Up @@ -91,7 +91,7 @@ fn mine_block_with_nrd_kernel_and_nrd_feature_enabled() {
let key_id2 = ExtKeychainPath::new(1, 2, 0, 0, 0).to_identifier();
let tx = build::transaction(
KernelFeatures::NoRecentDuplicate {
fee: 20000,
fee: 20000.into(),
relative_height: NRDRelativeHeight::new(1440).unwrap(),
},
&[
Expand Down Expand Up @@ -138,7 +138,7 @@ fn mine_invalid_block_with_nrd_kernel_and_nrd_feature_enabled_before_hf() {
let key_id2 = ExtKeychainPath::new(1, 2, 0, 0, 0).to_identifier();
let tx = build::transaction(
KernelFeatures::NoRecentDuplicate {
fee: 20000,
fee: 20000.into(),
relative_height: NRDRelativeHeight::new(1440).unwrap(),
},
&[
Expand Down
9 changes: 5 additions & 4 deletions chain/tests/mine_simple_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,7 @@ fn spend_rewind_spend() {
let key_id30 = ExtKeychainPath::new(1, 30, 0, 0, 0).to_identifier();

let tx1 = build::transaction(
KernelFeatures::Plain { fee: 20000 },
KernelFeatures::Plain { fee: 20000.into() },
&[
build::coinbase_input(consensus::MWC_FIRST_GROUP_REWARD, key_id_coinbase.clone()),
build::output(consensus::MWC_FIRST_GROUP_REWARD - 20000, key_id30.clone()),
Expand Down Expand Up @@ -645,7 +645,7 @@ fn spend_in_fork_and_compact() {
let key_id31 = ExtKeychainPath::new(1, 31, 0, 0, 0).to_identifier();

let tx1 = build::transaction(
KernelFeatures::Plain { fee: 20000 },
KernelFeatures::Plain { fee: 20000.into() },
&[
build::coinbase_input(consensus::MWC_FIRST_GROUP_REWARD, key_id2.clone()),
build::output(consensus::MWC_FIRST_GROUP_REWARD - 20000, key_id30.clone()),
Expand All @@ -663,7 +663,7 @@ fn spend_in_fork_and_compact() {
chain.validate(false).unwrap();

let tx2 = build::transaction(
KernelFeatures::Plain { fee: 20000 },
KernelFeatures::Plain { fee: 20000.into() },
&[
build::input(consensus::MWC_FIRST_GROUP_REWARD - 20000, key_id30.clone()),
build::output(consensus::MWC_FIRST_GROUP_REWARD - 40000, key_id31.clone()),
Expand Down Expand Up @@ -899,7 +899,8 @@ where
let proof_size = global::proofsize();
let key_id = ExtKeychainPath::new(1, key_idx, 0, 0, 0).to_identifier();

let fees = txs.iter().map(|tx| tx.fee()).sum();
let height = prev.height + 1;
let fees = txs.iter().map(|tx| tx.fee(height)).sum();
let reward = libtx::reward::output(
kc,
&libtx::ProofBuilder::new(kc),
Expand Down
8 changes: 4 additions & 4 deletions chain/tests/nrd_validation_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ where
{
let next_header_info =
consensus::next_difficulty(prev.height, chain.difficulty_iter().unwrap());
let fee = txs.iter().map(|x| x.fee()).sum();
let fee = txs.iter().map(|x| x.fee(prev.height + 1)).sum();
let reward = reward::output(
keychain,
&ProofBuilder::new(keychain),
Expand Down Expand Up @@ -107,7 +107,7 @@ fn process_block_nrd_validation() -> Result<(), Error> {
assert_eq!(chain.head()?.height, 8);

let mut kernel = TxKernel::with_features(KernelFeatures::NoRecentDuplicate {
fee: 20000,
fee: 20000.into(),
relative_height: NRDRelativeHeight::new(2)?,
});

Expand Down Expand Up @@ -223,7 +223,7 @@ fn process_block_nrd_validation_relative_height_1() -> Result<(), Error> {
assert_eq!(chain.head()?.height, 8);

let mut kernel = TxKernel::with_features(KernelFeatures::NoRecentDuplicate {
fee: 20000,
fee: 20000.into(),
relative_height: NRDRelativeHeight::new(1)?,
});

Expand Down Expand Up @@ -322,7 +322,7 @@ fn process_block_nrd_validation_fork() -> Result<(), Error> {
assert_eq!(header_8.height, 8);

let mut kernel = TxKernel::with_features(KernelFeatures::NoRecentDuplicate {
fee: 20000,
fee: 20000.into(),
relative_height: NRDRelativeHeight::new(2)?,
});

Expand Down
11 changes: 7 additions & 4 deletions chain/tests/process_block_cut_through.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use self::chain_test_helper::{clean_output_dir, genesis_block, init_chain};
use crate::chain::{pipe, Chain, Options};
use crate::core::core::verifier_cache::LruVerifierCache;
use crate::core::core::{block, pmmr, transaction};
use crate::core::core::{Block, KernelFeatures, Transaction, Weighting};
use crate::core::core::{Block, FeeFields, KernelFeatures, Transaction, Weighting};
use crate::core::libtx::{build, reward, ProofBuilder};
use crate::core::{consensus, global, pow};
use crate::keychain::{ExtKeychain, ExtKeychainPath, Keychain, SwitchCommitmentType};
Expand All @@ -43,7 +43,7 @@ where
let prev = chain.head_header().unwrap();
let next_height = prev.height + 1;
let next_header_info = consensus::next_difficulty(1, chain.difficulty_iter()?);
let fee = txs.iter().map(|x| x.fee()).sum();
let fee = txs.iter().map(|x| x.fee(next_height)).sum();
let key_id = ExtKeychainPath::new(1, next_height as u32, 0, 0, 0).to_identifier();
let reward = reward::output(
keychain,
Expand Down Expand Up @@ -114,7 +114,9 @@ fn process_block_cut_through() -> Result<(), chain::Error> {
// Note: We reuse key_ids resulting in an input and an output sharing the same commitment.
// The input is coinbase and the output is plain.
let tx = build::transaction(
KernelFeatures::Plain { fee: 0 },
KernelFeatures::Plain {
fee: FeeFields::zero(),
},
&[
build::coinbase_input(consensus::MWC_FIRST_GROUP_REWARD, key_id1.clone()),
build::coinbase_input(consensus::MWC_FIRST_GROUP_REWARD, key_id2.clone()),
Expand Down Expand Up @@ -143,8 +145,9 @@ fn process_block_cut_through() -> Result<(), chain::Error> {
let verifier_cache = Arc::new(RwLock::new(LruVerifierCache::new()));

// Transaction is invalid due to cut-through.
let height = 7;
assert_eq!(
tx.validate(Weighting::AsTransaction, verifier_cache.clone()),
tx.validate(Weighting::AsTransaction, verifier_cache.clone(), height),
Err(transaction::Error::CutThrough),
);

Expand Down
10 changes: 5 additions & 5 deletions chain/tests/test_coinbase_maturity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ fn test_coinbase_maturity() {
// here we build a tx that attempts to spend the earlier coinbase output
// this is not a valid tx as the coinbase output cannot be spent yet
let coinbase_txn = build::transaction(
KernelFeatures::Plain { fee: 2 },
KernelFeatures::Plain { fee: 2.into() },
&[
build::coinbase_input(amount, key_id1.clone()),
build::output(amount - 2, key_id2.clone()),
Expand All @@ -111,7 +111,7 @@ fn test_coinbase_maturity() {
.unwrap();

let txs = &[coinbase_txn.clone()];
let fees = txs.iter().map(|tx| tx.fee()).sum();
let fees = txs.iter().map(|tx| tx.fee(prev.height + 1)).sum();
let reward = libtx::reward::output(&keychain, &builder, &key_id3, fees, false, 1).unwrap();
let next_header_info =
consensus::next_difficulty(prev.height + 1, chain.difficulty_iter().unwrap());
Expand Down Expand Up @@ -186,7 +186,7 @@ fn test_coinbase_maturity() {
// here we build a tx that attempts to spend the earlier coinbase output
// this is not a valid tx as the coinbase output cannot be spent yet
let coinbase_txn = build::transaction(
KernelFeatures::Plain { fee: 2 },
KernelFeatures::Plain { fee: 2.into() },
&[
build::coinbase_input(amount, key_id1.clone()),
build::output(amount - 2, key_id2.clone()),
Expand All @@ -197,7 +197,7 @@ fn test_coinbase_maturity() {
.unwrap();

let txs = &[coinbase_txn.clone()];
let fees = txs.iter().map(|tx| tx.fee()).sum();
let fees = txs.iter().map(|tx| tx.fee(prev.height + 1)).sum();
let reward =
libtx::reward::output(&keychain, &builder, &key_id3, fees, false, 1).unwrap();
let next_header_info =
Expand Down Expand Up @@ -267,7 +267,7 @@ fn test_coinbase_maturity() {
.unwrap();

let txs = &[coinbase_txn];
let fees = txs.iter().map(|tx| tx.fee()).sum();
let fees = txs.iter().map(|tx| tx.fee(prev.height + 1)).sum();
let next_header_info =
consensus::next_difficulty(prev.height + 1, chain.difficulty_iter().unwrap());
let reward =
Expand Down
6 changes: 3 additions & 3 deletions core/src/consensus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,13 +113,13 @@ pub const CUT_THROUGH_HORIZON: u32 = WEEK_HEIGHT as u32;
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 BLOCK_INPUT_WEIGHT: u64 = 1;
pub const INPUT_WEIGHT: u64 = 1;

/// Weight of an output when counted against the max block weight capacity
pub const BLOCK_OUTPUT_WEIGHT: u64 = 21;
pub const OUTPUT_WEIGHT: u64 = 21;

/// Weight of a kernel when counted against the max block weight capacity
pub const BLOCK_KERNEL_WEIGHT: u64 = 3;
pub const KERNEL_WEIGHT: u64 = 3;

/// Total maximum block weight. At current sizes, this means a maximum
/// theoretical size of:
Expand Down
9 changes: 3 additions & 6 deletions core/src/core/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -503,11 +503,8 @@ impl Readable for UntrustedBlockHeader {
}

// Validate global output and kernel MMR sizes against upper bounds based on block height.
let global_weight = TransactionBody::weight_as_block(
0,
header.output_mmr_count(),
header.kernel_mmr_count(),
);
let global_weight =
TransactionBody::weight_by_iok(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(),
Expand Down Expand Up @@ -736,7 +733,7 @@ impl Block {

/// Sum of all fees (inputs less outputs) in the block
pub fn total_fees(&self) -> u64 {
self.body.fee()
self.body.fee(self.header.height)
}

/// "Lightweight" validation that we can perform quickly during read/deserialization.
Expand Down
Loading

0 comments on commit 566151f

Please sign in to comment.