fix: ensure time-based future operations are queued and executed

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
 	}
 }
 

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++
 	}
 

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)
+	}
+}