Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Remove unused and deprecated code #1431

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions actors/account/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ fil_actors_runtime::wasm_trampoline!(Actor);
pub enum Method {
Constructor = METHOD_CONSTRUCTOR,
PubkeyAddress = 2,
// Deprecated in v10
// AuthenticateMessage = 3,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should remove these (in any actor). They are useful documentation of method numbers that have been used, and relevant to interpreting old blockchain history. These lines aren't costing us anything to retain.

AuthenticateMessageExported = frc42_dispatch::method_hash!("AuthenticateMessage"),
}

Expand Down
15 changes: 0 additions & 15 deletions actors/datacap/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,21 +51,6 @@ lazy_static! {
#[repr(u64)]
pub enum Method {
Constructor = METHOD_CONSTRUCTOR,
// Deprecated in v10
// Mint = 2,
// Destroy = 3,
// Name = 10,
// Symbol = 11,
// TotalSupply = 12,
// BalanceOf = 13,
// Transfer = 14,
// TransferFrom = 15,
// IncreaseAllowance = 16,
// DecreaseAllowance = 17,
// RevokeAllowance = 18,
// Burn = 19,
// BurnFrom = 20,
// Allowance = 21,
// Method numbers derived from FRC-0042 standards
MintExported = frc42_dispatch::method_hash!("Mint"),
DestroyExported = frc42_dispatch::method_hash!("Destroy"),
Expand Down
3 changes: 1 addition & 2 deletions actors/market/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,7 @@ pub enum Method {
VerifyDealsForActivation = 5,
BatchActivateDeals = 6,
OnMinerSectorsTerminate = 7,
// ComputeDataCommitment = 8, // Deprecated
CronTick = 9,
CronTick = 8,
// Method numbers derived from FRC-0042 standards
AddBalanceExported = frc42_dispatch::method_hash!("AddBalance"),
WithdrawBalanceExported = frc42_dispatch::method_hash!("WithdrawBalance"),
Expand Down
29 changes: 11 additions & 18 deletions actors/miner/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1423,26 +1423,19 @@ impl Actor {
.sectors
.into_iter()
.map(|spci| {
if spci.replace_capacity {
Err(actor_error!(
forbidden,
"cc upgrade through precommit discontinued, use ProveReplicaUpdate"
))
} else {
Ok(SectorPreCommitInfoInner {
seal_proof: spci.seal_proof,
sector_number: spci.sector_number,
sealed_cid: spci.sealed_cid,
seal_rand_epoch: spci.seal_rand_epoch,
deal_ids: spci.deal_ids,
expiration: spci.expiration,
// This entry point computes the unsealed CID from deals via the market.
// A future one will accept it directly as a parameter.
unsealed_cid: None,
})
SectorPreCommitInfoInner {
seal_proof: spci.seal_proof,
sector_number: spci.sector_number,
sealed_cid: spci.sealed_cid,
seal_rand_epoch: spci.seal_rand_epoch,
deal_ids: spci.deal_ids,
expiration: spci.expiration,
// This entry point computes the unsealed CID from deals via the market.
// A future one will accept it directly as a parameter.
unsealed_cid: None,
}
})
.collect::<Result<_, _>>()?;
.collect::<Vec<SectorPreCommitInfoInner>>();
Self::pre_commit_sector_batch_inner(rt, sectors)
}

Expand Down
15 changes: 1 addition & 14 deletions actors/miner/src/testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -599,12 +599,6 @@ impl PartitionStateSummary {
struct ExpirationQueueStateSummary {
pub on_time_sectors: BitField,
pub early_sectors: BitField,
#[allow(dead_code)]
pub active_power: PowerPair,
#[allow(dead_code)]
pub faulty_power: PowerPair,
#[allow(dead_code)]
pub on_time_pledge: TokenAmount,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These being dead signals to me that we are missing state invariant checks upstream of this that should use the values. I don't think removing them now is helpful – we should strengthen the invariant checks to use them instead.

pub expiration_epochs: Vec<ChainEpoch>,
}

Expand Down Expand Up @@ -697,14 +691,7 @@ impl ExpirationQueueStateSummary {
let union_on_time = BitField::union(&all_on_time);
let union_early = BitField::union(&all_early);

Self {
on_time_sectors: union_on_time,
early_sectors: union_early,
active_power: all_active_power,
faulty_power: all_faulty_power,
on_time_pledge: all_on_time_pledge,
expiration_epochs,
}
Self { on_time_sectors: union_on_time, early_sectors: union_early, expiration_epochs }
}
}

Expand Down
6 changes: 0 additions & 6 deletions actors/miner/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -293,12 +293,6 @@ pub struct PreCommitSectorParams {
pub seal_rand_epoch: ChainEpoch,
pub deal_ids: Vec<DealID>,
pub expiration: ChainEpoch,
/// Deprecated:
/// Whether to replace a "committed capacity" no-deal sector (requires non-empty DealIDs)
pub replace_capacity: bool,
/// Deprecated:
/// The committed capacity sector to replace, and its deadline/partition location
pub replace_sector_deadline: u64,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can't change Params structs. This is a publicly exported API invoked from off-chain. This will break existing callers. We can one day deprecate the entire method, forcing every remaining caller to migrate to PreCommitSectorBatch2, but until then it needs to remain as-is. Deprecated doesn't mean we can delete it – it means we can't delete it but people should stop using it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@anorth For my ow understanding, what kind of offchain callers use this public API ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this one, storage providers. I haven't looked at stats recently to see how many have migrated to v2, but probably most have not yet.

pub replace_sector_partition: u64,
pub replace_sector_number: SectorNumber,
}
Expand Down
6 changes: 0 additions & 6 deletions actors/miner/tests/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -517,9 +517,6 @@ impl ActorHarness {
seal_rand_epoch: challenge,
deal_ids: sector_deal_ids,
expiration,
// unused
replace_capacity: false,
replace_sector_deadline: 0,
replace_sector_partition: 0,
replace_sector_number: 0,
}
Expand Down Expand Up @@ -578,9 +575,6 @@ impl ActorHarness {
seal_rand_epoch: s.seal_rand_epoch,
deal_ids: s.deal_ids.clone(),
expiration: s.expiration,
// unused
replace_capacity: false,
replace_sector_deadline: 0,
replace_sector_partition: 0,
replace_sector_number: 0,
}
Expand Down
6 changes: 2 additions & 4 deletions actors/power/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,8 @@ pub enum Method {
EnrollCronEvent = 4,
OnEpochTickEnd = 5,
UpdatePledgeTotal = 6,
// * Deprecated in v2
// OnConsensusFault = 7,
SubmitPoRepForBulkVerify = 8,
CurrentTotalPower = 9,
SubmitPoRepForBulkVerify = 7,
CurrentTotalPower = 8,
// Method numbers derived from FRC-0042 standards
CreateMinerExported = frc42_dispatch::method_hash!("CreateMiner"),
NetworkRawPowerExported = frc42_dispatch::method_hash!("NetworkRawPower"),
Expand Down
14 changes: 6 additions & 8 deletions actors/verifreg/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,12 @@ pub enum Method {
AddVerifier = 2,
RemoveVerifier = 3,
AddVerifiedClient = 4,
// UseBytes = 5, // Deprecated
// RestoreBytes = 6, // Deprecated
RemoveVerifiedClientDataCap = 7,
RemoveExpiredAllocations = 8,
ClaimAllocations = 9,
GetClaims = 10,
ExtendClaimTerms = 11,
RemoveExpiredClaims = 12,
RemoveVerifiedClientDataCap = 5,
RemoveExpiredAllocations = 6,
ClaimAllocations = 7,
GetClaims = 8,
ExtendClaimTerms = 9,
RemoveExpiredClaims = 10,
// Method numbers derived from FRC-0042 standards
AddVerifiedClientExported = frc42_dispatch::method_hash!("AddVerifiedClient"),
RemoveExpiredAllocationsExported = frc42_dispatch::method_hash!("RemoveExpiredAllocations"),
Expand Down
2 changes: 0 additions & 2 deletions integration_tests/src/util/workflows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,6 @@ pub fn miner_precommit_sector(
seal_rand_epoch: v.epoch() - 1,
deal_ids,
expiration,
replace_capacity: false,
replace_sector_deadline: 0,
replace_sector_partition: 0,
replace_sector_number: 0,
};
Expand Down
5 changes: 0 additions & 5 deletions runtime/src/runtime/fvm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,6 @@ impl<B> FvmRuntime<B> {
}
Ok(())
}

#[allow(dead_code)]
fn policy_mut(&mut self) -> &mut Policy {
&mut self.policy
}
}

/// A stub MessageInfo implementation performing FVM syscalls to obtain its fields.
Expand Down
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![warn(dead_code)]
/// The bundled CAR embedded as a byte slice for easy consumption by Rust programs.
///
/// The root CID of the CAR points to an actor index data structure. It is a
Expand Down
2 changes: 2 additions & 0 deletions src/main.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#![warn(dead_code)]

use clap::Parser;
use std::io::Write;

Expand Down