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
Trusted wallet peer enhancements #17872
base: main
Are you sure you want to change the base?
Trusted wallet peer enhancements #17872
Conversation
Is it possible to make the log warning only for trusted wallets? untrusted wallets from other users over the network might generate log spam. That being said it feels like that would be better placed in the wallets log instead of the full node log, yes |
@xearl4 mentioned that as well. If we're able to move it to the wallet logs with minimal code changes, I'd prefer that as well. I'll look into it later this week. |
I'd like to note that with CHIP-0026 once adopted, hitting the subscription limit will result in a rejection instead of silently failing. The wallet can then respond appropriately, such as subscribing via another node or warning the user. (Not saying this affects this PR really, just mentioning it since it's relevant) |
Thanks for bringing that up, that'll be a big improvement. I was looking into moving the warning to the wallet side, and concluded that it's not possible without protocol change. Happy to hear that CHIP-0026 will bring a related protocol change :) |
That sounds much better! I'll revert the log warning code. Would the trusted label still be of any value? Otherwise the PR can be closed. |
We don't yet use CHIP-0026 in the wallet , though it will be in the full node in the upcoming release if things go according to plan. So for now this might still be useful to have. |
Purpose:
Recently, the SpaceFamers pool encountered significant difficulties in maintaining wallet integrity. Over time, it gradually lost track of coins. The wallet's peer ID had been incorporated into the node configuration, rendering it trusted. However, due to an unknown cause, the incorrect peer ID was configured. It took us a considerable amount of time to identify the root of the issue.
This pull request aims to enhance the experience for farmers who also operate wallets with substantial coin holdings while being limited by the node without clear indication.
Current Behavior:
Neither the wallet nor the node notifies the user when limits have been reached. Additionally, the wallet is not marked as trusted in the
chia peer full_node -c
CLI command. Consequently, there is currently no method to verify if a trusted peer has been configured accurately.New Behavior:
This pull request indicates the peer as trusted, confirming the success of the configuration update. Additionally, it prompts a warning in the node log when subscription limits are exceeded. Perhaps there are better ways to warn the user (eg. in the wallet log), I'm looking forward to any suggestions.