CGovernanceManager::MasternodeRateCheck issue

[ivansib]

Hi guys!

In Sibcoin testnet network we faced with an error which I believe can be a problem for Dash network as well.

The problem was that a lot of nodes didn't accept superblock because they didn't have a proper gov object. Debug log shows that they received message with proper gobject but didn't accept it because of this:

CGovernanceManager::MasternodeRateCheck -- Rate too high: object hash = 1da8b06ac16de12cb5d0cf368da76152ab8c12d7646b9a8c8fb1b049621607e3, masternode vin = e59bfeca638fd1efb992a9780585c51b9190a7bc030d315e2fee788304c3ae4f-1, object timestamp = 1491243136, rate = 0.000641, max rate = 0.000611

and after that there were several such errors:

CGovernanceManager::MasternodeRateCheck -- Rate too high: object hash = 1da8b06ac16de12cb5d0cf368da76152ab8c12d7646b9a8c8fb1b049621607e3, masternode vin = e59bfeca638fd1efb992a9780585c51b9190a7bc030d315e2fee788304c3ae4f-1, object timestamp = 1491243136, rate = 0.000962, max rate = 0.000611

CGovernanceManager::MasternodeRateCheck -- Rate too high: object hash = 1da8b06ac16de12cb5d0cf368da76152ab8c12d7646b9a8c8fb1b049621607e3, masternode vin = e59bfeca638fd1efb992a9780585c51b9190a7bc030d315e2fee788304c3ae4f-1, object timestamp = 1491243136, rate = 0.001282, max rate = 0.000611

...

CGovernanceManager::MasternodeRateCheck -- Rate too high: object hash = 1da8b06ac16de12cb5d0cf368da76152ab8c12d7646b9a8c8fb1b049621607e3, masternode vin = e59bfeca638fd1efb992a9780585c51b9190a7bc030d315e2fee788304c3ae4f-1, object timestamp = 1491243136, rate = 10000000000.000000, max rate = 0.000611

I looked into the code and found this condition:

https://github.com/dashpay/dash/blob/master/src/governance.cpp#L883 // Allow 1 trigger per mn per cycle, with a small fudge factor

Maybe I'm wrong, but it seems that such condition is too restrictive. As far as I understand masternode elected to be watchdog winner using watchdog's gobject hash "lottery" and this is quite random process, so it's possible that one masternode will issue several superblock gobjects within a payment cycle. And if this will happen right before superblock creation it can cause a problem because other nodes won't accept this superblock message ("Rate too high").

In our case we have only few masternodes with sentinel on testnet so chance to face with this error was high. Of course in mainnet probability of such bad chain of events is really small but I think it's still possible.

Another problem is adding timestamp to buffer several times for the same gobject. It led to rate increasing without real reason for that. Maybe it was fixed here:

https://github.com/dashpay/dash/commit/86525601d5915f380976c9d2e686ad7f66db991f

But I looked though the code and I consider that it wasn't. And I'm not sure what is the best way to fix it.

[joshafest]

Have you checked "double dMaxRate"?

[Znuff]

Does anyone have any idea how this could be fixed? Just change the rate?

I'm not directly implicated in any coin, I just run a lot of masternodes for different coins based on this source-code (forks and whatever), and I notice this issue is creeping into a lot of coins.

[nmarley]

This can be "fixed" by changing the rate, yes, but this trigger rate limit is intentional to prevent possibly malicious MNs from submitting too many triggers and being able to DoS the network (as no collateral is necessary for triggers, but only MNs are able to submit them).