From 7623ce7567ce7f1614e23c73b7a48ec5d8d59d37 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 | 3 - actors/miner/src/policy.rs | 16 ++---- actors/miner/tests/aggregate_prove_commit.rs | 1 - actors/miner/tests/policy_test.rs | 58 ++++++++------------ actors/miner/tests/util.rs | 8 +-- 5 files changed, 28 insertions(+), 58 deletions(-) diff --git a/actors/miner/src/lib.rs b/actors/miner/src/lib.rs index 8ae14c7a8..152e7725f 100644 --- a/actors/miner/src/lib.rs +++ b/actors/miner/src/lib.rs @@ -3895,7 +3895,6 @@ fn extend_simple_qap_sector( 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( @@ -4286,7 +4285,6 @@ fn update_existing_sector_info( let qa_pow = qa_power_for_weight( sector_size, duration, - &new_sector_info.deal_weight, &new_sector_info.verified_deal_weight, ); @@ -5533,7 +5531,6 @@ fn activate_new_sector_infos( let power = qa_power_for_weight( info.sector_size, duration, - &deal_weight, &verified_deal_weight, ); diff --git a/actors/miner/src/policy.rs b/actors/miner/src/policy.rs index 66e20130a..43f9e2169 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); @@ -128,18 +125,14 @@ pub const MIN_SECTOR_EXPIRATION: i64 = 180 * EPOCHS_IN_DAY; 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 +151,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..385114ce2 100644 --- a/actors/miner/tests/policy_test.rs +++ b/actors/miner/tests/policy_test.rs @@ -1,7 +1,6 @@ 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::{EPOCHS_IN_DAY, SECONDS_IN_DAY}; use fvm_shared::bigint::{BigInt, Zero}; @@ -12,12 +11,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, @@ -36,17 +35,15 @@ fn quality_is_independent_of_size_and_duration() { for size in size_range { for duration in &duration_range { let sector_weight = weight(size, *duration); + assert_eq!(empty_quality, quality_for_weight(size, *duration, &BigInt::zero())); + assert_eq!(verified_quality, quality_for_weight(size, *duration, §or_weight)); assert_eq!( - empty_quality, - quality_for_weight(size, *duration, &BigInt::zero(), &BigInt::zero()), - ); - assert_eq!( - deal_quality, - quality_for_weight(size, *duration, §or_weight, &BigInt::zero()), - ); - assert_eq!( - verified_quality, - quality_for_weight(size, *duration, &BigInt::zero(), §or_weight), + half_verified_quality, + quality_for_weight( + size, + *duration, + §or_weight.checked_div(&BigInt::from(2)).unwrap() + ) ); } } @@ -86,7 +83,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 +106,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 +132,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 +177,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 +195,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 +213,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)) ); } diff --git a/actors/miner/tests/util.rs b/actors/miner/tests/util.rs index 861c75d66..520abc3e0 100644 --- a/actors/miner/tests/util.rs +++ b/actors/miner/tests/util.rs @@ -1203,17 +1203,13 @@ 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, ); expected_qa_power += &qa_power_delta; @@ -1383,7 +1379,7 @@ impl ActorHarness { } } let qa_power_delta = - qa_power_for_weight(self.sector_size, duration, &deal_size, &verified_size); + 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); } @@ -1597,7 +1593,7 @@ impl ActorHarness { } let qa_power_delta = - qa_power_for_weight(self.sector_size, duration, &deal_size, &verified_size) + 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);