search

celestiaorg / celestia-app

Celestia consensus node
https://celestiaorg.github.io/celestia-app/
Apache License 2.0
345 stars 292 forks source link

Reject tx in prepare proposal if shares overflow max square size #3986

Open rootulp opened 1 month ago

rootulp commented 1 month ago

Motivation

Discussion w/ @evan-forbes

Context

The blob share decorator ante handler rejects a transaction if it occupies more shares than available in the max square size. The ante handler is unaware of the number of shares occupied by other transactions in a block.

Problem

Assume pfbTxA and pfbTxB occupy 2100 shares each. Assume max square size is 64 (so max total shares is 64 * 64 = 4,096). Both pfbTxA and pfbTxB will pass the blob share decorator ante handler but they will fail to be included in the same block.

Proposal

Implement a mechanism to reject a tx in prepare proposal if adding it to the partially constructed block will overflow the maximum number of shares. This can be done via modifications to the blob share decorator or a new ante handler.

Note: The SDK has a gas meter that is responsible for accounting gas used per transaction such that the total gas in a block doesn't exceed consensus.block.MaxGas. We may consider adding a similar counter for the number of shares used by the partially constructed block.

cmwaters commented 1 month ago

Won't the square construction mechanism reject that second transaction in PrepareProposal?

rootulp commented 1 month ago

It should but AFAIK the square construction won't reject other transactions in the same block that have invalid nonces because the second transaction was removed. For example (assume all these txs are from the same account):

I think square construction will remove pfbTxB and include pfbTxA and msgSendC even though msgSendC skips a nonce. We should add a test for this scenario before trying to address this issue.

evan-forbes commented 1 month ago

It should but AFAIK the square construction won't reject other transactions in the same block that have invalid nonces because the second transaction was removed.

can confirm that we saw this occur on mocha, and a presumably honest proposer created an invalid block

evan-forbes commented 1 month ago

side note: imo this is blocking https://github.com/celestiaorg/celestia-core/issues/1007