Proposer Slashing

[zmanian]

During the Terra collapse, we observed a very large number of invalid transactions in proposed and confirmed blocks. ZKValidator

An assumption in Cosmos has been that block proposers would be running

There included

• Duplicate transactions posted in multiple blocks and in same blocks.
• Transactions with invalid sequence numbers

The default mmpool behavior should minimize the number of transactions that fail the antehandler that make it into blocks. The behavior on Terra seems to indicate that validators have started to run non-standard mempools. We expect this to become more common as MEV extraction becomes more common.

The Cosmos SDK should introduce slashing of proposers that waste network resources in the blocks they propose.

[alexanderbez]

Yeah, so the gist is here to slash a validator given that they have included a tx that failed during the AnteHandler phase during DeliverTx under certain conditions, right?

[alexanderbez]

ref: https://github.com/cosmos/cosmos-sdk/issues/10917

[aaronc]

The key thing is to distinguish between things which can succeed in checktx but fail during delivertx. For instance gas can change because it's dependent on state. Having sufficient funds for fees could change under certain conditions. Signature verification should never fail (unless we implement ADR 034). Not dealing with these distinctions correctly opens up another attack surface on validators because a malicious sequence of txs could cause validators to be unfairly slashed

[alexanderbez]

Correct, this is an implicit assumption, but should be stated clearly in the ADR.

[faddat]

Zaki, I think there's evidence of this happening on osmosis as well.

As I court enough controversey I won't name who I think is doing it but fair to say that they post some highly irregular blocks.

I like this idea.

[iramiller]

I have specific evidence of this behavior occurring (related to sequence numbers) on Provenance ... examples that have been occurring for the past year (or more, didn't look further back) from SDK 0.42.x forward. No custom ante handlers/mempools/configs, etc involved.

Edit: To be clear, these examples are failed signatures (error code 32) but the tx with the exact same txhash exists in two different blocks ... first time it is a failed tx, second time it is successful (as the out of order tx was retried).

[robert-zaremba]

I proposed few ideas in #8192

[jiseongnoh]

I am wondering why the validators did not catch the state pollution. Aren't detecting the incorrect block proposing the validators' role?

[alexanderbez]

I am wondering why the validators did not catch the state pollution. Aren't detecting the incorrect block proposing the validators' role?

The block isn't technically incorrect. It follow's the CometBFT consensus rules. The issue is that the proposer included txs that should not be included, the other validators simply execute the block and agree on the resulting merkle root. The proposal here to prevent such transactions from ever being included in blocks, which must happen at the application level.

With ABCI++ this becomes pretty easy actually. The validators during ProcessProposal will simply reject the proposal and a new proposer will propose a new block. We could slash that malicious validator which is pretty easy to do in ABCI++

[iramiller]

I agree that ABCI++ seems strictly superior to what we have today ... but I am simultaneously suspicious of the assertion that the behavior we have observed is from malicious validators. It feels more like a concurrency/timing issue because as least in our case we have observed this behavior with benign validators all running the same code in a naive way.

This has mostly been a curiosity for us with little real impact and I honestly expect that with the major ABCI++ refactors the dynamic multithreaded flows will shift enough to where this specific problem goes away even if it isn't directly addressed.

[alexanderbez]

Given you're running with ABCI++ and you have a proposer proposing a block via PrepareProposal, how would such a honest validator propose a block with invalid transactions? The honest validator must be up to the tip of the chain and it's mempool must be based on the latest state, s.t. it should only select txs that are valid. If any invalid tx is included, it must be that it is doing so in a non-honest way.

[iramiller]

If the tx was removed from the pool ... but the tx was resubmitted (by a user running a retry script for example) then it absolutely will appear again as the validators and mempools do not check for every previous txhash when a new one comes in... Seemingly the behavior we are seeing isn't a malicious validator but instead a client attempting to force the same tx in a few times in response to sequence errors.

I haven't seen a proposed change that would prevent this so long as sequence numbers are used and existing txhashes that have been fully processed are not checked before a new tx is accepted.

[alexanderbez]

If the tx was removed from the pool ... but the tx was resubmitted (by a user running a retry script for example) then it absolutely will appear again as the validators and mempools do not check for every previous txhash when a new one comes in...

If it was in the mempool at some point in time, then later removed (for whatever reason), then added back again, it must've gone through the entire CheckTx flow. So at this stage, it's in the mempool again and should be valid.

Am I missing something?