Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(sims): skip operations that use denoms that can't be sent #20399

Merged
merged 3 commits into from
May 15, 2024
+53 −3
Merged

fix(sims): skip operations that use denoms that can't be sent #20399

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9f7a801
fix\(sims\): skip operations that use denoms that can\'t be sent
facundomedica May 15, 2024
9560be7
remove test print
facundomedica May 15, 2024
80f57e2
make mocks
facundomedica May 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions x/distribution/simulation/operations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,18 @@ func SimulateMsgWithdrawDelegatorReward(txConfig client.TxConfig, ak types.Accou

msg := types.NewMsgWithdrawDelegatorReward(addr, validator.GetOperator())

// get outstanding rewards so we can first check if the withdrawable coins are sendable
outstanding, err := k.GetValidatorOutstandingRewardsCoins(ctx, sdk.ValAddress(delAddr))
if err != nil {
return simtypes.NoOpMsg(types.ModuleName, sdk.MsgTypeURL(&types.MsgWithdrawDelegatorReward{}), "error getting outstanding rewards"), nil, err
}

for _, v := range outstanding {
if !bk.IsSendEnabledDenom(ctx, v.Denom) {
return simtypes.NoOpMsg(types.ModuleName, sdk.MsgTypeURL(msg), "denom send not enabled: "+v.Denom), nil, nil
}
}
Comment on lines +167 to +171
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding a log message or metric increment here to track how often operations are skipped due to non-sendable denominations. This can help in monitoring and debugging.

+ // Log or metric increment can be added here for monitoring
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
for _, v := range outstanding {
if !bk.IsSendEnabledDenom(ctx, v.Denom) {
return simtypes.NoOpMsg(types.ModuleName, sdk.MsgTypeURL(msg), "denom send not enabled: "+v.Denom), nil, nil
}
}
for _, v := range outstanding {
if !bk.IsSendEnabledDenom(ctx, v.Denom) {
// Log or metric increment can be added here for monitoring
return simtypes.NoOpMsg(types.ModuleName, sdk.MsgTypeURL(msg), "denom send not enabled: "+v.Denom), nil, nil
}
}


txCtx := simulation.OperationInput{
R: r,
App: app,
Expand Down
1 change: 1 addition & 0 deletions x/distribution/types/expected_keepers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ type BankKeeper interface {
SendCoinsFromAccountToModule(ctx context.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error

BlockedAddr(addr sdk.AccAddress) bool
IsSendEnabledDenom(ctx context.Context, denom string) bool
}

// PoolKeeper defines the expected interface needed to fund & distribute pool balances.
Expand Down
8 changes: 8 additions & 0 deletions x/staking/simulation/operations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,10 @@ func SimulateMsgUndelegate(
delAddr, val.GetOperator(), sdk.NewCoin(bondDenom, unbondAmt),
)

if !bk.IsSendEnabledDenom(ctx, bondDenom) {
return simtypes.NoOpMsg(types.ModuleName, sdk.MsgTypeURL(msg), "bond denom send not enabled"), nil, nil
}

// need to retrieve the simulation account associated with delegation to retrieve PrivKey
var simAccount simtypes.Account

Expand Down Expand Up @@ -712,6 +716,10 @@ func SimulateMsgBeginRedelegate(
return simtypes.NoOpMsg(types.ModuleName, msgType, "bond denom not found"), nil, err
}

if !bk.IsSendEnabledDenom(ctx, bondDenom) {
return simtypes.NoOpMsg(types.ModuleName, msgType, "bond denom send not enabled"), nil, nil
}

msg := types.NewMsgBeginRedelegate(
delAddr, srcVal.GetOperator(), destVal.GetOperator(),
sdk.NewCoin(bondDenom, redAmt),
Expand Down
1 change: 1 addition & 0 deletions x/staking/types/expected_keepers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ type BankKeeper interface {
DelegateCoinsFromAccountToModule(ctx context.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error

BurnCoins(context.Context, []byte, sdk.Coins) error
IsSendEnabledDenom(ctx context.Context, denom string) bool
}

// ValidatorSet expected properties for the set of all validators (noalias)
Expand Down
Loading