fix(cli): make lotus-miner sectors extend command resilient to higher gas #11928

beck-8 · 2024-04-25T07:12:01Z

Related Issues

#11927

Proposed Changes

Additional Info

Checklist

Before you mark the PR ready for review, please make sure that:

Commits have a clear commit message.
PR title is in the form of of <PR type>: <area>: <change being made>
- example: fix: mempool: Introduce a cache for valid signatures
- PR type: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, test
- area, e.g. api, chain, state, market, mempool, multisig, networking, paych, proving, sealing, wallet, deps
If the PR affects users (e.g., new feature, bug fix, system requirements change), update the CHANGELOG.md and add details to the UNRELEASED section.
New features have usage guidelines and / or documentation updates in
- Lotus Documentation
- Discussion Tutorials
Tests exist for new functionality or change in behavior
CI is green

beck-8

This is still incorrect.

rjan90 · 2024-04-27T09:11:52Z

Thank you so much for the PR. I think this PR just needs a make gen (run the command while in your /lotus folder), to pass the final check.

LexLuthr

Mostly looks good with some questions

cli/spcli/sectors.go

BigLep · 2025-02-10T16:21:01Z

2025-02-10 triage conversation: Venus is going to pick this up to get it over the line.

beck-8 · 2025-06-30T15:08:10Z

Is there any news here?

BigLep · 2025-06-30T16:15:08Z

@beck-8 : thanks for the ping.

@tediou5 : has @filecoin-project/lotus-miner-maintainers looked at this?

tediou5

@beck-8 Thanks a lot for your PR; it looks good to me. However, you might need to resolve the conflicts first.

Sorry I overlooked it earlier. I’ll push it forward and get it merged ASAP.

cli/spcli/sectors.go

tediou5

Thank you so much for the PR!

Sorry this took so long. I’ll wait a bit in case others are interested, but it should be merged soon. Thanks again for your contribution!

beck-8 · 2025-07-01T03:40:11Z

Don't worry, the careful review is what we want to see.

tediou5 · 2025-07-01T03:51:04Z

@beck-8 By the way, with FIP-0100 implemented and the batch balancer removed, we probably don’t need to worry about that earlier issue anymore.

beck-8 · 2025-07-01T03:53:45Z

@beck-8 By the way, with FIP-0100 implemented and the batch balancer removed, we probably don’t need to worry about that earlier issue anymore.

out of gas ?
Is this part certain? FIP100 only reduces the gas proportion of some users, but the block limit of the blockchain does not change. I understand that if I have too many sectors in one batch, there will still be a limit.

tediou5 · 2025-07-01T04:44:59Z

but the block limit of the blockchain does not change. I understand that if I have too many sectors in one batch, there will still be a limit.

Yes, the block limit still exists, but since messages now contain only the basic gas and the rest has been shifted to a daily fee, a batch message is now almost impossible to hit the block limit.

I can go check the data to confirm. That said, I don’t see a problem with this PR; having it configurable is always good. We might just set a more reasonable default value.

tediou5

Alright, my bad. I checked again, and while it’s lower, it’s still quite high. Setting it to 500 seems fairly reasonable.

Here are something that could be improved, but I don’t think we need to handle it in this PR.

cli/spcli/sectors.go

tediou5

Very nice, thank you.

CHANGELOG.md

Co-authored-by: Rod Vagg <rod@vagg.org>

tediou5 · 2025-07-02T03:38:56Z

/usr/bin/ld: warning: 81a71fbc30f7fcce-sha256_x64.o: missing .note.GNU-stack section implies executable stack
/usr/bin/ld: NOTE: This behaviour is deprecated and will be removed in a future version of the linker

CI seems to be having issues, I’ve run into this a few times.

rvagg · 2025-07-02T04:31:39Z

I’ve run into this a few times

this has been my life for a while now .. Go compiler thing on some Linux variants; I'm blind to it these days but maybe I shouldn't be!

eddieperez88 · 2025-07-08T08:47:36Z

CHANGELOG.md

@@ -31,6 +31,7 @@
  - default to ProveCommit aggregation
  - remove config options: AggregateCommits, AggregateAboveBaseFee, BatchPreCommitAboveBaseFee
 - feat(paych): add EnablePaymentChannelManager config option and disable payment channel manager by default ([filecoin-project/lotus#13139](https://github.com/filecoin-project/lotus/pull/13139))


eddieperez88 · 2025-07-08T08:55:56Z

I can go check the data to confirm but looks great

beck-8 commented Apr 25, 2024

View reviewed changes

beck-8 force-pushed the fix/sectors_extend branch 2 times, most recently from 3c171e7 to 12e188c Compare April 25, 2024 08:04

rjan90 changed the title ~~fix sectors extend~~ fix: cli: lotus-miner sectors extend command Apr 27, 2024

jennijuju requested a review from magik6k May 28, 2024 08:47

LexLuthr approved these changes Jun 3, 2024

View reviewed changes

cli/spcli/sectors.go Outdated Show resolved Hide resolved

beck-8 force-pushed the fix/sectors_extend branch from 932765e to 7245483 Compare June 3, 2024 11:23

LexLuthr reviewed Jun 3, 2024

View reviewed changes

cli/spcli/sectors.go Outdated Show resolved Hide resolved

beck-8 force-pushed the fix/sectors_extend branch from 7245483 to 5349448 Compare June 3, 2024 13:36

rjan90 added the area/miner-maintenance label Feb 10, 2025

BigLep moved this from 🐱 Todo to 👊 Needs Commitment in FilOz Mar 17, 2025

BigLep requested review from tediou5 and removed request for magik6k June 30, 2025 16:15

tediou5 approved these changes Jul 1, 2025

View reviewed changes

github-project-automation bot moved this from 👊 Needs Commitment to ✔️ Approved by reviewer in FilOz Jul 1, 2025

beck-8 force-pushed the fix/sectors_extend branch from 5349448 to 75bdce0 Compare July 1, 2025 03:03

tediou5 reviewed Jul 1, 2025

View reviewed changes

cli/spcli/sectors.go Outdated Show resolved Hide resolved

beck-8 force-pushed the fix/sectors_extend branch from 75bdce0 to b4cf0ad Compare July 1, 2025 03:29

tediou5 approved these changes Jul 1, 2025

View reviewed changes

cli/spcli/sectors.go Outdated Show resolved Hide resolved

fix sectors extend

f44ff17

beck-8 force-pushed the fix/sectors_extend branch from b4cf0ad to f44ff17 Compare July 1, 2025 08:00

tediou5 approved these changes Jul 1, 2025

View reviewed changes

rvagg changed the title ~~fix: cli: lotus-miner sectors extend command~~ fix(cli): make lotus-miner sectors extend command resilient to higher gas Jul 2, 2025

rvagg reviewed Jul 2, 2025

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

Update CHANGELOG.md

60d49a8

Co-authored-by: Rod Vagg <rod@vagg.org>

rvagg approved these changes Jul 2, 2025

View reviewed changes

rvagg merged commit 662f8e2 into filecoin-project:master Jul 2, 2025
173 of 174 checks passed

github-project-automation bot moved this from ✔️ Approved by reviewer to 🎉 Done in FilOz Jul 2, 2025

beck-8 deleted the fix/sectors_extend branch July 2, 2025 04:33

rjan90 mentioned this pull request Jul 8, 2025

lotus-miner sectors extend unable to renew DC sectors #11927

Closed

11 tasks

eddieperez88 reviewed Jul 8, 2025

View reviewed changes

fix(cli): make lotus-miner sectors extend command resilient to higher gas #11928

fix(cli): make lotus-miner sectors extend command resilient to higher gas #11928

Uh oh!

Conversation

beck-8 commented Apr 25, 2024 • edited by rjan90 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Related Issues

Proposed Changes

Additional Info

Checklist

Uh oh!

beck-8 left a comment

Choose a reason for hiding this comment

Uh oh!

rjan90 commented Apr 27, 2024

Uh oh!

LexLuthr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

BigLep commented Feb 10, 2025

Uh oh!

beck-8 commented Jun 30, 2025

Uh oh!

BigLep commented Jun 30, 2025

Uh oh!

tediou5 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tediou5 left a comment

Choose a reason for hiding this comment

Uh oh!

beck-8 commented Jul 1, 2025

Uh oh!

tediou5 commented Jul 1, 2025

Uh oh!

beck-8 commented Jul 1, 2025

Uh oh!

tediou5 commented Jul 1, 2025

Uh oh!

tediou5 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tediou5 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tediou5 commented Jul 2, 2025

Uh oh!

rvagg commented Jul 2, 2025

Uh oh!

Uh oh!

eddieperez88 Jul 8, 2025

Choose a reason for hiding this comment

Uh oh!

eddieperez88 commented Jul 8, 2025

Uh oh!

Uh oh!

beck-8 commented Apr 25, 2024 •

edited by rjan90

Loading