MerosCrypto / Meros

An instant and feeless cryptocurrency for the future, secured by the Merit Caching Consensus Mechanism.
https://meroscrypto.io
Other
82 stars 19 forks source link

Possible to break syncing via creating a Block with 1 million transactions. #255

Open kayabaNerve opened 3 years ago

kayabaNerve commented 3 years ago

It is technically possible for any Merit Holder to do this as they can bypass the spam filter check, as it's not enforced by the blockchain, though it would take a very long time to create such a block. Any live nodes can avoid this via optimizing out the sketch hash syncing thanks to successfully resolving the sketch (though it would take flooding the network with all these transactions, which would cause a spam filter adjustment, which would prevent further downloading).

That said, if such a block did manage to be accepted, it wouldn't be syncable. This is an effectively impossible edge case, yet it's still one we should write some basic code for (reject blocks which have 1 million TXs).

kayabaNerve commented 3 years ago

The fact it takes this many transactions to trigger this limit, with a technical 16,666 TPS, proves a 8 MB cap is not effective DoS protection. Said limit should also be reduced to 1 MB, AKA ~2,083 TPS, which already will likely never be achieved. We coul.ld practically lower to 0.5 MB, which would support effectively every optimization possible with the current architecture, yet I don't see a point in going that low.

It should be noted elements in a block body can cause this same issue.

kayabaNerve commented 3 years ago

My math is horribly wrong. 1 MB would be 437 TPS, which also would be fine. With threading, SignedVerificationPackets, and overall optimizations, it is in the realm of feasibility though. Therefore the limit should be 2 MB (874; would require massive architectural reworks).

As we do write faster code, we can increase this easily. The plan always was to regularly upgrade.

kayabaNerve commented 3 years ago

The most frustrating part isn't that Elements need their own limitation added. It's that BlockBody sketches, with 100% capacity (will be used when syncing after #274) exist along WITH Elements. Because of this, when considering extreme activity, the system starts to struggle.

274 should be amended so any capacity is accepted in a response; not just the requested capacity. With the maximum amount of holders, which is incredibly improbably to the point of impossibility, this would mean every holder can get 4 Elements in a Block.

The problem is creating a Block with so many Elements, which can be broadcasted in the mempool, will cause miners to generate blocks which can't be propagated. This splits every miner onto their own chain. Because of that, simply setting a cap of 4 (carrying the rest into the next Block from a node-perspective), does seem optimal. It's blunt, and inefficient, yet can be revised later.

Then when responding with a BlockBody, the capacity must have a max set to prevent triggering the message limit. Then the header would have a packet quantity limit added of 262143.

kayabaNerve commented 3 years ago

Tests to pair with this issue: