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

Added get_keys_for_plot daemon server API #15772

Merged
merged 8 commits into from
Jul 18, 2023
Merged

Added get_keys_for_plot daemon server API #15772

merged 8 commits into from
Jul 18, 2023

Conversation

ChiaMineJP
Copy link
Contributor

This PR is a partial PR cut from cmj.plot2_0 branch.

@ChiaMineJP ChiaMineJP added the Changed Required label for PR that categorizes merge commit message as "Changed" for changelog label Jul 14, 2023
@ChiaMineJP ChiaMineJP requested a review from a team as a code owner July 14, 2023 15:08
@ChiaMineJP ChiaMineJP self-assigned this Jul 14, 2023
Copy link
Contributor

@arvidn arvidn left a comment

Choose a reason for hiding this comment

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

looks reasonable. do you think it's realistic to have tests? I don't think we have any infrastructure for it

chia/daemon/server.py Show resolved Hide resolved
@coveralls-official
Copy link

coveralls-official bot commented Jul 16, 2023

Pull Request Test Coverage Report for Build 5586985052

  • 82 of 82 (100.0%) changed or added relevant lines in 3 files are covered.
  • 1192 unchanged lines in 40 files lost coverage.
  • Overall coverage increased (+0.2%) to 87.65%

Files with Coverage Reduction New Missed Lines %
chia/cmds/chia.py 1 80.81%
chia/full_node/bundle_tools.py 1 98.59%
tests/core/test_full_node_rpc.py 1 99.69%
tests/build-job-matrix.py 2 89.9%
tests/wallet/db_wallet/test_dl_offers.py 2 99.19%
chia/util/errors.py 3 98.76%
chia/wallet/wallet_node.py 3 86.43%
chia/wallet/wallet_state_manager.py 3 95.23%
tests/wallet/cat_wallet/test_trades.py 3 99.36%
chia/wallet/util/puzzle_compression.py 4 92.98%
Totals Coverage Status
Change from base Build 5551480313: 0.2%
Covered Lines: 80725
Relevant Lines: 91988

💛 - Coveralls

tests/core/daemon/test_daemon.py Outdated Show resolved Hide resolved
tests/core/daemon/test_daemon.py Outdated Show resolved Hide resolved
@ChiaMineJP ChiaMineJP requested a review from arvidn July 17, 2023 09:13
@emlowe
Copy link
Contributor

emlowe commented Jul 17, 2023

This seems fine, but I don't like the name that much get_keys_for_plot would suggest to me that I provide a PLOT as a parameter to the call and get back the keys used for the plot.

However, I believe this call is returning the farmer and pool public keys for the given fingerprint. So it returns the public keys one would use for plotting (it is the same farmer and pool pk as chia keys show)

Some alternative names:
get_plotting_public_keys
get_plotting_pks
get_farmer_pool_public_keys
get_farmer_pool_pks
get_pks_for_plotting
get_keys_for_plotting

@ChiaMineJP
Copy link
Contributor Author

@emlowe I picked up get_keys_for_plotting.

@ChiaMineJP ChiaMineJP requested a review from emlowe July 18, 2023 11:45
@wallentx wallentx merged commit dd1fbeb into main Jul 18, 2023
196 checks passed
@wallentx wallentx deleted the cmj.plot2-6 branch July 18, 2023 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changed Required label for PR that categorizes merge commit message as "Changed" for changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants