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: SPTool #11788

Merged
merged 14 commits into from
Apr 1, 2024
Merged

feat: SPTool #11788

merged 14 commits into from
Apr 1, 2024

Conversation

snadrus
Copy link
Collaborator

@snadrus snadrus commented Mar 26, 2024

This PR starts implementing #11733

Proposed Changes
Setup the initial cmd structure
Small abstraction for sharing of command code between SPTool and lotus-miner
Bring over lotus-miner actor withdraw into the shared package
Additional Info
Not sure if we want to do this as one bigly PR bringing everything over at once, or if we want to land the almost empty binary first, then start bringing over more and more commands in smaller PRs.

I'm slightly leaning towards more smaller PRs, so this PR is ready for review as-is

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

@snadrus snadrus requested review from magik6k and LexLuthr March 26, 2024 21:08
@snadrus
Copy link
Collaborator Author

snadrus commented Mar 26, 2024

I remade this so @magik6k can review it.
@LexLuthr already fixed the lint issue in his guided-setup for new setups PR.

@magik6k magik6k changed the title Feat/sptool feat: SPTool scaffolding Mar 27, 2024
Copy link
Contributor

@LexLuthr LexLuthr left a comment

Choose a reason for hiding this comment

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

I got a few funky output from the sptool commands.

lexluthr@Lexs-Mac lotus % FULLNODE_API_INFO=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJBbGxvdyI6WyJyZWFkIiwid3JpdGUiLCJzaWduIiwiYWRtaW4iXX0.z0DhOtSBB05FArWJCPfqqYfS4zdSYT_n39bKa-6FoGY:/dns/localhost/tcp/1234/http ./sptool --actor t01000 actor control list
2024-03-27T16:17:33.145+0400	ERROR	sptool	sptool/main.go:71	stat /Users/lexluthr/.lotus-miner-local-net: no such file or directory
lexluthr@Lexs-Mac lotus % FULLNODE_API_INFO=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJBbGxvdyI6WyJyZWFkIiwid3JpdGUiLCJzaWduIiwiYWRtaW4iXX0.z0DhOtSBB05FArWJCPfqqYfS4zdSYT_n39bKa-6FoGY:/dns/localhost/tcp/1234/http ./sptool --actor t01000 actor compact-allocated --really-do-it
2024-03-27T16:23:07.431+0400	ERROR	sptool	sptool/main.go:71	must pass address of new owner address

Requires double --actor flag

lexluthr@Lexs-Mac lotus % FULLNODE_API_INFO=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJBbGxvdyI6WyJyZWFkIiwid3JpdGUiLCJzaWduIiwiYWRtaW4iXX0.z0DhOtSBB05FArWJCPfqqYfS4zdSYT_n39bKa-6FoGY:/dns/localhost/tcp/1234/http ./sptool --actor t01000 sectors compact-partitions --really-do-it --deadline 0 --partitions 0
2024-03-27T16:27:23.277+0400	ERROR	sptool	sptool/main.go:71	failed to load miner actor: GetActor called on undefined address

I am still testing some lotus commands and sptool.

cli/spcli/actor.go Outdated Show resolved Hide resolved
cli/spcli/actor.go Outdated Show resolved Hide resolved
cli/spcli/sectors.go Outdated Show resolved Hide resolved
cli/spcli/sectors.go Outdated Show resolved Hide resolved
cli/spcli/sectors.go Outdated Show resolved Hide resolved
cmd/lotus-miner/actor.go Outdated Show resolved Hide resolved
cmd/sptool/actor.go Outdated Show resolved Hide resolved
@magik6k magik6k changed the title feat: SPTool scaffolding feat: SPTool Mar 27, 2024
Copy link
Contributor

@magik6k magik6k left a comment

Choose a reason for hiding this comment

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

Not much to add over the review from Lex, CI needs to be made a little more green, but really this looks already almost done

cli/spcli/actor.go Outdated Show resolved Hide resolved
cli/spcli/sectors.go Show resolved Hide resolved
@snadrus
Copy link
Collaborator Author

snadrus commented Mar 28, 2024

@magik6k Tests are clean (except the typical chain flakes).

Copy link
Contributor

@magik6k magik6k left a comment

Choose a reason for hiding this comment

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

Looks good, would be good to go through each command on both lotus-miner and sptool just to make sure there are no obvious regressions.

@magik6k
Copy link
Contributor

magik6k commented Mar 30, 2024

Pushed a small fix to the sectors list command fixing the deal weight math

@snadrus snadrus merged commit b95e95f into master Apr 1, 2024
156 of 184 checks passed
@snadrus snadrus deleted the feat/sptool branch April 1, 2024 15:30
Nagaprasadvr pushed a commit to Nagaprasadvr/lotus that referenced this pull request Apr 4, 2024
* sptool: Initial structure

* sptool: Port lotus-miner actor withdraw

* sptool: Make cli docsgen happy

* actors are done

* info

* proving

* sptool the rest

* fixed gitignore

* lints

* oops

* 2

* terminate

* fixes

* sptool: improve sectors list

---------

Co-authored-by: Łukasz Magiera <magik6k@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants