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

chore: remove support of v1 proofs in Miner actor #1391

Merged
merged 5 commits into from
Sep 27, 2023
Merged
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
42 changes: 6 additions & 36 deletions actors/miner/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ use fil_actors_runtime::{
SYSTEM_ACTOR_ADDR, VERIFIED_REGISTRY_ACTOR_ADDR,
};
use fvm_ipld_encoding::ipld_block::IpldBlock;
use fvm_shared::version::NetworkVersion;
pub use monies::*;
pub use partition_state::*;
pub use policy::*;
Expand Down Expand Up @@ -163,14 +162,6 @@ impl Actor {
check_control_addresses(rt.policy(), &params.control_addresses)?;
check_peer_info(rt.policy(), &params.peer_id, &params.multi_addresses)?;
check_valid_post_proof_type(rt.policy(), params.window_post_proof_type)?;
// TODO: v12: cleanup https://github.com/filecoin-project/builtin-actors/issues/1260
if !is_window_post_proof_v1p1(params.window_post_proof_type) {
return Err(actor_error!(
illegal_argument,
"unsupported window post proof type: {:?}",
params.window_post_proof_type
));
}

let owner = rt.resolve_address(&params.owner).ok_or_else(|| {
actor_error!(illegal_argument, "unable to resolve owner address: {}", params.owner)
Expand Down Expand Up @@ -550,33 +541,12 @@ impl Actor {

// Make sure the miner is using the correct proof type.
if params.proofs[0].post_proof != info.window_post_proof_type {
// Special for nv19: Allow the v1 version of v1p1 post proof types
let nv = rt.network_version();
// TODO: v12: cleanup https://github.com/filecoin-project/builtin-actors/issues/1260
if nv == NetworkVersion::V19 {
let info_v1_proof_type =
convert_window_post_proof_v1p1_to_v1(info.window_post_proof_type)
.context_code(
ExitCode::USR_ILLEGAL_STATE,
"failed to convert to v1 window post proof",
)?;
if info_v1_proof_type != params.proofs[0].post_proof {
return Err(actor_error!(
illegal_argument,
"expected proof of type {:?} or {:?}, got {:?}",
info_v1_proof_type,
info.window_post_proof_type,
params.proofs[0].post_proof
));
}
} else {
return Err(actor_error!(
illegal_argument,
"expected proof of type {:?}, got {:?}",
info.window_post_proof_type,
params.proofs[0].post_proof
));
}
return Err(actor_error!(
illegal_argument,
"expected proof of type {:?}, got {:?}",
info.window_post_proof_type,
params.proofs[0].post_proof
));
}

// Make sure the proof size doesn't exceed the max. We could probably check for an exact match, but this is safer.
Expand Down
36 changes: 0 additions & 36 deletions actors/miner/src/policy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,42 +58,6 @@ pub fn can_extend_seal_proof_type(_proof: RegisteredSealProof) -> bool {
true
}

/// Convert the v1_1 PoSt Proof type to the older v1 types (used in nv18 and below)
pub fn convert_window_post_proof_v1p1_to_v1(
rpp: RegisteredPoStProof,
) -> Result<RegisteredPoStProof, String> {
match rpp {
RegisteredPoStProof::StackedDRGWindow2KiBV1P1 => {
Ok(RegisteredPoStProof::StackedDRGWindow2KiBV1)
}
RegisteredPoStProof::StackedDRGWindow8MiBV1P1 => {
Ok(RegisteredPoStProof::StackedDRGWindow8MiBV1)
}
RegisteredPoStProof::StackedDRGWindow512MiBV1P1 => {
Ok(RegisteredPoStProof::StackedDRGWindow512MiBV1)
}
RegisteredPoStProof::StackedDRGWindow32GiBV1P1 => {
Ok(RegisteredPoStProof::StackedDRGWindow32GiBV1)
}
RegisteredPoStProof::StackedDRGWindow64GiBV1P1 => {
Ok(RegisteredPoStProof::StackedDRGWindow64GiBV1)
}
i => Err(format!("not a v1p1 proof type: {:?}", i)),
}
}

/// Convert the v1_1 PoSt Proof type to the older v1 types (used in nv18 and below)
pub fn is_window_post_proof_v1p1(rpp: RegisteredPoStProof) -> bool {
matches!(
rpp,
RegisteredPoStProof::StackedDRGWindow2KiBV1P1
| RegisteredPoStProof::StackedDRGWindow8MiBV1P1
| RegisteredPoStProof::StackedDRGWindow512MiBV1P1
| RegisteredPoStProof::StackedDRGWindow32GiBV1P1
| RegisteredPoStProof::StackedDRGWindow64GiBV1P1
)
}

/// Maximum duration to allow for the sealing process for seal algorithms.
/// Dependent on algorithm and sector size
pub fn max_prove_commit_duration(
Expand Down
12 changes: 7 additions & 5 deletions actors/miner/tests/miner_actor_test_wpost.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ fn invalid_submissions() {
);
expect_abort_contains_message(
ExitCode::USR_ILLEGAL_ARGUMENT,
"expected proof of type",
"proof type StackedDRGWindow64GiBV1 not allowed",
result,
);
rt.reset();
Expand Down Expand Up @@ -1296,7 +1296,7 @@ fn bad_post_fails_when_verified() {
}

#[test]
fn can_submit_v1_proof_types_nv19() {
fn cannot_submit_v1_proof_types_nv19() {
struct TestCase {
desc: &'static str,
nv: NetworkVersion,
Expand All @@ -1308,12 +1308,14 @@ fn can_submit_v1_proof_types_nv19() {

let tests = [
TestCase {
desc: "can submit v1 proof in nv19",
desc: "cannot submit v1 proof in nv19",
nv: NetworkVersion::V19,
seal_proof_type: RegisteredSealProof::StackedDRG32GiBV1P1,
post_proof_type: RegisteredPoStProof::StackedDRGWindow32GiBV1,
exit_code: ExitCode::OK,
error_msg: "".to_string(),
exit_code: ExitCode::USR_ILLEGAL_ARGUMENT,
error_msg:
"expected proof of type StackedDRGWindow32GiBV1P1, got StackedDRGWindow32GiBV1"
.to_string(),
},
TestCase {
desc: "can submit v1p1 proof in nv19",
Expand Down
6 changes: 0 additions & 6 deletions runtime/src/runtime/policy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -358,30 +358,24 @@ impl ProofSet {
/// Create a `ProofSet` for enabled `RegisteredPoStProof`s
pub fn default_post_proofs() -> Self {
let mut proofs = vec![false; REGISTERED_POST_PROOF_VARIANTS];
Copy link
Contributor

Choose a reason for hiding this comment

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

We should be able to drop the size of this REGISTERED_POST_PROOF_VARIANTS field now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you mean exactly by dropping the size of REGISTERED_POST_PROOF_VARIANTS?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the number can change from 15 to 10?

Copy link
Contributor Author

@elmattic elmattic Sep 26, 2023

Choose a reason for hiding this comment

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

You mean from 15 to 5, no? Let me know if I am mistaken, I might not have all the context here.

I have also created counterpart PR in the FVM repo: filecoin-project/ref-fvm#1896

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, nvm, I was wrong -- we can't reduce this number at all, because it needs to be indexed up to StackedDRGWindow64GiBV1P1, which is 15.

Copy link
Contributor Author

@elmattic elmattic Sep 27, 2023

Choose a reason for hiding this comment

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

Yes but once the builtin-actors repo is making use of FVM4 this should be possible (the V1P1 proofs are indexed from 0 to 4 there).

Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately, these numbers are consensus-critical, so we can't change them (easily).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it!

// TODO: v12: cleanup https://github.com/filecoin-project/builtin-actors/issues/1260
#[cfg(feature = "sector-2k")]
{
proofs[i64::from(RegisteredPoStProof::StackedDRGWindow2KiBV1) as usize] = true;
proofs[i64::from(RegisteredPoStProof::StackedDRGWindow2KiBV1P1) as usize] = true;
}
#[cfg(feature = "sector-8m")]
{
proofs[i64::from(RegisteredPoStProof::StackedDRGWindow8MiBV1) as usize] = true;
proofs[i64::from(RegisteredPoStProof::StackedDRGWindow8MiBV1P1) as usize] = true;
}
#[cfg(feature = "sector-512m")]
{
proofs[i64::from(RegisteredPoStProof::StackedDRGWindow512MiBV1) as usize] = true;
proofs[i64::from(RegisteredPoStProof::StackedDRGWindow512MiBV1P1) as usize] = true;
}
#[cfg(feature = "sector-32g")]
{
proofs[i64::from(RegisteredPoStProof::StackedDRGWindow32GiBV1) as usize] = true;
proofs[i64::from(RegisteredPoStProof::StackedDRGWindow32GiBV1P1) as usize] = true;
}
#[cfg(feature = "sector-64g")]
{
proofs[i64::from(RegisteredPoStProof::StackedDRGWindow64GiBV1) as usize] = true;
proofs[i64::from(RegisteredPoStProof::StackedDRGWindow64GiBV1P1) as usize] = true;
}
ProofSet(proofs)
Expand Down