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: fix sim test state persistence issue #19771

Closed
wants to merge 2 commits into from
Closed

fix: fix sim test state persistence issue #19771

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
01a90ef
fix: fix sim test state persistence issue
hacheigriega Mar 18, 2024
0a9fecf
fix: fix accidental commenting
hacheigriega Mar 18, 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
6 changes: 3 additions & 3 deletions baseapp/abci.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -890,7 +890,7 @@ func (app *BaseApp) FinalizeBlock(req *abci.RequestFinalizeBlock) (res *abci.Res
// only return if we are not aborting
if !aborted {
if res != nil {
res.AppHash = app.workingHash()
res.AppHash = app.WorkingHash()
}

return res, err
Expand All @@ -904,7 +904,7 @@ func (app *BaseApp) FinalizeBlock(req *abci.RequestFinalizeBlock) (res *abci.Res
// if no OE is running, just run the block (this is either a block replay or a OE that got aborted)
res, err = app.internalFinalizeBlock(context.Background(), req)
if res != nil {
res.AppHash = app.workingHash()
res.AppHash = app.WorkingHash()
}

return res, err
Expand Down Expand Up @@ -990,7 +990,7 @@ func (app *BaseApp) Commit() (*abci.ResponseCommit, error) {
// disk in the Commit phase. This means when the ABCI client requests Commit(), the application
// state transitions will be flushed to disk and as a result, but we already have
// an application Merkle root.
func (app *BaseApp) workingHash() []byte {
func (app *BaseApp) WorkingHash() []byte {
Copy link
Member

Choose a reason for hiding this comment

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

We shouldn't make this one public, if calling this solves the issue then the problem is that there are operations running outside of a FinalizeBlock call context. Because this function is called at the end of FinalizeBlock.

// Write the FinalizeBlock state into branched storage and commit the MultiStore.
// The write to the FinalizeBlock state writes all state transitions to the root
// MultiStore (app.cms) so when Commit() is called it persists those values.
Expand Down
38 changes: 25 additions & 13 deletions x/gov/simulation/operations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ func simulateMsgSubmitProposal(
) (simtypes.OperationMsg, []simtypes.FutureOperation, error) {
simAccount, _ := simtypes.RandomAcc(r, accs)
expedited := r.Intn(2) == 0
deposit, skip, err := randomDeposit(r, ctx, ak, bk, k, simAccount.Address, true, expedited)
deposit, skip, err := randomDeposit(r, ctx, ak, bk, k, simAccount.Address, true, true, expedited)
switch {
case skip:
return simtypes.NoOpMsg(types.ModuleName, TypeMsgSubmitProposal, "skip deposit"), nil, nil
Expand Down Expand Up @@ -278,11 +278,20 @@ func simulateMsgSubmitProposal(

opMsg := simtypes.NewOperationMsg(msg, true, "")

// get the submitted proposal ID
// Get the submitted proposal ID (current sequence value minus one).
proposalID, err := k.ProposalID.Peek(ctx)
if err != nil {
return simtypes.NoOpMsg(types.ModuleName, sdk.MsgTypeURL(msg), "unable to generate proposalID"), nil, err
}
proposalID--
hacheigriega marked this conversation as resolved.
Show resolved Hide resolved

proposal, err := k.Proposals.Get(ctx, proposalID)
if err != nil {
return simtypes.NoOpMsg(types.ModuleName, sdk.MsgTypeURL(msg), "unable to retrieve proposal"), nil, err
}

unixDiff := proposal.VotingEndTime.Unix() - proposal.VotingStartTime.Unix()
voteTime := time.Unix(r.Int63n(unixDiff), 0)

// 2) Schedule operations for votes
// 2.1) first pick a number of people to vote.
Expand All @@ -294,14 +303,11 @@ func simulateMsgSubmitProposal(

// didntVote := whoVotes[numVotes:]
whoVotes = whoVotes[:numVotes]
params, _ := k.Params.Get(ctx)
votingPeriod := params.VotingPeriod

fops := make([]simtypes.FutureOperation, numVotes+1)
for i := 0; i < numVotes; i++ {
whenVote := ctx.HeaderInfo().Time.Add(time.Duration(r.Int63n(int64(votingPeriod.Seconds()))) * time.Second)
fops[i] = simtypes.FutureOperation{
BlockTime: whenVote,
BlockTime: voteTime,
Op: operationSimulateMsgVote(txGen, ak, bk, k, accs[whoVotes[i]], int64(proposalID)),
}
}
Expand Down Expand Up @@ -332,7 +338,7 @@ func SimulateMsgDeposit(
return simtypes.NoOpMsg(types.ModuleName, TypeMsgDeposit, "unable to get proposal"), nil, err
}

deposit, skip, err := randomDeposit(r, ctx, ak, bk, k, simAccount.Address, false, p.Expedited)
deposit, skip, err := randomDeposit(r, ctx, ak, bk, k, simAccount.Address, false, false, p.Expedited)
switch {
case skip:
return simtypes.NoOpMsg(types.ModuleName, TypeMsgDeposit, "skip deposit"), nil, nil
Expand Down Expand Up @@ -555,7 +561,8 @@ func randomDeposit(
bk types.BankKeeper,
k *keeper.Keeper,
addr sdk.AccAddress,
useMinAmount bool,
useMinInitAmt bool,
useMinAmt bool,
expedited bool,
) (deposit sdk.Coins, skip bool, err error) {
account := ak.GetAccount(ctx, addr)
Expand Down Expand Up @@ -587,21 +594,26 @@ func randomDeposit(

threshold := minDepositAmount.ToLegacyDec().Mul(minDepositRatio).TruncateInt()

minAmount := sdkmath.ZeroInt()
if useMinAmount {
minInitAmt := sdkmath.ZeroInt()
if useMinInitAmt {
Copy link
Member

Choose a reason for hiding this comment

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

can you explain this?

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, I was trying to keep the original code as much as possible. I made these changes to get the proposal to immediately enter the voting period. So the deposit amount has to satisfy both the "minimum initial deposit requirement" and the "minimum deposit requirement".

minDepositPercent, err := sdkmath.LegacyNewDecFromStr(params.MinInitialDepositRatio)
if err != nil {
return nil, false, err
}

minAmount = sdkmath.LegacyNewDecFromInt(minDepositAmount).Mul(minDepositPercent).TruncateInt()
minInitAmt = sdkmath.LegacyNewDecFromInt(minDepositAmount).Mul(minDepositPercent).TruncateInt()
}

minAmt := sdkmath.ZeroInt()
if useMinAmt {
minAmt = minDepositAmount
}

amount, err := simtypes.RandPositiveInt(r, minDepositAmount.Sub(minAmount))
amount, err := simtypes.RandPositiveInt(r, minDepositAmount)
if err != nil {
return nil, false, err
}
amount = amount.Add(minAmount)
amount = amount.Add(minInitAmt).Add(minAmt)

if amount.GT(spendableBalance) || amount.LT(threshold) {
return nil, true, nil
Expand Down
2 changes: 2 additions & 0 deletions x/simulation/simulate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,8 @@ func SimulateFromSeed(

logWriter.AddEntry(EndBlockEntry(blockHeight))

app.WorkingHash()
Copy link
Member

Choose a reason for hiding this comment

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

I think we should not make this function public but see if all operations are run in the context of a FinalizeBlock call. Given that this function is called in there


if config.Commit {
_, err := app.Commit()
if err != nil {
Expand Down
Loading