Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixfees #3481

Merged
merged 27 commits into from
Nov 26, 2020
Merged

Fixfees #3481

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
a4881c2
add FeeFields type
tromp Oct 23, 2020
e6bf3f1
use FeeFields with ::zero and try_into().unwrap()
tromp Oct 24, 2020
9ef55d2
fixed tests
tromp Oct 24, 2020
14716f5
avoid 0 accept_base_fee
tromp Oct 25, 2020
ed46e92
add aggregate_fee_fields method for transaction
tromp Oct 26, 2020
daa2004
implement std::fmt::Display trait for FeeFields
tromp Oct 26, 2020
29c797c
make base_fee argument non-optional in libtx::mod::tx_fee
tromp Oct 29, 2020
40a03c6
add global and thread local accept_fee_base; use to simplify tests
tromp Nov 1, 2020
23e563e
set unusually high fee base for a change
tromp Nov 1, 2020
e40751b
revert to optional base fee argument; default coming from either grin…
tromp Nov 2, 2020
f17c816
remove optional base fee argument; can be set with global::set_local_…
tromp Nov 2, 2020
d83fbb1
define constant global::DEFAULT_ACCEPT_FEE_BASE and set global value
tromp Nov 2, 2020
9b373ef
add Transaction::accept_fee() method and use
tromp Nov 4, 2020
843a6b7
Manual (de)ser impl on FeeFields
jaspervdm Nov 6, 2020
c4137cc
fix comment bug
tromp Nov 6, 2020
73995da
Serialize FeeFields as int in tx
jaspervdm Nov 11, 2020
d40b9c8
allow feefields: u32:into() for tests
tromp Nov 13, 2020
0fe1198
try adding height args everywhere
tromp Nov 22, 2020
bc5799a
make FeeFields shift/fee methods height dependent
tromp Nov 23, 2020
65aec68
prior to hf4 feefield testing
tromp Nov 23, 2020
3cfcb4d
rename selected fee_fields back to fee for serialization compatibility
tromp Nov 24, 2020
69f6e57
Merge branch 'master' into fixfees
tromp Nov 24, 2020
48dda85
Merge branch 'master' into fixfees
tromp Nov 25, 2020
0d59394
fix test_fee_fields test, merge conflict, and doctest use of obsolete…
tromp Nov 25, 2020
346dfb9
merge latest master
tromp Nov 25, 2020
f4b7c30
make accept_fee height dependent
tromp Nov 26, 2020
2e5a3dc
Accept any u64 in FeeFields deser
jaspervdm Nov 26, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions api/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@
// limitations under the License.

use crate::chain;
use crate::core::consensus::YEAR_HEIGHT;
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 @@ -499,6 +500,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 @@ -508,17 +510,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()),
};
let height = 2 * YEAR_HEIGHT; // print as if post-HF4
let fee = fee_fields.fee(height);
let fee_shift: u8 = fee_fields.fee_shift(height);
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 @@ -355,7 +355,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), key_id, fee, false).unwrap();

Expand Down Expand Up @@ -84,7 +84,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 @@ -131,7 +131,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::REWARD, key_id_coinbase.clone()),
build::output(consensus::REWARD - 20000, key_id30.clone()),
Expand Down Expand Up @@ -642,7 +642,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::REWARD, key_id2.clone()),
build::output(consensus::REWARD - 20000, key_id30.clone()),
Expand All @@ -660,7 +660,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::REWARD - 20000, key_id30.clone()),
build::output(consensus::REWARD - 40000, key_id31.clone()),
Expand Down Expand Up @@ -885,7 +885,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), &key_id, fees, false).unwrap();
let mut b = match core::core::Block::new(prev, txs, Difficulty::from_num(diff), reward) {
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), key_id, fee, false).unwrap();

Expand Down Expand Up @@ -100,7 +100,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 @@ -216,7 +216,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 @@ -315,7 +315,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, &ProofBuilder::new(keychain), &key_id, fee, false).unwrap();
Expand Down Expand Up @@ -104,7 +104,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::REWARD, key_id1.clone()),
build::coinbase_input(consensus::REWARD, key_id2.clone()),
Expand All @@ -129,8 +131,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).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).unwrap();
let next_header_info =
consensus::next_difficulty(prev.height + 1, chain.difficulty_iter().unwrap());
Expand Down Expand Up @@ -266,7 +266,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 = libtx::reward::output(&keychain, &builder, &key_id4, fees, false).unwrap();
Expand Down
6 changes: 3 additions & 3 deletions core/src/consensus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,13 +99,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 @@ -469,11 +469,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);
}
Expand Down Expand Up @@ -700,7 +697,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