Add sanity check for round destroyer threshold #12585
Conversation
|
Nevermind. A lot of things can be done. Let's restore the original behavior first. |
|
RoundDestroyer is solving a very complex problem of ensuring all the clients can register ALL their inputs into the round. Before the client registers, it sets up a plan. According to the wallet privacy status, number of coins, etc. it selects the best combination of coins. The best for the client the plan can be executed. However if the number of inputs in the round goes up and reaches the maximum, it is not guaranteed that all the inputs can be registered. RoundDestroyer solving this issue by splitting the round and making an optimal distribution by using I am sure it can be improved or there can be other solutions but the other solution must address the requirement to ensure the clients can register all of their inputs to the round. |
| @@ -497,7 +497,7 @@ private async Task CreateRoundsAsync(CancellationToken cancellationToken) | |||
| x.Phase == Phase.InputRegistration | |||
| && x is not BlameRound | |||
| && !x.IsInputRegistrationEnded(x.Parameters.MaxInputCountByRound) | |||
| && x.InputCount >= roundDestroyerInputCount).ToArray()) | |||
| && x.InputCount >= Math.Min(0.9 * x.Parameters.MaxInputCountByRound, roundDestroyerInputCount)).ToArray()) | |||
There was a problem hiding this comment.
roundDestroyerInputCount has no use after this change and at LOC540 we will log false data.
How about this?
var roundDestroyerInputCount = 0.9 * Config.MaxInputCountByRound
Then, we don't complicate further this Where predicate.
There was a problem hiding this comment.
Where does the x come from? :D
There was a problem hiding this comment.
hehe, Config.MaxInputCountByRound is better.
Edited my prev message.
There was a problem hiding this comment.
For the logging, we could create a function instead, I was tired, didn't see we where using it again for logging purposes
There was a problem hiding this comment.
hehe, Config.MaxInputCountByRound is better.
No, it has to be tested against the Round Parameter, not the Config.
There was a problem hiding this comment.
I tried to fix this while respecting that we want use the Round Parameter and not the Config
#12591
|
Deployed to TestNet and MainNet. |
Round destroyer is broken since deployment of #12524 because
roundDestroyerInputCount = 440, which is higher thanMaxInputCountByRound(400). However, it must always be lower.This clearly shows how unintuitive is this code, those multipliers etc etc... We should remove all that and refactor more simply.
Something else, we had extremely good rounds while having this bug. Maybe we should reconsider how this round destroyer is working.