Skip to content

Commit

Permalink
Update weights
Browse files Browse the repository at this point in the history
The existing transaction weights were placeholder values.

This PR updates the weights to be more representative of their cost in
disk and bandwidth usage.

A single input is 57 bytes, and output is 737 B and a kernel is 147 B.
Normalising an input weight to one, an output would be 12.9 and a kernel 2.6.

There is one kernel per transaction; and one header per block. We can't
know what the header contribution is per transaction, but by rounding
the kernel weight to 3, each transaction can "pay" for a little part of
the header.

Rounding the output weight to 13 makes very little difference to
the overall arithmetic.

Overall, this makes every transaction, regardless of number of in- or
outputs roughly the same weight per byte; around 57 ±  bytes/weight.

Several tests were updated that used fees in theor assertions to account
for the new weights; typically by reducing the fee per gram specified in
the test transactions.
  • Loading branch information
CjS77 committed Apr 2, 2020
1 parent 63e1cb9 commit a5f3790
Show file tree
Hide file tree
Showing 15 changed files with 62 additions and 52 deletions.
12 changes: 6 additions & 6 deletions base_layer/core/src/mempool/orphan_pool/orphan_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,12 +168,12 @@ mod test {

#[test]
fn test_insert_rlu_and_ttl() {
let tx1 = Arc::new(tx!(MicroTari(10_000), fee: MicroTari(500), lock: 4000, inputs: 2, outputs: 1).0);
let tx2 = Arc::new(tx!(MicroTari(10_000), fee: MicroTari(300), lock: 3000, inputs: 2, outputs: 1).0);
let tx3 = Arc::new(tx!(MicroTari(10_000), fee: MicroTari(100), lock: 2500, inputs: 2, outputs: 1).0);
let tx4 = Arc::new(tx!(MicroTari(10_000), fee: MicroTari(200), lock: 1000, inputs: 2, outputs: 1).0);
let tx5 = Arc::new(tx!(MicroTari(10_000), fee: MicroTari(500), lock: 2000, inputs: 2, outputs: 1).0);
let tx6 = Arc::new(tx!(MicroTari(10_000), fee: MicroTari(600), lock: 5500, inputs: 2, outputs: 1).0);
let tx1 = Arc::new(tx!(MicroTari(100_000), fee: MicroTari(500), lock: 4000, inputs: 2, outputs: 1).0);
let tx2 = Arc::new(tx!(MicroTari(100_000), fee: MicroTari(300), lock: 3000, inputs: 2, outputs: 1).0);
let tx3 = Arc::new(tx!(MicroTari(100_000), fee: MicroTari(100), lock: 2500, inputs: 2, outputs: 1).0);
let tx4 = Arc::new(tx!(MicroTari(100_000), fee: MicroTari(200), lock: 1000, inputs: 2, outputs: 1).0);
let tx5 = Arc::new(tx!(MicroTari(100_000), fee: MicroTari(500), lock: 2000, inputs: 2, outputs: 1).0);
let tx6 = Arc::new(tx!(MicroTari(100_000), fee: MicroTari(600), lock: 5500, inputs: 2, outputs: 1).0);
let network = Network::LocalNet;
let consensus_manager = ConsensusManagerBuilder::new(network).build();
let store = create_mem_db(&consensus_manager);
Expand Down
6 changes: 3 additions & 3 deletions base_layer/core/src/mempool/pending_pool/pending_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ mod test {

#[test]
fn test_insert_and_lru() {
let tx1 = Arc::new(tx!(MicroTari(10_000), fee: MicroTari(50), lock: 500, inputs: 2, outputs: 1).0);
let tx1 = Arc::new(tx!(MicroTari(10_000), fee: MicroTari(49), lock: 500, inputs: 2, outputs: 1).0);
let tx2 = Arc::new(tx!(MicroTari(10_000), fee: MicroTari(20), lock: 2150, inputs: 1, outputs: 2).0);
let tx3 = Arc::new(tx!(MicroTari(10_000), fee: MicroTari(100), lock: 1000, inputs: 2, outputs: 1).0);
let tx4 = Arc::new(tx!(MicroTari(10_000), fee: MicroTari(30), lock: 2450, inputs: 2, outputs: 2).0);
Expand All @@ -276,7 +276,7 @@ mod test {
assert_eq!(pending_pool.len(), 3);
assert_eq!(
pending_pool.has_tx_with_excess_sig(&tx1.body.kernels()[0].excess_sig),
true
false
);
assert_eq!(
pending_pool.has_tx_with_excess_sig(&tx2.body.kernels()[0].excess_sig),
Expand All @@ -292,7 +292,7 @@ mod test {
);
assert_eq!(
pending_pool.has_tx_with_excess_sig(&tx5.body.kernels()[0].excess_sig),
false
true
);
assert_eq!(
pending_pool.has_tx_with_excess_sig(&tx6.body.kernels()[0].excess_sig),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ pub struct FeePriority(Vec<u8>);

impl FeePriority {
pub fn try_from(transaction: &Transaction) -> Result<Self, PriorityError> {
// The weights have been normalised, so the fee priority is now equal to the fee per gram ± a few pct points
let fee_per_byte = (transaction.calculate_ave_fee_per_gram() * 1000.0) as usize; // Include 3 decimal places before flooring
let mut fee_priority = fee_per_byte.to_binary()?;
fee_priority.reverse(); // Requires Big-endian for BtreeMap sorting
Expand Down
12 changes: 6 additions & 6 deletions base_layer/core/src/mempool/reorg_pool/reorg_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,12 +146,12 @@ mod test {

#[test]
fn test_insert_rlu_and_ttl() {
let tx1 = Arc::new(tx!(MicroTari(10_000), fee: MicroTari(500), lock: 4000, inputs: 2, outputs: 1).0);
let tx2 = Arc::new(tx!(MicroTari(10_000), fee: MicroTari(300), lock: 3000, inputs: 2, outputs: 1).0);
let tx3 = Arc::new(tx!(MicroTari(10_000), fee: MicroTari(100), lock: 2500, inputs: 2, outputs: 1).0);
let tx4 = Arc::new(tx!(MicroTari(10_000), fee: MicroTari(200), lock: 1000, inputs: 2, outputs: 1).0);
let tx5 = Arc::new(tx!(MicroTari(10_000), fee: MicroTari(500), lock: 2000, inputs: 2, outputs: 1).0);
let tx6 = Arc::new(tx!(MicroTari(10_000), fee: MicroTari(600), lock: 5500, inputs: 2, outputs: 1).0);
let tx1 = Arc::new(tx!(MicroTari(100_000), fee: MicroTari(500), lock: 4000, inputs: 2, outputs: 1).0);
let tx2 = Arc::new(tx!(MicroTari(100_000), fee: MicroTari(300), lock: 3000, inputs: 2, outputs: 1).0);
let tx3 = Arc::new(tx!(MicroTari(100_000), fee: MicroTari(100), lock: 2500, inputs: 2, outputs: 1).0);
let tx4 = Arc::new(tx!(MicroTari(100_000), fee: MicroTari(200), lock: 1000, inputs: 2, outputs: 1).0);
let tx5 = Arc::new(tx!(MicroTari(100_000), fee: MicroTari(500), lock: 2000, inputs: 2, outputs: 1).0);
let tx6 = Arc::new(tx!(MicroTari(100_000), fee: MicroTari(600), lock: 5500, inputs: 2, outputs: 1).0);

let reorg_pool = ReorgPool::new(ReorgPoolConfig {
storage_capacity: 3,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ mod test {
let tx2 = Arc::new(tx!(MicroTari(5_000), fee: MicroTari(20), inputs: 4, outputs: 1).0);
let tx3 = Arc::new(tx!(MicroTari(5_000), fee: MicroTari(100), inputs: 5, outputs: 1).0);
let tx4 = Arc::new(tx!(MicroTari(5_000), fee: MicroTari(30), inputs: 3, outputs: 1).0);
let tx5 = Arc::new(tx!(MicroTari(5_000), fee: MicroTari(50), inputs: 5, outputs: 1).0);
let tx5 = Arc::new(tx!(MicroTari(5_000), fee: MicroTari(55), inputs: 5, outputs: 1).0);

let mut unconfirmed_pool = UnconfirmedPool::new(UnconfirmedPoolConfig {
storage_capacity: 4,
Expand Down Expand Up @@ -270,12 +270,12 @@ mod test {
true
);
// Retrieve the set of highest priority unspent transactions
let desired_weight = tx1.calculate_weight() + tx3.calculate_weight() + tx4.calculate_weight();
let desired_weight = tx1.calculate_weight() + tx3.calculate_weight() + tx5.calculate_weight();
let selected_txs = unconfirmed_pool.highest_priority_txs(desired_weight).unwrap();
assert_eq!(selected_txs.len(), 3);
assert!(selected_txs.contains(&tx1));
assert!(selected_txs.contains(&tx3));
assert!(selected_txs.contains(&tx4));
assert!(selected_txs.contains(&tx5));
// Note that transaction tx5 could not be included as its weight was to big to fit into the remaining allocated
// space, the second best transaction was then included

Expand Down
2 changes: 1 addition & 1 deletion base_layer/core/src/transactions/aggregated_body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ impl AggregateBody {

/// Returns the byte size or weight of a body
pub fn calculate_weight(&self) -> u64 {
Fee::calculate_weight(self.inputs().len(), self.outputs().len())
Fee::calculate_weight(self.kernels().len(), self.inputs().len(), self.outputs().len())
}
}

Expand Down
24 changes: 16 additions & 8 deletions base_layer/core/src/transactions/fee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,25 @@ use crate::transactions::{tari_amount::*, transaction::MINIMUM_TRANSACTION_FEE};
pub struct Fee {}

pub const WEIGHT_PER_INPUT: u64 = 1;
pub const WEIGHT_PER_OUTPUT: u64 = 4;
pub const BASE_COST: u64 = 1;
pub const WEIGHT_PER_OUTPUT: u64 = 13;
pub const KERNEL_WEIGHT: u64 = 3; // Constant weight per transaction; covers kernel and part of header.

impl Fee {
/// Computes the absolute transaction fee given the fee-per-gram, and the size of the transaction
pub fn calculate(fee_per_gram: MicroTari, num_inputs: usize, num_outputs: usize) -> MicroTari {
(BASE_COST + Fee::calculate_weight(num_inputs, num_outputs) * u64::from(fee_per_gram)).into()
pub fn calculate(fee_per_gram: MicroTari, num_kernels: usize, num_inputs: usize, num_outputs: usize) -> MicroTari {
(Fee::calculate_weight(num_kernels, num_inputs, num_outputs) * u64::from(fee_per_gram)).into()
}

/// Computes the absolute transaction fee using `calculate`, but the resulting fee will always be at least the
/// minimum network transaction fee.
pub fn calculate_with_minimum(fee_per_gram: MicroTari, num_inputs: usize, num_outputs: usize) -> MicroTari {
let fee = Fee::calculate(fee_per_gram, num_inputs, num_outputs);
pub fn calculate_with_minimum(
fee_per_gram: MicroTari,
num_kernels: usize,
num_inputs: usize,
num_outputs: usize,
) -> MicroTari
{
let fee = Fee::calculate(fee_per_gram, num_kernels, num_inputs, num_outputs);
if fee < MINIMUM_TRANSACTION_FEE {
MINIMUM_TRANSACTION_FEE
} else {
Expand All @@ -46,7 +52,9 @@ impl Fee {
}

/// Calculate the weight of a transaction based on the number of inputs and outputs
pub fn calculate_weight(num_inputs: usize, num_outputs: usize) -> u64 {
WEIGHT_PER_INPUT * num_inputs as u64 + WEIGHT_PER_OUTPUT * num_outputs as u64
pub fn calculate_weight(num_kernels: usize, num_inputs: usize, num_outputs: usize) -> u64 {
KERNEL_WEIGHT * num_kernels as u64 +
WEIGHT_PER_INPUT * num_inputs as u64 +
WEIGHT_PER_OUTPUT * num_outputs as u64
}
}
2 changes: 1 addition & 1 deletion base_layer/core/src/transactions/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ pub fn create_tx(
unblinded_inputs.push(input.clone());
stx_builder.with_input(utxo, input);

let estimated_fee = Fee::calculate(fee_per_gram, input_count as usize, output_count as usize);
let estimated_fee = Fee::calculate(fee_per_gram, 1, input_count as usize, output_count as usize);
let amount_per_output = (amount - estimated_fee) / output_count;
let amount_for_last_output = (amount - estimated_fee) - amount_per_output * (output_count - 1);
for i in 0..output_count {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -557,7 +557,7 @@ mod test {
let b = TestParams::new();
let (utxo, input) = make_input(&mut OsRng, MicroTari(1200), &factories.commitment);
let mut builder = SenderTransactionProtocol::builder(1);
let fee = Fee::calculate(MicroTari(20), 1, 1);
let fee = Fee::calculate(MicroTari(20), 1, 1, 1);
builder
.with_lock_height(0)
.with_fee_per_gram(MicroTari(20))
Expand Down Expand Up @@ -615,7 +615,7 @@ mod test {
let b = TestParams::new();
let (utxo, input) = make_input(&mut OsRng, MicroTari(2500), &factories.commitment);
let mut builder = SenderTransactionProtocol::builder(1);
let fee = Fee::calculate(MicroTari(20), 1, 2);
let fee = Fee::calculate(MicroTari(20), 1, 1, 2);
builder
.with_lock_height(0)
.with_fee_per_gram(MicroTari(20))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,8 @@ impl SenderTransactionInitializer {

let total_amount = self.amounts.sum().ok_or_else(|| "Not all amounts have been provided")?;
let fee_per_gram = self.fee_per_gram.ok_or_else(|| "Fee per gram was not provided")?;
let fee_without_change = Fee::calculate(fee_per_gram, num_inputs, num_outputs);
let fee_with_change = Fee::calculate(fee_per_gram, num_inputs, num_outputs + 1);
let fee_without_change = Fee::calculate(fee_per_gram, 1, num_inputs, num_outputs);
let fee_with_change = Fee::calculate(fee_per_gram, 1, num_inputs, num_outputs + 1);
let extra_fee = fee_with_change - fee_without_change;
// Subtract with a check on going negative
let change_amount = total_being_spent.checked_sub(total_to_self + total_amount + fee_without_change);
Expand Down Expand Up @@ -313,7 +313,7 @@ impl SenderTransactionInitializer {
#[cfg(test)]
mod test {
use crate::transactions::{
fee::{Fee, BASE_COST, WEIGHT_PER_INPUT, WEIGHT_PER_OUTPUT},
fee::{Fee, KERNEL_WEIGHT, WEIGHT_PER_INPUT, WEIGHT_PER_OUTPUT},
helpers::{make_input, TestParams},
tari_amount::*,
transaction::{UnblindedOutput, MAX_TRANSACTION_INPUTS},
Expand Down Expand Up @@ -347,10 +347,10 @@ mod test {
.with_offset(p.offset)
.with_private_nonce(p.nonce);
builder.with_output(UnblindedOutput::new(MicroTari(100), p.spend_key, None));
let (utxo, input) = make_input(&mut OsRng, MicroTari(500), &factories.commitment);
let (utxo, input) = make_input(&mut OsRng, MicroTari(5_000), &factories.commitment);
builder.with_input(utxo, input);
builder.with_fee_per_gram(MicroTari(20));
let expected_fee = Fee::calculate(MicroTari(20), 1, 2);
let expected_fee = Fee::calculate(MicroTari(20), 1, 1, 2);
// We needed a change input, so this should fail
let err = builder.build::<Blake256>(&factories).unwrap_err();
assert_eq!(err.message, "Change spending key was not provided");
Expand Down Expand Up @@ -380,7 +380,7 @@ mod test {
let factories = CryptoFactories::default();
let p = TestParams::new();
let (utxo, input) = make_input(&mut OsRng, MicroTari(500), &factories.commitment);
let expected_fee = Fee::calculate(MicroTari(20), 1, 1);
let expected_fee = Fee::calculate(MicroTari(20), 1, 1, 1);
let output = UnblindedOutput::new(MicroTari(500) - expected_fee, p.spend_key, None);
// Start the builder
let mut builder = SenderTransactionInitializer::new(0);
Expand Down Expand Up @@ -414,8 +414,9 @@ mod test {
let factories = CryptoFactories::default();
let p = TestParams::new();
let (utxo, input) = make_input(&mut OsRng, MicroTari(500), &factories.commitment);
let expected_fee = MicroTari::from(BASE_COST + (WEIGHT_PER_INPUT + 1 * WEIGHT_PER_OUTPUT) * 20); // 101, output = 80
// Pay out so that I should get change, but not enough to pay for the output
let expected_fee = MicroTari::from((KERNEL_WEIGHT + WEIGHT_PER_INPUT + 1 * WEIGHT_PER_OUTPUT) * 20);
// fee == 340, output = 80
// Pay out so that I should get change, but not enough to pay for the output
let output = UnblindedOutput::new(MicroTari(500) - expected_fee - MicroTari(50), p.spend_key, None);
// Start the builder
let mut builder = SenderTransactionInitializer::new(0);
Expand Down Expand Up @@ -511,7 +512,7 @@ mod test {
// Create some inputs
let factories = CryptoFactories::default();
let p = TestParams::new();
let (utxo, input) = make_input(&mut OsRng, MicroTari(1000), &factories.commitment);
let (utxo, input) = make_input(&mut OsRng, MicroTari(100_000), &factories.commitment);
let output = UnblindedOutput::new(MicroTari(150), p.spend_key, None);
// Start the builder
let mut builder = SenderTransactionInitializer::new(2);
Expand Down Expand Up @@ -542,7 +543,7 @@ mod test {
let (utxo1, input1) = make_input(&mut OsRng, MicroTari(2000), &factories.commitment);
let (utxo2, input2) = make_input(&mut OsRng, MicroTari(3000), &factories.commitment);
let weight = MicroTari(30);
let expected_fee = Fee::calculate(weight, 2, 3);
let expected_fee = Fee::calculate(weight, 1, 2, 3);
let output = UnblindedOutput::new(MicroTari(1500) - expected_fee, p.spend_key, None);
// Start the builder
let mut builder = SenderTransactionInitializer::new(1);
Expand Down
10 changes: 5 additions & 5 deletions base_layer/core/tests/mempool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ fn test_insert_and_process_published_block() {
assert_eq!(stats.orphan_txs, 1);
assert_eq!(stats.timelocked_txs, 2);
assert_eq!(stats.published_txs, 0);
assert_eq!(stats.total_weight, 36);
assert_eq!(stats.total_weight, 120);

// Spend tx2, so it goes in Reorg pool, tx5 matures, so goes in Unconfirmed pool
generate_block(
Expand Down Expand Up @@ -219,7 +219,7 @@ fn test_insert_and_process_published_block() {
assert_eq!(stats.orphan_txs, 1);
assert_eq!(stats.timelocked_txs, 1);
assert_eq!(stats.published_txs, 1);
assert_eq!(stats.total_weight, 36);
assert_eq!(stats.total_weight, 120);
}

#[test]
Expand Down Expand Up @@ -501,15 +501,15 @@ fn request_response_get_stats() {
bob.mempool.insert(orphan1.clone()).unwrap();
bob.mempool.insert(orphan2.clone()).unwrap();

// The coinbase tx cannot be spent until maturity, so rxn1 will be in the timelocked pool. The other 2 txns are
// The coinbase tx cannot be spent until maturity, so txn1 will be in the timelocked pool. The other 2 txns are
// orphans.
let stats = bob.mempool.stats().unwrap();
assert_eq!(stats.total_txs, 3);
assert_eq!(stats.orphan_txs, 2);
assert_eq!(stats.unconfirmed_txs, 0);
assert_eq!(stats.timelocked_txs, 1);
assert_eq!(stats.published_txs, 0);
assert_eq!(stats.total_weight, 35);
assert_eq!(stats.total_weight, 116);

runtime.block_on(async {
// Alice will request mempool stats from Bob, and thus should be identical
Expand All @@ -519,7 +519,7 @@ fn request_response_get_stats() {
assert_eq!(received_stats.orphan_txs, 2);
assert_eq!(received_stats.timelocked_txs, 1);
assert_eq!(received_stats.published_txs, 0);
assert_eq!(received_stats.total_weight, 35);
assert_eq!(received_stats.total_weight, 116);

alice.comms.shutdown().await;
bob.comms.shutdown().await;
Expand Down
6 changes: 3 additions & 3 deletions base_layer/wallet/src/output_manager_service/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,7 @@ where
);
}

let fee_without_change = Fee::calculate(fee_per_gram, outputs.len(), 1);
let fee_without_change = Fee::calculate(fee_per_gram, 1, outputs.len(), 1);
let mut change_key: Option<PrivateKey> = None;
// If the input values > the amount to be sent + fees_without_change then we will need to include a change
// output
Expand Down Expand Up @@ -681,8 +681,8 @@ where
outputs.push(o.clone());
total += o.value;
// I am assuming that the only output will be the payment output and change if required
fee_without_change = Fee::calculate(fee_per_gram, outputs.len(), 1);
fee_with_change = Fee::calculate(fee_per_gram, outputs.len(), 2);
fee_without_change = Fee::calculate(fee_per_gram, 1, outputs.len(), 1);
fee_with_change = Fee::calculate(fee_per_gram, 1, outputs.len(), 2);

if total == amount + fee_without_change || total >= amount + fee_with_change {
break;
Expand Down
2 changes: 1 addition & 1 deletion base_layer/wallet/src/testnet_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -678,7 +678,7 @@ pub fn receive_test_transaction<
.runtime
.block_on(wallet.transaction_service.test_accept_transaction(
OsRng.next_u64(),
MicroTari::from(10_000 + OsRng.next_u64() % 10_1000),
MicroTari::from(10_000 + OsRng.next_u64() % 101_000),
public_key,
))?;

Expand Down
2 changes: 1 addition & 1 deletion base_layer/wallet/src/transaction_service/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1718,7 +1718,7 @@ where
fake_oms.add_output(uo).await?;

let mut stp = fake_oms
.prepare_transaction_to_send(amount, MicroTari::from(100), None, "".to_string())
.prepare_transaction_to_send(amount, MicroTari::from(25), None, "".to_string())
.await?;

let msg = stp.build_single_round_message()?;
Expand Down
4 changes: 2 additions & 2 deletions base_layer/wallet/tests/output_manager_service/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ fn send_no_change<T: OutputManagerBackend + 'static>(backend: T) {
let (mut oms, _, _shutdown, _) = setup_output_manager_service(&mut runtime, backend);

let fee_per_gram = MicroTari::from(20);
let fee_without_change = Fee::calculate(fee_per_gram, 2, 1);
let fee_without_change = Fee::calculate(fee_per_gram, 1, 2, 1);
let key1 = PrivateKey::random(&mut OsRng);
let value1 = 500;
runtime
Expand Down Expand Up @@ -347,7 +347,7 @@ fn send_not_enough_for_change<T: OutputManagerBackend + 'static>(backend: T) {
let (mut oms, _, _shutdown, _) = setup_output_manager_service(&mut runtime, backend);

let fee_per_gram = MicroTari::from(20);
let fee_without_change = Fee::calculate(fee_per_gram, 2, 1);
let fee_without_change = Fee::calculate(fee_per_gram, 1, 2, 1);
let key1 = PrivateKey::random(&mut OsRng);
let value1 = 500;
runtime
Expand Down

0 comments on commit a5f3790

Please sign in to comment.