Decide on the default max transaction size accepted by the mempool

[evan-forbes]

We want to consider changing the default config for the mempool to accept larger transactions.

https://github.com/celestiaorg/celestia-core/blob/094cd2c631a5b970ce89e04a81f404c99c38bdf8/config/config.go#L744

related to #243

and there are concerns as well https://github.com/celestiaorg/celestia-core/issues/243#issuecomment-804273440 https://github.com/informalsystems/audit-celestia/issues/6

[musalbas]

Can potential DoS vectors be fixed by somehow rate limiting transactions by IP or blacklisting IPs that repeatedly send invalid or very low fee transactions?

[evan-forbes]

yeah that makes sense, and is likely something we will need to build in the future. afaiu this would require some significant changes to the p2p stack. I believe this is actually something that the tendermint v0.35 p2p stack was attempting to address.

[adlerjohn]

This isn't really a DoS vector (it's still a DDoS vector, but those are unavoidable) since the cost of validating even a large transaction really isn't that much, and a blacklist would be applied swiftly. Of course, this means we need a blacklist on invalid transactions, but that's needed anyways. Each invalid transaction can only consume a bounded amount of resources for finite time.

[musalbas]

The point is that right now I don't think there's a blacklist, and also the problem is not just with invalid transactions, but with valid but very low/zero fee transactions.

[evan-forbes]

afaiu this would require some significant changes to the p2p stack

after doing further digging, the state sync reactor has some form of peer blacklist https://github.com/celestiaorg/celestia-core/blob/094cd2c631a5b970ce89e04a81f404c99c38bdf8/statesync/snapshots.go#L54

other reactors do not have this afaik, but as discussed elsewhere it shouldn't actually be that difficult to do when we don't generalize it across reactors

[rootulp]

I may be echoing what @adlerjohn said above but it seems like there are two separate issues being discussed here:

1. Decide on a value for MaxTxBytes (this issue)
2. Design a peer blacklist mechanism for the mempool (partially discussed in https://github.com/celestiaorg/celestia-core/issues/243)

I think the issues are distinct and we need to reach consensus on (1) and can punt (2) until later. It also seems like (2) shouldn't impact (1) because an attacker could construct several small transactions or one large transaction. Peers of the validator can be forced to check the validity of the several small transactions or one large transaction so (2) is desirable regardless of what we decide for (1).

The point is that right now I don't think there's a blacklist, and also the problem is not just with invalid transactions, but with valid but very low/zero fee transactions.

If we are concerned about the mempool being flooded with low/zero fee transactions, a naive mitigation to this is to implement tx prioritization https://github.com/celestiaorg/celestia-app/issues/513#issuecomment-1282984922 and then configure either or both of these mempool parameters (both default to 0):

# ttl-duration, if non-zero, defines the maximum amount of time a transaction
# can exist for in the mempool.
#
# Note, if ttl-num-blocks is also defined, a transaction will be removed if it
# has existed in the mempool at least ttl-num-blocks number of blocks or if it's
# insertion time into the mempool is beyond ttl-duration.
ttl-duration = 0

# ttl-num-blocks, if non-zero, defines the maximum number of blocks a transaction
# can exist for in the mempool.
#
# Note, if ttl-duration is also defined, a transaction will be removed if it
# has existed in the mempool at least ttl-num-blocks number of blocks or if
# it's insertion time into the mempool is beyond ttl-duration.
ttl-num-blocks = 0

[evan-forbes]

If we are concerned about the mempool being flooded with low/zero fee transactions

This might be in some other issue somewhere, but what about setting the default minimum fee in the app.toml to something other than zero?

[rootulp]

It seems like the default minimum gas price is set by a full-node operator here but we can provide a default higher than 0 here. Based on the docs below, I propose we rely on the default mempool configuration of:

ttl-duration = 0
ttl-num-blocks = 0

and encourage full-node operators to use a minimum gas fee > 0

Screenshot from Cosmos SDK AnteHandler:

Screenshot from Cosmos SDK auth module:

[rootulp]

Just as a ballpark, 1 MB messages should probably a good starting point upper bound I think

Sourced from https://github.com/celestiaorg/celestia-core/issues/243

but given the default is 1 MiB I expect that comment is outdated. Do we have an updated estimate for the the size of messages we intend on supporting?

[evan-forbes]

I think we should change the default value for ttl-num-blocks to something non-zero, as per suggested in #812

as discussed elsewhere, without a blocklist, the size of the tx isn't that important as a malicious node could spam their peers with many small txs and have the same effect. Projects have expressed having the ability to submit up to 2MB blocks, so imo we should just stick with that for now.

@rootulp does that sound reasonable? can we add this as a default in celestia-app and close this?

[rootulp]

Yea, SGTM.

Projects have expressed having the ability to submit up to 2MB blocks

Out of curiosity, which projects?