-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
make lotus-miner net
commands hit markets subsystem.
#7042
Conversation
lotus-miner net
commands hit markets subsystem.
Before $ lotus-miner net peers
ERROR: method not supported After $ lotus-miner net peers
12D3KooW..., [/ip4/1.2.3.4/tcp/1]
12D3KooW..., [/ip4/2.3.4.5/tcp/2]
12D3KooW..., [/ip4/3.4.5.6/tcp/3]
12D3KooW..., [/ip4/4.5.6.7/tcp/4] Note how I don't need to use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We also need to add NetCmd
to cmd/lotus
, otherwise we'll lose net
commands on the chain process
Nice! thanks for this - would be really nice to get in v1.11.1. |
The libp2p host on the miner is now only started where the markets subsystem runs. This PR makes
lotus-miner net
commands hit the node where the markets subsystem is running.Honestly,
Net
no longer belongs in theCommon
commands, nor should the endpoint be resolved throughGetCommonAPI
/GetAPI
, but there is some larger untangling to do to tidy this up, and it's associated with pre-existing tech debt.Choosing not to do it now, as there are other decisions to make first with regards to providing the optimal UX experience for miners, now and as we introduce more miner services.
Testing: unfortunately we don't have a decent framework/suite for CLI testing. All itests hit the JSON-RPC API or the native API. So I can't add a test for this. Instead, I performed manual testing and all looks good.