Heuristic tx censorship detection

[Arindam2407]

Changes

Censorship Detection Using Heuristics

• We maintain a cache CensorshipDetector of size 4 which indicates whether newly cached blocks are potentially censoring transactions.
• On caching of a block, we check if the maximum gas price among transactions included in the block is lower than the maximum gas price in the tx pool.
• If so, we mark PotentialCensorship true and check if all the 4 latest cached blocks indicate PotentialCensorship to be true.
• If so, ShouldOverrideBuilder becomes true.

Types of changes

What types of changes does your code introduce?

• New feature (a non-breaking change that adds functionality)

Requires testing

• Yes
• No

If yes, did you write tests?

• Yes
• No

[LukaszRozmej]

Good first try and place to polish it a bit!

[LukaszRozmej]

Love the simplifications!
Few minor things to polish.

Next steps:

• Write tests for CensorshipDetector class - either unit or proper integration using BlockchainTests or descendant class.
• Test in real world.
• Consider implementing different censorship detection strategies.
• Run it on mainnet - @kamilchodola can you help with that?

[marcindsobczak]

In the way you wrote it - it will be working, but you overcomplicated it. You are taking snapshot of the whole pool, then selecting lowest nonce from each bucket, then reading accounts and looking for next-nonce-to-be-executed and then iterating for the biggest gas price (which post-1559 is equal to maxPriorityFeePerGas).

Instead, you can simplify it and do a snapshot of lowest-nonce-per-sender only and then just look for the tx with biggest gasBottleneck.

[marcindsobczak]

One more potential issue - there are 2 ways of paying tx fee. New way, post London fork, you are paying baseFee + priority fee. Priority fee is just tx.GasPrice. But there are pre-London tx types as well - legacy and access list. These types have only GasPrice, which all is consumed when tx is executed. For these types, under tx.GasPrice, you have baseFee + priority fee. Post 1559 tx.GasPrice means priority fee only. So you probably need take it into consideration when looking for potentially censored txs.
One example: Let's say that we have in the pool legacy tx with GasPrice equal 10 Gwei. In the last block, the most expensive tx is 1559-type and has tx.GasPrice (priorityFee) equal 1 Gwei. BaseFee is 20Gwei.
In the current form, you will consider it as censorship. But in reality, our legacy tx is too cheap to be included (as baseFee is 20 Gwei and tx is offering only 10 Gwei).

So what I will recommend here is to prepare maxGasPriceInBlock as tx.GasPrice + block base fee if best tx was supporting 1559 or just tx.GasPrice if was legacy or access list.

Then you only need to find tx in mempool with biggest gasBottleneck - gas bottleneck has inside current base fee + priority fee

[marcindsobczak]

And one more thing - blob transactions. There is a separate market with blob date fee and a hard limit of max 6 blob txs per block. We probably don't need to check censorship in the context of blob transactions. But we can miss some censorship attempts because of it.

Example: Our most expensive tx in the pool is 1559-type tx with tx.GasPrice equal 2 Gwei.
In the block you found the most expensive tx - a blob tx with tx.GasPrice equal 3 Gwei. You don't consider it as censorship - our best tx is cheper than best tx in the block.
But if we exclude all blob transactions, let's say next most expensive tx in the block pays only 1 Gwei. And it was actually censorship on the tx which we have in the pool.

My recommendation - exclude blob-type txs when setting maxGasPriceInBlock

[marcindsobczak]

Good changes, but still a bit to improve.
Also it would be great to add some unit tests - tests will help you to catch potential mistakes/inconsistences in implementation and we will be protected from broking it by accident in the future

[marcindsobczak]

LGTM

[LukaszRozmej]

we probably shouldn't even do 8 threshold, as here we should only check tx pool if we are processing current head block, right?

[marcindsobczak]

Right, my bad

[LukaszRozmej]

Shouldn't we enable it by default? Or do we plan to start slowly?

[marcindsobczak]

I don't knw why I disabled it by default. It should be enabled ofc

[marcindsobczak]

Looks good, let's just add a metric for potentially censored blocks

[marcindsobczak]

Actually more useful will be not block number, but number of blocks. Maybe let's have both?