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

feat: CLI: add claim-extend cli #11711

Merged
merged 13 commits into from
Mar 20, 2024
Merged

feat: CLI: add claim-extend cli #11711

merged 13 commits into from
Mar 20, 2024

Conversation

LexLuthr
Copy link
Contributor

@LexLuthr LexLuthr commented Mar 12, 2024

Related Issues

Fixes #10001

Proposed Changes

The new command creates extend message[s] based on the following conditions

  1. Extend all claims for a miner ID
  2. Extend all claims for multiple miner IDs
  3. Extend specified claims for a miner ID
  4. Extend specific claims for specific miner ID
  5. Extend all claims for a miner ID with different client address (2 messages)
  6. Extend all claims for multiple miner IDs with different client address (2 messages)
  7. Extend specified claims for a miner ID with different client address (2 messages)
  8. Extend specific claims for specific miner ID with different client address (2 messages)

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
  • Tests exist for new functionality or change in behavior
  • CI is green

cli/filplus.go Outdated Show resolved Hide resolved
cli/filplus.go Outdated Show resolved Hide resolved
@rvagg
Copy link
Member

rvagg commented Mar 13, 2024

maybe hold off until we make it work: filecoin-project/go-state-types#254

cli/filplus.go Outdated Show resolved Hide resolved
cli/filplus.go Outdated Show resolved Hide resolved
cli/filplus.go Outdated Show resolved Hide resolved
@Stebalien Stebalien requested review from rvagg and removed request for masih, Stebalien, aarshkshah1992 and magik6k March 14, 2024 20:51
@rvagg
Copy link
Member

rvagg commented Mar 18, 2024

merged types changes in filecoin-project/go-state-types#254, @rjan90 I think probably a new rc from go-state-types that we can pull in to master and release/v1.26.0?

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.

This looks really nice, if it works as advertised. I like that it's asking for confirmation when needing to spend datacap.

Did you have a draft itest somewhere for this when you found that encoding discrepancy originally on ClaimExtensionRequest? I know this is really hard to test, but perhaps there's something minimal we could do at least?

@LexLuthr
Copy link
Contributor Author

I was testing e2e in devnet but an itest sounds good. I can create one which quickly creates a claim and then extends it.

@LexLuthr LexLuthr requested a review from rvagg March 19, 2024 16:07
@LexLuthr LexLuthr enabled auto-merge (squash) March 20, 2024 09:23
@LexLuthr LexLuthr merged commit 02a8848 into master Mar 20, 2024
88 of 92 checks passed
@LexLuthr LexLuthr deleted the feat/claim-extend-cli branch March 20, 2024 09:34
@beck-8
Copy link
Contributor

beck-8 commented Mar 21, 2024

Hello, I took the time to look at it today.

I found that there is no out of gas processing, I don't know if I missed it.

This message is the same as the renewal sector. When the number is large, a large amount of gas will be used, exceeding the block limit, so an error will be reported when renewing a large number of deals.

So please confirm whether there is batching logic. If not, you may need to add it.

ps: When I was renewing the deal, I found that it was already out of gas around 2000-3000

@LexLuthr
Copy link
Contributor Author

@beck-8 You are right. There is no batching logic. I will add it to #11764 with another fix.

LexLuthr added a commit that referenced this pull request Mar 26, 2024
* add claim-extend cli

* fix arg usage

* add missing question

* fix client addr, datacap prompt

* replace waitGrp with errGrp

* use promptUI

* replace fmt.ErrorF with xerror

* apply var name suggestion

* GST rc3, update types

* add itest

* make gen

* add changelog
ribasushi pushed a commit to ribasushi/ci-abusing-lotus-fork that referenced this pull request May 11, 2024
* add claim-extend cli

* fix arg usage

* add missing question

* fix client addr, datacap prompt

* replace waitGrp with errGrp

* use promptUI

* replace fmt.ErrorF with xerror

* apply var name suggestion

* GST rc3, update types

* add itest

* make gen

* add changelog
ribasushi pushed a commit to ribasushi/ci-abusing-lotus-fork that referenced this pull request May 11, 2024
* add claim-extend cli

* fix arg usage

* add missing question

* fix client addr, datacap prompt

* replace waitGrp with errGrp

* use promptUI

* replace fmt.ErrorF with xerror

* apply var name suggestion

* GST rc3, update types

* add itest

* make gen

* add changelog
ribasushi pushed a commit to ribasushi/ci-abusing-lotus-fork that referenced this pull request May 11, 2024
* add claim-extend cli

* fix arg usage

* add missing question

* fix client addr, datacap prompt

* replace waitGrp with errGrp

* use promptUI

* replace fmt.ErrorF with xerror

* apply var name suggestion

* GST rc3, update types

* add itest

* make gen
ribasushi pushed a commit to ribasushi/ci-abusing-lotus-fork that referenced this pull request May 16, 2024
* add claim-extend cli

* fix arg usage

* add missing question

* fix client addr, datacap prompt

* replace waitGrp with errGrp

* use promptUI

* replace fmt.ErrorF with xerror

* apply var name suggestion

* GST rc3, update types

* add itest

* make gen
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ☑️ Done (Archive)
Development

Successfully merging this pull request may close these issues.

add method support Allocation: DataCap allocated by a client to a specific piece of data.
4 participants