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

Check observer addresses in farm rewards dialog #11259

Merged
merged 1 commit into from
Apr 21, 2022
Merged

Conversation

emlowe
Copy link
Contributor

@emlowe emlowe commented Apr 21, 2022

The GUI farmer rewards check was not checking for observer keys and the code was duplicated from farmer and wallet, which is one reason this was missed.

Consolidated some code into new function called by both the wallet and farmer and added tests for observer keys.

Also added the ability to pass in the number of derivations to the RPC call - primarily this was so the tests can pass in much smaller numbers than the default (500) and so the tests can run much faster. However, this flexibility seems worth while regardless

(new PR because git hates me, but it is mutual)

Fixes #11036

@emlowe emlowe requested review from xdustinface and paninaro April 21, 2022 21:47
Copy link
Contributor

@paninaro paninaro left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor

@mariano54 mariano54 left a comment

Choose a reason for hiding this comment

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

Thanks for doing this, it's a good cleanup.
The performance of this can also be optimized significantly, by doing the same thing as init funcs and keeping the intermediate values:
https://github.com/Chia-Network/chia-blockchain/blob/main/chia/cmds/init_funcs.py#L96

master_sk_to_wallet_sk repeats a lot of work (3/4 of the derivations are being done over and over)

@emlowe emlowe added the ready_to_merge Submitter and reviewers think this is ready label Apr 21, 2022
@wjblanke wjblanke merged commit cb98258 into main Apr 21, 2022
@wjblanke wjblanke deleted the EL.farm_rewards branch April 21, 2022 23:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready_to_merge Submitter and reviewers think this is ready
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Farming rewards dialog incorrectly claims there is no private key for address
4 participants