feat: blockstm check pre-state in transaction state transition

CHANGELOG.md

@@ -78,6 +78,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (server) [#25632](https://github.com/cosmos/cosmos-sdk/pull/25632) Add missing call to close the app on shutdown.
 * (server) [#25740](https://github.com/cosmos/cosmos-sdk/pull/25740) Add variadic `grpc.DialOption` parameter to `StartGrpcServer` for custom gRPC client connection options.
 * (blockstm) [#25765](https://github.com/cosmos/cosmos-sdk/pull/25765) Minor code readability improvement in block-stm.
+* (blockstm) [#](https://github.com/cosmos/cosmos-sdk/pull/) Add pre-state checking in transaction state transition.
 
 ### Bug Fixes
 

blockstm/README.md

@@ -14,6 +14,8 @@ func ExecuteBlock(
 ) error
 ```
 
+Broken internal invariants will cause panics.
+
 The main deviations from the paper are:
 
 ### Optimisation

blockstm/mvmemory_test.go

@@ -102,6 +102,7 @@ func TestMVMemoryRecord(t *testing.T) {
 		require.False(t, wroteNewLocation)
 		require.True(t, mv.ValidateReadSet(2))
 
+		scheduler.TryIncarnate(version.Index)
 		scheduler.FinishExecution(version, wroteNewLocation)
 
 		// wait for dependency to finish

blockstm/status.go

@@ -1,6 +1,9 @@
 package blockstm
 
-import "sync"
+import (
+	"fmt"
+	"sync"
+)
 
 type Status uint
 
@@ -60,16 +63,29 @@
 	return incarnation, ok
 }
 
-func (s *StatusEntry) setStatus(status Status) {
+// setStatus sets the status to the given status if the current status is preStatus.
+// preStatus invariant must be held by the caller.
+func (s *StatusEntry) setStatus(status Status, preStatus Status) {
 	s.Lock()
+
+	if s.status != preStatus {
+		s.Unlock()
+		panic(fmt.Sprintf("invalid status transition: %v -> %v, current: %v", preStatus, status, s.status))
+	}
+
 	s.status = status
 	s.Unlock()
 }
 
 func (s *StatusEntry) Resume() {
-	// status must be SUSPENDED and cond != nil
 	s.Lock()
 
+	// status must be SUSPENDED and cond != nil
+	if s.status != StatusSuspended || s.cond == nil {
+		s.Unlock()
+		panic(fmt.Sprintf("invalid resume: status=%v", s.status))
+	}
+
 	s.status = StatusExecuting
 	s.cond.Notify()
 	s.cond = nil
@@ -79,7 +95,7 @@
 
 func (s *StatusEntry) SetExecuted() {
 	// status must have been EXECUTING
-	s.setStatus(StatusExecuted)
+	s.setStatus(StatusExecuted, StatusExecuting)
 }
 
 func (s *StatusEntry) TryValidationAbort(incarnation Incarnation) (ok bool) {
@@ -97,8 +113,13 @@
 func (s *StatusEntry) SetReadyStatus() {
 	s.Lock()
 
-	s.incarnation++
 	// status must be ABORTING
+	if s.status != StatusAborting {
+		s.Unlock()
+		panic(fmt.Sprintf("invalid status transition: %v -> %v, current: %v", StatusAborting, StatusReadyToExecute, s.status))
+	}
+
+	s.incarnation++
 	s.status = StatusReadyToExecute
 
 	s.Unlock()
@@ -107,6 +128,11 @@
 func (s *StatusEntry) Suspend(cond *Condvar) {
 	s.Lock()
 
+	if s.status != StatusExecuting {
+		s.Unlock()
+		panic(fmt.Sprintf("invalid suspend: status=%v", s.status))
+	}
+
 	s.cond = cond
 	s.status = StatusSuspended