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.

Sign up for GitHub

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

feat(miner): no batch fee in PCD & lower batch balancer to 2 #1595

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
+65 −319
Draft

feat(miner): no batch fee in PCD & lower batch balancer to 2 #1595

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 0 additions & 13 deletions actors/miner/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1586,19 +1586,6 @@ impl Actor {
let mut fee_to_burn = TokenAmount::zero();
let mut needs_cron = false;
rt.transaction(|state: &mut State, rt| {
// Aggregate fee applies only when batching.
if sectors.len() > 1 {
let aggregate_fee = aggregate_pre_commit_network_fee(sectors.len(), &rt.base_fee());
// AggregateFee applied to fee debt to consolidate burn with outstanding debts
state.apply_penalty(&aggregate_fee)
.map_err(|e| {
actor_error!(
illegal_state,
"failed to apply penalty: {}",
e
)
})?;
}
// available balance already accounts for fee debt so it is correct to call
// this before RepayDebts. We would have to
// subtract fee debt explicitly if we called this after.
Expand Down
10 changes: 1 addition & 9 deletions actors/miner/src/monies.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ pub mod detail {
use super::*;

lazy_static! {
pub static ref BATCH_BALANCER: TokenAmount = TokenAmount::from_nano(5);
pub static ref BATCH_BALANCER: TokenAmount = TokenAmount::from_nano(2);
}

// BR but zero values are clamped at 1 attofil
Expand Down Expand Up @@ -328,7 +328,6 @@ const BATCH_DISCOUNT_DENOM: u32 = 20;

lazy_static! {
static ref ESTIMATED_SINGLE_PROVE_COMMIT_GAS_USAGE: BigInt = BigInt::from(49299973);
static ref ESTIMATED_SINGLE_PRE_COMMIT_GAS_USAGE: BigInt = BigInt::from(16433324);
}

pub fn aggregate_prove_commit_network_fee(
Expand All @@ -338,13 +337,6 @@ pub fn aggregate_prove_commit_network_fee(
aggregate_network_fee(aggregate_size, &ESTIMATED_SINGLE_PROVE_COMMIT_GAS_USAGE, base_fee)
}

pub fn aggregate_pre_commit_network_fee(
aggregate_size: usize,
base_fee: &TokenAmount,
) -> TokenAmount {
aggregate_network_fee(aggregate_size, &ESTIMATED_SINGLE_PRE_COMMIT_GAS_USAGE, base_fee)
}

pub fn aggregate_network_fee(
aggregate_size: usize,
gas_usage: &BigInt,
Expand Down
91 changes: 32 additions & 59 deletions actors/miner/tests/aggregate_network_fee_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,89 +1,62 @@
use fil_actor_miner::aggregate_prove_commit_network_fee;
use fil_actor_miner::detail::BATCH_BALANCER;
use fil_actor_miner::{aggregate_pre_commit_network_fee, aggregate_prove_commit_network_fee};
use fvm_shared::econ::TokenAmount;
use num_traits::zero;

#[test]
fn constant_fee_per_sector_when_base_fee_is_below_5_nfil() {
for fee_func in [aggregate_prove_commit_network_fee, aggregate_pre_commit_network_fee] {
let one_sector_fee = fee_func(1, &zero());
let ten_sector_fee = fee_func(10, &zero());
assert_eq!(&one_sector_fee * 10, ten_sector_fee);
fn constant_fee_per_sector_when_base_fee_is_below_batch_balancer() {
let one_sector_fee = aggregate_prove_commit_network_fee(1, &zero());
let ten_sector_fee = aggregate_prove_commit_network_fee(10, &zero());
assert_eq!(&one_sector_fee * 10, ten_sector_fee);

let forty_sector_fee = fee_func(40, &TokenAmount::from_nano(1));
assert_eq!(&one_sector_fee * 40, forty_sector_fee);
let forty_sector_fee = aggregate_prove_commit_network_fee(40, &TokenAmount::from_nano(1));
assert_eq!(&one_sector_fee * 40, forty_sector_fee);

let two_hundred_sector_fee = fee_func(200, &TokenAmount::from_nano(3));
assert_eq!(one_sector_fee * 200, two_hundred_sector_fee);
}
let two_hundred_sector_fee =
aggregate_prove_commit_network_fee(200, &TokenAmount::from_nano(2));
assert_eq!(one_sector_fee * 200, two_hundred_sector_fee);
}

#[test]
fn fee_increases_if_basefee_crosses_threshold() {
for fee_func in [aggregate_prove_commit_network_fee, aggregate_pre_commit_network_fee] {
let at_no_base_fee = fee_func(10, &zero());
let at_balance_minus_one_base_fee =
fee_func(10, &(&*BATCH_BALANCER - TokenAmount::from_atto(1)));
let at_balance_base_fee = fee_func(10, &BATCH_BALANCER);
let at_balance_plus_one_base_fee =
fee_func(10, &(&*BATCH_BALANCER + TokenAmount::from_nano(1)));
let at_balance_plus_two_base_fee =
fee_func(10, &(&*BATCH_BALANCER + TokenAmount::from_nano(2)));
let at_balance_times_two_base = fee_func(10, &(2 * &*BATCH_BALANCER));

assert_eq!(at_no_base_fee, at_balance_minus_one_base_fee);
assert_eq!(at_no_base_fee, at_balance_base_fee);
assert!(at_balance_base_fee < at_balance_plus_one_base_fee);
assert!(at_balance_plus_one_base_fee < at_balance_plus_two_base_fee);
assert_eq!(at_balance_times_two_base, 2 * at_balance_base_fee);
}
let at_no_base_fee = aggregate_prove_commit_network_fee(10, &zero());
let at_balance_minus_one_base_fee =
aggregate_prove_commit_network_fee(10, &(&*BATCH_BALANCER - TokenAmount::from_atto(1)));
let at_balance_base_fee = aggregate_prove_commit_network_fee(10, &BATCH_BALANCER);
let at_balance_plus_one_base_fee =
aggregate_prove_commit_network_fee(10, &(&*BATCH_BALANCER + TokenAmount::from_nano(1)));
let at_balance_plus_two_base_fee =
aggregate_prove_commit_network_fee(10, &(&*BATCH_BALANCER + TokenAmount::from_nano(2)));
let at_balance_times_two_base = aggregate_prove_commit_network_fee(10, &(2 * &*BATCH_BALANCER));

assert_eq!(at_no_base_fee, at_balance_minus_one_base_fee);
assert_eq!(at_no_base_fee, at_balance_base_fee);
assert!(at_balance_base_fee < at_balance_plus_one_base_fee);
assert!(at_balance_plus_one_base_fee < at_balance_plus_two_base_fee);
assert_eq!(at_balance_times_two_base, 2 * at_balance_base_fee);
}

#[test]
fn regression_tests() {
let magic_number = 65733297;
let magic_number = 49299973;
let fee = |aggregate_size, base_fee_multiplier| {
aggregate_prove_commit_network_fee(
aggregate_size,
&TokenAmount::from_nano(base_fee_multiplier),
) + aggregate_pre_commit_network_fee(
aggregate_size,
&TokenAmount::from_nano(base_fee_multiplier),
)
};

// (5/20) * x * 10 = (5/2) * x
let expected = (TokenAmount::from_nano(5) * magic_number).div_floor(2);
// Under batch balancer (2), so these two are the same:
// 2/20 * x * 10 = (2/2) * x = x
let expected = TokenAmount::from_nano(magic_number);
assert_eq!(expected, fee(10, 0));
assert_eq!(expected, fee(10, 1));

let expected = TokenAmount::from_nano(25) * magic_number;
// 3/20 * x * 100 = 15 * x
let expected = TokenAmount::from_nano(15) * magic_number;
assert_eq!(expected, fee(100, 3));

// 6/20 * x * 100 = 30 * x
let expected = TokenAmount::from_nano(30) * magic_number;
assert_eq!(expected, fee(100, 6));
}

#[test]
fn split_25_75() {
// check 25/75% split up to uFIL precision
let one_micro_fil = TokenAmount::from_nano(1000);

for base_fee_multiplier in [0, 5, 20] {
for aggregate_size in [13, 303] {
let fee_pre = aggregate_pre_commit_network_fee(
aggregate_size,
&TokenAmount::from_nano(base_fee_multiplier),
)
.atto()
/ one_micro_fil.atto();
let fee_prove = aggregate_prove_commit_network_fee(
aggregate_size,
&TokenAmount::from_nano(base_fee_multiplier),
)
.atto()
/ one_micro_fil.atto();
assert_eq!(fee_prove, 3 * fee_pre);
}
}
}
184 changes: 5 additions & 179 deletions actors/miner/tests/batch_method_network_fees_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
use std::collections::HashMap;

use fil_actor_miner::{
aggregate_pre_commit_network_fee, aggregate_prove_commit_network_fee,
pre_commit_deposit_for_power, qa_power_max, PreCommitSectorBatchParams, State,
aggregate_prove_commit_network_fee, pre_commit_deposit_for_power, qa_power_max,
PreCommitSectorBatchParams, State,
};
use fil_actors_runtime::test_utils::{expect_abort, expect_abort_contains_message};
use fil_actors_runtime::test_utils::expect_abort;
use fvm_ipld_bitfield::BitField;
use fvm_shared::{clock::ChainEpoch, econ::TokenAmount, error::ExitCode};
use lazy_static::lazy_static;
Expand Down Expand Up @@ -67,166 +65,6 @@ fn insufficient_funds_for_aggregated_prove_commit_network_fee() {
rt.reset();
}

#[test]
fn insufficient_funds_for_batch_precommit_network_fee() {
let actor = ActorHarness::new(*PERIOD_OFFSET);
let rt = actor.new_runtime();
rt.set_balance(BIG_BALANCE.clone());
let precommit_epoch = *PERIOD_OFFSET + 1;
rt.set_epoch(precommit_epoch);
actor.construct_and_verify(&rt);
let dl_info = actor.deadline(&rt);
// something on deadline boundary but > 180 days
let expiration =
dl_info.period_end() + rt.policy.wpost_proving_period * DEFAULT_SECTOR_EXPIRATION as i64;

let mut precommits = Vec::new();
let mut sector_nos_bf = BitField::new();
for i in 0..4 {
sector_nos_bf.set(i);
let precommit = actor.make_pre_commit_params(i, precommit_epoch - 1, expiration, vec![]);
precommits.push(precommit);
}

// set base fee extremely high so AggregateProveCommitNetworkFee is > 1000 FIL. Set balance to 1000 FIL to easily cover PCD but not network fee
let balance = TokenAmount::from_whole(1000);
rt.set_balance(balance.clone());
let base_fee = TokenAmount::from_atto(10u64.pow(16));
rt.base_fee.replace(base_fee.clone());
assert!(aggregate_pre_commit_network_fee(precommits.len(), &base_fee) > balance);

let res = actor.pre_commit_sector_batch(
&rt,
PreCommitSectorBatchParams { sectors: precommits },
&PreCommitBatchConfig { first_for_miner: true, ..Default::default() },
&base_fee,
);

// state untouched
let state: State = rt.get_state();
assert!(state.pre_commit_deposits.is_zero());
let expirations = actor.collect_precommit_expirations(&rt, &state);
assert_eq!(HashMap::new(), expirations);

expect_abort_contains_message(
ExitCode::USR_INSUFFICIENT_FUNDS,
"unlocked balance can not repay fee debt",
res,
);
rt.reset();
}

#[test]
fn insufficient_funds_for_batch_precommit_in_combination_of_fee_debt_and_network_fee() {
let actor = ActorHarness::new(*PERIOD_OFFSET);
let rt = actor.new_runtime();
rt.set_balance(BIG_BALANCE.clone());
let precommit_epoch = *PERIOD_OFFSET + 1;
rt.set_epoch(precommit_epoch);
actor.construct_and_verify(&rt);
let dl_info = actor.deadline(&rt);
// something on deadline boundary but > 180 days
let expiration =
dl_info.period_end() + rt.policy.wpost_proving_period * DEFAULT_SECTOR_EXPIRATION as i64;

let mut precommits = Vec::new();
let mut sector_nos_bf = BitField::new();
for i in 0..4 {
sector_nos_bf.set(i);
let precommit = actor.make_pre_commit_params(i, precommit_epoch - 1, expiration, vec![]);
precommits.push(precommit);
}

// set base fee extremely high so AggregateProveCommitNetworkFee is > 1000 FIL. Set balance to 1000 FIL to easily cover PCD but not network fee
let base_fee = TokenAmount::from_atto(10u64.pow(16));
rt.base_fee.replace(base_fee.clone());
let net_fee = aggregate_pre_commit_network_fee(precommits.len(), &base_fee);

// setup miner to have fee debt equal to net fee
let mut state: State = rt.get_state();
state.fee_debt = net_fee.clone();
rt.replace_state(&state);

// give miner almost enough balance to pay both
let balance = (2 * net_fee) - TokenAmount::from_atto(1);
rt.set_balance(balance);

let res = actor.pre_commit_sector_batch(
&rt,
PreCommitSectorBatchParams { sectors: precommits },
&PreCommitBatchConfig { first_for_miner: true, ..Default::default() },
&base_fee,
);

// state untouched
let state: State = rt.get_state();
assert!(state.pre_commit_deposits.is_zero());
let expirations = actor.collect_precommit_expirations(&rt, &state);
assert_eq!(HashMap::new(), expirations);

expect_abort_contains_message(
ExitCode::USR_INSUFFICIENT_FUNDS,
"unlocked balance can not repay fee debt",
res,
);
rt.reset();
}

#[test]
fn enough_funds_for_fee_debt_and_network_fee_but_not_for_pcd() {
let actor = ActorHarness::new(*PERIOD_OFFSET);
let rt = actor.new_runtime();
rt.set_balance(BIG_BALANCE.clone());
let precommit_epoch = *PERIOD_OFFSET + 1;
rt.set_epoch(precommit_epoch);
actor.construct_and_verify(&rt);
let dl_info = actor.deadline(&rt);
// something on deadline boundary but > 180 days
let expiration =
dl_info.period_end() + rt.policy.wpost_proving_period * DEFAULT_SECTOR_EXPIRATION as i64;

let mut precommits = Vec::new();
let mut sector_nos_bf = BitField::new();
for i in 0..4 {
sector_nos_bf.set(i);
let precommit = actor.make_pre_commit_params(i, precommit_epoch - 1, expiration, vec![]);
precommits.push(precommit);
}

// set base fee and fee debt high
let base_fee = TokenAmount::from_atto(10u64.pow(16));
rt.base_fee.replace(base_fee.clone());
let net_fee = aggregate_pre_commit_network_fee(precommits.len(), &base_fee);
// setup miner to have feed debt equal to net fee
let mut state: State = rt.get_state();
state.fee_debt = net_fee.clone();
rt.replace_state(&state);

// give miner enough balance to pay both but not any extra for pcd
let balance = 2 * net_fee;
rt.set_balance(balance);

let res = actor.pre_commit_sector_batch(
&rt,
PreCommitSectorBatchParams { sectors: precommits },
&PreCommitBatchConfig { first_for_miner: true, ..Default::default() },
&base_fee,
);

expect_abort_contains_message(
ExitCode::USR_INSUFFICIENT_FUNDS,
"insufficient funds 0.0 for pre-commit deposit",
res,
);
rt.reset();

// state untouched
let state: State = rt.get_state();
assert!(state.pre_commit_deposits.is_zero());
let expirations = actor.collect_precommit_expirations(&rt, &state);
assert_eq!(HashMap::new(), expirations);
}

#[test]
fn enough_funds_for_everything() {
let actor = ActorHarness::new(*PERIOD_OFFSET);
Expand All @@ -248,32 +86,20 @@ fn enough_funds_for_everything() {
precommits.push(precommit);
}

// set base fee extremely high so AggregateProveCommitNetworkFee is > 1000 FIL. Set balance to 1000 FIL to easily cover PCD but not network fee
let base_fee = TokenAmount::from_atto(10u64.pow(16));
rt.base_fee.replace(base_fee.clone());
let net_fee = aggregate_pre_commit_network_fee(precommits.len(), &base_fee);

// setup miner to have fee debt equal to net fee
let mut state: State = rt.get_state();
state.fee_debt = net_fee.clone();
rt.replace_state(&state);

// give miner enough balance to pay both and pcd
let mut balance = 2 * net_fee;
// give miner enough balance to pay pcd
let expected_deposit = pre_commit_deposit_for_power(
&actor.epoch_reward_smooth,
&actor.epoch_qa_power_smooth,
&qa_power_max(actor.sector_size),
) * precommits.len();
balance += expected_deposit.clone();
let balance = expected_deposit.clone();
rt.set_balance(balance);

actor
.pre_commit_sector_batch(
&rt,
PreCommitSectorBatchParams { sectors: precommits },
&PreCommitBatchConfig { first_for_miner: true, ..Default::default() },
&base_fee,
)
.unwrap();

Expand Down
Loading
Loading