Skip to content

Commit

Permalink
Revert activation bug fix
Browse files Browse the repository at this point in the history
  • Loading branch information
ZenGround0 committed Apr 17, 2023
1 parent f28bfd0 commit 43f5303
Show file tree
Hide file tree
Showing 12 changed files with 57 additions and 189 deletions.
119 changes: 28 additions & 91 deletions actors/miner/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use itertools::Itertools;
use log::{error, info, warn};
use multihash::Code::Blake2b256;
use num_derive::FromPrimitive;
use num_traits::{Signed, Zero};
use num_traits::Zero;

pub use beneficiary::*;
pub use bitfield_queue::*;
Expand Down Expand Up @@ -1315,9 +1315,9 @@ impl Actor {
// Skip checking if CID is defined because it cannot be so in Rust

new_sector_info.deal_ids = with_details.update.deals.clone();
new_sector_info.power_base_epoch = rt.curr_epoch();
new_sector_info.activation = rt.curr_epoch();

let duration = new_sector_info.expiration - new_sector_info.power_base_epoch;
let duration = new_sector_info.expiration - new_sector_info.activation;

new_sector_info.deal_weight =
with_details.deal_spaces.deal_space.clone() * duration;
Expand All @@ -1332,26 +1332,22 @@ impl Actor {
&new_sector_info.verified_deal_weight,
);

new_sector_info.replaced_day_reward = max(
&with_details.sector_info.expected_day_reward,
&with_details.sector_info.replaced_day_reward,
)
.clone();
new_sector_info.replaced_day_reward =
with_details.sector_info.expected_day_reward.clone();
new_sector_info.expected_day_reward = expected_reward_for_power(
&rew.this_epoch_reward_smoothed,
&pow.quality_adj_power_smoothed,
&qa_pow,
fil_actors_runtime::network::EPOCHS_IN_DAY,
);
new_sector_info.expected_storage_pledge = max(
new_sector_info.expected_storage_pledge,
expected_reward_for_power(
&rew.this_epoch_reward_smoothed,
&pow.quality_adj_power_smoothed,
&qa_pow,
INITIAL_PLEDGE_PROJECTION_PERIOD,
),
new_sector_info.expected_storage_pledge = expected_reward_for_power(
&rew.this_epoch_reward_smoothed,
&pow.quality_adj_power_smoothed,
&qa_pow,
INITIAL_PLEDGE_PROJECTION_PERIOD,
);
new_sector_info.replaced_sector_age =
ChainEpoch::max(0, rt.curr_epoch() - with_details.sector_info.activation);

new_sector_info.initial_pledge = max(
new_sector_info.initial_pledge,
Expand Down Expand Up @@ -2232,8 +2228,6 @@ impl Actor {
kind: ExtensionKind,
) -> Result<(), ActorError> {
let curr_epoch = rt.curr_epoch();
let reward_stats = &request_current_epoch_block_reward(rt)?;
let power_stats = &request_current_total_power(rt)?;

/* Loop over sectors and do extension */
let (power_delta, pledge_delta) = rt.transaction(|state: &mut State, rt| {
Expand Down Expand Up @@ -2326,11 +2320,8 @@ impl Actor {
Some(claim_space_by_sector) => extend_sector_committment(
rt.policy(),
curr_epoch,
reward_stats,
power_stats,
decl.new_expiration,
sector,
info.sector_size,
claim_space_by_sector,
),
},
Expand Down Expand Up @@ -3718,31 +3709,18 @@ fn validate_extension_declarations(
})
}

#[allow(clippy::too_many_arguments)]
fn extend_sector_committment(
policy: &Policy,
curr_epoch: ChainEpoch,
reward_stats: &ThisEpochRewardReturn,
power_stats: &ext::power::CurrentTotalPowerReturn,
new_expiration: ChainEpoch,
sector: &SectorOnChainInfo,
sector_size: SectorSize,
claim_space_by_sector: &BTreeMap<SectorNumber, (u64, u64)>,
) -> Result<SectorOnChainInfo, ActorError> {
validate_extended_expiration(policy, curr_epoch, new_expiration, sector)?;

// all simple_qa_power sectors with VerifiedDealWeight > 0 MUST check all claims
if sector.simple_qa_power {
extend_simple_qap_sector(
policy,
new_expiration,
curr_epoch,
reward_stats,
power_stats,
sector,
sector_size,
claim_space_by_sector,
)
extend_simple_qap_sector(policy, new_expiration, curr_epoch, sector, claim_space_by_sector)
} else {
extend_non_simple_qap_sector(new_expiration, curr_epoch, sector)
}
Expand Down Expand Up @@ -3809,35 +3787,17 @@ fn validate_extended_expiration(
Ok(())
}

#[allow(clippy::too_many_arguments)]
fn extend_simple_qap_sector(
policy: &Policy,
new_expiration: ChainEpoch,
curr_epoch: ChainEpoch,
reward_stats: &ThisEpochRewardReturn,
power_stats: &ext::power::CurrentTotalPowerReturn,
sector: &SectorOnChainInfo,
sector_size: SectorSize,
claim_space_by_sector: &BTreeMap<SectorNumber, (u64, u64)>,
) -> Result<SectorOnChainInfo, ActorError> {
let mut new_sector = sector.clone();

new_sector.expiration = new_expiration;
new_sector.power_base_epoch = curr_epoch;
let old_duration = sector.expiration - sector.power_base_epoch;
let new_duration = new_sector.expiration - new_sector.power_base_epoch;

// Update the non-verified deal weights. This won't change power, it'll just keep it the same
// relative to the updated power base epoch.
if sector.deal_weight.is_positive() {
// (old_deal_weight) / old_duration -> old_space
// old_space * (old_expiration - curr_epoch) -> remaining spacetime in the deals.
new_sector.deal_weight =
&sector.deal_weight * (sector.expiration - curr_epoch) / old_duration;
}

// Update the verified deal weights, and pledge if necessary.
if sector.verified_deal_weight.is_positive() {
if sector.verified_deal_weight > BigInt::zero() {
let old_duration = sector.expiration - sector.activation;
let deal_space = &sector.deal_weight / old_duration;
let old_verified_deal_space = &sector.verified_deal_weight / old_duration;
let (expected_verified_deal_space, new_verified_deal_space) = match claim_space_by_sector
.get(&sector.sector_number)
Expand Down Expand Up @@ -3869,35 +3829,14 @@ fn extend_simple_qap_sector(
));
}

new_sector.verified_deal_weight = BigInt::from(*new_verified_deal_space) * new_duration;

// We only bother updating the expected_day_reward, expected_storage_pledge, and replaced_day_reward
// for verified deals, as it can increase power.
let qa_pow = qa_power_for_weight(
sector_size,
new_duration,
&new_sector.deal_weight,
&new_sector.verified_deal_weight,
);
new_sector.expected_day_reward = expected_reward_for_power(
&reward_stats.this_epoch_reward_smoothed,
&power_stats.quality_adj_power_smoothed,
&qa_pow,
fil_actors_runtime::network::EPOCHS_IN_DAY,
);
new_sector.expected_storage_pledge = max(
sector.expected_storage_pledge.clone(),
expected_reward_for_power(
&reward_stats.this_epoch_reward_smoothed,
&power_stats.quality_adj_power_smoothed,
&qa_pow,
INITIAL_PLEDGE_PROJECTION_PERIOD,
),
);
new_sector.replaced_day_reward =
max(sector.expected_day_reward.clone(), sector.replaced_day_reward.clone());
new_sector.expiration = new_expiration;
// update deal weights to account for new duration
new_sector.deal_weight = deal_space * (new_sector.expiration - new_sector.activation);
new_sector.verified_deal_weight = BigInt::from(*new_verified_deal_space)
* (new_sector.expiration - new_sector.activation);
} else {
new_sector.expiration = new_expiration
}

Ok(new_sector)
}

Expand All @@ -3909,17 +3848,15 @@ fn extend_non_simple_qap_sector(
let mut new_sector = sector.clone();
// Remove "spent" deal weights for non simple_qa_power sectors with deal weight > 0
let new_deal_weight = (&sector.deal_weight * (sector.expiration - curr_epoch))
.div_floor(&BigInt::from(sector.expiration - sector.power_base_epoch));
.div_floor(&BigInt::from(sector.expiration - sector.activation));

let new_verified_deal_weight = (&sector.verified_deal_weight
* (sector.expiration - curr_epoch))
.div_floor(&BigInt::from(sector.expiration - sector.power_base_epoch));
.div_floor(&BigInt::from(sector.expiration - sector.activation));

new_sector.expiration = new_expiration;
new_sector.deal_weight = new_deal_weight;
new_sector.verified_deal_weight = new_verified_deal_weight;
new_sector.power_base_epoch = curr_epoch;

Ok(new_sector)
}

Expand Down Expand Up @@ -4651,13 +4588,13 @@ fn termination_penalty(
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,
current_epoch - sector.activation,
&sector.expected_storage_pledge,
network_qa_power_estimate,
&sector_power,
reward_estimate,
&sector.replaced_day_reward,
sector.power_base_epoch - sector.activation,
sector.replaced_sector_age,
);
total_fee += fee;
}
Expand Down Expand Up @@ -4906,7 +4843,7 @@ fn confirm_sector_proofs_valid_internal(
initial_pledge,
expected_day_reward: day_reward,
expected_storage_pledge: storage_pledge,
power_base_epoch: activation,
replaced_sector_age: ChainEpoch::zero(),
replaced_day_reward: TokenAmount::zero(),
sector_key_cid: None,
simple_qa_power: true,
Expand Down
2 changes: 1 addition & 1 deletion actors/miner/src/policy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ pub fn qa_power_for_weight(

/// Returns the quality-adjusted power for a sector.
pub fn qa_power_for_sector(size: SectorSize, sector: &SectorOnChainInfo) -> StoragePower {
let duration = sector.expiration - sector.power_base_epoch;
let duration = sector.expiration - sector.activation;
qa_power_for_weight(size, duration, &sector.deal_weight, &sector.verified_deal_weight)
}

Expand Down
8 changes: 0 additions & 8 deletions actors/miner/src/testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,14 +87,6 @@ pub fn check_state_invariants<BS: Blockstore>(
if !sector.deal_ids.is_empty() {
miner_summary.sectors_with_deals.insert(sector_number);
}
acc.require(
sector.activation <= sector.power_base_epoch,
format!("invalid power base for {sector_number}"),
);
acc.require(
sector.power_base_epoch < sector.expiration,
format!("power base epoch is not before the sector expiration {sector_number}"),
);
Ok(())
});

Expand Down
10 changes: 5 additions & 5 deletions actors/miner/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,13 +347,13 @@ pub struct SectorOnChainInfo {
pub verified_deal_weight: DealWeight,
/// Pledge collected to commit this sector
pub initial_pledge: TokenAmount,
/// Expected one day projection of reward for sector computed at activation / update / extension time
/// Expected one day projection of reward for sector computed at activation time
pub expected_day_reward: TokenAmount,
/// Expected twenty day projection of reward for sector computed at activation / update / extension time
/// Expected twenty day projection of reward for sector computed at activation time
pub expected_storage_pledge: TokenAmount,
/// Epoch at which this sector's power was most recently updated
pub power_base_epoch: ChainEpoch,
/// Maximum day reward this sector has had in previous iterations (zero for brand new sectors)
/// Age of sector this sector replaced or zero
pub replaced_sector_age: ChainEpoch,
/// Day reward of sector this sector replace or zero
pub replaced_day_reward: TokenAmount,
/// The original SealedSectorCID, only gets set on the first ReplicaUpdate
pub sector_key_cid: Option<Cid>,
Expand Down
3 changes: 1 addition & 2 deletions actors/miner/tests/extend_sector_expiration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ use fvm_shared::{
use std::collections::HashMap;

mod util;

use fil_actors_runtime::runtime::Policy;
use itertools::Itertools;
use test_case::test_case;
Expand Down Expand Up @@ -947,7 +946,7 @@ fn assert_sector_verified_space(
let new_sector = h.get_sector(rt, sector_number);
assert_eq!(
DealWeight::from(v_deal_space),
new_sector.verified_deal_weight / (new_sector.expiration - new_sector.power_base_epoch)
new_sector.verified_deal_weight / (new_sector.expiration - new_sector.activation)
);
}

Expand Down
1 change: 0 additions & 1 deletion actors/miner/tests/sector_assignment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ fn new_sector_on_chain_info(
sealed_cid,
activation,
expiration: 1,
power_base_epoch: activation,
deal_weight: weight.clone(),
verified_deal_weight: weight,
..SectorOnChainInfo::default()
Expand Down
1 change: 0 additions & 1 deletion actors/miner/tests/sectors_stores_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,6 @@ fn new_sector_on_chain_info(
sealed_cid,
deal_ids: vec![],
activation,
power_base_epoch: activation,
expiration: ChainEpoch::from(1),
deal_weight: weight.clone(),
verified_deal_weight: weight,
Expand Down
8 changes: 2 additions & 6 deletions actors/miner/tests/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2385,7 +2385,6 @@ impl ActorHarness {
) -> Result<Option<IpldBlock>, ActorError> {
rt.set_caller(*ACCOUNT_ACTOR_CODE_ID, self.worker);
rt.expect_validate_caller_addr(self.caller_addrs());
self.expect_query_network_info(rt);

let mut qa_delta = BigInt::zero();
for extension in params.extensions.iter_mut() {
Expand Down Expand Up @@ -2470,14 +2469,12 @@ impl ActorHarness {
}
}

self.expect_query_network_info(rt);
// Handle QA power updates
for extension in params.extensions.iter_mut() {
for sector_nr in extension.sectors.validate().unwrap().iter() {
let sector = self.get_sector(&rt, sector_nr);
let mut new_sector = sector.clone();
new_sector.expiration = extension.new_expiration;
new_sector.power_base_epoch = *rt.epoch.borrow();
qa_delta += qa_power_for_sector(self.sector_size, &new_sector)
- qa_power_for_sector(self.sector_size, &sector);
}
Expand All @@ -2490,14 +2487,13 @@ impl ActorHarness {
}
}
let sector = self.get_sector(&rt, sector_claim.sector_number);
let old_duration = sector.expiration - sector.power_base_epoch;
let old_duration = sector.expiration - sector.activation;
let old_verified_deal_space = &sector.verified_deal_weight / old_duration;
let new_verified_deal_space = old_verified_deal_space - dropped_space;
let mut new_sector = sector.clone();
new_sector.expiration = extension.new_expiration;
new_sector.power_base_epoch = *rt.epoch.borrow();
new_sector.verified_deal_weight = BigInt::from(new_verified_deal_space)
* (new_sector.expiration - new_sector.power_base_epoch);
* (new_sector.expiration - new_sector.activation);
qa_delta += qa_power_for_sector(self.sector_size, &new_sector)
- qa_power_for_sector(self.sector_size, &sector);
}
Expand Down
2 changes: 1 addition & 1 deletion state/src/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ fn check_deal_states_against_sectors(
};

acc.require(
deal.sector_start_epoch >= sector_deal.sector_start,
deal.sector_start_epoch == sector_deal.sector_start,
format!(
"deal state start {} does not match sector start {} for miner {}",
deal.sector_start_epoch, sector_deal.sector_start, deal.provider
Expand Down
10 changes: 0 additions & 10 deletions test_vm/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -545,16 +545,6 @@ pub fn miner_extend_sector_expiration2<BS: Blockstore>(
..Default::default()
})
}
subinvocs.push(ExpectInvocation {
to: REWARD_ACTOR_ADDR,
method: RewardMethod::ThisEpochReward as u64,
..Default::default()
});
subinvocs.push(ExpectInvocation {
to: STORAGE_POWER_ACTOR_ADDR,
method: PowerMethod::CurrentTotalPower as u64,
..Default::default()
});
if !power_delta.is_zero() {
subinvocs.push(ExpectInvocation {
to: STORAGE_POWER_ACTOR_ADDR,
Expand Down
Loading

0 comments on commit 43f5303

Please sign in to comment.