-
Notifications
You must be signed in to change notification settings - Fork 178
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
Remove CLI option --restart from tumbler #1546
base: master
Are you sure you want to change the base?
Conversation
Implemented the most obvious test on regtest; ran the tumbler and Ctrl-C after a few transactions. Then ran again and the process completed successfully. |
Prior to this commit, the --restart option was still present in the help documentation of tumbler.py, however this functionality has been removed from the algorithm (restarting now just means "running again", since the algorithm now functions the same way independent of the location of the coins in mixdepths). Moreover, running with this option could lead to crash conditions. After this commit, the --restart option is removed from the help text, and no longer executed in code. Further, a slight change is made to the wording of the tumblerguide.md documentation, section "Restarting", to make the situation clearer.
5e325e1
to
0ea722d
Compare
Following from the telegram discussion. As we discussed, there are two obvious ways to do a "restart":
I was under the impression that both ways are supported by JM for "free". Meaning, as said, 1. by definition, and 2. because it's a schedule like any other. Am I mistaken? BTW, I initially thought the restart option was pointless because of option 2., i.e., by being already implicit in the schedule file, we do not need a special case for it. Assuming what I said above is the case, I think we should mention in the |
I can definitely see that. You correctly perceived my intention in going the schedule route, and I'm glad that it works. As you point out, it's still possible to footgun, in either approach.
Yes after this and the preceding discussions, it's now very clear that the restarting section of the doc needs to be substantially improved. I guess I'll try to do that and update the doc shortly. We should be clear in this discussion though, that you're advocating for I was quite enthusiastic about originally creating the schedule file approach for this reason (flexibility, especially for power users), and while it had some meaningful limitations (see #429 , not quite what I think, but what I do totally understand is that even if users can edit such a file, it doesn't mean that most of them will want to, or feel comfortable doing it), it's good that it exists.
Yes. Note the tumblerguide doc does currently address this quite a bit by showing examples and referring to it. Feel free to suggest further additions to the doc to make that clearer. I think we're in vague agreement. |
Yes. I guess
Absolutely, I think it's great to have it. I think labels already would go a long way. As I see it, this is a case where the schedule looks much more complicated/intimidating than it really is. I mean, it's literally just a few rows of a few values each, and most of these values (if not all really) are trivial to understand (and a taker should already be familiar with them). I'm fairly confident our users can handle it no problem.
Sorry, the point I was trying to make was referred to what you said on telegram about what's the easiest/safest approach for user. So I was trying to say that I don't mind telling user "hey, if you want to restart the tumbler you can use the schedule" and that I don't think we should spend too much time hiding the schedule from the users in an attempt to make things easier. |
Linking some closely related discussion in #1352, in particular #1352 (comment) |
Needs rebase. |
Prior to this commit, the --restart option was still present in the help documentation of tumbler.py, however this functionality has been removed from the algorithm (restarting now just means "running again", since the algorithm now functions the same way independent of the location of the coins in mixdepths). Moreover, running with this option could lead to crash conditions.
After this commit, the --restart option is removed from the help text, and no longer executed in code. Further, a slight change is made to the wording of the tumblerguide.md documentation, section "Restarting", to make the situation clearer.