Network activity toggle #5314
Network activity toggle #5314
Conversation
Added the function SetNetworkActive() which when called with argument set to false disconnects all nodes and sets the flag fNetworkActive to false. As long as this flag is false no new connections are attempted and no incoming connections are accepted. Network activity is reenabled by calling the function with argument true.
RPC command "togglenetwork" toggles network and returns new state after command. RPC command "getinfo" returns "networkactive" field in output.
Add getNetworkActive()/setNetworkActive() method to client model. Send network active status through NotifyNetworkActiveChanged. Indicate in tool tip of gui status bar network indicator whether network activity is disabled. Indicate in debug window whether network activity is disabled and add button to allow user to toggle network activity state.
- rename rpc command "togglenetwork" to "setnetworkactive (true|false)" - add simple testcase - GUI toggle added to connections icon in statusbar
|
Can you write up a proposed testing procedure? |
|
I started a very simple unit-test case in this pull (here). In general |
|
Any opinion on that? |
|
I always liked that, didn't look at the pull lately... |
|
I'm still too focused on the 0.10 release to even think about what should make it into 0.11, which is half a year away. I'd prefer traffic throttling or "quotas per time unit" a la Tor (see https://www.torproject.org/docs/faq.html.en#LimitTotalBandwidth) to a manual, complete shutoff. But this may be better than nothing in that regard, and much easier to test. |
|
@laanwj no worries. You should/can focus on 0.10. IMO: Throttling is another thing and can address other user scenarios. Disabling != throttling. |
|
I'd like to see a host network friendlyness features in upcoming versions. This should be one them. There are some corner cases to contemplate here. Say a transaction has one confirmation and you network off a host and the chain reorgs and drops the transaction. The wallet will be giving incorret information until the network is turned back on. So, should such a patch be disabling confirmation counts on newly confirmed transactions? |
|
Disabling the confirmation count during disabled network state would be possible. Adding code to handle the toggled off state in case of UI, etc. could bring in more effort when it comes to maintainability. IMO, let's just allow users cut themselves off the nodes, brutal and with all known sudden-disconnect issues. Or an option would be: |
There was a problem hiding this comment.
Can we find an icon that doesn't require a new license?
There was a problem hiding this comment.
In fact, NACK merging with this license. Section 4(c) seems to give the author special privileges that are non-free and could conceivably be problematic.
There was a problem hiding this comment.
@jonasschnelli Can you make a new one that fits better with #5219 ? :)
There was a problem hiding this comment.
Yes. The icon should not be the problem.
Will do a new one soon.
|
The "toggle" rpc is a bad interface. It depends on the caller knowing the current state, and even if the caller checks it could race with another caller. Can we make the api/interface so that it can have more than two states? A useful middle ground is "make outbound connections every X hours, fetch blocks, go back offline". This ensures you won't be waiting a long time when you go to actually use it. So, e.g. off/scheduled/on I think it's likely that when we add support for bandwidth rate limiting we'll add code to parse cron style schedules.
Our responsiblity to the user is to help them not footgun themselvels. If they understood the software and risks as well as we do, they've be writing it, not us. :) That doesn't mean it's reasonable to pack every potential protection we can think of into every pull. But if we have something that increases risk to the user we ought to give some thought as to how we can mitigate that risk.
I think overall we should have more affordances for risky situations. E.g. making it clear that confirmations are uncertian when we know the network has forked. Presumably all these modes of cautionary handling can be covered by a single piece of interface infrastrcture. Some hurestic of "if you're off line N minutes, then blocks N*2 minutes prior to when you went off now have a displayed confirm count of '?'" would be simple to add if there was already code that hid confirmation counts under other dangerous condtions. |
|
one thing i see is that this is a good first step for the issue with the in-wallet paper wallet generator |
|
There is no consensus about this. Closing for now. |
|
@gmaxwell: I still think a pause/resume feature could be useful.
|
|
I agree with @jonasschnelli in supporting this PR. For example, some have data plans that have limited day useage with unlimited night use, in such cases (very common around here) one would appreciate the option to pause access in the morning and resume after hours. Closing the app and restarting is always a scary thing because if it gets corrupted, you have to start from scratch. Also this ties in well with the pull request to add an internal paper wallet printer, another PR that i think would make a great addition to the wallet. |
6 participants
brings back @jonls #2412.
Rebased and overhauled: