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

Miner notifies market of termination only of sectors with non-zero data #1387

Merged
merged 2 commits into from
Aug 30, 2023
Merged

Miner notifies market of termination only of sectors with non-zero data #1387

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1d1f2f7
Miner notifies market of termination only of sectors with non-zero data
anorth Aug 29, 2023
c812763
Merge branch 'integration/direct-onboarding' into anorth/ddo-terminat…
ZenGround0 Aug 30, 2023
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
174 changes: 72 additions & 102 deletions actors/miner/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4061,91 +4061,87 @@ fn process_early_terminations(
reward_smoothed: &FilterEstimate,
quality_adj_power_smoothed: &FilterEstimate,
) -> Result</* more */ bool, ActorError> {
let (result, more, sectors_terminated, penalty, pledge_delta) =
rt.transaction(|state: &mut State, rt| {
let store = rt.store();
let policy = rt.policy();

let (result, more) = state
.pop_early_terminations(
policy,
store,
policy.addressed_partitions_max,
policy.addressed_sectors_max,
)
.map_err(|e| {
e.downcast_default(
ExitCode::USR_ILLEGAL_STATE,
"failed to pop early terminations",
)
})?;

// Nothing to do, don't waste any time.
// This can happen if we end up processing early terminations
// before the cron callback fires.
if result.is_empty() {
info!("no early terminations (maybe cron callback hasn't happened yet?)");
return Ok((result, more, Vec::new(), TokenAmount::zero(), TokenAmount::zero()));
}

let info = get_miner_info(rt.store(), state)?;
let sectors = Sectors::load(store, &state.sectors).map_err(|e| {
e.downcast_default(ExitCode::USR_ILLEGAL_STATE, "failed to load sectors array")
})?;
let mut sectors_with_data = vec![];
let (result, more, penalty, pledge_delta) = rt.transaction(|state: &mut State, rt| {
let store = rt.store();
let policy = rt.policy();

let mut sectors_terminated: Vec<BitField> = Vec::new();
let mut total_initial_pledge = TokenAmount::zero();
let mut penalty = TokenAmount::zero();
let (result, more) = state
.pop_early_terminations(
policy,
store,
policy.addressed_partitions_max,
policy.addressed_sectors_max,
)
.context_code(ExitCode::USR_ILLEGAL_STATE, "failed to pop early terminations")?;

// Nothing to do, don't waste any time.
// This can happen if we end up processing early terminations
// before the cron callback fires.
if result.is_empty() {
info!("no early terminations (maybe cron callback hasn't happened yet?)");
return Ok((result, more, TokenAmount::zero(), TokenAmount::zero()));
}

for (epoch, sector_numbers) in result.iter() {
sectors_terminated.push(sector_numbers.clone());
let sectors = sectors
.load_sector(sector_numbers)
.map_err(|e| e.wrap("failed to load sector infos"))?;
let info = get_miner_info(rt.store(), state)?;
let sectors = Sectors::load(store, &state.sectors).map_err(|e| {
e.downcast_default(ExitCode::USR_ILLEGAL_STATE, "failed to load sectors array")
})?;

penalty += termination_penalty(
info.sector_size,
epoch,
reward_smoothed,
let mut total_initial_pledge = TokenAmount::zero();
let mut total_penalty = TokenAmount::zero();

for (epoch, sector_numbers) in result.iter() {
let sectors = sectors
.load_sector(sector_numbers)
.map_err(|e| e.wrap("failed to load sector infos"))?;

for sector in &sectors {
total_initial_pledge += &sector.initial_pledge;
let sector_power = qa_power_for_sector(info.sector_size, sector);
total_penalty += pledge_penalty_for_termination(
&sector.expected_day_reward,
epoch - sector.power_base_epoch,
&sector.expected_storage_pledge,
quality_adj_power_smoothed,
&sectors,
&sector_power,
reward_smoothed,
&sector.replaced_day_reward,
sector.power_base_epoch - sector.activation,
);

// estimate ~one deal per sector.
let mut deal_ids = Vec::<DealID>::with_capacity(sectors.len());
anorth marked this conversation as resolved.
Show resolved Hide resolved
for sector in sectors {
deal_ids.extend(sector.deal_ids);
total_initial_pledge += sector.initial_pledge;
if sector.deal_weight.is_positive() || sector.verified_deal_weight.is_positive() {
sectors_with_data.push(sector.sector_number);
}
}
}

// Pay penalty
state
.apply_penalty(&penalty)
.map_err(|e| actor_error!(illegal_state, "failed to apply penalty: {}", e))?;
// Apply penalty (add to fee debt)
state
.apply_penalty(&total_penalty)
.map_err(|e| actor_error!(illegal_state, "failed to apply penalty: {}", e))?;

// Remove pledge requirement.
let mut pledge_delta = -total_initial_pledge;
state.add_initial_pledge(&pledge_delta).map_err(|e| {
actor_error!(illegal_state, "failed to add initial pledge {}: {}", pledge_delta, e)
})?;
// Remove pledge requirement.
let mut pledge_delta = -total_initial_pledge;
state.add_initial_pledge(&pledge_delta).map_err(|e| {
actor_error!(illegal_state, "failed to add initial pledge {}: {}", pledge_delta, e)
})?;

// Use unlocked pledge to pay down outstanding fee debt
let (penalty_from_vesting, penalty_from_balance) = state
.repay_partial_debt_in_priority_order(
rt.store(),
rt.curr_epoch(),
&rt.current_balance(),
)
.map_err(|e| {
e.downcast_default(ExitCode::USR_ILLEGAL_STATE, "failed to repay penalty")
})?;
// Use unlocked pledge to pay down outstanding fee debt
let (penalty_from_vesting, penalty_from_balance) = state
.repay_partial_debt_in_priority_order(
rt.store(),
rt.curr_epoch(),
&rt.current_balance(),
)
.map_err(|e| {
e.downcast_default(ExitCode::USR_ILLEGAL_STATE, "failed to repay penalty")
})?;

penalty = &penalty_from_vesting + penalty_from_balance;
pledge_delta -= penalty_from_vesting;
let penalty = &penalty_from_vesting + penalty_from_balance;
pledge_delta -= penalty_from_vesting;

Ok((result, more, sectors_terminated, penalty, pledge_delta))
})?;
Ok((result, more, penalty, pledge_delta))
})?;

// We didn't do anything, abort.
if result.is_empty() {
Expand All @@ -4165,8 +4161,9 @@ fn process_early_terminations(
notify_pledge_changed(rt, &pledge_delta)?;

// Terminate deals.
let all_terminated = BitField::union(sectors_terminated.iter());
request_terminate_deals(rt, rt.curr_epoch(), &all_terminated)?;
let terminated_data = BitField::try_from_bits(sectors_with_data)
.context_code(ExitCode::USR_ILLEGAL_STATE, "invalid sector number")?;
request_terminate_deals(rt, rt.curr_epoch(), &terminated_data)?;

// reschedule cron worker, if necessary.
Ok(more)
Expand Down Expand Up @@ -4960,33 +4957,6 @@ fn validate_partition_contains_sectors(
}
}

fn termination_penalty(
sector_size: SectorSize,
current_epoch: ChainEpoch,
reward_estimate: &FilterEstimate,
network_qa_power_estimate: &FilterEstimate,
sectors: &[SectorOnChainInfo],
) -> TokenAmount {
let mut total_fee = TokenAmount::zero();

for sector in sectors {
let sector_power = qa_power_for_sector(sector_size, sector);
let fee = pledge_penalty_for_termination(
&sector.expected_day_reward,
current_epoch - sector.power_base_epoch,
&sector.expected_storage_pledge,
network_qa_power_estimate,
&sector_power,
reward_estimate,
&sector.replaced_day_reward,
sector.power_base_epoch - sector.activation,
);
total_fee += fee;
}

total_fee
}

fn consensus_fault_active(info: &MinerInfo, curr_epoch: ChainEpoch) -> bool {
// For penalization period to last for exactly finality epochs
// consensus faults are active until currEpoch exceeds ConsensusFaultElapsed
Expand Down
79 changes: 69 additions & 10 deletions actors/miner/tests/terminate_sectors_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,26 @@ use fil_actor_miner::{
use fil_actors_runtime::{
runtime::Runtime,
test_utils::{expect_abort_contains_message, MockRuntime, ACCOUNT_ACTOR_CODE_ID},
BURNT_FUNDS_ACTOR_ADDR, EPOCHS_IN_DAY, STORAGE_MARKET_ACTOR_ADDR, STORAGE_POWER_ACTOR_ADDR,
SYSTEM_ACTOR_ADDR,
DealWeight, BURNT_FUNDS_ACTOR_ADDR, EPOCHS_IN_DAY, STORAGE_MARKET_ACTOR_ADDR,
STORAGE_POWER_ACTOR_ADDR, SYSTEM_ACTOR_ADDR,
};
use fvm_ipld_bitfield::BitField;
use fvm_shared::{econ::TokenAmount, error::ExitCode, METHOD_SEND};
use std::collections::HashMap;

mod util;

use fil_actor_market::VerifiedDealInfo;
use fil_actor_miner::ext::market::{
OnMinerSectorsTerminateParams, ON_MINER_SECTORS_TERMINATE_METHOD,
};
use fil_actor_miner::ext::power::UPDATE_PLEDGE_TOTAL_METHOD;
use fil_actors_runtime::test_utils::POWER_ACTOR_CODE_ID;
use fvm_ipld_encoding::ipld_block::IpldBlock;
use fvm_ipld_encoding::RawBytes;
use num_traits::Zero;
use fvm_shared::piece::PaddedPieceSize;
use fvm_shared::sector::SectorNumber;
use num_traits::{Signed, Zero};
use util::*;

fn setup() -> (ActorHarness, MockRuntime) {
Expand Down Expand Up @@ -56,7 +60,7 @@ fn removes_sector_with_correct_accounting() {
let state: State = rt.get_state();
let initial_locked_funds = state.locked_funds;

let expected_fee = calc_expected_fee_for_termination(&h, &rt, sector.clone());
let expected_fee = calc_expected_fee_for_termination(&h, &rt, &sector);

let sectors = bitfield_from_slice(&[sector.sector_number]);
h.terminate_sectors(&rt, &sectors, expected_fee.clone());
Expand All @@ -79,6 +83,50 @@ fn removes_sector_with_correct_accounting() {
h.check_state(&rt);
}

#[test]
fn removes_sector_with_without_deals() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would add a 3rd sector with only a verified deal to cover that case as well

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙏

let (mut h, rt) = setup();
// One sector with no data, one with a deal, one with a verified deal
let sectors = h.commit_and_prove_sectors_with_cfgs(
&rt,
3,
DEFAULT_SECTOR_EXPIRATION,
vec![vec![], vec![1], vec![2]],
true,
ProveCommitConfig {
verify_deals_exit: Default::default(),
claim_allocs_exit: Default::default(),
deal_space: HashMap::from_iter(vec![(1, DealWeight::from(1024))]),
verified_deal_infos: HashMap::from_iter(vec![(
2,
vec![VerifiedDealInfo {
client: 0,
allocation_id: 0,
data: Default::default(),
size: PaddedPieceSize(1024),
}],
)]),
},
);
let snos: Vec<SectorNumber> = sectors.iter().map(|s| s.sector_number).collect();
assert!(sectors[0].deal_weight.is_zero());
assert!(sectors[1].deal_weight.is_positive());
assert!(sectors[2].verified_deal_weight.is_positive());

h.advance_and_submit_posts(&rt, &sectors);
// Add locked funds to ensure correct fee calculation is used.
h.apply_rewards(&rt, BIG_REWARDS.clone(), TokenAmount::zero());

// Expectations about the correct call to market actor are in the harness method.
let expected_fee: TokenAmount = sectors
.iter()
.fold(TokenAmount::zero(), |acc, s| acc + calc_expected_fee_for_termination(&h, &rt, s));
h.terminate_sectors(&rt, &bitfield_from_slice(&snos), expected_fee);
let state: State = rt.get_state();
assert!(state.initial_pledge.is_zero());
h.check_state(&rt);
}

#[test]
fn cannot_terminate_a_sector_when_the_challenge_window_is_open() {
let (mut h, rt) = setup();
Expand Down Expand Up @@ -121,12 +169,23 @@ fn cannot_terminate_a_sector_when_the_challenge_window_is_open() {
}

#[test]
fn owner_cannot_terminate_if_market_cron_fails() {
fn owner_cannot_terminate_if_market_fails() {
let (mut h, rt) = setup();

let deal_ids = vec![10];
let sector_info =
h.commit_and_prove_sectors(&rt, 1, DEFAULT_SECTOR_EXPIRATION, vec![deal_ids], true);
let sector_info = h.commit_and_prove_sectors_with_cfgs(
&rt,
1,
DEFAULT_SECTOR_EXPIRATION,
vec![deal_ids],
true,
ProveCommitConfig {
verify_deals_exit: Default::default(),
claim_allocs_exit: Default::default(),
deal_space: HashMap::from_iter(vec![(0, DealWeight::from(1024))]),
verified_deal_infos: Default::default(),
},
);

assert_eq!(sector_info.len(), 1);

Expand All @@ -137,7 +196,7 @@ fn owner_cannot_terminate_if_market_cron_fails() {
let (deadline_index, partition_index) =
state.find_sector(rt.store(), sector.sector_number).unwrap();

let expected_fee = calc_expected_fee_for_termination(&h, &rt, sector.clone());
let expected_fee = calc_expected_fee_for_termination(&h, &rt, &sector);

rt.expect_validate_caller_addr(h.caller_addrs());

Expand Down Expand Up @@ -236,9 +295,9 @@ fn system_can_terminate_if_market_cron_fails() {
fn calc_expected_fee_for_termination(
h: &ActorHarness,
rt: &MockRuntime,
sector: SectorOnChainInfo,
sector: &SectorOnChainInfo,
) -> TokenAmount {
let sector_power = qa_power_for_sector(sector.seal_proof.sector_size().unwrap(), &sector);
let sector_power = qa_power_for_sector(sector.seal_proof.sector_size().unwrap(), sector);
let day_reward = expected_reward_for_power(
&h.epoch_reward_smooth,
&h.epoch_qa_power_smooth,
Expand Down
31 changes: 21 additions & 10 deletions actors/miner/tests/util.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ use fvm_shared::{MethodNum, HAMT_BIT_WIDTH, METHOD_SEND};
use itertools::Itertools;
use lazy_static::lazy_static;
use multihash::derive::Multihash;
use num_traits::Signed;

use fil_actor_account::Method as AccountMethod;
use fil_actor_market::{
Expand Down Expand Up @@ -1966,8 +1967,14 @@ impl ActorHarness {
}

// notify change to initial pledge
let mut sectors_with_data = vec![];
for sector_info in &sector_infos {
pledge_delta -= sector_info.initial_pledge.to_owned();
if sector_info.deal_weight.is_positive()
|| sector_info.verified_deal_weight.is_positive()
{
sectors_with_data.push(sector_info.sector_number);
}
}

if !pledge_delta.is_zero() {
Expand All @@ -1981,16 +1988,20 @@ impl ActorHarness {
);
}

let params =
OnMinerSectorsTerminateParams { epoch: *rt.epoch.borrow(), sectors: sectors.clone() };
rt.expect_send_simple(
STORAGE_MARKET_ACTOR_ADDR,
ON_MINER_SECTORS_TERMINATE_METHOD,
IpldBlock::serialize_cbor(&params).unwrap(),
TokenAmount::zero(),
None,
ExitCode::OK,
);
if !sectors_with_data.is_empty() {
rt.expect_send_simple(
STORAGE_MARKET_ACTOR_ADDR,
ON_MINER_SECTORS_TERMINATE_METHOD,
IpldBlock::serialize_cbor(&OnMinerSectorsTerminateParams {
epoch: *rt.epoch.borrow(),
sectors: bitfield_from_slice(&sectors_with_data),
})
.unwrap(),
TokenAmount::zero(),
None,
ExitCode::OK,
);
}

let sector_power = power_for_sectors(self.sector_size, &sector_infos);
let params = UpdateClaimedPowerParams {
Expand Down