feat(sims): Add simsx support to modules #24145
Conversation
github-actions
bot
added
C:x/staking
C:x/gov
C:x/slashing
C:x/distribution
alpe
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
alpe
force-pushed
the
alex/23762_simsx_4
branch
from
8748d09 to
1d535e9
Compare
alpe
force-pushed
the
alex/23762_simsx_4
branch
from
7c46764 to
c80c5be
Compare
alpe
force-pushed
the
alex/23762_simsx_4
branch
from
c80c5be to
676b292
Compare
alpe
force-pushed
the
alex/23762_simsx_4
branch
from
670a026 to
d141149
Compare
|
@alpe looks like this https://github.com/cosmos/cosmos-sdk/actions/runs/14199605068/job/39784010203?pr=24145#step:6:71 is consistently failing |
📝 WalkthroughWalkthroughThis pull request backports the new simulation framework “simsx” across a wide range of modules. It introduces new simulation message factory functions and methods (e.g., ProposalMsgsX, WeightedOperationsX) while adding deprecation comments to legacy implementations. In several simulation genesis functions, JSON marshalling and console logging have been removed to streamline state generation. Minor adjustments to parameters (e.g., reducing random generation counts) and additional helper methods (such as AddressCodec) are also included. Overall, the changes update simulation operation flows and message construction to leverage the simsx framework. Changes
Sequence Diagram(s)sequenceDiagram
participant Simulator
participant Factory
participant Keeper
participant Reporter
Simulator->>Factory: Request simulation message
Factory->>Keeper: Retrieve account/state data
Keeper-->>Factory: Return necessary data
Factory->>Reporter: Log/report any errors
Factory-->>Simulator: Return constructed simulation message
Assessment against linked issues
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (2)
simsx/environment.go (2)
430-432:⚠️ Potential issueBe cautious of potential integer overflow in Uint64InRange function.
The function converts uint64 to int64 in
int64(max-min)which could lead to integer overflow for large values near uint64 max.func (r *XRand) Uint64InRange(min, max uint64) uint64 { if min >= max { panic("min must be less than max") } - return uint64(r.Int63n(int64(max-min)) + int64(min)) + // Handle potential overflow + diff := max - min + if diff > math.MaxInt64 { + // For large ranges, split the range or use a different approach + return min + (uint64(r.Int63()) % diff) + } + return uint64(r.Int63n(int64(diff)) + int64(min)) }This addresses the integer overflow warning from code scanning.
436-441:⚠️ Potential issuePotential integer overflow in Uint32InRange function.
The function converts uint32 to int in
int(max-min)which could lead to integer overflow for large uint32 values.func (r *XRand) Uint32InRange(min, max uint32) uint32 { if min >= max { panic("min must be less than max") } - return uint32(r.Intn(int(max-min))) + min + // Handle potential overflow + diff := max - min + if diff > math.MaxInt32 { + // For large ranges, split the range or use a different approach + return min + uint32(r.Int31())%diff + } + return uint32(r.Intn(int(diff))) + min }This addresses the integer overflow warning from code scanning.
🧹 Nitpick comments (17)
x/params/simulation/operations.go (1)
22-23: Deprecation Notice Added: The addition of the deprecation comment clearly signals thatSimulateParamChangeProposalContentis slated for removal in the future. This aligns with the broader migration toward the new simsx simulation framework. For improved clarity, consider including a reference or migration note to point users toward the new simulation message factory methods or relevant documentation if available.x/protocolpool/simulation/msg_factory.go (2)
20-29: Improve the MsgCommunityPoolSpendFactory implementation.While the factory function correctly constructs a community pool spend message, there are two issues to address:
The function returns nil for accounts, but should return the authority account since it's required for governance proposal simulation tracking.
The amount is hardcoded to "100stake,2testtoken" which may not be appropriate for all simulation environments.
- return nil, &types.MsgCommunityPoolSpend{ - Authority: testData.ModuleAccountAddress(reporter, "gov"), - Recipient: testData.AnyAccount(reporter).AddressBech32, - Amount: must(sdk.ParseCoinsNormalized("100stake,2testtoken")), - } + govAccount := testData.GetModuleAccount(reporter, "gov") + recipient := testData.AnyAccount(reporter) + // Generate random amounts instead of hardcoded values + randomAmount := testData.RandModuleAccountBalance(reporter, "protocolpool").RandSubsetCoins(reporter) + return []simsx.SimAccount{govAccount}, &types.MsgCommunityPoolSpend{ + Authority: govAccount.Address, + Recipient: recipient.AddressBech32, + Amount: randomAmount, + }
31-36: Consider a more graceful error handling approach in the must function.The current implementation panics on error, which is appropriate for initialization code but could be problematic during simulation.
Consider modifying the function to use the reporter for error handling instead of panicking:
- func must[T any](r T, err error) T { - if err != nil { - panic(err) - } - return r - } + func must[T any](r T, err error) T { + if err != nil { + // This assumes must is only called from within factory functions that have access to a reporter + // If that's not always the case, this approach would need to be adjusted + panic(fmt.Sprintf("unexpected error: %v", err)) + } + return r + }Or alternatively, pass the reporter to the must function:
func must[T any](r T, err error, reporter simsx.SimulationReporter) T { if err != nil { reporter.Skip(fmt.Sprintf("unexpected error: %v", err)) return r // This assumes T has a zero value that indicates failure } return r }simsx/environment.go (3)
110-110: Consider the impact of reducing retry attempts from 5 to 1.Reducing retry attempts from 5 to 1 will make simulations faster but could increase the frequency of skipped operations since there's only one chance to find a matching amount. This might affect simulation coverage and thoroughness.
- amount := b.randomAmount(1, reporter, b.Coins, filters...) + amount := b.randomAmount(3, reporter, b.Coins, filters...)Consider using a middle ground of 3 retries to balance performance and simulation quality.
126-126: Consider the impact of reducing retry attempts from 5 to 1.Similar to the change in RandSubsetCoins, reducing retry attempts to 1 could lead to more skipped operations in your simulations, affecting test coverage.
- amounts := b.randomAmount(1, reporter, sdk.Coins{coin}, filters...) + amounts := b.randomAmount(3, reporter, sdk.Coins{coin}, filters...)Consider using a middle ground of 3 retries for a better balance.
292-292: Consider the impact of reducing retry attempts from 5 to 1.Reducing randomAccount retry attempts to 1 may significantly increase the likelihood of skipped account selections when filters are applied, potentially affecting simulation diversity.
- acc := c.randomAccount(r, 1, filters...) + acc := c.randomAccount(r, 3, filters...)Consider using a middle ground of 3 retries for better selection diversity.
x/feegrant/simulation/msg_factory.go (3)
62-65: Use existing simsx.BlockTime instead of creating a temporary function.A BlockTime function already exists in simsx/context.go that provides the same functionality.
-// temporary solution. use simsx.BlockTime when available -func blockTime(ctx context.Context) time.Time { - return sdk.UnwrapSDKContext(ctx).BlockTime() -} +// Import simsx and use simsx.BlockTime insteadThen update the usage in line 27:
- oneYear := blockTime(ctx).AddDate(1, 0, 0) + oneYear := simsx.BlockTime(ctx).AddDate(1, 0, 0)
41-60: MsgRevokeAllowanceFactory always selects the first grant found.The current implementation always selects the first grant it finds in the iterator, which may lead to non-random simulation behavior.
func MsgRevokeAllowanceFactory(k keeper.Keeper) simsx.SimMsgFactoryFn[*feegrant.MsgRevokeAllowance] { return func(ctx context.Context, testData *simsx.ChainDataSource, reporter simsx.SimulationReporter) ([]simsx.SimAccount, *feegrant.MsgRevokeAllowance) { - var gotGrant *feegrant.Grant - if err := k.IterateAllFeeAllowances(ctx, func(grant feegrant.Grant) bool { - gotGrant = &grant - return true - }); err != nil { + var grants []feegrant.Grant + if err := k.IterateAllFeeAllowances(ctx, func(grant feegrant.Grant) bool { + grants = append(grants, grant) + return false // continue iteration + }); err != nil { reporter.Skip(err.Error()) return nil, nil } - if gotGrant == nil { + if len(grants) == 0 { reporter.Skip("no grant found") return nil, nil } + // Select a random grant + r := testData.Rand() + gotGrant := &grants[r.Intn(len(grants))] granter := testData.GetAccount(reporter, gotGrant.Granter) grantee := testData.GetAccount(reporter, gotGrant.Grantee) msg := feegrant.NewMsgRevokeAllowance(granter.Address, grantee.Address) return []simsx.SimAccount{granter}, &msg } }This change collects all grants and then selects one randomly, improving simulation diversity.
14-39: Grant allowance expiration set to exactly one year.Using a fixed expiration time of one year from now might lead to less diverse simulation scenarios.
func MsgGrantAllowanceFactory(k keeper.Keeper) simsx.SimMsgFactoryFn[*feegrant.MsgGrantAllowance] { return func(ctx context.Context, testData *simsx.ChainDataSource, reporter simsx.SimulationReporter) ([]simsx.SimAccount, *feegrant.MsgGrantAllowance) { granter := testData.AnyAccount(reporter, simsx.WithSpendableBalance()) grantee := testData.AnyAccount(reporter, simsx.ExcludeAccounts(granter)) if reporter.IsSkipped() { return nil, nil } if f, _ := k.GetAllowance(ctx, granter.Address, grantee.Address); f != nil { reporter.Skip("fee allowance exists") return nil, nil } coins := granter.LiquidBalance().RandSubsetCoins(reporter, simsx.WithSendEnabledCoins()) - oneYear := blockTime(ctx).AddDate(1, 0, 0) + // Add some randomness to expiration for more diverse scenarios + daysToAdd := testData.Rand().IntInRange(180, 730) // Between ~6 months and 2 years + expirationTime := simsx.BlockTime(ctx).AddDate(0, 0, daysToAdd) msg, err := feegrant.NewMsgGrantAllowance( - &feegrant.BasicAllowance{SpendLimit: coins, Expiration: &oneYear}, + &feegrant.BasicAllowance{SpendLimit: coins, Expiration: &expirationTime}, granter.Address, grantee.Address, ) if err != nil { reporter.Skip(err.Error()) return nil, nil } return []simsx.SimAccount{granter}, msg } }This change introduces random expiration times between ~6 months and 2 years, creating more diverse simulation scenarios.
x/group/simulation/msg_factory.go (1)
25-52: Consider randomizing the decision policy for broader test coverage.
Currently, the threshold and voting period are fixed. If you want more coverage, randomizing could be beneficial.x/mint/simulation/msg_factory.go (2)
12-29: Well-implemented message factory functionThe
MsgUpdateParamsFactoryfunction follows the same pattern as other modules' factory functions, ensuring consistency across the codebase. The parameter generation uses appropriate random ranges.However, there's a potential issue with the min/max inflation parameters:
Consider adding a check to ensure that InflationMin is always less than InflationMax, since they're generated independently:
- params.InflationMin = sdkmath.LegacyNewDecWithPrec(int64(r.IntInRange(1, 50)), 2) - params.InflationMax = sdkmath.LegacyNewDecWithPrec(int64(r.IntInRange(50, 100)), 2) + // Generate min and max inflation ensuring min < max + minInflation := int64(r.IntInRange(1, 50)) + maxInflation := int64(r.IntInRange(minInflation + 1, 100)) + params.InflationMin = sdkmath.LegacyNewDecWithPrec(minInflation, 2) + params.InflationMax = sdkmath.LegacyNewDecWithPrec(maxInflation, 2)
13-28: Consider adding module-specific documentationWhile the function name is clear, the documentation comment is not as detailed as in other modules.
Consider expanding the comment to be more specific about mint parameters, similar to how other modules document their parameters:
- // MsgUpdateParamsFactory creates a gov proposal for param updates + // MsgUpdateParamsFactory creates a gov proposal for param updates specific to mint. + // It randomizes BlocksPerYear, GoalBonded, inflation rates, and MintDenom.x/slashing/module.go (1)
178-181: Incorrect migration reference in deprecation comment.The deprecation comment incorrectly states to migrate to WeightedOperationsX instead of ProposalMsgsX, which doesn't match the actual replacement method.
-// migrate to WeightedOperationsX. This method is ignored when WeightedOperationsX exists and will be removed in the future +// migrate to ProposalMsgsX. This method is ignored when ProposalMsgsX exists and will be removed in the future.x/gov/simulation/msg_factory.go (4)
71-79: Clarify skip message in cancellation factory.At line 77, the function can randomly target deposit or voting period proposals, but the skip message still reads
"no proposal in deposit state". Consider updating it to reflect both states it checks (deposit or voting).- reporter.Skip("no proposal in deposit state") + reporter.Skip("no proposal in deposit/voting state")
101-101: Consider removing the staticcheck ignore.The
//nolint:staticcheck // used for legacy testingcomment bypasses static analysis warnings. If feasible, address the underlying reason for the warning instead of ignoring it long-term.
123-156: Remove or revise the commented-out deposit code.Line 156 has commented-out code (
// deposit := sdk.Coin{Amount: amount.Add(minAmount), Denom: ...}). If it’s no longer needed, consider removing it to reduce clutter. Otherwise, clarify why it needs to remain.
175-231: Large-scale future ops scheduling could affect performance.Lining up many vote operations (especially if there are hundreds of accounts) can be costly. Consider whether it’s necessary to schedule a vote for each selected account or if batching or sampling fewer accounts would be beneficial.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (52)
CHANGELOG.md(1 hunks)simsx/context.go(1 hunks)simsx/environment.go(3 hunks)x/auth/module.go(2 hunks)x/auth/simulation/genesis.go(0 hunks)x/auth/simulation/msg_factory.go(1 hunks)x/auth/simulation/proposals.go(2 hunks)x/authz/module/module.go(2 hunks)x/authz/simulation/msg_factory.go(1 hunks)x/authz/simulation/operations.go(4 hunks)x/bank/simulation/genesis.go(0 hunks)x/bank/simulation/operations.go(1 hunks)x/bank/simulation/proposals.go(2 hunks)x/distribution/module.go(3 hunks)x/distribution/simulation/genesis.go(0 hunks)x/distribution/simulation/msg_factory.go(1 hunks)x/distribution/simulation/operations.go(6 hunks)x/distribution/simulation/proposals.go(2 hunks)x/evidence/simulation/genesis.go(0 hunks)x/feegrant/module/module.go(2 hunks)x/feegrant/simulation/msg_factory.go(1 hunks)x/feegrant/simulation/operations.go(3 hunks)x/gov/module.go(3 hunks)x/gov/simulation/genesis.go(0 hunks)x/gov/simulation/msg_factory.go(1 hunks)x/gov/simulation/operations.go(13 hunks)x/gov/simulation/proposals.go(2 hunks)x/group/keeper/keeper.go(2 hunks)x/group/module/module.go(3 hunks)x/group/simulation/msg_factory.go(1 hunks)x/group/simulation/operations.go(24 hunks)x/mint/module.go(2 hunks)x/mint/simulation/genesis.go(0 hunks)x/mint/simulation/msg_factory.go(1 hunks)x/mint/simulation/proposals.go(2 hunks)x/nft/module/module.go(3 hunks)x/nft/simulation/msg_factory.go(1 hunks)x/nft/simulation/operations.go(3 hunks)x/params/simulation/operations.go(1 hunks)x/params/simulation/proposals.go(2 hunks)x/protocolpool/module.go(4 hunks)x/protocolpool/simulation/msg_factory.go(1 hunks)x/slashing/module.go(4 hunks)x/slashing/simulation/genesis.go(0 hunks)x/slashing/simulation/msg_factory.go(1 hunks)x/slashing/simulation/operations.go(2 hunks)x/slashing/simulation/proposals.go(2 hunks)x/staking/module.go(2 hunks)x/staking/simulation/genesis.go(0 hunks)x/staking/simulation/msg_factory.go(1 hunks)x/staking/simulation/operations.go(8 hunks)x/staking/simulation/proposals.go(2 hunks)
💤 Files with no reviewable changes (8)
- x/slashing/simulation/genesis.go
- x/distribution/simulation/genesis.go
- x/auth/simulation/genesis.go
- x/evidence/simulation/genesis.go
- x/mint/simulation/genesis.go
- x/bank/simulation/genesis.go
- x/gov/simulation/genesis.go
- x/staking/simulation/genesis.go
🧰 Additional context used
🧬 Code Definitions (14)
x/feegrant/module/module.go (2)
x/feegrant/simulation/operations.go (1)
WeightedOperations(35-74)x/feegrant/simulation/msg_factory.go (2)
MsgGrantAllowanceFactory(14-39)MsgRevokeAllowanceFactory(41-60)
x/group/module/module.go (1)
x/group/simulation/msg_factory.go (14)
MsgCreateGroupFactory(16-23)MsgUpdateGroupAdminFactory(279-294)MsgUpdateGroupMetadataFactory(263-277)MsgUpdateGroupMembersFactory(296-323)MsgCreateGroupPolicyFactory(25-52)MsgCreateGroupWithPolicyFactory(54-78)MsgUpdateGroupPolicyAdminFactory(325-339)MsgUpdateGroupPolicyDecisionPolicyFactory(341-365)MsgUpdateGroupPolicyMetadataFactory(367-380)MsgSubmitProposalFactory(180-220)MsgWithdrawProposalFactory(80-122)MsgVoteFactory(124-178)MsgExecFactory(222-250)MsgLeaveGroupFactory(382-405)
x/mint/simulation/msg_factory.go (6)
x/distribution/simulation/msg_factory.go (1)
MsgUpdateParamsFactory(113-125)x/auth/simulation/msg_factory.go (1)
MsgUpdateParamsFactory(10-25)x/slashing/simulation/msg_factory.go (1)
MsgUpdateParamsFactory(76-91)x/staking/simulation/msg_factory.go (1)
MsgUpdateParamsFactory(292-309)simsx/environment.go (2)
ChainDataSource(248-255)SimAccount(32-37)x/mint/simulation/genesis.go (4)
GoalBonded(18-18)InflationMin(17-17)InflationMax(16-16)InflationRateChange(15-15)
x/auth/simulation/msg_factory.go (5)
x/distribution/simulation/msg_factory.go (1)
MsgUpdateParamsFactory(113-125)x/mint/simulation/msg_factory.go (1)
MsgUpdateParamsFactory(13-29)x/slashing/simulation/msg_factory.go (1)
MsgUpdateParamsFactory(76-91)x/staking/simulation/msg_factory.go (1)
MsgUpdateParamsFactory(292-309)x/auth/simulation/genesis.go (3)
TxSigLimit(16-16)TxSizeCostPerByte(17-17)SigVerifyCostED25519(18-18)
x/slashing/simulation/msg_factory.go (3)
simsx/environment.go (2)
ChainDataSource(248-255)SimAccount(32-37)simsx/context.go (1)
BlockTime(11-14)x/slashing/simulation/genesis.go (5)
DowntimeJailDuration(18-18)SignedBlocksWindow(16-16)MinSignedPerWindow(17-17)SlashFractionDoubleSign(19-19)SlashFractionDowntime(20-20)
x/authz/simulation/msg_factory.go (2)
simsx/environment.go (5)
ChainDataSource(248-255)SimAccount(32-37)WithSpendableBalance(231-235)ExcludeAccounts(180-186)WithSendEnabledCoins(68-77)simsx/context.go (1)
BlockTime(11-14)
x/gov/module.go (2)
x/gov/simulation/proposals.go (3)
ProposalContents(38-46)ProposalMsgs(18-26)SimulateLegacyTextProposalContent(51-56)x/gov/simulation/operations.go (1)
WeightedOperations(48-142)
x/feegrant/simulation/msg_factory.go (2)
simsx/environment.go (5)
ChainDataSource(248-255)SimAccount(32-37)WithSpendableBalance(231-235)ExcludeAccounts(180-186)WithSendEnabledCoins(68-77)simsx/context.go (1)
BlockTime(11-14)
x/gov/simulation/operations.go (2)
x/gov/simulation/msg_factory.go (2)
NewSharedState(358-362)SharedState(353-355)x/group/simulation/msg_factory.go (2)
NewSharedState(478-482)SharedState(473-475)
x/staking/module.go (3)
x/staking/simulation/proposals.go (1)
ProposalMsgs(26-34)x/staking/simulation/msg_factory.go (6)
MsgUpdateParamsFactory(292-309)MsgCreateValidatorFactory(17-67)MsgDelegateFactory(69-86)MsgUndelegateFactory(88-124)MsgBeginRedelegateFactory(149-229)MsgCancelUnbondingDelegationFactory(231-289)x/staking/simulation/operations.go (1)
WeightedOperations(41-108)
x/distribution/simulation/msg_factory.go (6)
simsx/environment.go (3)
ChainDataSource(248-255)SimAccount(32-37)ExcludeAccounts(180-186)x/auth/simulation/msg_factory.go (1)
MsgUpdateParamsFactory(10-25)x/mint/simulation/msg_factory.go (1)
MsgUpdateParamsFactory(13-29)x/slashing/simulation/msg_factory.go (1)
MsgUpdateParamsFactory(76-91)x/staking/simulation/msg_factory.go (1)
MsgUpdateParamsFactory(292-309)x/distribution/simulation/genesis.go (1)
CommunityTax(14-14)
x/staking/simulation/msg_factory.go (8)
x/group/keeper/keeper.go (1)
Keeper(49-84)simsx/msg_factory.go (1)
SimMsgFactoryFn(115-115)simsx/environment.go (5)
ChainDataSource(248-255)SimAccount(32-37)SimAccountFilterFn(174-174)WithDenomBalance(218-222)XRand(395-397)simsx/context.go (1)
BlockTime(11-14)x/distribution/simulation/msg_factory.go (1)
MsgUpdateParamsFactory(113-125)x/auth/simulation/msg_factory.go (1)
MsgUpdateParamsFactory(10-25)x/mint/simulation/msg_factory.go (1)
MsgUpdateParamsFactory(13-29)x/slashing/simulation/msg_factory.go (1)
MsgUpdateParamsFactory(76-91)
x/gov/simulation/msg_factory.go (6)
x/group/keeper/keeper.go (1)
Keeper(49-84)simsx/msg_factory.go (4)
SimMsgFactoryFn(115-115)SimMsgFactoryX(10-25)NewSimMsgFactoryWithFutureOps(94-101)FactoryMethod(44-44)simsx/environment.go (2)
ChainDataSource(248-255)SimAccount(32-37)simsx/context.go (1)
BlockTime(11-14)simsx/registry.go (1)
FutureOpsRegistry(28-30)x/gov/simulation/genesis.go (3)
MinDeposit(18-18)ExpeditedMinDeposit(19-19)VotingPeriod(22-22)
x/group/simulation/operations.go (1)
x/group/simulation/msg_factory.go (2)
NewSharedState(478-482)SharedState(473-475)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: test-system
- GitHub Check: test-system-legacy
- GitHub Check: tests (01)
- GitHub Check: tests (00)
- GitHub Check: test-sim-nondeterminism
- GitHub Check: test-integration
- GitHub Check: test-e2e
- GitHub Check: Analyze
- GitHub Check: Gosec
- GitHub Check: Summary
🔇 Additional comments (180)
x/bank/simulation/operations.go (1)
19-20: Properly documenting future deprecation.The comment indicates these operation weights constants will be removed in a future update, which aligns with the migration to simsx framework. This helps prepare developers for eventual removal.
simsx/context.go (1)
10-14: Helpful utility function added for accessing block time.The
BlockTimefunction provides a clean way to retrieve block time from a context.Context, which is useful for modules that use context.Context rather than sdk.Context directly. This supports the decoupling efforts mentioned in previous discussions.The utility abstraction is appropriate as many keepers use
context.Contextalready, as mentioned in a previous comment by alpe: "Many keepers usecontext.Contextalready. It is part of the decoupling. I would prefer to keep this helper method."x/params/simulation/proposals.go (2)
8-8: Appropriate deprecation notice added.Adding this comment clearly indicates these constants will be removed in the future as part of the simsx migration.
17-18: Good practice for documenting future removal.Explicitly marking the
ProposalContentsfunction as "will be removed in the future" helps prepare developers for the eventual migration to simsx alternatives.CHANGELOG.md (1)
43-43: Properly documented the new simsx framework in CHANGELOG.The addition to the "Features" section correctly references both PRs (#24062 and #24145) and provides a clear, concise description of the new simsx framework. This helps users understand what's being added and where to find more information.
x/group/keeper/keeper.go (2)
7-7: Added import for address codec support.The import for
cosmossdk.io/core/addressis added to support the new address codec method. This is consistent with the PR objective of adding simsx support.
225-227: Well-designed accessor method for address codec.This method provides clean access to the account keeper's address codec, making it accessible to simulation code. The implementation is simple and follows the module's patterns for accessor methods.
x/nft/simulation/operations.go (4)
19-26: Marked constants for future removal.These constants have been appropriately marked for future removal as part of the transition to the simsx framework, providing clear guidance for developers.
28-29: Marked type declaration for future removal.The TypeMsgSend variable has been marked for deprecation, consistent with the module's transition to message factories.
31-33: Marked WeightedOperations for migration to message factories.The comment clearly indicates the migration path from the current operations to the new message factory approach.
58-60: Marked SimulateMsgSend for migration to message factories.Similar to the WeightedOperations function, this simulation method is marked for replacement with message factories.
x/distribution/simulation/operations.go (3)
21-33: Marked simulation constants for future removal.The operation weight constants are annotated with deprecation comments, providing clear guidance on future code direction.
35-37: Marked WeightedOperations for migration to message factories.The comment clearly indicates the transition strategy to move from the current operations implementation to message factories.
91-93: Consistent deprecation notices across simulation methods.All simulation methods are consistently marked with the same deprecation notice, providing a unified transition path to message factories.
Also applies to: 132-134, 184-186, 246-248
x/authz/module/module.go (3)
22-22: Added simsx import for new simulation framework.The added import supports the new WeightedOperationsX method for the simsx framework.
199-207: Clear deprecation notice for existing WeightedOperations method.The comment indicates that this method will be ignored when WeightedOperationsX exists and will be removed in the future, providing a clear transition path.
209-214: Well-implemented WeightedOperationsX for simsx support.This new method properly registers three weighted operations with their respective factories, following the simsx framework pattern. The approach is consistent with the PR objective of adding simsx support across modules.
x/authz/simulation/operations.go (7)
22-22: Good practice marking deprecated codeThis comment clearly indicates that the defined variables will be removed in the future, which helps prepare developers for the upcoming changes when migrating to the simsx framework.
30-31: Good practice marking deprecated codeAdding the deprecation notice to these simulation operation weights helps developers understand that they should start using the new message factory approach instead.
38-39: Good practice marking deprecated codeConsistent deprecation comments across all related constants is a good approach for maintaining clarity during the transition to simsx.
46-47: Detailed deprecation commentThis provides more specific guidance by mentioning that the future replacement will be a message factory, which is helpful for developers who will need to migrate their code.
93-94: Consistent deprecation commentConsistently marking all related simulation functions with deprecation notices maintains a clear migration path for the codebase.
167-168: Consistent deprecation commentThe consistency in the deprecation comments across all simulation methods helps guide the eventual migration to the simsx message factory approach.
237-238: Consistent deprecation commentThe comment style is consistent with the other simulation methods, maintaining a clear pattern throughout the file.
x/bank/simulation/proposals.go (4)
14-15: Good practice marking deprecated codeThis comment clearly indicates that the simulation operation weight constant will be removed in the future, helping prepare for the simsx transition.
17-18: Minor alignment fixThe alignment has been improved for better readability, which is a good practice.
21-22: Specific migration guidanceThis comment provides more detailed guidance by explicitly mentioning the migration path to message factories, which is more helpful than a generic deprecation notice.
33-34: Consistent deprecation commentThe consistency in deprecation comments throughout the file maintains a clear pattern for developers to follow.
x/group/module/module.go (3)
21-22: Good addition of the simsx importAdding the import for the simsx package is necessary for implementing the new simulation framework support.
165-166: Clear migration path commentThis comment is particularly helpful as it not only marks the method for future removal but also explicitly states that it will be ignored when the new
WeightedOperationsXmethod exists, providing a clear migration path.
174-192: Well-implemented new simulation framework supportThe new
WeightedOperationsXmethod is well-structured and comprehensive. It:
- Creates a shared state for simulations
- Uses backward-compatible keys for weights
- Registers all necessary message factories with appropriate weights
- Follows the pattern established across other modules for consistency
This implementation properly supports all group module operations in the new simsx framework.
x/nft/module/module.go (3)
23-24: Good addition of the simsx importAdding the import for the simsx package is necessary for implementing the new simulation framework support.
148-149: Clear migration path commentThis comment clearly indicates that the method will eventually be removed and that
WeightedOperationsXwill take precedence when both exist, providing a smooth transition path.
157-160: Simple and clean simsx implementationThe new
WeightedOperationsXmethod is simple but effective. It properly registers the NFT module's send message factory with an appropriate weight, following the same pattern used in other modules for consistency.x/feegrant/module/module.go (2)
25-25: Import for new simsx package.The simsx package is added to support the new simulation framework, which aligns with the PR objective of implementing simsx support across modules.
204-205: Appropriate deprecation notice for WeightedOperations method.The comment clearly indicates that this method will be migrated to the newer WeightedOperationsX approach, which follows good deprecation practices by informing developers of the migration path.
x/nft/simulation/msg_factory.go (1)
13-34:✅ Verification successful
Well-implemented message factory function for NFT sending simulation.
The MsgSendFactory function follows the simsx pattern correctly by:
- Selecting appropriate accounts (sender with balance, different receiver)
- Properly handling simulation skipping
- Using the randNFT function to generate test data
- Correctly constructing the message with all required fields
- Returning the sender account that needs to sign the transaction
Let's verify that the randNFT function exists and is implemented correctly:
🏁 Script executed:
#!/bin/bash # Check for the randNFT function implementation rg -A 10 "func randNFT" --type goLength of output: 676
Verified: NFT Message Factory and randNFT Implementation Are Correct
The simulation factory function is implemented according to the simsx pattern and operates as expected. Our verification confirms that:
- The
randNFTfunction is defined inx/nft/simulation/operations.goand follows the intended logic by first fetching a class and then returning an existing NFT (if available) or proceeding to create one.- The factory function correctly selects accounts, handles simulation skips, and constructs the message with all required fields.
No further changes are necessary.
x/gov/simulation/proposals.go (3)
13-14: Clear deprecation notice for OpWeightSubmitTextProposal constant.This comment appropriately indicates that the constant will be removed in the future, which helps prepare developers for the upcoming changes as part of the simsx migration.
17-18: Clear deprecation notice for ProposalMsgs function.This comment appropriately indicates that the function will be removed in the future, aligning with the simsx migration strategy.
30-31: Clear deprecation notice for SimulateTextProposal function.This comment appropriately indicates that the function will be removed in the future as part of the simsx migration.
x/protocolpool/simulation/msg_factory.go (1)
11-19: Well-implemented message factory for community pool funding.The MsgFundCommunityPoolFactory function follows the simsx pattern correctly and creates realistic simulation scenarios by:
- Selecting an account with spendable balance
- Using a random subset of the funder's liquid balance
- Properly constructing the message with appropriate parameters
- Returning the funder account that needs to sign the transaction
x/protocolpool/module.go (3)
145-146: LGTM: Clear deprecation notice.Good practice to clearly mark the deprecation path and note that this method will be ignored when ProposalMsgsX exists.
154-156: LGTM: Clear deprecation notice.Good practice to clearly mark the deprecation path and note that this method will be ignored when WeightedOperationsX exists.
166-174: LGTM: Well-implemented simsx integration.The implementation of ProposalMsgsX and WeightedOperationsX follows the simsx framework design pattern correctly, registering operations with appropriate weights.
x/feegrant/simulation/operations.go (1)
20-21: LGTM: Well-documented deprecation notices.The deprecation comments clearly indicate that these constants, variables, and functions will be replaced by the message factory approach in the future, providing a clear path forward for developers.
Also applies to: 28-29, 34-35, 77-78, 133-134
x/staking/simulation/operations.go (8)
22-22: Acknowledged comment for future removal
No functional changes are introduced here, and the comment aligns with migrating away from these constants.
40-40: Acknowledged comment for future removal
Referencing the new message factories is a good practice. No functional changes to flag.
111-111: Acknowledged comment for future removal
This marker effectively communicates deprecation plans.
199-199: Acknowledged comment for future removal
No concerns: the plan to move this logic to factories looks consistent.
275-275: Acknowledged comment for future removal
Retaining the existing logic for now appears fine.
354-354: Acknowledged comment for future removal
This comment ensures the team is aware of the eventual deprecation.
470-470: Acknowledged comment for future removal
All future operations will presumably rely on the new factories.
571-571: Acknowledged comment for future removal
No functional impact; consistent with the modularization plan.x/authz/simulation/msg_factory.go (4)
14-39: Review of MsgGrantFactory
This factory correctly selects separate granter and grantee with spendable balance, optionally sets expirations, and randomizes authorization types (Send/G¯eneric). It properly skips on error to avoid simulation crashes. Consider whether indefinite grants (no expiration) are acceptable when the random timestamp is before the current block time. Otherwise, looks solid.
41-63: Review of MsgExecFactory
Nicely filters for bank-related authorizations and ensures the spend limit is respected. You could explore multi-msg execution in future improvements, but for a single message scenario, this is fine.
65-76: Review of MsgRevokeFactory
Straightforward revocation approach. It cleanly handles missing or inapplicable grants viareporter.Skip. This meets the simulation needs.
78-107: Review of findGrant helper
The iteration and filter approach is effective for discovering a suitable authorization. Only the first matching grant is used, which is typical in this context. No concerns here.x/staking/simulation/msg_factory.go (10)
17-67: Review of MsgCreateValidatorFactory
Selecting an account that does not already have a validator or used consensus address is a good safeguard. Randomizing commission rates and descriptions is aligned with simulation needs. The self-delegation logic is consistent with typical staking constraints.
69-86: Review of MsgDelegateFactory
Retrieving a random validator and verifying its exchange rate, then selecting an account to delegate is clean. The approach is minimal yet sufficient for simulation coverage.
88-124: Review of MsgUndelegateFactory
The logic properly checks for maximum unbonding entries and ensures the delegation is nonzero before undelegating. This is a solid approach for simulating undelegation scenarios.
126-147: Review of MsgEditValidatorFactory
Fetching a random validator, verifying commission changes, and constructing the edit validator message is correct and consistent with the module’s constraints.
149-229: Review of MsgBeginRedelegateFactory
The flow for selecting a source validator, checking for a valid amount, and then picking a distinct destination validator is carefully implemented. The skipping conditions (if rates are invalid or if the entire balance would be redelegated) reflect real-world constraints.
231-289: Review of MsgCancelUnbondingDelegationFactory
The logic thoroughly checks unbonding entries and ensures that processed or zero-balance unbondings are skipped. This prevents simulation breaks. The fallback approach of picking the first matching creation height is a sensible workaround.
291-309: Review of MsgUpdateParamsFactory
Allowing randomization of certain parameters but preserving critical fields (e.g., denom) helps test reconfiguration without destabilizing core staking. This approach is consistent with other modules' param factories.
311-319: Review of randomValidator
Returns a random validator or skips if none are available. Straightforward approach and appropriate for simulation demands.
320-327: Review of assertKeyUnused
Checks for existing usage of the consensus key to prevent collisions in the simulation. This skip logic is beneficial for preventing double usage in a single block.
329-334: Review of must helper
Panic-based error handling is acceptable in this simulation context, as normal usage is guarded by skip logic elsewhere. This is a concise utility.x/group/simulation/msg_factory.go (18)
16-23: FunctionMsgCreateGroupFactorylooks good.
No major problems or suggestions.
54-78: Randomization approach is well done.
The usage ofr.Float32() < 0.5forGroupPolicyAsAdminis a good approach to test both possibilities.
80-122: Proper error handling and skip logic
The approach to skip when no proposals are found or if the proposal is not in a suitable state is correct.
124-178: Skipping when a user has already voted is a good approach
Ensures we do not produce invalid double votes.
180-220: Logic for random group policy and random group member selection
Ensures coverage of various group configurations.
222-250: Good approach to test accepted proposal execution
Skipping if none are accepted is consistent with the logic.
252-261: Multiple attempts ensure coverage
Tries up to five times to find a group policy with an account-based admin. Looks good.
263-277: Metadata update is straightforward
No issues found.
279-294: Admin update logic
This approach (selecting a new admin) is consistent with typical group admin changes.
296-323: Allows both additions and removals
Setting an existing member's weight to zero for removal is a valid approach.
325-339: Implementation for updating group policy admin
Looks consistent with the group logic.
341-365: Use of random threshold and voting period
Promotes broader coverage in simulation.
367-380: Policy metadata update
No concerns.
382-405: Leaving group simulation
Implementation appears coherent with the group membership logic.
407-421: Member generation strategy
Configuration of random weights from 1–10 is sufficient for simulation.
423-444: Group selection logic
The skip logic is correct in the scenario where no groups exist.
446-470: Tries multiple times to find a group policy
Ok approach for ensuring coverage.
472-490: Shared state usage
Atomic usage is correct for concurrency safety.x/group/simulation/operations.go (18)
25-25: Noting the planned removal
All these lines are comments indicating these constants or references "will be removed in the future." No immediate impact.Also applies to: 44-44, 64-64
83-83: Migration plan
MarkingWeightedOperationsfor future removal is consistent with the new approach using message factories.
152-152: Switch toNewSharedState()
This usage is consistent with the new shared state approach inmsg_factory.go.
224-224: Deprecation notice
SimulateMsgCreateGroupis marked for removal in favor of the new factories approach.
263-263: Deprecation notice
Clear messaging about the eventual removal ofSimulateMsgCreateGroupWithPolicyis helpful.
346-346: Refactored call tosimulateMsgCreateGroupPolicy
UsingNewSharedState()in place of the older approach ensures consistency with the new architecture.
425-425: Refactored call tosimulateMsgSubmitProposal
Aligned with the new shared state initialization approach.
517-517: New shared state usage
Ensures a consistent approach with the new constructor pattern.
593-593: Use ofNewSharedState()
Maintains a uniform pattern across simulation message methods.
661-661: Consistent usage
The new constructor is used in all relevantsimulateMsgUpdateGroupMemberscalls.
755-755: Refactored call
UsingNewSharedState()for policy admin changes is correct.
831-831: Consistent with the new pattern
No concerns here forsimulateMsgUpdateGroupPolicyDecisionPolicy.
905-905: Refactored approach
Aligns with message factory architecture for updating group policy metadata.
973-973: Refactored usage
Retains consistent usage ofNewSharedState()for withdrawal.
1091-1091: Refactor
Correct usage of the new shared state approach insimulateMsgVote.
1209-1209: Use ofNewSharedState()
Remains consistent with the simulation approach forsimulateMsgExec.
1300-1300: Final function uses the new approach
UsingNewSharedState()forsimulateMsgLeaveGroupensures consistency.
1367-1367: Added*SharedStateparameter
This aligns the helper functions with the new stateful approach in simulations.Also applies to: 1405-1405
x/mint/simulation/proposals.go (3)
16-16: Future removal note
Indicates thatDefaultWeightMsgUpdateParamsis planned for deprecation.
24-25: Deprecation notice
ProposalMsgswill be replaced by new message factories.
36-37: Migration plan
SimulateMsgUpdateParamsis slated to be replaced by the new factories approach.x/staking/simulation/proposals.go (3)
17-17: Proper deprecation comment appliedThe comment clearly indicates this constant will be removed in the future, which is in line with the simsx migration effort.
25-25: Clear migration guidance providedGood comment that explicitly suggests migrating to message factories while indicating future removal. This helps developers understand the transition path to the new simsx framework.
37-37: Consistent deprecation annotationThe comment matches the one above ProposalMsgs, providing a consistent migration message across the codebase.
x/auth/simulation/proposals.go (4)
14-14: Proper deprecation comment appliedThe comment clearly indicates this constant will be removed in the future, which is in line with the simsx migration effort.
17-17: Formatting change aligns code styleThe equal sign alignment improves code readability while maintaining functionality.
21-21: Specific migration target identifiedThis comment provides more specific guidance by naming the exact function (
MsgUpdateParamsFactory) to migrate to, which is helpful for developers.
33-33: Deprecation comment addedThe comment clearly indicates future removal without specifying an alternative, which is slightly different from the other migration comments but still appropriate.
x/slashing/simulation/proposals.go (3)
17-17: Proper deprecation comment appliedThe comment clearly indicates this constant will be removed in the future, which is in line with the simsx migration effort.
25-25: Clear migration guidance providedGood comment that explicitly suggests migrating to message factories while indicating future removal. This helps developers understand the transition path to the new simsx framework.
37-37: Consistent deprecation annotationThe comment matches the one above ProposalMsgs, providing a consistent migration message across the codebase.
x/mint/simulation/msg_factory.go (1)
1-11: Clean package structure and importsThe file has a good organization with appropriately grouped imports. The usage of the simsx package indicates integration with the new simulation framework.
x/auth/simulation/msg_factory.go (1)
1-25: LGTM! Implementation aligns well with simsx frameworkThe
MsgUpdateParamsFactoryfollows the pattern established across other modules (distribution, staking, slashing, mint) for simsx integration. It correctly generates random parameter values and properly sets up the governance authority.x/auth/module.go (3)
19-19: LGTM! Required import for simsx supportProperly imports the simsx package needed for the new simulation framework integration.
175-178: LGTM! Clear deprecation noticeThe comment properly informs developers that this method will be ignored when ProposalMsgsX exists and will be removed in the future, aligning with the simsx migration strategy.
180-183: LGTM! ProposalMsgsX implementation matches simsx patternThe implementation correctly registers the MsgUpdateParams message with the simulation registry using the factory defined in simulation/msg_factory.go, matching the pattern used in other modules.
x/slashing/simulation/operations.go (3)
21-22: LGTM! Clear deprecation noticeThis comment properly indicates that these operation weight constants will be removed in the future as part of the simsx migration.
29-30: LGTM! Helpful migration guidanceThe comment clearly indicates the migration path to message factories, helping developers understand the transition to the simsx framework.
54-55: LGTM! Consistent deprecation noticeThis comment aligns with the other deprecation notices, maintaining consistency in communicating the migration strategy.
x/distribution/simulation/proposals.go (4)
16-17: LGTM! Clear deprecation noticeComment properly indicates that these constants will be removed in the future as part of the simsx migration.
19-20: LGTM! Simple alignment fixMinor whitespace adjustment to improve code alignment.
23-24: LGTM! Clear migration guidanceThe comment effectively directs developers to use MsgUpdateParamsFactory going forward.
35-36: LGTM! Consistent deprecation noticeThis comment aligns with the other deprecation notices throughout the PR, maintaining a consistent message about migration strategy.
x/mint/module.go (3)
18-18: Correctly added simsx import for new functionality.The import is properly added to support the new ProposalMsgsX method.
174-177: LGTM: Clear deprecation notice added.The deprecation comment clearly communicates that this method will be ignored when ProposalMsgsX exists and will eventually be removed.
179-182: LGTM: New ProposalMsgsX implementation.The implementation correctly uses the simsx framework to register the weighted proposal message for parameter updates. The method signature follows the pattern used across other modules in this PR.
x/slashing/module.go (3)
18-18: Correctly added simsx import for new functionality.The import is properly added to support the new simulation methods.
188-191: LGTM: New ProposalMsgsX implementation.The implementation correctly uses the simsx framework to register the weighted proposal message for parameter updates.
201-205: LGTM: New WeightedOperationsX implementation.The implementation correctly registers weighted slashing module operations for simulation with appropriate comment about backward compatibility.
x/slashing/simulation/msg_factory.go (4)
1-14: LGTM: Proper package structure and imports.The package declaration and imports are correctly set up for the new simulation message factory.
15-73: Well-implemented MsgUnjailFactory with comprehensive validation.The MsgUnjailFactory implementation is thorough, with appropriate validation checks for:
- Validator jailed status
- Exchange rate validity
- Signing information retrieval
- Self-delegation status
- Various failure conditions (tombstoned, still in jail period, insufficient self-delegation)
The delivery result handler properly validates the success/failure conditions.
75-91: LGTM: Well-structured MsgUpdateParamsFactory.The factory correctly generates a governance proposal for parameter updates with randomized values for the various slashing parameters.
93-98: Appropriate helper function for error handling.The
musthelper function provides a clean way to handle errors in the factories.x/distribution/module.go (5)
20-20: Correctly added simsx import for new functionality.The import is properly added to support the new simulation methods.
177-180: LGTM: Clear deprecation notice added.The deprecation comment clearly communicates that this method will be ignored when ProposalMsgsX exists and will eventually be removed.
188-194: LGTM: Clear deprecation notice for WeightedOperations.The deprecation comment clearly communicates the migration path to WeightedOperationsX.
196-199: LGTM: New ProposalMsgsX implementation.The implementation correctly uses the simsx framework to register the weighted proposal message for parameter updates.
201-206: LGTM: Comprehensive WeightedOperationsX implementation.The implementation correctly registers multiple weighted operations for the distribution module:
- Set withdraw address
- Withdraw delegator reward
- Withdraw validator commission
All with appropriate weights and factory functions.
x/gov/module.go (5)
25-25: Add new simsx package import for simulation supportThe newly imported
simsxpackage provides enhanced simulation capabilities for the governance module, which aligns with the backported simulation framework mentioned in the PR objectives.
329-330: Deprecation notice for ProposalContents methodGood documentation practice to indicate that this method will be deprecated in favor of the newer
ProposalMsgsXmethod, providing a clear migration path for developers.
335-336: Deprecation notice for ProposalMsgs methodThis comment clearly indicates that this method will be deprecated, helping developers understand the migration path to the new
ProposalMsgsXimplementation.
346-347: Deprecation notice for WeightedOperations methodThis comment clearly documents that this method will be migrated to
WeightedOperationsXand will be removed in the future, providing necessary guidance for code evolution.
355-358: New ProposalMsgsX method implementationThis new method leverages the
simsxframework to register governance proposal messages with appropriate weights in the simulation registry. The implementation is concise and follows the new simulation pattern.x/staking/module.go (5)
24-24: Add new simsx package import for simulation supportThe newly imported
simsxpackage provides enhanced simulation capabilities for the staking module, which is consistent with the changes being made across modules for the simsx backport.
284-285: Deprecation notice for ProposalMsgs methodGood documentation practice to indicate that this method will be deprecated in favor of the newer
ProposalMsgsXmethod, providing a clear migration path for developers.
289-292: New ProposalMsgsX method implementationThis new method registers the parameter update message factory with the simulation registry. The implementation is clean and uses the weights source to get the appropriate weight for the message.
300-301: Deprecation notice for WeightedOperations methodThis comment clearly documents that this method will be migrated to
WeightedOperationsXand will be removed in the future, providing necessary guidance for code evolution.
308-316: Implement WeightedOperationsX method for staking simulationsThis implementation properly registers all staking-related operations (creating validator, delegating, undelegating, etc.) with appropriate weights for simulation testing. Each operation is registered with a factory function from the staking simulation package.
x/distribution/simulation/msg_factory.go (5)
1-14: Add new simulation message factory file with necessary importsThis new file properly sets up the necessary imports for simulation message factories, including context, collections, math utilities, and the simsx package.
15-30: Implement MsgSetWithdrawAddressFactory for simulationThis factory function creates simulation messages for setting withdrawal addresses. It properly checks if withdrawal is enabled before attempting to generate the message, and selects different accounts for delegator and withdrawer.
32-81: Implement MsgWithdrawDelegatorRewardFactory for simulationThis factory function handles delegator reward withdrawals in simulations. It includes thorough error handling at each step of the process:
- Getting delegations
- Checking if delegations exist
- Getting validator addresses
- Retrieving validator information
- Checking outstanding rewards
- Verifying that the reward denoms are sendable
The error reporting through the simulation reporter is comprehensive, making debugging easier.
83-111: Implement MsgWithdrawValidatorCommissionFactory for simulationThis factory function properly handles validator commission withdrawals in simulations, with appropriate error handling at each step:
- Getting all validators
- Selecting a random validator
- Converting validator addresses
- Checking accumulated commission
- Verifying non-zero commission amount
The error checks are thorough and the function will skip message generation when conditions aren't met.
113-125: Implement MsgUpdateParamsFactory for simulationThis factory function creates parameter update messages with randomized values:
- Community tax is set to a random decimal between 0 and 1
- WithdrawAddrEnabled is randomly set to true or false with 50% probability
The message sets the governance module as the authority, which is the correct approach for parameter changes through governance.
x/gov/simulation/operations.go (16)
22-23: Add deprecation comment for message types constantsThis comment makes it clear that these message type constants will be removed in the future, helping developers prepare for upcoming changes in the codebase.
32-33: Add deprecation comment for operation weights constantsThis comment indicates that these operation weight constants will be removed in the future, providing developers with a heads-up about upcoming changes to the codebase.
47-48: Add migration note for WeightedOperations functionThis comment explains that the function will be replaced by the message factory approach in the future, helping to document the transition strategy.
121-121: Update shared state initializationChanged from
newSharedState()toNewSharedState()to use the new constructor function. This is part of the transition to the new simulation framework.
147-148: Add migration note for SimulateMsgSubmitProposal functionThis comment explains that the function will be replaced by the message factory approach in the future, documenting the transition strategy.
170-171: Add migration note for SimulateMsgSubmitLegacyProposal functionThis comment explains that the function will be replaced by the message factory approach in the future, documenting the transition strategy.
295-295: Update shared state initialization in simulateMsgSubmitProposalChanged from
newSharedState()toNewSharedState()to use the new constructor function. This is part of the transition to the new simulation framework.
310-318: Update SimulateMsgDeposit function with new shared stateAdded a migration comment and updated the function to use the new
NewSharedState()constructor instead of the deprecatednewSharedState(). This is part of the transition to the new simulation framework.
325-326: Update simulateMsgDeposit parameter typeChanged the parameter type from
*sharedStateto*SharedStateto match the new type in the simulation framework.
381-389: Update SimulateMsgVote function with new shared stateAdded a deprecation comment and updated the function to use the new
NewSharedState()constructor. This aligns with the transition to the new simulation framework.
396-397: Update simulateMsgVote parameter typeChanged the parameter type from
*sharedStateto*SharedStateto match the new type in the simulation framework.
408-409: Update operationSimulateMsgVote parameter typeChanged the parameter type from
*sharedStateto*SharedStateto match the new type in the simulation framework.
456-464: Update SimulateMsgVoteWeighted function with new shared stateAdded a migration comment and updated the function to use the new
NewSharedState()constructor instead of the deprecatednewSharedState(). This is part of the transition to the new simulation framework.
471-472: Update simulateMsgVoteWeighted parameter typeChanged the parameter type from
*sharedStateto*SharedStateto match the new type in the simulation framework.
483-484: Update operationSimulateMsgVoteWeighted parameter typeChanged the parameter type from
*sharedStateto*SharedStateto match the new type in the simulation framework.
531-532: Add migration note for SimulateMsgCancelProposal functionThis comment explains that the function will be replaced by the message factory approach in the future, documenting the transition strategy.
x/gov/simulation/msg_factory.go (6)
1-19: Overall file structure looks good.The package declaration, imports, and basic setup appear correct. No immediate concerns are noted in this segment.
20-41: Implementation looks correct for deposit message creation.The logic for retrieving a proposal in deposit state and computing a random deposit amount is clear. Skipping when no suitable proposal is found or when other conditions fail is appropriate. No suggested changes in this block.
57-69: Weighted voting logic is straightforward.No issues found with the weighted voting approach. The use of random weighted vote options for simulation is correct.
158-160: Double-check the block amount logic and skip condition.Here, the simulation skips if either
reporter.IsSkipped()is already set or if!proposer.LiquidBalance().BlockAmount(deposit). The second condition modifies the account’s balance, but no explicit skip message is generated. Verify that this is intended and does not mask partial deposit blocks.Do you want to confirm the exact interplay of
BlockAmountand skipping logic across other simulation code to ensure consistent deposit handling?
275-292: Random proposal selection logic is solid, but watch for missed proposals.By picking a random ID between the minimum known proposal and the latest, there’s a chance some proposals are skipped. This is appropriate for random coverage but verify you don’t inadvertently miss relevant proposals.
343-349: Utility function is correct.Panic-based error handling is acceptable in simulation code to reveal unexpected states. No issues here.
Thanks for the info! I had reduced the number of retries for a matching account. Unfortunately, I missed the test covering this. Sorry for the inconvenience. |
Close #23762
Last part of simsx backport
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.
I have...
Summary by CodeRabbit