diff --git a/x/simulation/operation.go b/x/simulation/operation.go index ec58ece3f15b..0c2c8f103d90 100644 --- a/x/simulation/operation.go +++ b/x/simulation/operation.go @@ -73,7 +73,7 @@ func NewOperationQueue() OperationQueue { } // queueOperations adds all future operations into the operation queue. -func queueOperations(queuedOps OperationQueue, queuedTimeOps, futureOps []simulation.FutureOperation) { +func queueOperations(queuedOps OperationQueue, queuedTimeOps *[]simulation.FutureOperation, futureOps []simulation.FutureOperation) { if futureOps == nil { return } @@ -89,18 +89,11 @@ func queueOperations(queuedOps OperationQueue, queuedTimeOps, futureOps []simula continue } - // TODO: Replace with proper sorted data structure, so don't have the - // copy entire slice - index := sort.Search( - len(queuedTimeOps), - func(i int) bool { - return queuedTimeOps[i].BlockTime.After(futureOp.BlockTime) - }, - ) - - queuedTimeOps = append(queuedTimeOps, simulation.FutureOperation{}) - copy(queuedTimeOps[index+1:], queuedTimeOps[index:]) - queuedTimeOps[index] = futureOp + queue := append(*queuedTimeOps, futureOp) + sort.Slice(queue, func(i, j int) bool { + return queue[i].BlockTime.Before(queue[j].BlockTime) + }) + *queuedTimeOps = queue } } diff --git a/x/simulation/simulate.go b/x/simulation/simulate.go index 1531cee3d249..a5fd7a47eda1 100644 --- a/x/simulation/simulate.go +++ b/x/simulation/simulate.go @@ -164,7 +164,7 @@ func SimulateFromSeedX( eventStats.Tally, ops, operationQueue, - timeOperationQueue, + &timeOperationQueue, logWriter, config, ) @@ -217,13 +217,13 @@ func SimulateFromSeedX( ) numQueuedTimeOpsRan, timeFutureOps := runQueuedTimeOperations(tb, - timeOperationQueue, int(blockHeight), blockTime, + &timeOperationQueue, int(blockHeight), blockTime, r, app, ctx, accs, logWriter, eventStats.Tally, config.Lean, config.ChainID, ) futureOps = append(futureOps, timeFutureOps...) - queueOperations(operationQueue, timeOperationQueue, futureOps) + queueOperations(operationQueue, &timeOperationQueue, futureOps) // run standard operations operations := blockSimulator(r, app, ctx, accs, cmtproto.Header{ @@ -297,7 +297,7 @@ type blockSimFn func( // parameters being passed every time, to minimize memory overhead. func createBlockSimulator(tb testing.TB, printProgress bool, w io.Writer, params Params, event func(route, op, evResult string), ops WeightedOperations, - operationQueue OperationQueue, timeOperationQueue []simulation.FutureOperation, + operationQueue OperationQueue, timeOperationQueue *[]simulation.FutureOperation, logWriter LogWriter, config simulation.Config, ) blockSimFn { tb.Helper() @@ -409,7 +409,7 @@ func runQueuedOperations( return numOpsRan, allFutureOps } -func runQueuedTimeOperations(tb testing.TB, queueOps []simulation.FutureOperation, +func runQueuedTimeOperations(tb testing.TB, queueOps *[]simulation.FutureOperation, height int, currentTime time.Time, r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, accounts []simulation.Account, logWriter LogWriter, event func(route, op, evResult string), @@ -420,8 +420,8 @@ func runQueuedTimeOperations(tb testing.TB, queueOps []simulation.FutureOperatio allFutureOps = make([]simulation.FutureOperation, 0) numOpsRan = 0 - for len(queueOps) > 0 && currentTime.After(queueOps[0].BlockTime) { - opMsg, futureOps, err := queueOps[0].Op(r, app, ctx, accounts, chainID) + for len(*queueOps) > 0 && currentTime.After((*queueOps)[0].BlockTime) { + opMsg, futureOps, err := (*queueOps)[0].Op(r, app, ctx, accounts, chainID) opMsg.LogEvent(event) @@ -438,7 +438,7 @@ func runQueuedTimeOperations(tb testing.TB, queueOps []simulation.FutureOperatio allFutureOps = append(allFutureOps, futureOps...) } - queueOps = queueOps[1:] + *queueOps = (*queueOps)[1:] numOpsRan++ } diff --git a/x/simulation/simulate_test.go b/x/simulation/simulate_test.go new file mode 100644 index 000000000000..24efb9b41bf8 --- /dev/null +++ b/x/simulation/simulate_test.go @@ -0,0 +1,77 @@ +package simulation + +import ( + "math/rand" + "testing" + "time" + + "github.com/cosmos/cosmos-sdk/baseapp" + sdk "github.com/cosmos/cosmos-sdk/types" + simtypes "github.com/cosmos/cosmos-sdk/types/simulation" +) + +// Test that time-based FutureOperation values are enqueued into the shared +// timeOperationQueue and executed/cleared by runQueuedTimeOperations. +func TestTimeOperationQueue_EnqueueAndExecute(t *testing.T) { + t.Helper() + + var ( + opCalled int + now = time.Now() + ) + + // Future operation scheduled in the past relative to currentTime so that it + // must be executed by runQueuedTimeOperations. + futureOp := simtypes.FutureOperation{ + BlockTime: now.Add(-1 * time.Second), + Op: func(r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, accounts []simtypes.Account, chainID string, + ) (simtypes.OperationMsg, []simtypes.FutureOperation, error) { + opCalled++ + return simtypes.NewOperationMsgBasic("test", "time-op", "", true, nil), nil, nil + }, + } + + operationQueue := NewOperationQueue() + timeOperationQueue := []simtypes.FutureOperation{} + + // Enqueue the time-based operation via queueOperations. + queueOperations(operationQueue, &timeOperationQueue, []simtypes.FutureOperation{futureOp}) + + if len(operationQueue) != 0 { + t.Fatalf("expected height-based operationQueue to be empty, got %d entries", len(operationQueue)) + } + if got := len(timeOperationQueue); got != 1 { + t.Fatalf("expected timeOperationQueue length 1 after enqueue, got %d", got) + } + + // Run queued time operations; the single past-due operation should execute + // exactly once and be removed from the queue. + r := rand.New(rand.NewSource(1)) + numRan, futureOps := runQueuedTimeOperations( + t, + &timeOperationQueue, + 1, // height + now, // currentTime + r, + nil, // app + sdk.Context{}, // ctx + nil, // accounts + &DummyLogWriter{}, + func(_, _, _ string) {}, // event logger + true, // lean + "", // chainID + ) + + if numRan != 1 { + t.Fatalf("expected numOpsRan = 1, got %d", numRan) + } + if opCalled != 1 { + t.Fatalf("expected time-based operation to be called once, got %d", opCalled) + } + if len(futureOps) != 0 { + t.Fatalf("expected no new future operations from time-based op, got %d", len(futureOps)) + } + if got := len(timeOperationQueue); got != 0 { + t.Fatalf("expected timeOperationQueue to be empty after execution, got %d", got) + } +}