diff --git a/actors/miner/src/lib.rs b/actors/miner/src/lib.rs index 65f974a0d..bbce1d679 100644 --- a/actors/miner/src/lib.rs +++ b/actors/miner/src/lib.rs @@ -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::*; @@ -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; @@ -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, @@ -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| { @@ -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, ), }, @@ -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, ) -> Result { 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) } @@ -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, ) -> Result { 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 = - §or.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 = §or.deal_weight / old_duration; let old_verified_deal_space = §or.verified_deal_weight / old_duration; let (expected_verified_deal_space, new_verified_deal_space) = match claim_space_by_sector .get(§or.sector_number) @@ -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) } @@ -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 = (§or.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 = (§or.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) } @@ -4651,13 +4588,13 @@ fn termination_penalty( let sector_power = qa_power_for_sector(sector_size, sector); let fee = pledge_penalty_for_termination( §or.expected_day_reward, - current_epoch - sector.power_base_epoch, + current_epoch - sector.activation, §or.expected_storage_pledge, network_qa_power_estimate, §or_power, reward_estimate, §or.replaced_day_reward, - sector.power_base_epoch - sector.activation, + sector.replaced_sector_age, ); total_fee += fee; } @@ -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, diff --git a/actors/miner/src/policy.rs b/actors/miner/src/policy.rs index 1c6f1bda3..10042a2b1 100644 --- a/actors/miner/src/policy.rs +++ b/actors/miner/src/policy.rs @@ -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, §or.deal_weight, §or.verified_deal_weight) } diff --git a/actors/miner/src/testing.rs b/actors/miner/src/testing.rs index 79a0ece39..cc8060be3 100644 --- a/actors/miner/src/testing.rs +++ b/actors/miner/src/testing.rs @@ -87,14 +87,6 @@ pub fn check_state_invariants( 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(()) }); diff --git a/actors/miner/src/types.rs b/actors/miner/src/types.rs index e20c55d96..dcaa3c603 100644 --- a/actors/miner/src/types.rs +++ b/actors/miner/src/types.rs @@ -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, diff --git a/actors/miner/tests/extend_sector_expiration_test.rs b/actors/miner/tests/extend_sector_expiration_test.rs index 814729d59..70612627e 100644 --- a/actors/miner/tests/extend_sector_expiration_test.rs +++ b/actors/miner/tests/extend_sector_expiration_test.rs @@ -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; @@ -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) ); } diff --git a/actors/miner/tests/sector_assignment.rs b/actors/miner/tests/sector_assignment.rs index 51faabb37..855822746 100644 --- a/actors/miner/tests/sector_assignment.rs +++ b/actors/miner/tests/sector_assignment.rs @@ -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() diff --git a/actors/miner/tests/sectors_stores_test.rs b/actors/miner/tests/sectors_stores_test.rs index df8e1928e..16233379b 100644 --- a/actors/miner/tests/sectors_stores_test.rs +++ b/actors/miner/tests/sectors_stores_test.rs @@ -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, diff --git a/actors/miner/tests/util.rs b/actors/miner/tests/util.rs index b982562f6..a1da962c8 100644 --- a/actors/miner/tests/util.rs +++ b/actors/miner/tests/util.rs @@ -2385,7 +2385,6 @@ impl ActorHarness { ) -> Result, 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() { @@ -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, §or); } @@ -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 = §or.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, §or); } diff --git a/state/src/check.rs b/state/src/check.rs index 9283dd040..92096fd52 100644 --- a/state/src/check.rs +++ b/state/src/check.rs @@ -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 diff --git a/test_vm/src/util.rs b/test_vm/src/util.rs index cd8b2f805..25dea01e3 100644 --- a/test_vm/src/util.rs +++ b/test_vm/src/util.rs @@ -545,16 +545,6 @@ pub fn miner_extend_sector_expiration2( ..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, diff --git a/test_vm/tests/extend_sectors_test.rs b/test_vm/tests/extend_sectors_test.rs index 5b5e1c6ac..6f022c34c 100644 --- a/test_vm/tests/extend_sectors_test.rs +++ b/test_vm/tests/extend_sectors_test.rs @@ -1,5 +1,4 @@ -use fil_actor_market::State as MarketState; -use fil_actor_market::{DealMetaArray, Method as MarketMethod}; +use fil_actor_market::{DealMetaArray, Method as MarketMethod, State as MarketState}; use fil_actor_miner::{ max_prove_commit_duration, power_for_sector, ExpirationExtension, ExpirationExtension2, ExtendSectorExpiration2Params, ExtendSectorExpirationParams, Method as MinerMethod, PowerPair, @@ -99,32 +98,15 @@ fn extend( } }; - let mut expect_invoke = vec![ - ExpectInvocation { - to: REWARD_ACTOR_ADDR, - method: RewardMethod::ThisEpochReward as u64, - ..Default::default() - }, - ExpectInvocation { - to: STORAGE_POWER_ACTOR_ADDR, - method: PowerMethod::CurrentTotalPower as u64, - ..Default::default() - }, - ]; - - if power_update_params.is_some() { - expect_invoke.push(ExpectInvocation { + ExpectInvocation { + to: maddr, + method: extension_method, + subinvocs: Some(vec![ExpectInvocation { to: STORAGE_POWER_ACTOR_ADDR, method: PowerMethod::UpdateClaimedPower as u64, params: Some(power_update_params), ..Default::default() - }); - } - - ExpectInvocation { - to: maddr, - method: extension_method, - subinvocs: Some(expect_invoke), + }]), ..Default::default() } .matches(v.take_invocations().last().unwrap()); @@ -248,17 +230,10 @@ fn extend_legacy_sector_with_deals_inner(do_extend2: bool) { deadline_info.index + 1 % policy.wpost_period_deadlines, ); - // Advance halfway through life and extend another 6 months. We need to spread the remaining 90 - // days of 10x power over 90 + 180 days - // subtract half the remaining deal weight: - // - verified deal weight /= 2 - // - // normalize 90 days of 10x power plus 180 days of 1x power over 90+180 days: - // - multiplier = ((10 * 90) + (1 * 180)) / (90 + 180) - // - multiplier = 4 - // - // delta from the previous 10x power multiplier: - // - power delta = (10-4)*32GiB = 6*32GiB + // advance halfway through life and extend another 6 months + // verified deal weight /= 2 + // power multiplier = (1/4)*10 + (3/4)*1 = 3.25 + // power delta = (10-3.25)*32GiB = 6.75*32GiB advance_by_deadline_to_epoch_while_proving( &v, &miner_id, @@ -271,7 +246,7 @@ fn extend_legacy_sector_with_deals_inner(do_extend2: bool) { let mut expected_update_claimed_power_params = UpdateClaimedPowerParams { raw_byte_delta: StoragePower::zero(), - quality_adjusted_delta: StoragePower::from(-6 * (32i64 << 30)), + quality_adjusted_delta: StoragePower::from(-675 * (32i64 << 30) / 100), }; let mut expected_update_claimed_power_params_ser = IpldBlock::serialize_cbor(&expected_update_claimed_power_params).unwrap().unwrap(); @@ -288,23 +263,10 @@ fn extend_legacy_sector_with_deals_inner(do_extend2: bool) { do_extend2, ); - miner_state = v.get_state::(&miner_id).unwrap(); - sector_info = miner_state.get_sector(&store, sector_number).unwrap().unwrap(); - assert_eq!(180 * 2 * EPOCHS_IN_DAY, sector_info.expiration - sector_info.activation); - assert_eq!(initial_deal_weight, sector_info.deal_weight); // 0 space time, unchanged - assert_eq!(&initial_verified_deal_weight / 2, sector_info.verified_deal_weight); - // advance to 6 months (original expiration) and extend another 6 months - // - // We're 1/3rd of the way through the last extension, so keep 2/3 of the power. - // - verified deal weight *= 2/3 - // - // normalize 180 days of 4x power plus 180 days of 1x power over 180+180 days: - // - multiplier = ((4 * 180) + (1 * 180)) / (90 + 180) - // - multiplier = 2.5 - // - // delta from the previous 4x power multiplier: - // - power delta = (4-2.5)*32GiB = 1.5*32GiB + // verified deal weight /= 2 + // power multiplier = (1/3)*3.25 + (2/3)*1 = 1.75 + // power delta = (3.25 - 1.75)*32GiB = 1.5*32GiB advance_by_deadline_to_epoch_while_proving( &v, @@ -338,8 +300,8 @@ fn extend_legacy_sector_with_deals_inner(do_extend2: bool) { sector_info = miner_state.get_sector(&store, sector_number).unwrap().unwrap(); assert_eq!(180 * 3 * EPOCHS_IN_DAY, sector_info.expiration - sector_info.activation); assert_eq!(initial_deal_weight, sector_info.deal_weight); // 0 space time, unchanged - assert_eq!(initial_verified_deal_weight / 3, sector_info.verified_deal_weight); - // 1/2 * 2/3 -> 1/3 + assert_eq!(initial_verified_deal_weight / 4, sector_info.verified_deal_weight); + // two halvings => 1/4 initial verified deal weight v.expect_state_invariants( &[invariant_failure_patterns::REWARD_STATE_EPOCH_MISMATCH.to_owned()], @@ -572,12 +534,7 @@ fn extend_updated_sector_with_claim() { sector_info_after_extension.verified_deal_weight ); // 32 GiB * the remaining life of the sector - assert_eq!(sector_info_after_extension.power_base_epoch, v.epoch()); assert_eq!(sector_info_after_update.activation, sector_info_after_extension.activation); - assert_eq!( - sector_info_after_extension.replaced_day_reward, - sector_info_after_update.expected_day_reward - ); } #[test] diff --git a/test_vm/tests/replica_update_test.rs b/test_vm/tests/replica_update_test.rs index 8ca352a15..70d5841c4 100644 --- a/test_vm/tests/replica_update_test.rs +++ b/test_vm/tests/replica_update_test.rs @@ -681,7 +681,7 @@ fn terminate_after_upgrade() { v.assert_state_invariants(); } -// Tests that an active CC sector can be correctly upgraded, and then the sector can be extended +// Tests that an active CC sector can be correctly upgraded, and then the sector can be terminated #[test] fn extend_after_upgrade() { let store = &MemoryBlockstore::new(); @@ -699,13 +699,12 @@ fn extend_after_upgrade() { st.sectors = sectors.amt.flush().unwrap(); }); - let extension_epoch = v.epoch(); let extension_params = ExtendSectorExpirationParams { extensions: vec![ExpirationExtension { deadline: deadline_index, partition: partition_index, sectors: make_bitfield(&[sector_number]), - new_expiration: extension_epoch + policy.max_sector_expiration_extension - 1, + new_expiration: v.epoch() + policy.max_sector_expiration_extension - 1, }], }; @@ -722,7 +721,7 @@ fn extend_after_upgrade() { let final_sector_info = miner_state.get_sector(store, sector_number).unwrap().unwrap(); assert_eq!( policy.max_sector_expiration_extension - 1, - final_sector_info.expiration - extension_epoch, + final_sector_info.expiration - final_sector_info.activation ); v.assert_state_invariants(); }