-
Notifications
You must be signed in to change notification settings - Fork 27
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
Redistribute wallet into 100 outputs #1829
base: master
Are you sure you want to change the base?
Conversation
a515583
to
cd19e70
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should improve things but first we might need to merge SiaFoundation/coreutils#170 since >10 is the magic threshold where we currently have a chance of producing invalid transaction sets.
d367e19
to
7135f70
Compare
} | ||
|
||
// skip maintenance if wallet balance is too low | ||
if numOutputs < 10 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(nit) do you want to update the warning now that we check for outputs instead of balance?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's fine?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition is numOutputs < 10
but the warning mentions the balance being too low with the current balance rather than the fact that < 10
outputs triggered the warning
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean < 10
didn't really trigger the warning. Why would we want to specify "10" as a meaningful number. We want to split into 100, lower is fine, below 10 we don't bother. This warning essentially says: your wallet is virtually empty, we won't even bother redistributing it. We're talking sub 1KS wallets here. I don't mind updating the warning but I feel we would be needless specifying a very arbitrary condition that doesn't mean anything.
edit:
I updated it, but feel free to revert the commit. I don't see the point in providing so much information simply because the threshold is so low (~1KS). I consider this a shortcoming of renterd
though, in general, when we're out of money we drown the user in alerts that are essentially side-effects. I had it happen to my node, and when you log in you see a lot of alerts, but it's not immediately apparent the balance is 0... That's one of the things I would probably want to do differently next time around UX wise
No description provided.