[Bug]: Consensus Params fail to update via MsgUpdateParams

[MSalopek]

Is there an existing issue for this?

• I have searched the existing issues

What happened?

During gaia release testing, an update to ConsensusParams was attempted via a governance proposal. The proposal was passed but it failed to execute with a panic:

proposal:
  deposit_end_time: "2024-09-13T14:48:06.64993Z"
  failed_reason: 'handling x/gov proposal msg [authority:"cosmos10d07y265gmmuvt4z0w9aw880jnsr700j6zn9kn"
    block:<max_bytes:2000000 max_gas:75000000 > evidence:<max_age_num_blocks:1000000
    max_age_duration:<seconds:172800 > max_bytes:50000 > validator:<pub_key_types:"ed25519"
    > abci:<> ] PANICKED: runtime error: invalid memory address or nil pointer dereference'
  final_tally_result:

Upon further inspection, it was noticed that on cosmos-sdk v0.50.x and main the ConsensusParams.Version information was not initialized properly. The system was expecting a zero-value but the fields were in fact nil (unset) when the params were fetched from the consensus params collection.

Further context:
This may affect affects all networks that migrated from v0.47 to v0.50 as changes are related to cometbft v0.38.

• cosmos sdk param processing
• comet code attempts to access an uninitialized (nil) field

This can be mitigated by initializing the consensus params in UpgradeHandler and setting the version information to zero-value.

Additionally, this could be patched so the execution flow does not panic and instead sets a zero-value for the ConsensusParams.Version.

This does not affect "new" chains created with cosmos-sdk v0.50.

Cosmos SDK Version

v0.50.x, main

How to reproduce?

Submit a consensus upgrade proposal to a chain that migrated from v0.47 to v0.50.

{
    "messages": [
        {
            "@type": "/cosmos.consensus.v1.MsgUpdateParams",
            "authority": "cosmos10d07y265gmmuvt4z0w9aw880jnsr700j6zn9kn",
            "block": {
                "max_bytes": "2000000",
                "max_gas": "75000000"
            },
            "evidence": {
                "max_age_num_blocks": "1000000",
                "max_age_duration": "48h0m0s",
                "max_bytes": "50000"
            },
            "validator": {
                "pub_key_types": [
                    "ed25519"
                ]
            }
        }
    ],
    "metadata": "ipfs://CID",
    "deposit": "200000000stake",
    "title": "Update consensus params",
    "summary": "Update consensus params - expect fail",
    "expedited": false
}

simd tx gov submit-proposal <proposal>.json --keyring-backend=test --from user --fees 10000000stake --gas 700000
simd tx gov vote 2 yes --from user --keyring-backend test --fees 50000stake --gas 300000

Upon proposal passing, execution will fail:

proposal:
  deposit_end_time: "2024-09-13T14:48:06.64993Z"
  failed_reason: 'handling x/gov proposal msg [authority:"cosmos10d07y265gmmuvt4z0w9aw880jnsr700j6zn9kn"
    block:<max_bytes:2000000 max_gas:75000000 > evidence:<max_age_num_blocks:1000000
    max_age_duration:<seconds:172800 > max_bytes:50000 > validator:<pub_key_types:"ed25519"
    > abci:<> ] PANICKED: runtime error: invalid memory address or nil pointer dereference'
  final_tally_result:

[julienrbrt]

To add context on how that happened let me paste my slack message:

I see what happened, it wasn't required before, and baseapp.MigrateParams didn't set a default value either. So you could migrate to v0.50 without it.
Then comet found an issue with our consensus params msg update handling and hardened it: #20381.
That made x/consensus use their function, and that one expected a version being set.

[julienrbrt]

Thanks for finding this!