Skip to content

feat: add ExtendSectorExpiration3 to update the existing sectors to 10x QAP - #1775

Draft
akaladarshi wants to merge 4 commits into
rvagg/fip-0118-filplus-fixesfrom
akaladarshi/solstice-upgrade-path
Draft

feat: add ExtendSectorExpiration3 to update the existing sectors to 10x QAP#1775
akaladarshi wants to merge 4 commits into
rvagg/fip-0118-filplus-fixesfrom
akaladarshi/solstice-upgrade-path

Conversation

@akaladarshi

@akaladarshi akaladarshi commented Aug 7, 2026

Copy link
Copy Markdown
Contributor
  • This PR adds a new method ExtendSectorExpiration3 in the miner actor.
  • ExtendSectorExpiration3 updates the existing active sectors to the 10x QAP.
    • Right now we are updating all the active sectors using their ids.
    • (Clarification needed) FIP mentions that this method only needs to update the CC sectors but current methods update all the sectors not just CC sectors (Should this be changed?).
    • ExtendSectorExpiration3Params accepts new_expiration which is an optional value and it allows the method to extended the sectors if set to some value.

This PR is assisted by Claude code (Fable and Opus5)

@github-project-automation github-project-automation Bot moved this to 📌 Triage in FilOz Aug 7, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 82.44275% with 46 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.17%. Comparing base (1ff11fe) to head (1025a9c).

Files with missing lines Patch % Lines
actors/miner/src/lib.rs 82.44% 46 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                        Coverage Diff                         @@
##           fix/fips1249-1744-review-fixes    #1775      +/-   ##
==================================================================
- Coverage                           88.22%   88.17%   -0.05%     
==================================================================
  Files                                 140      140              
  Lines                               26555    26817     +262     
==================================================================
+ Hits                                23428    23646     +218     
- Misses                               3127     3171      +44     
Files with missing lines Coverage Δ
actors/miner/src/types.rs 100.00% <ø> (ø)
actors/miner/src/lib.rs 82.38% <82.44%> (+<0.01%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@rjan90 rjan90 added this to Solstice Aug 7, 2026
@github-project-automation github-project-automation Bot moved this to Backlog in Solstice Aug 7, 2026
@rjan90 rjan90 linked an issue Aug 7, 2026 that may be closed by this pull request
@rjan90 rjan90 moved this from Backlog to In progress in Solstice Aug 7, 2026
@rjan90 rjan90 removed this from FilOz Aug 7, 2026
@github-project-automation github-project-automation Bot moved this to 📌 Triage in FilOz Aug 7, 2026
@akaladarshi
akaladarshi marked this pull request as ready for review August 11, 2026 04:15
@akaladarshi

Copy link
Copy Markdown
Contributor Author

I am continuing the work on the testing and some changes, but It would be great to have a first review on this, so we can align on the required changes.

@FilOzzy FilOzzy moved this from 📌 Triage to 🔎 Awaiting Review in FilOz Aug 11, 2026
@FilOzzy FilOzzy moved this from In progress to In review in Solstice Aug 11, 2026
@BigLep BigLep removed this from FilOz Aug 19, 2026
@BigLep BigLep moved this from In review to In progress in Solstice Aug 19, 2026
Comment thread actors/miner/src/lib.rs
SectorOnChainInfoFlags::SIMPLE_QA_POWER | SectorOnChainInfoFlags::FULL_QA_POWER;

// Pledge for the upgraded power, never lowered below what is already held.
new_sector.initial_pledge = max(new_sector.initial_pledge, full_power_pledge.clone());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

needs a FULL_QA_POWER around this, don't ratchet it up for already-upgraded sectors; FIP says "raised at most once"

Comment thread actors/miner/src/lib.rs
Comment on lines +2609 to +2613
for decl in &params.extensions {
for sector_number in decl.sectors.iter() {
emit::sector_updated(rt, sector_number, None, &[])?;
}
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Two things here: 1) they're not emitted the deduped list so you could get duplicate events. But more importantly 2) this is a snap event that comes out of PRU3 and isn't appropriate for extensions. So for now let's remove events entirely because I want to have that discussion separately.

Comment thread actors/miner/src/lib.rs
sector: &SectorOnChainInfo,
sector_size: SectorSize,
circulating_supply: &TokenAmount,
full_qa_power: &StoragePower,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
full_qa_power: &StoragePower,

you only use this after you set the flags to make full QA be max QA, so it's not variable, it's just qa_power_max

@rvagg

rvagg commented Aug 21, 2026

Copy link
Copy Markdown
Member

@akaladarshi see items in #1763, I think mostly we've got resolution. Getting near-enough for now would be good and if there are outstanding details or tweaks we can do them on top of this later.
If you could rebase it on top of #1778 that would be helpful, there's a lot of changes in there but there shouldn't be many conflicts. But your tests will probably fail because of the switch from deal weight to verified weight as the signal-bearer, and I also merged the two extend_*_sector() methods into a single extend_sector_weights.
In my latest commit in #1763 I added a refactor that's waiting for you to dedupe the work you're doing here, the method replace_sector_records should have the common mechanisms that you're already doing in your traversal so you can just call it; rebase and then use that method and a bunch of your code can disappear.
Also another thing to consider: your upgrade_sector_to_full_power and the existing update_existing_sector_info are two copies of "grant full QA power", they need to agree per the FIP, it's fine to do the work separately but you could consider a small helper to do the common things: flag, pledge, fee. See if that cleans things up at all.

@akaladarshi
akaladarshi marked this pull request as draft August 21, 2026 16:26
@akaladarshi
akaladarshi changed the base branch from fix/fips1249-1744-review-fixes to rvagg/fip-0118-filplus-fixes August 21, 2026 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In progress

Development

Successfully merging this pull request may close these issues.

[Solstice] Provide pathway for existing sectors to upgrade to 10x QAP

6 participants