UnpackAny limit exceeded for historical txs

[rootulp]

Context

Hey! We think there might be a bug in Cosmos SDK. A few months ago, a constant MaxUnpackAnySubCalls‎ = 100 was added, which - if we understand correctly - sets a limit on the number of calls inside MsgExec. However, in this transaction (link), there seem to be around 175 calls. As a result, when trying to parse the transaction, we get a call limit exceeded error. For now, we’ve rolled back to version v1.25.0-sdk-v0.46.16, where this change wasn’t present. But if there are breaking changes in the future, we won’t be able to re-index using the newer versions. Is this expected behavior or a bug? And what would be the best way to handle this?

From @vvuwei

Problem

https://github.com/celestiaorg/cosmos-sdk/releases/tag/v1.25.1-sdk-v0.46.16 included a fix that addressed a security vulnerability but celestia-app releases with that change won't be able to process https://www.mintscan.io/celestia/block/7013 because https://www.mintscan.io/celestia/tx/999b89260b384256f32722f7a1c54b319889cc6dd34c73b99ec1f47e560c5b66?height=7013. This effectively breaks single-binary syncs.

Options

1. Identify txs included on-chain with a UnpackAny >= 100. If it's not that many, consider modifying the constant in celestia-app v4 to handle the max UnpackAnys already included on-chain.
2. Consider wrapping fix: transaction decoding may result in a stack overflow or resource exhaustion cosmos-sdk#422 in a version gate so that it only applies to app version >= 3
3. Consider wrapping fix: transaction decoding may result in a stack overflow or resource exhaustion cosmos-sdk#422 in a block height gate so that it only applies to block heights >= N

[tac0turtle]

height gating seems sufficient. You shouldnt need to worry about the vulnerability in the historical context

[rootulp]

Height based gating has the downside that we must add height constants for each supported chain-id (arabica, mocha, mainnet beta). If we pursue option 2 then we can avoid any chain-id specific logic.

Given we bumped to cosmos-sdk v1.25.1 in https://github.com/celestiaorg/celestia-app/releases/tag/v3.1.1, we need to verify that all celestia-app v3 blocks adhere to the 100 UnpackAny limit and then we can add the version gate into the binary.

[evan-forbes]

imo we should simply add the version gate like all other consensus breaking changes, even if that means adding the gate in the fork of the sdk.

[adlerjohn]

I would add the height gating alone, at least to start. That at least guarantees nothing can go wrong. The gating can be loosened to a version gating in the future without breaking anything if we determine there are no such txs in the history.

[rootulp]

On Mainnet Beta celestia-app v3 there have been no transactions with > 50 MsgExecs (see query). Numia doesn't support Arabica or Mocha (yet, I just submitted a feature request) so I can't easily verify the same claim on those networks.

[cmwaters]

How difficult will it be to apply the limit based on either of the app version or height? On the surface it doesn't look like a simple change