Skip to content

Commit

Permalink
Revert activation bug fix and FIP 0052 (#1281)
Browse files Browse the repository at this point in the history
* Revert activation bug fix

* Revert FIP 0052

* Speed up slow market test

* Fix test to match reverted activation bug fix

---------

Co-authored-by: zenground0 <ZenGround0@users.noreply.github.com>
Co-authored-by: Steven Allen <steven@stebalien.com>
  • Loading branch information
3 people authored Apr 21, 2023
1 parent 6aa1782 commit cd9ac2b
Show file tree
Hide file tree
Showing 17 changed files with 102 additions and 447 deletions.
2 changes: 1 addition & 1 deletion actors/market/src/policy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub mod detail {

/// Bounds (inclusive) on deal duration.
pub(super) fn deal_duration_bounds(_size: PaddedPieceSize) -> (ChainEpoch, ChainEpoch) {
(180 * EPOCHS_IN_DAY, 1278 * EPOCHS_IN_DAY)
(180 * EPOCHS_IN_DAY, 540 * EPOCHS_IN_DAY)
}

pub(super) fn deal_price_per_epoch_bounds(
Expand Down
27 changes: 25 additions & 2 deletions actors/market/tests/harness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,29 @@ pub fn setup() -> MockRuntime {
rt
}

pub fn setup_with_policy(policy: Policy) -> MockRuntime {
let actor_code_cids = HashMap::from([
(OWNER_ADDR, *ACCOUNT_ACTOR_CODE_ID),
(WORKER_ADDR, *ACCOUNT_ACTOR_CODE_ID),
(PROVIDER_ADDR, *MINER_ACTOR_CODE_ID),
(CLIENT_ADDR, *ACCOUNT_ACTOR_CODE_ID),
]);

let rt = MockRuntime {
receiver: STORAGE_MARKET_ACTOR_ADDR,
caller: RefCell::new(SYSTEM_ACTOR_ADDR),
caller_type: RefCell::new(*INIT_ACTOR_CODE_ID),
actor_code_cids: RefCell::new(actor_code_cids),
balance: RefCell::new(TokenAmount::from_whole(10)),
policy,
..Default::default()
};

construct_and_verify(&rt);

rt
}

/// Checks internal invariants of market state asserting none of them are broken.
pub fn check_state(rt: &MockRuntime) {
let (_, acc) = check_state_invariants(
Expand Down Expand Up @@ -757,11 +780,11 @@ pub fn expect_query_network_info(rt: &MockRuntime) {
);
}

pub fn assert_n_good_deals<BS>(dobe: &SetMultimap<BS>, epoch: ChainEpoch, n: isize)
pub fn assert_n_good_deals<BS>(dobe: &SetMultimap<BS>, epoch: ChainEpoch, n: isize, policy: &Policy)
where
BS: fvm_ipld_blockstore::Blockstore,
{
let deal_updates_interval = Policy::default().deal_updates_interval;
let deal_updates_interval = policy.deal_updates_interval;
let mut count = 0;
dobe.for_each(epoch, |id| {
assert_eq!(epoch % deal_updates_interval, (id as i64) % deal_updates_interval);
Expand Down
18 changes: 9 additions & 9 deletions actors/market/tests/market_actor_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -555,12 +555,12 @@ fn fails_if_withdraw_from_provider_funds_is_not_initiated_by_the_owner_or_worker

#[test]
fn deal_starts_on_day_boundary() {
let deal_updates_interval = Policy::default().deal_updates_interval;
let start_epoch = deal_updates_interval; // 2880
let deal_updates_interval = 101; // small number for testing
let start_epoch = deal_updates_interval;
let end_epoch = start_epoch + 200 * EPOCHS_IN_DAY;
let publish_epoch = ChainEpoch::from(1);

let rt = setup();
let rt = setup_with_policy(Policy { deal_updates_interval, ..Default::default() });
rt.set_epoch(publish_epoch);

for i in 0..(3 * deal_updates_interval) {
Expand All @@ -582,15 +582,15 @@ fn deal_starts_on_day_boundary() {
let store = &rt.store;
let dobe = SetMultimap::from_root(store, &st.deal_ops_by_epoch).unwrap();
for e in deal_updates_interval..(2 * deal_updates_interval) {
assert_n_good_deals(&dobe, e, 3);
assert_n_good_deals(&dobe, e, 3, &rt.policy);
}

// DOBE has no deals scheduled in the previous or next day
for e in 0..deal_updates_interval {
assert_n_good_deals(&dobe, e, 0);
assert_n_good_deals(&dobe, e, 0, &rt.policy);
}
for e in (2 * deal_updates_interval)..(3 * deal_updates_interval) {
assert_n_good_deals(&dobe, e, 0);
assert_n_good_deals(&dobe, e, 0, &rt.policy);
}
}

Expand Down Expand Up @@ -622,11 +622,11 @@ fn deal_starts_partway_through_day() {
let store = &rt.store;
let dobe = SetMultimap::from_root(store, &st.deal_ops_by_epoch).unwrap();
for e in interval..(interval + start_epoch) {
assert_n_good_deals(&dobe, e, 1);
assert_n_good_deals(&dobe, e, 1, &rt.policy);
}
// Nothing scheduled between 0 and interval
for e in 0..interval {
assert_n_good_deals(&dobe, e, 0);
assert_n_good_deals(&dobe, e, 0, &rt.policy);
}

// Now add another 500 deals
Expand All @@ -647,7 +647,7 @@ fn deal_starts_partway_through_day() {
let store = &rt.store;
let dobe = SetMultimap::from_root(store, &st.deal_ops_by_epoch).unwrap();
for e in start_epoch..(start_epoch + 500) {
assert_n_good_deals(&dobe, e, 1);
assert_n_good_deals(&dobe, e, 1, &rt.policy);
}
}

Expand Down
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
Loading

0 comments on commit cd9ac2b

Please sign in to comment.