Skip to content

feat: add settle-deal to lotus-shed #13189

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

beck-8
Copy link
Contributor

@beck-8 beck-8 commented Jun 30, 2025

Related Issues

https://filecoinproject.slack.com/archives/C03CKDLEWG1/p1751257247291959?thread_ts=1745286086.505189&cid=C03CKDLEWG1

Proposed Changes

Additional Info

Checklist

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

@github-project-automation github-project-automation bot moved this to 📌 Triage in FilOz Jun 30, 2025
@BigLep
Copy link
Member

BigLep commented Jul 1, 2025

@beck-8 : looks like there's a small lint error to fix.

@BigLep BigLep requested a review from rvagg July 1, 2025 06:19
@BigLep BigLep moved this from 📌 Triage to 🔎 Awaiting Review in FilOz Jul 1, 2025
Copy link
Member

@rvagg rvagg left a comment

Choose a reason for hiding this comment

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

there's too much duplicated code in here from cli/spcli/actor.go, we need to reduce the overlap because they're doing almost identical things so we can share the code even if we have it in two difference places.

I'm assuming that the intention here is that you'd like to have it available as a lotus-shed and not need to have the full lotus-miner setup available and you shouldn't be tied to a specific miner address? That's fine, but we need to dedupe.

@github-project-automation github-project-automation bot moved this from 🔎 Awaiting Review to ⌨️ In Progress in FilOz Jul 1, 2025
@beck-8
Copy link
Contributor Author

beck-8 commented Jul 1, 2025

I originally wanted to change it on the original foundation (lotus-miner). I don't think anyone will need to check the details of the message. Now everyone who uses this tool urgently wants to take out the locked amount.

However, considering that someone may really ask to check the payment, I changed the logic to lotus-shed. This is my idea. Do you have any suggestions?

@beck-8
Copy link
Contributor Author

beck-8 commented Jul 1, 2025

Or should I ask if we really need this part of logic? Or put this small part elsewhere in the future?

lotus/cli/spcli/actor.go

Lines 141 to 171 in cfb4c8e

// wait for it to get mined into a block
wait, err := api.StateWaitMsg(ctx, res, uint64(cctx.Int("confidence")), lapi.LookbackNoLimit, true)
if err != nil {
return xerrors.Errorf("Timeout waiting for deal settlement message %s", res)
}
if wait.Receipt.ExitCode.IsError() {
return xerrors.Errorf("Failed to execute withdrawal message %s: %w", wait.Message, wait.Receipt.ExitCode.Error())
}
var settlementReturn markettypes14.SettleDealPaymentsReturn
if err = settlementReturn.UnmarshalCBOR(bytes.NewReader(wait.Receipt.Return)); err != nil {
return err
}
fmt.Printf("Settled %d out of %d deals\n", settlementReturn.Results.SuccessCount, len(dealIDs))
var (
totalPayment = big.Zero()
totalCompletedDeals = 0
)
for _, s := range settlementReturn.Settlements {
totalPayment = big.Add(totalPayment, s.Payment)
if s.Completed {
totalCompletedDeals++
}
}
fmt.Printf("Total payment: %s\n", types.FIL(totalPayment))
fmt.Printf("Total number of deals finished their lifetime: %d\n", totalCompletedDeals)

@rvagg
Copy link
Member

rvagg commented Jul 2, 2025

Actually @beck-8, hold off from putting in all that work. I think maybe you should just explain, both briefly in the CHANGELOG and a little more verbose in the Description of the new command what it does and why it's useful instead of the lotus-miner variant. I think maybe you've got a very narrow use-case here.

The alternative is to just go ahead and adapt your use-case to the lotus-miner one, make one command but make it do what you want to do on top of its existing flexibility -- or make a judgement about whether its existing flexibility is even very useful. I see complaints about it, if you just want to fix it then we can get additional SP feedback before doing so but we shouldn't feel like these things are not open to changing.

@beck-8
Copy link
Contributor Author

beck-8 commented Jul 2, 2025

In fact, no matter which one, SP will still complain, because StateMarketDeals is as high as 29G, and the premise of executing settlement is to wait for a few minutes to pull this data.

@beck-8
Copy link
Contributor Author

beck-8 commented Jul 2, 2025

I will give more explanations later. I know that the use case is relatively narrow, but I have no other good way to help SP. Some experienced people will use lotus-miner sectors status to extract deal, but it only applies to certain environments of some people.

As we discussed at that time, I put forward my ideas and cases. But you don't have to merge this PR. You can close it and restructure it, but you should know my idea.

@beck-8 beck-8 marked this pull request as draft July 10, 2025 02:07
@beck-8
Copy link
Contributor Author

beck-8 commented Jul 10, 2025

  1. Quickly obtain the relationship between sectors and deals
  2. Reasonably handle batches and send messages.

I suggest exporting this method or transferring it to a reasonable place, and then GetMinerAllDeals -> GetMarketDealIDs
https://github.com/sumitvekariya/lotus/blob/7f2a7b7179dd75b5e379ac78e2af1f9caca04d58/cli/state.go#L1752

@BigLep
Copy link
Member

BigLep commented Jul 10, 2025

My 2025-07-10 understanding of where this PR is at:

  • Related conversation: https://filecoinproject.slack.com/archives/C03CKDLEWG1/p1745286086505189
  • @beck-8's core goal is to make this API faster. It's currently too slow because it has to load and transfer all the deals.
  • Maintainers want to avoid a duplicated lotus-shed settle-deal when there’s a lotus-miner settle-deal
  • @beck-8 and @tediou5 will work to land this so it has a faster API and that it's leveraged in lotus-miner settle-deal
  • For now while it's still being worked on, it's in draft.

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.

3 participants