Skip to content

Conversation

@sudo-shashank
Copy link
Contributor

@sudo-shashank sudo-shashank commented Jul 11, 2025

Removed deprecated ExtendSectorExpiration as a new method already exist ExtendSectorExpiration2

  • Remove ExtendSectorExpiration and ExtensionKind from the miner actor.
  • Ensure that tests for ExtendSectorExpiration2 effectively cover the same functionality.
  • Ensure that removal of tests previously covering ExtendSectorExpiration doesn't result in an overall reduction in test coverage.
  • Ensure all systems behave normally with the removal of ExtendSectorExpiration.

FIP-0103: https://github.com/filecoin-project/FIPs/blob/master/FIPS/fip-0103.md

@github-project-automation github-project-automation bot moved this to 📌 Triage in FilOz Jul 11, 2025
@sudo-shashank sudo-shashank marked this pull request as ready for review July 15, 2025 12:31
@BigLep BigLep requested a review from rvagg July 15, 2025 14:22
@BigLep BigLep moved this from 📌 Triage to 🔎 Awaiting Review in FilOz Jul 15, 2025
@BigLep BigLep requested a review from Copilot July 15, 2025 14:22
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes the deprecated ExtendSectorExpiration method and all associated legacy handling, consolidating on the new ExtendSectorExpiration2 API.

  • Eliminate all vestiges of the old ExtendSectorExpiration method in actor code and tests
  • Update tests and utilities to only invoke ExtendSectorExpiration2 and drop version flags
  • Ensure test coverage remains equivalent by renaming and refactoring legacy test cases

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
test_vm/tests/suite/extend_sectors_test.rs Renamed and simplified the legacy extension test to use only v2
integration_tests/src/tests/replica_update_test.rs Updated method enum use to call the new v2 method ID
integration_tests/src/tests/extend_sectors_test.rs Removed v1 branches and tests; all calls now target v2
actors/miner/tests/util.rs Dropped legacy helper and parameter types; versioned helper now only v2
actors/miner/tests/extend_sector_expiration_test.rs Stripped out all v1 test cases and updated remaining tests to v2
actors/miner/src/lib.rs Commented out deprecated enum variant and removed legacy handler code
Comments suppressed due to low confidence (4)

test_vm/tests/suite/extend_sectors_test.rs:9

  • [nitpick] The test name extend_sector_with_deals no longer distinguishes which extension version it covers. Consider renaming it to extend_sector_with_deals_extend2 to make it clear that only the v2 API is exercised.
fn extend_sector_with_deals() {

actors/miner/tests/util.rs:2669

  • [nitpick] This helper no longer handles multiple versions and always calls the v2 path. Consider renaming it (for example to extend_sectors2_no_claims) to reflect that it invokes only the new API.
    pub fn extend_sectors_versioned(

actors/miner/src/lib.rs:115

  • [nitpick] The deprecated method enum variant is commented out but not marked as deprecated in the public API docs. Consider annotating it with #[deprecated] or adding a clear comment in the Method enum documentation to guide integrators.
    //ExtendSectorExpiration = 8, // Deprecated

actors/miner/tests/extend_sector_expiration_test.rs:76

  • [nitpick] The construction of ExtendSectorExpiration2Params with a single-element vector is repeated across many tests. Consider extracting a small helper or builder function to reduce duplication and improve readability.
    let params = ExtendSectorExpiration2Params {

@sudo-shashank sudo-shashank marked this pull request as draft July 15, 2025 18:49
@sudo-shashank sudo-shashank marked this pull request as ready for review July 15, 2025 19:52
@sudo-shashank sudo-shashank requested a review from ZenGround0 July 16, 2025 08:29
@sudo-shashank sudo-shashank requested a review from ZenGround0 July 17, 2025 08:48
Copy link
Contributor

@ZenGround0 ZenGround0 left a comment

Choose a reason for hiding this comment

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

LGTM

@github-project-automation github-project-automation bot moved this from 🔎 Awaiting Review to ✔️ Approved by reviewer in FilOz Jul 23, 2025
@sudo-shashank
Copy link
Contributor Author

@ZenGround0 can i merge this PR?

@ZenGround0 ZenGround0 added this pull request to the merge queue Jul 24, 2025
@ZenGround0
Copy link
Contributor

Yes, added it to the queue for you

Merged via the queue into master with commit 5f1f5b0 Jul 24, 2025
12 checks passed
@ZenGround0 ZenGround0 deleted the shashank/fip-0103 branch July 24, 2025 15:19
@github-project-automation github-project-automation bot moved this from ✔️ Approved by reviewer to 🎉 Done in FilOz Jul 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 🎉 Done

Development

Successfully merging this pull request may close these issues.

3 participants