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

refactor(x/gov): log error when deposit refund error (#18976)

Browse Source
This commit is contained in:
Julien Robert 2024-01-08 22:13:46 +01:00 committed by GitHub
parent 20b1da7a2e
commit 6a2329fc93
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 88 additions and 91 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
x/gov/CHANGELOG.md
View File

@ -34,6 +34,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
### Improvements
* [#18976](https://github.com/cosmos/cosmos-sdk/pull/18976) Log and send an event when a proposal deposit refund or burn has failed.
* [#18856](https://github.com/cosmos/cosmos-sdk/pull/18856) Add `ProposalCancelMaxPeriod` for modifying how long a proposal can be cancelled after it has been submitted.
* [#18445](https://github.com/cosmos/cosmos-sdk/pull/18445) Extend gov config.
* [#18532](https://github.com/cosmos/cosmos-sdk/pull/18532) Repurpose `govcliutils.NormalizeProposalType` to work for gov v1 proposal types.

14
x/gov/abci.go
View File

@ -137,7 +137,19 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error {
err = keeper.RefundAndDeleteDeposits(ctx, proposal.Id)
}
if err != nil {
return false, err
// in case of an error, log it and emit an event
// we do not want to halt the chain if the refund/burn fails
// as it could happen due to a governance mistake (governance has let a proposal pass that sends gov funds that were from proposal deposits)
keeper.Logger(ctx).Error("failed to refund or burn deposits", "error", err)
ctx.EventManager().EmitEvent(
sdk.NewEvent(
types.EventTypeProposalDeposit,
sdk.NewAttribute(types.AttributeKeyProposalID, fmt.Sprintf("%d", proposal.Id)),
sdk.NewAttribute(types.AttributeKeyProposalDepositError, "failed to refund or burn deposits"),
sdk.NewAttribute("error", err.Error()),
),
)
}
if err = keeper.ActiveProposalsQueue.Remove(ctx, collections.Join(*proposal.VotingEndTime, proposal.Id)); err != nil {

122
x/gov/abci_test.go
View File

@ -41,8 +41,6 @@ func TestUnregisteredProposal_InactiveProposalFails(t *testing.T) {
err = suite.GovKeeper.InactiveProposalsQueue.Set(ctx, collections.Join(endTime, proposal.Id), proposal.Id)
require.NoError(t, err)
checkInactiveProposalsQueue(t, ctx, suite.GovKeeper)
err = gov.EndBlocker(ctx, suite.GovKeeper)
require.NoError(t, err)
@ -71,8 +69,6 @@ func TestUnregisteredProposal_ActiveProposalFails(t *testing.T) {
err = suite.GovKeeper.ActiveProposalsQueue.Set(ctx, collections.Join(endTime, proposal.Id), proposal.Id)
require.NoError(t, err)
checkActiveProposalsQueue(t, ctx, suite.GovKeeper)
err = gov.EndBlocker(ctx, suite.GovKeeper)
require.NoError(t, err)
@ -89,8 +85,6 @@ func TestTickExpiredDepositPeriod(t *testing.T) {
govMsgSvr := keeper.NewMsgServerImpl(suite.GovKeeper)
checkInactiveProposalsQueue(t, ctx, suite.GovKeeper)
newProposalMsg, err := v1.NewMsgSubmitProposal(
[]sdk.Msg{mkTestLegacyContent(t)},
sdk.Coins{sdk.NewInt64Coin(sdk.DefaultBondDenom, 100000)},
@ -106,25 +100,17 @@ func TestTickExpiredDepositPeriod(t *testing.T) {
require.NoError(t, err)
require.NotNil(t, res)
checkInactiveProposalsQueue(t, ctx, suite.GovKeeper)
newHeader := ctx.HeaderInfo()
newHeader.Time = ctx.HeaderInfo().Time.Add(time.Duration(1) * time.Second)
ctx = ctx.WithHeaderInfo(newHeader)
checkInactiveProposalsQueue(t, ctx, suite.GovKeeper)
params, _ := suite.GovKeeper.Params.Get(ctx)
newHeader = ctx.HeaderInfo()
newHeader.Time = ctx.HeaderInfo().Time.Add(*params.MaxDepositPeriod)
ctx = ctx.WithHeaderInfo(newHeader)
checkInactiveProposalsQueue(t, ctx, suite.GovKeeper)
err = gov.EndBlocker(ctx, suite.GovKeeper)
require.NoError(t, err)
checkInactiveProposalsQueue(t, ctx, suite.GovKeeper)
}
func TestTickMultipleExpiredDepositPeriod(t *testing.T) {
@ -134,8 +120,6 @@ func TestTickMultipleExpiredDepositPeriod(t *testing.T) {
addrs := simtestutil.AddTestAddrs(suite.BankKeeper, suite.StakingKeeper, ctx, 10, valTokens)
govMsgSvr := keeper.NewMsgServerImpl(suite.GovKeeper)
checkInactiveProposalsQueue(t, ctx, suite.GovKeeper)
newProposalMsg, err := v1.NewMsgSubmitProposal(
[]sdk.Msg{mkTestLegacyContent(t)},
sdk.Coins{sdk.NewInt64Coin(sdk.DefaultBondDenom, 100000)},
@ -151,14 +135,10 @@ func TestTickMultipleExpiredDepositPeriod(t *testing.T) {
require.NoError(t, err)
require.NotNil(t, res)
checkInactiveProposalsQueue(t, ctx, suite.GovKeeper)
newHeader := ctx.HeaderInfo()
newHeader.Time = ctx.HeaderInfo().Time.Add(time.Duration(2) * time.Second)
ctx = ctx.WithHeaderInfo(newHeader)
checkInactiveProposalsQueue(t, ctx, suite.GovKeeper)
newProposalMsg2, err := v1.NewMsgSubmitProposal(
[]sdk.Msg{mkTestLegacyContent(t)},
sdk.Coins{sdk.NewInt64Coin(sdk.DefaultBondDenom, 100000)},
@ -179,17 +159,12 @@ func TestTickMultipleExpiredDepositPeriod(t *testing.T) {
newHeader.Time = ctx.HeaderInfo().Time.Add(*params.MaxDepositPeriod).Add(time.Duration(-1) * time.Second)
ctx = ctx.WithHeaderInfo(newHeader)
checkInactiveProposalsQueue(t, ctx, suite.GovKeeper)
require.NoError(t, gov.EndBlocker(ctx, suite.GovKeeper))
checkInactiveProposalsQueue(t, ctx, suite.GovKeeper)
newHeader = ctx.HeaderInfo()
newHeader.Time = ctx.HeaderInfo().Time.Add(time.Duration(5) * time.Second)
ctx = ctx.WithHeaderInfo(newHeader)
checkInactiveProposalsQueue(t, ctx, suite.GovKeeper)
require.NoError(t, gov.EndBlocker(ctx, suite.GovKeeper))
checkInactiveProposalsQueue(t, ctx, suite.GovKeeper)
}
func TestTickPassedDepositPeriod(t *testing.T) {
@ -216,21 +191,68 @@ func TestTickPassedDepositPeriod(t *testing.T) {
proposalID := res.ProposalId
checkInactiveProposalsQueue(t, ctx, suite.GovKeeper)
newHeader := ctx.HeaderInfo()
newHeader.Time = ctx.HeaderInfo().Time.Add(time.Duration(1) * time.Second)
ctx = ctx.WithHeaderInfo(newHeader)
checkInactiveProposalsQueue(t, ctx, suite.GovKeeper)
newDepositMsg := v1.NewMsgDeposit(addrs[1], proposalID, sdk.Coins{sdk.NewInt64Coin(sdk.DefaultBondDenom, 100000)})
res1, err := govMsgSvr.Deposit(ctx, newDepositMsg)
require.NoError(t, err)
require.NotNil(t, res1)
}
checkActiveProposalsQueue(t, ctx, suite.GovKeeper)
func TestProposalDepositRefundFailEndBlocker(t *testing.T) {
suite := createTestSuite(t)
app := suite.App
ctx := app.BaseApp.NewContext(false)
addrs := simtestutil.AddTestAddrs(suite.BankKeeper, suite.StakingKeeper, ctx, 10, valTokens)
govMsgSvr := keeper.NewMsgServerImpl(suite.GovKeeper)
depositMultiplier := getDepositMultiplier(v1.ProposalType_PROPOSAL_TYPE_STANDARD)
proposalCoins := sdk.Coins{sdk.NewCoin(sdk.DefaultBondDenom, suite.StakingKeeper.TokensFromConsensusPower(ctx, 15*depositMultiplier))}
// create a proposal that empties the gov module account
// which will cause the proposal deposit refund to fail
newProposalMsg, err := v1.NewMsgSubmitProposal(
[]sdk.Msg{},
proposalCoins,
addrs[0].String(),
"metadata",
"proposal",
"description of proposal",
v1.ProposalType_PROPOSAL_TYPE_STANDARD,
)
require.NoError(t, err)
res, err := govMsgSvr.SubmitProposal(ctx, newProposalMsg)
require.NoError(t, err)
require.NotNil(t, res)
proposal, err := suite.GovKeeper.Proposals.Get(ctx, res.ProposalId)
require.NoError(t, err)
require.Equal(t, v1.StatusVotingPeriod, proposal.Status)
proposalID := res.ProposalId
err = suite.GovKeeper.AddVote(ctx, proposalID, addrs[0], v1.NewNonSplitVoteOption(v1.OptionYes), "metadata")
require.NoError(t, err)
// empty the gov module account before the proposal ends
err = suite.BankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, addrs[1], proposalCoins)
require.NoError(t, err)
// fast forward to the end of the voting period
newHeader := ctx.HeaderInfo()
newHeader.Time = proposal.VotingEndTime.Add(time.Duration(100) * time.Second)
ctx = ctx.WithHeaderInfo(newHeader)
err = gov.EndBlocker(ctx, suite.GovKeeper)
require.NoError(t, err) // no error, means does not halt the chain
events := ctx.EventManager().Events()
attr, ok := events.GetAttributes(types.AttributeKeyProposalDepositError)
require.True(t, ok)
require.Contains(t, attr[0].Value, "failed to refund or burn deposits")
}
func TestTickPassedVotingPeriod(t *testing.T) {
@ -258,9 +280,6 @@ func TestTickPassedVotingPeriod(t *testing.T) {
SortAddresses(addrs)
govMsgSvr := keeper.NewMsgServerImpl(suite.GovKeeper)
checkInactiveProposalsQueue(t, ctx, suite.GovKeeper)
checkActiveProposalsQueue(t, ctx, suite.GovKeeper)
proposalCoins := sdk.Coins{sdk.NewCoin(sdk.DefaultBondDenom, suite.StakingKeeper.TokensFromConsensusPower(ctx, 5*depositMultiplier))}
newProposalMsg, err := v1.NewMsgSubmitProposal([]sdk.Msg{mkTestLegacyContent(t)}, proposalCoins, addrs[0].String(), "", "Proposal", "description of proposal", tc.proposalType)
require.NoError(t, err)
@ -291,9 +310,6 @@ func TestTickPassedVotingPeriod(t *testing.T) {
newHeader.Time = ctx.HeaderInfo().Time.Add(*params.MaxDepositPeriod).Add(*votingPeriod)
ctx = ctx.WithHeaderInfo(newHeader)
checkInactiveProposalsQueue(t, ctx, suite.GovKeeper)
checkActiveProposalsQueue(t, ctx, suite.GovKeeper)
proposal, err := suite.GovKeeper.Proposals.Get(ctx, res.ProposalId)
require.NoError(t, err)
require.Equal(t, v1.StatusVotingPeriod, proposal.Status)
@ -302,13 +318,10 @@ func TestTickPassedVotingPeriod(t *testing.T) {
require.NoError(t, err)
if tc.proposalType != v1.ProposalType_PROPOSAL_TYPE_EXPEDITED {
checkActiveProposalsQueue(t, ctx, suite.GovKeeper)
return
}
// If expedited, it should be converted to a regular proposal instead.
checkActiveProposalsQueue(t, ctx, suite.GovKeeper)
proposal, err = suite.GovKeeper.Proposals.Get(ctx, res.ProposalId)
require.Nil(t, err)
require.Equal(t, v1.StatusVotingPeriod, proposal.Status)
@ -490,8 +503,6 @@ func TestExpeditedProposal_PassAndConversionToRegular(t *testing.T) {
createValidators(t, stakingMsgSvr, ctx, []sdk.ValAddress{valAddr}, []int64{10})
_, err = suite.StakingKeeper.EndBlocker(ctx)
require.NoError(t, err)
checkInactiveProposalsQueue(t, ctx, suite.GovKeeper)
checkActiveProposalsQueue(t, ctx, suite.GovKeeper)
macc := suite.GovKeeper.GetGovernanceAccount(ctx)
require.NotNil(t, macc)
@ -524,9 +535,6 @@ func TestExpeditedProposal_PassAndConversionToRegular(t *testing.T) {
newHeader.Time = ctx.HeaderInfo().Time.Add(*params.MaxDepositPeriod).Add(*params.ExpeditedVotingPeriod)
ctx = ctx.WithHeaderInfo(newHeader)
checkInactiveProposalsQueue(t, ctx, suite.GovKeeper)
checkActiveProposalsQueue(t, ctx, suite.GovKeeper)
proposal, err := suite.GovKeeper.Proposals.Get(ctx, res.ProposalId)
require.Nil(t, err)
require.Equal(t, v1.StatusVotingPeriod, proposal.Status)
@ -541,8 +549,6 @@ func TestExpeditedProposal_PassAndConversionToRegular(t *testing.T) {
err = gov.EndBlocker(ctx, suite.GovKeeper)
require.NoError(t, err)
if tc.expeditedPasses {
checkActiveProposalsQueue(t, ctx, suite.GovKeeper)
proposal, err = suite.GovKeeper.Proposals.Get(ctx, res.ProposalId)
require.Nil(t, err)
@ -562,7 +568,6 @@ func TestExpeditedProposal_PassAndConversionToRegular(t *testing.T) {
}
// Expedited proposal should be converted to a regular proposal instead.
checkActiveProposalsQueue(t, ctx, suite.GovKeeper)
proposal, err = suite.GovKeeper.Proposals.Get(ctx, res.ProposalId)
require.Nil(t, err)
require.Equal(t, v1.StatusVotingPeriod, proposal.Status)
@ -583,9 +588,6 @@ func TestExpeditedProposal_PassAndConversionToRegular(t *testing.T) {
newHeader.Time = ctx.HeaderInfo().Time.Add(*params.MaxDepositPeriod).Add(*params.VotingPeriod)
ctx = ctx.WithHeaderInfo(newHeader)
checkInactiveProposalsQueue(t, ctx, suite.GovKeeper)
checkActiveProposalsQueue(t, ctx, suite.GovKeeper)
if tc.regularEventuallyPassing {
// Validator votes YES, letting the converted regular proposal pass.
err = suite.GovKeeper.AddVote(ctx, proposal.Id, addrs[0], v1.NewNonSplitVoteOption(v1.OptionYes), "metadata")
@ -602,8 +604,6 @@ func TestExpeditedProposal_PassAndConversionToRegular(t *testing.T) {
submitterEventualBalance := suite.BankKeeper.GetAllBalances(ctx, addrs[0])
depositorEventualBalance := suite.BankKeeper.GetAllBalances(ctx, addrs[1])
checkActiveProposalsQueue(t, ctx, suite.GovKeeper)
proposal, err = suite.GovKeeper.Proposals.Get(ctx, res.ProposalId)
require.Nil(t, err)
@ -655,21 +655,3 @@ func getDepositMultiplier(proposalType v1.ProposalType) int64 {
return 1
}
}
func checkActiveProposalsQueue(t *testing.T, ctx sdk.Context, k *keeper.Keeper) {
t.Helper()
err := k.ActiveProposalsQueue.Walk(ctx, collections.NewPrefixUntilPairRange[time.Time, uint64](ctx.HeaderInfo().Time), func(key collections.Pair[time.Time, uint64], value uint64) (stop bool, err error) {
return false, err
})
require.NoError(t, err)
}
func checkInactiveProposalsQueue(t *testing.T, ctx sdk.Context, k *keeper.Keeper) {
t.Helper()
err := k.InactiveProposalsQueue.Walk(ctx, collections.NewPrefixUntilPairRange[time.Time, uint64](ctx.HeaderInfo().Time), func(key collections.Pair[time.Time, uint64], value uint64) (stop bool, err error) {
return false, err
})
require.NoError(t, err)
}

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

@ -48,6 +48,19 @@ func (k Keeper) DeleteAndBurnDeposits(ctx context.Context, proposalID uint64) er
return k.bankKeeper.BurnCoins(ctx, k.authKeeper.GetModuleAddress(types.ModuleName), coinsToBurn)
}
// RefundAndDeleteDeposits refunds and deletes all the deposits on a specific proposal.
func (k Keeper) RefundAndDeleteDeposits(ctx context.Context, proposalID uint64) error {
return k.IterateDeposits(ctx, proposalID, func(key collections.Pair[uint64, sdk.AccAddress], deposit v1.Deposit) (bool, error) {
depositor := key.K2()
err := k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, depositor, deposit.Amount)
if err != nil {
return false, err
}
err = k.Deposits.Remove(ctx, key)
return false, err
})
}
// IterateDeposits iterates over all the proposals deposits and performs a callback function
func (k Keeper) IterateDeposits(ctx context.Context, proposalID uint64, cb func(key collections.Pair[uint64, sdk.AccAddress], value v1.Deposit) (bool, error)) error {
rng := collections.NewPrefixedPairRange[uint64, sdk.AccAddress](proposalID)
@ -265,19 +278,6 @@ func (k Keeper) ChargeDeposit(ctx context.Context, proposalID uint64, destAddres
return nil
}
// RefundAndDeleteDeposits refunds and deletes all the deposits on a specific proposal.
func (k Keeper) RefundAndDeleteDeposits(ctx context.Context, proposalID uint64) error {
return k.IterateDeposits(ctx, proposalID, func(key collections.Pair[uint64, sdk.AccAddress], deposit v1.Deposit) (bool, error) {
depositor := key.K2()
err := k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, depositor, deposit.Amount)
if err != nil {
return false, err
}
err = k.Deposits.Remove(ctx, key)
return false, err
})
}
// 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.

16
x/gov/types/events.go
View File

@ -9,13 +9,15 @@ const (
EventTypeActiveProposal = "active_proposal"
EventTypeCancelProposal = "cancel_proposal"
AttributeKeyProposalResult = "proposal_result"
AttributeKeyVoter = "voter"
AttributeKeyOption = "option"
AttributeKeyProposalID = "proposal_id"
AttributeKeyProposalMessages = "proposal_messages" // Msg type_urls in the proposal
AttributeKeyVotingPeriodStart = "voting_period_start"
AttributeKeyProposalLog = "proposal_log" // log of proposal execution
AttributeKeyProposalResult = "proposal_result"
AttributeKeyVoter = "voter"
AttributeKeyOption = "option"
AttributeKeyProposalID = "proposal_id"
AttributeKeyProposalMessages = "proposal_messages" // Msg type_urls in the proposal
AttributeKeyVotingPeriodStart = "voting_period_start"
AttributeKeyProposalLog = "proposal_log" // log of proposal execution
AttributeKeyProposalDepositError = "proposal_deposit_error" // error on proposal deposit refund/burn
AttributeValueProposalDropped = "proposal_dropped" // didn't meet min deposit
AttributeValueProposalPassed = "proposal_passed" // met vote quorum
AttributeValueProposalRejected = "proposal_rejected" // didn't meet vote quorum