LaconicNetwork/cosmos-sdk
7
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 2 Wiki Activity

feat(x/gov): limit deposited coins to accepted proposal denom (#18189)

Browse Source
This commit is contained in:
Julien Robert 2023-10-20 18:05:12 +02:00 committed by GitHub
parent 5cd5003a7a
commit 2fbc547ce9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 106 additions and 141 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -55,6 +55,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
### Improvements
* (x/gov) [#18189](https://github.com/cosmos/cosmos-sdk/pull/18189) Limit the accepted deposit coins for a proposal to the minimum proposal deposit denoms.
* (x/gov) [#18025](https://github.com/cosmos/cosmos-sdk/pull/18025) Improve `<appd> q gov proposer` by querying directly a proposal instead of tx events. It is an alias of `q gov proposal` as the proposer is a field of the proposal.
* (x/staking/keeper) [#18049](https://github.com/cosmos/cosmos-sdk/pull/18049) return early if Slash encounters zero tokens to burn.
* (x/staking/keeper) [#18035](https://github.com/cosmos/cosmos-sdk/pull/18035) Hoisted out of the redelegation loop, the non-changing validator and delegator addresses parsing.

4
tests/integration/bank/app_test.go
View File

@ -373,14 +373,14 @@ func TestMsgSetSendEnabled(t *testing.T) {
s := createTestSuite(t, genAccs)
ctx := s.App.BaseApp.NewContext(false)
require.NoError(t, testutil.FundAccount(ctx, s.BankKeeper, addr1, sdk.NewCoins(sdk.NewInt64Coin("foocoin", 101))))
require.NoError(t, testutil.FundAccount(ctx, s.BankKeeper, addr1, sdk.NewCoins(sdk.NewInt64Coin("stake", 101))))
addr1Str := addr1.String()
govAddr := s.BankKeeper.GetAuthority()
goodGovProp, err := govv1.NewMsgSubmitProposal(
[]sdk.Msg{
types.NewMsgSetSendEnabled(govAddr, nil, nil),
},
sdk.Coins{{Denom: "foocoin", Amount: sdkmath.NewInt(5)}},
sdk.Coins{{Denom: "stake", Amount: sdkmath.NewInt(5)}},
addr1Str,
"set default send enabled to true",
"Change send enabled",

134
x/gov/README.md
View File

@ -518,6 +518,7 @@ All `sdk.Msgs` passed into the `messages` field of a `MsgSubmitProposal` message
must be registered in the app's `MsgServiceRouter`. Each of these messages must
have one signer, namely the gov module account. And finally, the metadata length
must not be larger than the `maxMetadataLen` config passed into the gov keeper.
The `initialDeposit` must be strictly positive and conform to the accepted denom of the `MinDeposit` param.
**State modifications:**
@ -529,58 +530,17 @@ must not be larger than the `maxMetadataLen` config passed into the gov keeper.
* Push `proposalID` in `ProposalProcessingQueue`
* Transfer `InitialDeposit` from the `Proposer` to the governance `ModuleAccount`
A `MsgSubmitProposal` transaction can be handled according to the following
pseudocode.
```go
// PSEUDOCODE //
// Check if MsgSubmitProposal is valid. If it is, create proposal //
upon receiving txGovSubmitProposal from sender do
if !correctlyFormatted(txGovSubmitProposal)
// check if proposal is correctly formatted and the messages have routes to other modules. Includes fee payment.
// check if all messages' unique Signer is the gov acct.
// check if the metadata is not too long.
throw
initialDeposit = txGovSubmitProposal.InitialDeposit
if (initialDeposit.Atoms <= 0) OR (sender.AtomBalance < initialDeposit.Atoms)
// InitialDeposit is negative or null OR sender has insufficient funds
throw
if (txGovSubmitProposal.Type != ProposalTypePlainText) OR (txGovSubmitProposal.Type != ProposalTypeSoftwareUpgrade)
sender.AtomBalance -= initialDeposit.Atoms
depositParam = load(GlobalParams, 'DepositParam')
proposalID = generate new proposalID
proposal = NewProposal()
proposal.Messages = txGovSubmitProposal.Messages
proposal.Metadata = txGovSubmitProposal.Metadata
proposal.TotalDeposit = initialDeposit
proposal.SubmitTime = <CurrentTime>
proposal.DepositEndTime = <CurrentTime>.Add(depositParam.MaxDepositPeriod)
proposal.Deposits.append({initialDeposit, sender})
proposal.Submitter = sender
proposal.YesVotes = 0
proposal.NoVotes = 0
proposal.NoWithVetoVotes = 0
proposal.AbstainVotes = 0
proposal.CurrentStatus = ProposalStatusOpen
store(Proposals, <proposalID|'proposal'>, proposal) // Store proposal in Proposals mapping
return proposalID
```
### Deposit
Once a proposal is submitted, if
`Proposal.TotalDeposit < ActiveParam.MinDeposit`, Atom holders can send
Once a proposal is submitted, if `Proposal.TotalDeposit < ActiveParam.MinDeposit`, Atom holders can send
`MsgDeposit` transactions to increase the proposal's deposit.
A deposit is accepted iff:
* The proposal exists
* The proposal is not in the voting period
* The deposited coins are conform to the accepted denom from the `MinDeposit` param
```protobuf reference
https://github.com/cosmos/cosmos-sdk/blob/v0.47.0-rc1/proto/cosmos/gov/v1/tx.proto#L134-L147
```
@ -594,55 +554,6 @@ https://github.com/cosmos/cosmos-sdk/blob/v0.47.0-rc1/proto/cosmos/gov/v1/tx.pro
* Push `proposalID` in `ProposalProcessingQueueEnd`
* Transfer `Deposit` from the `proposer` to the governance `ModuleAccount`
A `MsgDeposit` transaction has to go through a number of checks to be valid.
These checks are outlined in the following pseudocode.
```go
// PSEUDOCODE //
// Check if MsgDeposit is valid. If it is, increase deposit and check if MinDeposit is reached
upon receiving txGovDeposit from sender do
// check if proposal is correctly formatted. Includes fee payment.
if !correctlyFormatted(txGovDeposit)
throw
proposal = load(Proposals, <txGovDeposit.ProposalID|'proposal'>) // proposal is a const key, proposalID is variable
if (proposal == nil)
// There is no proposal for this proposalID
throw
if (txGovDeposit.Deposit.Atoms <= 0) OR (sender.AtomBalance < txGovDeposit.Deposit.Atoms) OR (proposal.CurrentStatus != ProposalStatusOpen)
// deposit is negative or null
// OR sender has insufficient funds
// OR proposal is not open for deposit anymore
throw
depositParam = load(GlobalParams, 'DepositParam')
if (CurrentBlock >= proposal.SubmitBlock + depositParam.MaxDepositPeriod)
proposal.CurrentStatus = ProposalStatusClosed
else
// sender can deposit
sender.AtomBalance -= txGovDeposit.Deposit.Atoms
proposal.Deposits.append({txGovVote.Deposit, sender})
proposal.TotalDeposit.Plus(txGovDeposit.Deposit)
if (proposal.TotalDeposit >= depositParam.MinDeposit)
// MinDeposit is reached, vote opens
proposal.VotingStartBlock = CurrentBlock
proposal.CurrentStatus = ProposalStatusActive
ProposalProcessingQueue.push(txGovDeposit.ProposalID)
store(Proposals, <txGovVote.ProposalID|'proposal'>, proposal)
```
### Vote
Once `ActiveParam.MinDeposit` is reached, voting period starts. From there,
@ -661,35 +572,6 @@ https://github.com/cosmos/cosmos-sdk/blob/v0.47.0-rc1/proto/cosmos/gov/v1/tx.pro
Gas cost for this message has to take into account the future tallying of the vote in EndBlocker.
:::
Next is a pseudocode outline of the way `MsgVote` transactions are handled:
```go
// PSEUDOCODE //
// Check if MsgVote is valid. If it is, count vote//
upon receiving txGovVote from sender do
// check if proposal is correctly formatted. Includes fee payment.
if !correctlyFormatted(txGovDeposit)
throw
proposal = load(Proposals, <txGovDeposit.ProposalID|'proposal'>)
if (proposal == nil)
// There is no proposal for this proposalID
throw
if (proposal.CurrentStatus == ProposalStatusActive)
// Sender can vote if
// Proposal is active
// Sender has some bonds
store(Governance, <txGovVote.ProposalID|'addresses'|sender>, txGovVote.Vote) // Voters can vote multiple times. Re-voting overrides previous vote. This is ok because tallying is done once at the end.
```
## Events
The governance module emits the following events:

41
x/gov/keeper/deposit.go
View File

@ -70,6 +70,16 @@ func (keeper Keeper) AddDeposit(ctx context.Context, proposalID uint64, deposito
return false, errors.Wrapf(types.ErrInactiveProposal, "%d", proposalID)
}
// Check coins to be deposited match the proposal's deposit params
params, err := keeper.Params.Get(ctx)
if err != nil {
return false, err
}
if err := keeper.validateDepositDenom(ctx, params, depositAmount); err != nil {
return false, err
}
// update the governance module's account coins pool
err = keeper.bankKeeper.SendCoinsFromAccountToModule(ctx, depositorAddr, types.ModuleName, depositAmount)
if err != nil {
@ -84,13 +94,9 @@ func (keeper Keeper) AddDeposit(ctx context.Context, proposalID uint64, deposito
}
// Check if deposit has provided sufficient total funds to transition the proposal into the voting period
activatedVotingPeriod := false
params, err := keeper.Params.Get(ctx)
if err != nil {
return false, err
}
minDepositAmount := proposal.GetMinDepositFromParams(params)
activatedVotingPeriod := false
if proposal.Status == v1.StatusDepositPeriod && sdk.NewCoins(proposal.TotalDeposit...).IsAllGTE(minDepositAmount) {
err = keeper.ActivateVotingPeriod(ctx, proposal)
if err != nil {
@ -237,12 +243,7 @@ func (keeper Keeper) RefundAndDeleteDeposits(ctx context.Context, proposalID uin
// validateInitialDeposit validates if initial deposit is greater than or equal to the minimum
// required at the time of proposal submission. This threshold amount is determined by
// the deposit parameters. Returns nil on success, error otherwise.
func (keeper Keeper) validateInitialDeposit(ctx context.Context, initialDeposit sdk.Coins, expedited bool) error {
params, err := keeper.Params.Get(ctx)
if err != nil {
return err
}
func (keeper Keeper) validateInitialDeposit(ctx context.Context, params v1.Params, initialDeposit sdk.Coins, expedited bool) error {
minInitialDepositRatio, err := sdkmath.LegacyNewDecFromStr(params.MinInitialDepositRatio)
if err != nil {
return err
@ -266,3 +267,21 @@ func (keeper Keeper) validateInitialDeposit(ctx context.Context, initialDeposit
}
return nil
}
// validateDepositDenom validates if the deposit denom is accepted by the governance module.
func (keeper Keeper) validateDepositDenom(ctx context.Context, params v1.Params, depositAmount sdk.Coins) error {
denoms := []string{}
acceptedDenoms := make(map[string]bool, len(params.MinDeposit))
for _, coin := range params.MinDeposit {
acceptedDenoms[coin.Denom] = true
denoms = append(denoms, coin.Denom)
}
for _, coin := range depositAmount {
if _, ok := acceptedDenoms[coin.Denom]; !ok {
return errors.Wrapf(types.ErrInvalidDepositDenom, "deposited %s, but gov accepts only the following denom(s): %v", coin, denoms)
}
}
return nil
}

7
x/gov/keeper/export_test.go
View File

@ -5,5 +5,10 @@ import sdk "github.com/cosmos/cosmos-sdk/types"
// ValidateInitialDeposit is a helper function used only in deposit tests which returns the same
// functionality of validateInitialDeposit private function.
func (k Keeper) ValidateInitialDeposit(ctx sdk.Context, initialDeposit sdk.Coins, expedited bool) error {
return k.validateInitialDeposit(ctx, initialDeposit, expedited)
params, err := k.Params.Get(ctx)
if err != nil {
return err
}
return k.validateInitialDeposit(ctx, params, initialDeposit, expedited)
}

11
x/gov/keeper/msg_server.go
View File

@ -75,7 +75,16 @@ func (k msgServer) SubmitProposal(goCtx context.Context, msg *v1.MsgSubmitPropos
ctx := sdk.UnwrapSDKContext(goCtx)
initialDeposit := msg.GetInitialDeposit()
if err := k.validateInitialDeposit(ctx, initialDeposit, msg.Expedited); err != nil {
params, err := k.Params.Get(ctx)
if err != nil {
return nil, fmt.Errorf("failed to get governance parameters: %w", err)
}
if err := k.validateInitialDeposit(ctx, params, initialDeposit, msg.Expedited); err != nil {
return nil, err
}
if err := k.validateDepositDenom(ctx, params, initialDeposit); err != nil {
return nil, err
}

48
x/gov/keeper/msg_server_test.go
View File

@ -207,6 +207,36 @@ func (suite *KeeperTestSuite) TestMsgSubmitProposal() {
expErr: true,
expErrMsg: "proposal message not recognized by router",
},
"invalid deposited coin": {
preRun: func() (*v1.MsgSubmitProposal, error) {
return v1.NewMsgSubmitProposal(
[]sdk.Msg{bankMsg},
[]sdk.Coin{sdk.NewCoin("invalid", sdkmath.NewInt(100))},
proposer.String(),
"",
"Proposal",
"description of proposal",
false,
)
},
expErr: true,
expErrMsg: "deposited 100invalid, but gov accepts only the following denom(s): [stake]: invalid deposit denom",
},
"invalid deposited coin (multiple)": {
preRun: func() (*v1.MsgSubmitProposal, error) {
return v1.NewMsgSubmitProposal(
[]sdk.Msg{bankMsg},
append(initialDeposit, sdk.NewCoin("invalid", sdkmath.NewInt(100))),
proposer.String(),
"",
"Proposal",
"description of proposal",
false,
)
},
expErr: true,
expErrMsg: "deposited 100invalid, but gov accepts only the following denom(s): [stake]: invalid deposit denom",
},
"all good": {
preRun: func() (*v1.MsgSubmitProposal, error) {
return v1.NewMsgSubmitProposal(
@ -792,6 +822,24 @@ func (suite *KeeperTestSuite) TestMsgDeposit() {
expErr: true,
expErrMsg: "invalid depositor address",
},
"invalid deposited coin ": {
preRun: func() uint64 {
return pID
},
depositor: proposer,
deposit: []sdk.Coin{sdk.NewCoin("ibc/badcoin", sdkmath.NewInt(1000))},
expErr: true,
expErrMsg: "deposited 1000ibc/badcoin, but gov accepts only the following denom(s): [stake]",
},
"invalid deposited coin (multiple)": {
preRun: func() uint64 {
return pID
},
depositor: proposer,
deposit: append(minDeposit, sdk.NewCoin("ibc/badcoin", sdkmath.NewInt(1000))),
expErr: true,
expErrMsg: "deposited 1000ibc/badcoin, but gov accepts only the following denom(s): [stake]",
},
"all good": {
preRun: func() uint64 {
return pID

1
x/gov/types/errors.go
View File

@ -24,4 +24,5 @@ var (
ErrVotingPeriodEnded = errors.Register(ModuleName, 20, "voting period already ended")
ErrInvalidProposal = errors.Register(ModuleName, 21, "invalid proposal")
ErrSummaryTooLong = errors.Register(ModuleName, 22, "summary too long")
ErrInvalidDepositDenom = errors.Register(ModuleName, 23, "invalid deposit denom")
)