chia-blockchain
chia-blockchain copied to clipboard
Trusted wallet peer enhancements
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.
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
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)
I'd like to note that with CHIP-0026 once adopted, hitting the subscription limit will result in a rejection instead of silently failing.
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 :)
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)
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.
I do think the trusted is a reasonable addition if you want to dust this off
This PR has been flagged as stale due to no activity for over 60 days. It will not be automatically closed, but it has been given a stale-pr label and should be manually reviewed by the relevant parties.
Coverage exemption