From 92a7559dd0d26080533fc0c4e2007f4ae1e87b18 Mon Sep 17 00:00:00 2001 From: Rod Vagg Date: Mon, 19 Aug 2024 17:06:10 +1000 Subject: [PATCH] fix(miner)!: remove DEAL_WEIGHT_MULTIPLIER and its input to QAP calc Closes: https://github.com/filecoin-project/builtin-actors/issues/1573 --- actors/miner/src/lib.rs | 22 +--- actors/miner/src/policy.rs | 23 ++-- actors/miner/tests/aggregate_prove_commit.rs | 1 - actors/miner/tests/policy_test.rs | 108 +++++++++++++------ actors/miner/tests/util.rs | 19 +--- 5 files changed, 92 insertions(+), 81 deletions(-) diff --git a/actors/miner/src/lib.rs b/actors/miner/src/lib.rs index 8ae14c7a8..c1a132f20 100644 --- a/actors/miner/src/lib.rs +++ b/actors/miner/src/lib.rs @@ -3892,12 +3892,8 @@ fn extend_simple_qap_sector( // 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, - ); + let qa_pow = + qa_power_for_weight(sector_size, new_duration, &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, @@ -4283,12 +4279,7 @@ fn update_existing_sector_info( new_sector_info.verified_deal_weight = activated_data.verified_space.clone() * duration; // compute initial pledge - let qa_pow = qa_power_for_weight( - sector_size, - duration, - &new_sector_info.deal_weight, - &new_sector_info.verified_deal_weight, - ); + let qa_pow = qa_power_for_weight(sector_size, duration, &new_sector_info.verified_deal_weight); new_sector_info.replaced_day_reward = max(§or_info.expected_day_reward, §or_info.replaced_day_reward).clone(); @@ -5530,12 +5521,7 @@ fn activate_new_sector_infos( let deal_weight = &deal_spaces.unverified_space * duration; let verified_deal_weight = &deal_spaces.verified_space * duration; - let power = qa_power_for_weight( - info.sector_size, - duration, - &deal_weight, - &verified_deal_weight, - ); + let power = qa_power_for_weight(info.sector_size, duration, &verified_deal_weight); let day_reward = expected_reward_for_power( &pledge_inputs.epoch_reward, diff --git a/actors/miner/src/policy.rs b/actors/miner/src/policy.rs index 66e20130a..97cb52796 100644 --- a/actors/miner/src/policy.rs +++ b/actors/miner/src/policy.rs @@ -27,9 +27,6 @@ lazy_static! { /// Quality multiplier for committed capacity (no deals) in a sector pub static ref QUALITY_BASE_MULTIPLIER: BigInt = BigInt::from(10); - /// Quality multiplier for unverified deals in a sector - pub static ref DEAL_WEIGHT_MULTIPLIER: BigInt = BigInt::from(10); - /// Quality multiplier for verified deals in a sector pub static ref VERIFIED_DEAL_WEIGHT_MULTIPLIER: BigInt = BigInt::from(100); @@ -119,27 +116,22 @@ pub fn seal_proof_sector_maximum_lifetime(proof: RegisteredSealProof) -> Option< /// minimum number of epochs past the current epoch a sector may be set to expire pub const MIN_SECTOR_EXPIRATION: i64 = 180 * EPOCHS_IN_DAY; -/// DealWeight and VerifiedDealWeight are spacetime occupied by regular deals and verified deals in a sector. -/// Sum of DealWeight and VerifiedDealWeight should be less than or equal to total SpaceTime of a sector. +/// VerifiedDealWeight is spacetime occupied by verified pieces in a sector. +/// VerifiedDealWeight should be less than or equal to total SpaceTime of a sector. /// Sectors full of VerifiedDeals will have a BigInt of VerifiedDealWeightMultiplier/QualityBaseMultiplier. -/// Sectors full of Deals will have a BigInt of DealWeightMultiplier/QualityBaseMultiplier. -/// Sectors with neither will have a BigInt of QualityBaseMultiplier/QualityBaseMultiplier. +/// Sectors without VerifiedDeals will have a BigInt of QualityBaseMultiplier/QualityBaseMultiplier. /// BigInt of a sector is a weighted average of multipliers based on their proportions. pub fn quality_for_weight( size: SectorSize, duration: ChainEpoch, - deal_weight: &DealWeight, verified_weight: &DealWeight, ) -> BigInt { let sector_space_time = BigInt::from(size as u64) * BigInt::from(duration); - let total_deal_space_time = deal_weight + verified_weight; let weighted_base_space_time = - (§or_space_time - total_deal_space_time) * &*QUALITY_BASE_MULTIPLIER; - let weighted_deal_space_time = deal_weight * &*DEAL_WEIGHT_MULTIPLIER; + (§or_space_time - verified_weight) * &*QUALITY_BASE_MULTIPLIER; let weighted_verified_space_time = verified_weight * &*VERIFIED_DEAL_WEIGHT_MULTIPLIER; - let weighted_sum_space_time = - weighted_base_space_time + weighted_deal_space_time + weighted_verified_space_time; + let weighted_sum_space_time = weighted_base_space_time + weighted_verified_space_time; let scaled_up_weighted_sum_space_time: BigInt = weighted_sum_space_time << SECTOR_QUALITY_PRECISION; @@ -158,17 +150,16 @@ pub fn qa_power_max(size: SectorSize) -> StoragePower { pub fn qa_power_for_weight( size: SectorSize, duration: ChainEpoch, - deal_weight: &DealWeight, verified_weight: &DealWeight, ) -> StoragePower { - let quality = quality_for_weight(size, duration, deal_weight, verified_weight); + let quality = quality_for_weight(size, duration, verified_weight); (BigInt::from(size as u64) * quality) >> SECTOR_QUALITY_PRECISION } /// 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; - qa_power_for_weight(size, duration, §or.deal_weight, §or.verified_deal_weight) + qa_power_for_weight(size, duration, §or.verified_deal_weight) } pub fn raw_power_for_sector(size: SectorSize) -> StoragePower { diff --git a/actors/miner/tests/aggregate_prove_commit.rs b/actors/miner/tests/aggregate_prove_commit.rs index 942eccbc9..c999fceb6 100644 --- a/actors/miner/tests/aggregate_prove_commit.rs +++ b/actors/miner/tests/aggregate_prove_commit.rs @@ -91,7 +91,6 @@ fn valid_precommits_then_aggregate_provecommit() { let qa_power = qa_power_for_weight( actor.sector_size, expiration - *rt.epoch.borrow(), - &deal_weight, &verified_deal_weight, ); assert_eq!(expected_power, qa_power); diff --git a/actors/miner/tests/policy_test.rs b/actors/miner/tests/policy_test.rs index ac2ea58da..ef56479e3 100644 --- a/actors/miner/tests/policy_test.rs +++ b/actors/miner/tests/policy_test.rs @@ -1,10 +1,10 @@ use fil_actor_miner::{qa_power_for_weight, quality_for_weight}; use fil_actor_miner::{ - DEAL_WEIGHT_MULTIPLIER, QUALITY_BASE_MULTIPLIER, SECTOR_QUALITY_PRECISION, - VERIFIED_DEAL_WEIGHT_MULTIPLIER, + QUALITY_BASE_MULTIPLIER, SECTOR_QUALITY_PRECISION, VERIFIED_DEAL_WEIGHT_MULTIPLIER, }; +use fil_actors_runtime::DealWeight; use fil_actors_runtime::{EPOCHS_IN_DAY, SECONDS_IN_DAY}; -use fvm_shared::bigint::{BigInt, Zero}; +use fvm_shared::bigint::{BigInt, Integer, Zero}; use fvm_shared::clock::ChainEpoch; use fvm_shared::sector::SectorSize; @@ -12,12 +12,12 @@ use fvm_shared::sector::SectorSize; fn quality_is_independent_of_size_and_duration() { // Quality of space with no deals. This doesn't depend on either the sector size or duration. let empty_quality = BigInt::from(1 << SECTOR_QUALITY_PRECISION); - // Quality space filled with non-verified deals. - let deal_quality = - &empty_quality * (DEAL_WEIGHT_MULTIPLIER.clone() / QUALITY_BASE_MULTIPLIER.clone()); // Quality space filled with verified deals. let verified_quality = &empty_quality * (VERIFIED_DEAL_WEIGHT_MULTIPLIER.clone() / QUALITY_BASE_MULTIPLIER.clone()); + // Quality space half filled with verified deals. + let half_verified_quality = + &empty_quality / BigInt::from(2) + &verified_quality / BigInt::from(2); let size_range: Vec = vec![ SectorSize::_2KiB, @@ -32,21 +32,50 @@ fn quality_is_independent_of_size_and_duration() { ChainEpoch::from(1000), 1000 * EPOCHS_IN_DAY, ]; + let zero = &BigInt::zero(); for size in size_range { for duration in &duration_range { - let sector_weight = weight(size, *duration); + let full_weight = weight(size, *duration); + let half_weight = &full_weight.checked_div(&BigInt::from(2)).unwrap(); + + assert_eq!(empty_quality, quality_for_weight(size, *duration, zero)); + assert_eq!(verified_quality, quality_for_weight(size, *duration, &full_weight)); + assert_eq!(half_verified_quality, quality_for_weight(size, *duration, half_weight)); + + // test against old form that takes a deal_weight argument + assert_eq!(empty_quality, original_quality_for_weight(size, *duration, zero, zero)); + assert_eq!( + empty_quality, + original_quality_for_weight(size, *duration, &full_weight, zero) + ); assert_eq!( empty_quality, - quality_for_weight(size, *duration, &BigInt::zero(), &BigInt::zero()), + original_quality_for_weight(size, *duration, half_weight, zero) + ); + assert_eq!( + verified_quality, + original_quality_for_weight(size, *duration, zero, &full_weight) ); assert_eq!( - deal_quality, - quality_for_weight(size, *duration, §or_weight, &BigInt::zero()), + verified_quality, + original_quality_for_weight(size, *duration, half_weight, &full_weight) ); assert_eq!( verified_quality, - quality_for_weight(size, *duration, &BigInt::zero(), §or_weight), + original_quality_for_weight(size, *duration, &full_weight, &full_weight) + ); + assert_eq!( + half_verified_quality, + original_quality_for_weight(size, *duration, zero, half_weight) + ); + assert_eq!( + half_verified_quality, + original_quality_for_weight(size, *duration, half_weight, half_weight) + ); + assert_eq!( + half_verified_quality, + original_quality_for_weight(size, *duration, &full_weight, half_weight) ); } } @@ -86,7 +115,7 @@ fn quality_scales_with_verified_weight_proportion() { let expected_quality = (eq + vq) / §or_weight; assert_eq!( expected_quality, - quality_for_weight(sector_size, sector_duration, &BigInt::zero(), &verified_weight), + quality_for_weight(sector_size, sector_duration, &verified_weight), ); } } @@ -109,10 +138,7 @@ fn empty_sector_has_power_equal_to_size() { for size in size_range { for duration in &duration_range { let expected_power = BigInt::from(size as i64); - assert_eq!( - expected_power, - qa_power_for_weight(size, *duration, &BigInt::zero(), &BigInt::zero()) - ); + assert_eq!(expected_power, qa_power_for_weight(size, *duration, &BigInt::zero())); } } } @@ -138,10 +164,7 @@ fn verified_sector_has_power_a_multiple_of_size() { for duration in &duration_range { let verified_weight = weight(size, *duration); let expected_power = size as i64 * &verified_multiplier; - assert_eq!( - expected_power, - qa_power_for_weight(size, *duration, &BigInt::zero(), &verified_weight) - ); + assert_eq!(expected_power, qa_power_for_weight(size, *duration, &verified_weight)); } } } @@ -186,12 +209,7 @@ fn verified_weight_adds_proportional_power() { let ep = empty_weight * &fully_empty_power; let vp = &verified_weight * &fully_verified_power; let expected_power = (ep + vp) / §or_weight; - let power = qa_power_for_weight( - sector_size, - sector_duration, - &BigInt::zero(), - &verified_weight, - ); + let power = qa_power_for_weight(sector_size, sector_duration, &verified_weight); let power_error = expected_power - power; assert!(power_error <= max_error); } @@ -209,16 +227,16 @@ fn demonstrate_standard_sectors() { assert_eq!( BigInt::from(sector_size as u64), - qa_power_for_weight(sector_size, sector_duration, &BigInt::zero(), &BigInt::zero()) + qa_power_for_weight(sector_size, sector_duration, &BigInt::zero()) ); assert_eq!( &vmul * sector_size as u64, - qa_power_for_weight(sector_size, sector_duration, &BigInt::zero(), §or_weight) + qa_power_for_weight(sector_size, sector_duration, §or_weight) ); let half_verified_power = ((sector_size as u64) / 2) + (&vmul * (sector_size as u64) / 2); assert_eq!( half_verified_power, - qa_power_for_weight(sector_size, sector_duration, &BigInt::zero(), &(sector_weight / 2)) + qa_power_for_weight(sector_size, sector_duration, &(sector_weight / 2)) ); // 64GiB @@ -227,16 +245,16 @@ fn demonstrate_standard_sectors() { assert_eq!( BigInt::from(sector_size as u64), - qa_power_for_weight(sector_size, sector_duration, &BigInt::zero(), &BigInt::zero()) + qa_power_for_weight(sector_size, sector_duration, &BigInt::zero()) ); assert_eq!( &vmul * sector_size as u64, - qa_power_for_weight(sector_size, sector_duration, &BigInt::zero(), §or_weight) + qa_power_for_weight(sector_size, sector_duration, §or_weight) ); let half_verified_power = ((sector_size as u64) / 2) + (&vmul * (sector_size as u64) / 2); assert_eq!( half_verified_power, - qa_power_for_weight(sector_size, sector_duration, &BigInt::zero(), &(sector_weight / 2)) + qa_power_for_weight(sector_size, sector_duration, &(sector_weight / 2)) ); } @@ -247,3 +265,29 @@ fn weight(size: SectorSize, duration: ChainEpoch) -> BigInt { fn weight_with_size_as_bigint(size: BigInt, duration: ChainEpoch) -> BigInt { size * BigInt::from(duration) } + +// Original form of quality_for_weight prior to removing the deal weight multiplier; retained here +// for testing purposes. Since the deal weight multipler has remained fixed at the same value as +// the quality base multiplier (10) it has never had an effect on the result. +fn original_quality_for_weight( + size: SectorSize, + duration: ChainEpoch, + deal_weight: &DealWeight, + verified_weight: &DealWeight, +) -> BigInt { + let sector_space_time = BigInt::from(size as u64) * BigInt::from(duration); + let total_deal_space_time = deal_weight + verified_weight; + + let weighted_base_space_time = + (§or_space_time - total_deal_space_time) * &*QUALITY_BASE_MULTIPLIER; + let weighted_deal_space_time = deal_weight * BigInt::from(10); + let weighted_verified_space_time = verified_weight * &*VERIFIED_DEAL_WEIGHT_MULTIPLIER; + let weighted_sum_space_time = + weighted_base_space_time + weighted_deal_space_time + weighted_verified_space_time; + let scaled_up_weighted_sum_space_time: BigInt = + weighted_sum_space_time << SECTOR_QUALITY_PRECISION; + + scaled_up_weighted_sum_space_time + .div_floor(§or_space_time) + .div_floor(&QUALITY_BASE_MULTIPLIER) +} diff --git a/actors/miner/tests/util.rs b/actors/miner/tests/util.rs index 861c75d66..8a9bf9ded 100644 --- a/actors/miner/tests/util.rs +++ b/actors/miner/tests/util.rs @@ -1203,19 +1203,12 @@ impl ActorHarness { let mut expected_raw_power = BigInt::from(0); for pc in valid_pcs { - let deal_space = cfg.deal_space(pc.info.sector_number); let verified_deal_space = cfg.verified_deal_space(pc.info.sector_number); - let duration = pc.info.expiration - *rt.epoch.borrow(); - let deal_weight = deal_space * duration; let verified_deal_weight = verified_deal_space * duration; if duration >= rt.policy.min_sector_expiration { - let qa_power_delta = qa_power_for_weight( - self.sector_size, - duration, - &deal_weight, - &verified_deal_weight, - ); + let qa_power_delta = + qa_power_for_weight(self.sector_size, duration, &verified_deal_weight); expected_qa_power += &qa_power_delta; expected_raw_power += self.sector_size as u64; expected_pledge += self.initial_pledge_for_power(rt, &qa_power_delta); @@ -1382,8 +1375,7 @@ impl ActorHarness { deal_size += piece.size.0 * duration as u64; } } - let qa_power_delta = - qa_power_for_weight(self.sector_size, duration, &deal_size, &verified_size); + let qa_power_delta = qa_power_for_weight(self.sector_size, duration, &verified_size); expected_qa_power += &qa_power_delta; expected_pledge += self.initial_pledge_for_power(rt, &qa_power_delta); } @@ -1596,9 +1588,8 @@ impl ActorHarness { } } - let qa_power_delta = - qa_power_for_weight(self.sector_size, duration, &deal_size, &verified_size) - - qa_power_for_sector(self.sector_size, §or); + let qa_power_delta = qa_power_for_weight(self.sector_size, duration, &verified_size) + - qa_power_for_sector(self.sector_size, §or); expected_qa_power += &qa_power_delta; expected_pledge += self.initial_pledge_for_power(rt, &qa_power_delta); }