Merge remote-tracking branch 'origin/main' into tzdybal/tx_injector

execution.go

@@ -7,17 +7,81 @@ import (
 	"github.com/rollkit/go-execution/types"
 )
 
-// Executor defines a common interface for interacting with the execution client.
+// Executor defines the interface that execution clients must implement to be compatible with Rollkit.
+// This interface enables the separation between consensus and execution layers, allowing for modular
+// and pluggable execution environments.
 type Executor interface {
-	// InitChain initializes the blockchain with genesis information.
+	// InitChain initializes a new blockchain instance with genesis parameters.
+	// Requirements:
+	// - Must generate initial state root representing empty/genesis state
+	// - Must validate and store genesis parameters for future reference
+	// - Must ensure idempotency (repeated calls with identical parameters should return same results)
+	// - Must return error if genesis parameters are invalid
+	// - Must return maxBytes indicating maximum allowed bytes for a set of transactions in a block
+	//
+	// Parameters:
+	// - ctx: Context for timeout/cancellation control
+	// - genesisTime: timestamp marking chain start time in UTC
+	// - initialHeight: First block height (must be > 0)
+	// - chainID: Unique identifier string for the blockchain
+	//
+	// Returns:
+	// - stateRoot: Hash representing initial state
+	// - maxBytes: Maximum allowed bytes for transacitons in a block
+	// - err: Any initialization errors
 	InitChain(ctx context.Context, genesisTime time.Time, initialHeight uint64, chainID string) (stateRoot types.Hash, maxBytes uint64, err error)
 
-	// GetTxs retrieves all available transactions from the execution client's mempool.
+	// GetTxs fetches available transactions from the execution layer's mempool.
+	// Requirements:
+	// - Must return currently valid transactions only
+	// - Must handle empty mempool case gracefully
+	// - Must respect context cancellation/timeout
+	// - Should perform basic transaction validation
+	// - Should not remove transactions from mempool
+	// - May remove invalid transactions from mempool
+	//
+	// Parameters:
+	// - ctx: Context for timeout/cancellation control
+	//
+	// Returns:
+	// - []types.Tx: Slice of valid transactions
+	// - error: Any errors during transaction retrieval
 	GetTxs(ctx context.Context) ([]types.Tx, error)
 
-	// ExecuteTxs executes a set of transactions to produce a new block header.
+	// ExecuteTxs processes transactions to produce a new block state.
+	// Requirements:
+	// - Must validate state transition against previous state root
+	// - Must handle empty transaction list
+	// - Must maintain deterministic execution
+	// - Must respect context cancellation/timeout
+	// - The rest of the rules are defined by the specific execution layer
+	//
+	// Parameters:
+	// - ctx: Context for timeout/cancellation control
+	// - txs: Ordered list of transactions to execute
+	// - blockHeight: Height of block being created (must be > 0)
+	// - timestamp: Block creation time in UTC
+	// - prevStateRoot: Previous block's state root hash
+	//
+	// Returns:
+	// - updatedStateRoot: New state root after executing transactions
+	// - maxBytes: Maximum allowed transaction size (may change with protocol updates)
+	// - err: Any execution errors
 	ExecuteTxs(ctx context.Context, txs []types.Tx, blockHeight uint64, timestamp time.Time, prevStateRoot types.Hash) (updatedStateRoot types.Hash, maxBytes uint64, err error)
 
-	// SetFinal marks a block at the given height as final.
+	// SetFinal marks a block as finalized at the specified height.
+	// Requirements:
+	// - Must verify block exists at specified height
+	// - Must be idempotent
+	// - Must maintain finality guarantees (no reverting finalized blocks)
+	// - Must respect context cancellation/timeout
+	// - Should clean up any temporary state/resources
+	//
+	// Parameters:
+	// - ctx: Context for timeout/cancellation control
+	// - blockHeight: Height of block to finalize
+	//
+	// Returns:
+	// - error: Any errors during finalization
 	SetFinal(ctx context.Context, blockHeight uint64) error
 }

test/dummy.go

@@ -3,20 +3,23 @@ package test
 import (
 	"bytes"
 	"crypto/rand"
+	"fmt"
 
 	"context"
 	"crypto/sha512"
-	"fmt"
+	"regexp"
 	"slices"
 	"sync"
 	"time"
 
 	"github.com/rollkit/go-execution/types"
 )
 
+var validChainIDRegex = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9-]*`)
+
 // DummyExecutor is a dummy implementation of the DummyExecutor interface for testing
 type DummyExecutor struct {
-	mu           sync.RWMutex // Add mutex for thread safety
+	mu           sync.RWMutex
 	stateRoot    types.Hash
 	pendingRoots map[uint64]types.Hash
 	maxBytes     uint64
@@ -38,6 +41,22 @@ func (e *DummyExecutor) InitChain(ctx context.Context, genesisTime time.Time, in
 	e.mu.Lock()
 	defer e.mu.Unlock()
 
+	if initialHeight == 0 {
+		return types.Hash{}, 0, types.ErrZeroInitialHeight
+	}
+	if chainID == "" {
+		return types.Hash{}, 0, types.ErrEmptyChainID
+	}
+	if !validChainIDRegex.MatchString(chainID) {
+		return types.Hash{}, 0, types.ErrInvalidChainID
+	}
+	if genesisTime.After(time.Now()) {
+		return types.Hash{}, 0, types.ErrFutureGenesisTime
+	}
+	if len(chainID) > 32 {
+		return types.Hash{}, 0, types.ErrChainIDTooLong
+	}
+
 	hash := sha512.New()
 	hash.Write(e.stateRoot)
 	e.stateRoot = hash.Sum(nil)
@@ -50,7 +69,7 @@ func (e *DummyExecutor) GetTxs(context.Context) ([]types.Tx, error) {
 	defer e.mu.RUnlock()
 
 	txs := make([]types.Tx, len(e.injectedTxs))
-	copy(txs, e.injectedTxs) // Create a copy to avoid external modifications
+	copy(txs, e.injectedTxs)
 	return txs, nil
 }
 
@@ -85,9 +104,26 @@ func (e *DummyExecutor) ExecuteTxs(ctx context.Context, txs []types.Tx, blockHei
 	e.mu.Lock()
 	defer e.mu.Unlock()
 
-	if len(txs) == 0 {
-		e.pendingRoots[blockHeight] = prevStateRoot
-		return prevStateRoot, e.maxBytes, nil
+	if bytes.Equal(prevStateRoot, types.Hash{}) {
+		return types.Hash{}, 0, types.ErrEmptyStateRoot
+	}
+
+	// Don't really allow future block times, but allow up to 5 minutes in the future
+	// for testing purposes.
+	if timestamp.After(time.Now().Add(5 * time.Minute)) {
+		return types.Hash{}, 0, types.ErrFutureBlockTime
+	}
+	if blockHeight == 0 {
+		return types.Hash{}, 0, types.ErrInvalidBlockHeight
+	}
+
+	for _, tx := range txs {
+		if len(tx) == 0 {
+			return types.Hash{}, 0, types.ErrEmptyTx
+		}
+		if uint64(len(tx)) > e.maxBytes {
+			return types.Hash{}, 0, types.ErrTxTooLarge
+		}
 	}
 
 	hash := sha512.New()
@@ -111,7 +147,7 @@ func (e *DummyExecutor) SetFinal(ctx context.Context, blockHeight uint64) error
 		delete(e.pendingRoots, blockHeight)
 		return nil
 	}
-	return fmt.Errorf("cannot set finalized block at height %d", blockHeight)
+	return types.ErrBlockNotFound
 }
 
 func (e *DummyExecutor) removeExecutedTxs(txs []types.Tx) {

test/dummy_test.go

@@ -2,6 +2,9 @@ package test
 
 import (
 	"context"
+	"fmt"
+	"strings"
+	"sync"
 	"testing"
 	"time"
 
@@ -25,7 +28,8 @@ func TestDummySuite(t *testing.T) {
 	suite.Run(t, new(DummyTestSuite))
 }
 
-func TestTxRemoval(t *testing.T) {
+func (s *DummyTestSuite) TestTxRemoval() {
+	t := s.T()
 	exec := NewDummyExecutor()
 
 	// Generate random transactions using GetRandomTxs
@@ -51,7 +55,8 @@ func TestTxRemoval(t *testing.T) {
 	require.Contains(t, txs, tx1)
 	require.Contains(t, txs, tx2)
 
-	state, _, err := exec.ExecuteTxs(context.Background(), []types.Tx{tx1}, 1, time.Now(), nil)
+	dummyStateRoot := []byte("dummy-state-root")
+	state, _, err := exec.ExecuteTxs(context.Background(), []types.Tx{tx1}, 1, time.Now(), dummyStateRoot)
 	require.NoError(t, err)
 	require.NotEmpty(t, state)
 
@@ -62,3 +67,168 @@ func TestTxRemoval(t *testing.T) {
 	require.NotContains(t, txs, tx1)
 	require.Contains(t, txs, tx2)
 }
+
+func (s *DummyTestSuite) TestExecuteTxsComprehensive() {
+	t := s.T()
+	tests := []struct {
+		name          string
+		txs           []types.Tx
+		blockHeight   uint64
+		timestamp     time.Time
+		prevStateRoot types.Hash
+		expectedErr   error
+	}{
+		{
+			name:          "valid multiple transactions",
+			txs:           []types.Tx{[]byte("tx1"), []byte("tx2"), []byte("tx3")},
+			blockHeight:   1,
+			timestamp:     time.Now().UTC(),
+			prevStateRoot: types.Hash{1, 2, 3},
+			expectedErr:   nil,
+		},
+		{
+			name:          "empty state root",
+			txs:           []types.Tx{[]byte("tx1")},
+			blockHeight:   1,
+			timestamp:     time.Now().UTC(),
+			prevStateRoot: types.Hash{},
+			expectedErr:   types.ErrEmptyStateRoot,
+		},
+		{
+			name:          "future timestamp",
+			txs:           []types.Tx{[]byte("tx1")},
+			blockHeight:   1,
+			timestamp:     time.Now().Add(24 * time.Hour),
+			prevStateRoot: types.Hash{1, 2, 3},
+			expectedErr:   types.ErrFutureBlockTime,
+		},
+		{
+			name:          "empty transaction",
+			txs:           []types.Tx{[]byte("")},
+			blockHeight:   1,
+			timestamp:     time.Now().UTC(),
+			prevStateRoot: types.Hash{1, 2, 3},
+			expectedErr:   types.ErrEmptyTx,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			stateRoot, maxBytes, err := s.Exec.ExecuteTxs(context.Background(), tt.txs, tt.blockHeight, tt.timestamp, tt.prevStateRoot)
+			if tt.expectedErr != nil {
+				require.ErrorIs(t, err, tt.expectedErr)
+				return
+			}
+			require.NoError(t, err)
+			require.NotEqual(t, types.Hash{}, stateRoot)
+			require.Greater(t, maxBytes, uint64(0))
+		})
+	}
+}
+
+func (s *DummyTestSuite) TestInitChain() {
+	t := s.T()
+	tests := []struct {
+		name          string
+		genesisTime   time.Time
+		initialHeight uint64
+		chainID       string
+		expectedErr   error
+	}{
+		{
+			name:          "valid case",
+			genesisTime:   time.Now().UTC(),
+			initialHeight: 1,
+			chainID:       "test-chain",
+			expectedErr:   nil,
+		},
+		{
+			name:          "very large initial height",
+			genesisTime:   time.Now().UTC(),
+			initialHeight: 1000000,
+			chainID:       "test-chain",
+			expectedErr:   nil,
+		},
+		{
+			name:          "zero height",
+			genesisTime:   time.Now().UTC(),
+			initialHeight: 0,
+			chainID:       "test-chain",
+			expectedErr:   types.ErrZeroInitialHeight,
+		},
+		{
+			name:          "empty chain ID",
+			genesisTime:   time.Now().UTC(),
+			initialHeight: 1,
+			chainID:       "",
+			expectedErr:   types.ErrEmptyChainID,
+		},
+		{
+			name:          "future genesis time",
+			genesisTime:   time.Now().Add(1 * time.Hour),
+			initialHeight: 1,
+			chainID:       "test-chain",
+			expectedErr:   types.ErrFutureGenesisTime,
+		},
+		{
+			name:          "invalid chain ID characters",
+			genesisTime:   time.Now().UTC(),
+			initialHeight: 1,
+			chainID:       "@invalid",
+			expectedErr:   types.ErrInvalidChainID,
+		},
+		{
+			name:          "invalid chain ID length",
+			genesisTime:   time.Now().UTC(),
+			initialHeight: 1,
+			chainID:       strings.Repeat("a", 50),
+			expectedErr:   types.ErrChainIDTooLong,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			stateRoot, maxBytes, err := s.Exec.InitChain(context.Background(), tt.genesisTime, tt.initialHeight, tt.chainID)
+			if tt.expectedErr != nil {
+				require.ErrorIs(t, err, tt.expectedErr)
+				return
+			}
+			require.NoError(t, err)
+			require.NotEqual(t, types.Hash{}, stateRoot)
+			require.Greater(t, maxBytes, uint64(0))
+		})
+	}
+}
+
+func (s *DummyTestSuite) TestGetTxsWithConcurrency() {
+	t := s.T()
+	const numGoroutines = 10
+	const txsPerGoroutine = 100
+
+	var wg sync.WaitGroup
+	wg.Add(numGoroutines)
+
+	// Inject transactions concurrently
+	for i := 0; i < numGoroutines; i++ {
+		go func(id int) {
+			defer wg.Done()
+			for j := 0; j < txsPerGoroutine; j++ {
+				tx := types.Tx([]byte(fmt.Sprintf("tx-%d-%d", id, j)))
+				s.TxInjector.InjectTx(tx)
+			}
+		}(i)
+	}
+	wg.Wait()
+
+	// Verify all transactions are retrievable
+	txs, err := s.Exec.GetTxs(context.Background())
+	require.NoError(t, err)
+	require.Len(t, txs, numGoroutines*txsPerGoroutine)
+
+	// Verify transaction uniqueness
+	txMap := make(map[string]struct{})
+	for _, tx := range txs {
+		txMap[string(tx)] = struct{}{}
+	}
+	require.Len(t, txMap, numGoroutines*txsPerGoroutine)
+}