[Neo Core Memory Pool] fix flooding attack#3631
[Neo Core Memory Pool] fix flooding attack#3631Jim8y wants to merge 1 commit intoneo-project:masterfrom
Conversation
|
Can I still spam billions of txs and let them fail due to RateLimitExceeded, almost for free? In this case others cannot send any more tx, or have to send them at a high price. |
to that i think we can only fix in network level. like defend against DOS or DDOS in p2p network and rpc server. |
|
An attacker may send txs from different corners of the whole network, and we need to stop the attacker's txs, without dropping too many normal txs. Also we always have to let the consensus nodes communicate with others, because the attacker can always let the consensus nodes receive different txs. We know the impossible CAP triangle among consistency, availability and partition tolerance. In dBFT we prioritize C, and then in a mempool flood it seems we want to keep A. Then we have to sacrifice P. In a robust blockchain system there should be enough nodes to sacrifice for P, and I do think the current methodology in this PR is good. But it is just dropping too many ordinary txs. And it is not saving the bandwidth among consensus nodes. The bandwidth among consensus nodes is the key to the quick availability for a new block. We may additionally ban too many txs from a single signer, and too many txs from a single peer before a new block is generated. This forms bastions around the consensus nodes, before too many txs overflow the bandwidth of consensus network bandwidth. Ordinary nodes should try to relay the txs in the PrepareRequest, after relaying the PrepareRequest, if they do know the contents of the txs. A node can forget the too many txs from a single peer or a single signer, unless it is included in a PrepareRequest in a coming block. Unfortunately current ordinary nodes are unable to decode PrepareRequest, and this may require a careful refactor. The smart mempool throttler was almost the correct way to do this, but it was not handling the good txs well, and was not able to let the txs in the PrepareRequest to be spread in the whole network for the next block to be generated quickly. |
| { | ||
| _system = system; | ||
| Capacity = system.Settings.MemoryPoolMaxTransactions; | ||
| _metrics = new MempoolMetrics(Capacity) { BaseMinFee = 1_0000 }; // 0.0001 GAS |
There was a problem hiding this comment.
It might be better to define BaseMinFee = 1_0000 with MaxFeeMultiplier and MetricsUpdateIntervalMs.
| } | ||
| } | ||
|
|
||
| private class MempoolMetrics(int capacity) |
There was a problem hiding this comment.
The file of MemPool has too many lines, so it's better for this part to be a separate file?
| const double BaseMultiplier = 2.0; | ||
| const double ExponentScale = 10.0; | ||
|
|
||
| return Math.Pow(BaseMultiplier, CongestionLevel * ExponentScale); |
There was a problem hiding this comment.
normally this will not be triggered, just used for defending flooding attacks.
|
Besides, a sender of good txs is unable to judge whether a tx marked RateLimitExceeded will be included in a future block. Other nodes may still possess the tx. |
vncoelho
left a comment
There was a problem hiding this comment.
I am still not in favor of this solution.
For that to be sucess, we would need to be more certain that these txs with more than 0,1 netfee gas will be processed and not replaced or dropped.
Reality is, this problem may not be |
|
it might better be defended in network level. |
Description
This one is to replace #3364, in 3364 i proposed a solution as smart throttler, which is not favored by core dev team, thus in this pr, i propose a much simpler solution that will set a minumum transaction fee barrier when the memory pool is over half filled. and the bar will by dynamicly updated based on the congestion factor.
When the memorypool is full, you have to pay 0.1 GAS to add a transaction.
Fixes # #2862
Type of change
Checklist: