DRAFT: sdk v0.50 upgrade

[niilptr]

No description provided.

[koofree]

Hi all, I am a member of Firmachain. I just mention about a small peace.

        upgrade_test.go:91: 
            	Error Trace:	/home/runner/work/firmachain/firmachain/app/upgrades/v05/upgrade_test.go:91
            	            				/home/runner/work/firmachain/firmachain/app/upgrades/v05/upgrade_test.go:52
            	Error:      	Not equal: 
            	            	expected: "0.520000000000000000"
            	            	actual  : "0.500000000000000000"
            	            	
            	            	Diff:
            	            	--- Expected
            	            	+++ Actual
            	            	@@ -1 +1 @@
            	            	-0.520000000000000000
            	            	+0.500000000000000000
            	Test:       	TestKeeperTestSuite/TestUpgrade

Check this one, plz

[koofree]

First, I appreciate all these contributions @dimiandre.

I have a few questions to make sure I understand these updates.

1. Do you plan to use ProposalCancelRatio and ExpeditedMinDeposit? It is currently not usable. Or I don't have a sense of what they are.

2. burnmodule was deprecated. Isn't it not needed anymore?

[koofree]

We don't need to set the Allow Messages directly not "*"

like below example

Suggested change

	AllowMessages: []string{"*"},
	AllowMessages: ["/cosmos.gov.v1.Vote"],

[niilptr]

If you want just the cosmos.gov.v1.Vote message to be allowed, we can change it to this. Can I proceed to set AllowMessages to ["/cosmos.gov.v1.Vote"]?

[koofree]

I wonder about the vulnerability when we set this as a *. If it doesn't, * is okay.
And, I don't exactly know what governance features require us to use messages when operating interchain. I think we could leave it as a * for now.

[niilptr]

@koofree There should be no vulnerability in using * (at least in theory), since this is the configuration used by gaia and juno for example.
If you want to be more conservative, we could use the approach that is used by osmosis, which allows the following cosmos/ibc/wasm messages at the moment:

- /ibc.applications.transfer.v1.MsgTransfer

- /cosmos.bank.v1beta1.MsgSend

- /cosmos.staking.v1beta1.MsgDelegate
- /cosmos.staking.v1beta1.MsgBeginRedelegate
- /cosmos.staking.v1beta1.MsgCreateValidator
- /cosmos.staking.v1beta1.MsgEditValidator
- /cosmos.staking.v1beta1.MsgUndelegate

- /cosmos.distribution.v1beta1.MsgWithdrawDelegatorReward
- /cosmos.distribution.v1beta1.MsgSetWithdrawAddress
- /cosmos.distribution.v1beta1.MsgWithdrawValidatorCommission
- /cosmos.distribution.v1beta1.MsgFundCommunityPool

- /cosmos.gov.v1beta1.MsgVote

- /cosmwasm.wasm.v1.MsgExecuteContract
- /cosmwasm.wasm.v1.MsgInstantiateContract

- /cosmos.authz.v1beta1.MsgGrant
- /cosmos.authz.v1beta1.MsgRevoke

[koofree]

Hi @niilptr! I clearly understood. If we can choose the conservative way, I have no doubt doing that way. Can we go on your suggestion?

[niilptr]

Hi @koofree, thanks for the feedbacks! I will answer to your previous questions here:

1. [TestUpgrade failing] The test was failing because we updated the MinInitialDepositRatio param but did not change the expected value in the test case. We tested it in our localnet and it is working properly. I've update the check, thank you for this catch!
2. [ProposalCancelRatio and ExpeditedMinDeposit] The ProposalCancelRatio and ExpeditedMinDeposit params are new gov parameters that manage two new features available with the Cosmos SDK v0.50 (respectively, cancelling proposals, and expedited proposals). The current version of Firmachain, v04, is using the old Cosmos SDK v0.46, which has no support for these new features. Once the upgrade will be executed, users will have the possibility to start using the cancelling-proposals and expedited-proposals features. If you want to further tune the expedited-proposals behaviour we can also set custom values for the ExpeditedThreshold and ExpeditedVotingPeriod params in this upgrade (these, as any other param, can anyway be changed via governance proposals if you don't want to include their value now in this upgrade). You can find all the information for these new features in the cancelling proposals documentation and the expedited proposals documentation.
3. [x/burn module] After rechecking the burn module, I think we made a mistake here. We removed it because it looked like an unused module. But after reviewing it again, its endblock functionality is indeed called by the end-blocker which burns all the burn-module address balances. If you still need this feature we can re-add it back for sure. Sorry for having overlooked this.

[koofree]

@niilptr I appreciate all the answers about this PR. Through it, I highly understand this upgrade.

At the answer 3, I don't think it needs to be back to this upgrade.