Skip to content
Closed
Show file tree
Hide file tree
Changes from 13 commits
Commits
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
2 changes: 1 addition & 1 deletion apps/testapp/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ replace (
require (
github.com/ipfs/go-datastore v0.8.2
github.com/rollkit/rollkit v0.0.0-00010101000000-000000000000
github.com/rollkit/rollkit/core v0.0.0-20250312114929-104787ba1a4c
github.com/rollkit/rollkit/da v0.0.0-00010101000000-000000000000
github.com/rollkit/rollkit/sequencers/single v0.0.0-00010101000000-000000000000
github.com/spf13/cobra v1.9.1
Expand Down Expand Up @@ -141,7 +142,6 @@ require (
github.com/quic-go/quic-go v0.50.1 // indirect
github.com/quic-go/webtransport-go v0.8.1-0.20241018022711-4ac2c9250e66 // indirect
github.com/raulk/go-watchdog v1.3.0 // indirect
github.com/rollkit/rollkit/core v0.0.0-20250312114929-104787ba1a4c // indirect
github.com/sagikazarmark/locafero v0.4.0 // indirect
github.com/sagikazarmark/slog-shim v0.1.0 // indirect
github.com/sourcegraph/conc v0.3.0 // indirect
Expand Down
7 changes: 7 additions & 0 deletions apps/testapp/kv/kvexecutor.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

ds "github.com/ipfs/go-datastore"
"github.com/ipfs/go-datastore/query"
"github.com/rollkit/rollkit/core/execution"
"github.com/rollkit/rollkit/pkg/store"
)

Expand Down Expand Up @@ -243,6 +244,12 @@ func (k *KVExecutor) SetFinal(ctx context.Context, blockHeight uint64) error {
return k.db.Put(ctx, ds.NewKey("/finalizedHeight"), []byte(fmt.Sprintf("%d", blockHeight)))
}

// GetExecutionMode returns the execution mode for this executor.
// KVExecutor uses delayed execution mode.
func (k *KVExecutor) GetExecutionMode() execution.ExecutionMode {
return execution.ExecutionModeDelayed
}

// InjectTx adds a transaction to the mempool channel.
// Uses a non-blocking send to avoid blocking the caller if the channel is full.
func (k *KVExecutor) InjectTx(tx []byte) {
Expand Down
149 changes: 114 additions & 35 deletions block/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,8 @@ type Manager struct {
metrics *Metrics

exec coreexecutor.Executor
// executionMode caches the execution mode from the executor
executionMode coreexecutor.ExecutionMode

// daIncludedHeight is rollkit height at which all blocks have been included
// in the DA
Expand Down Expand Up @@ -351,6 +353,8 @@ func NewManager(
daH := atomic.Uint64{}
daH.Store(s.DAHeight)

executionMode := exec.GetExecutionMode()

m := &Manager{
signer: signer,
config: config,
Expand Down Expand Up @@ -380,6 +384,7 @@ func NewManager(
metrics: seqMetrics,
sequencer: sequencer,
exec: exec,
executionMode: executionMode,
da: da,
gasPrice: gasPrice,
gasMultiplier: gasMultiplier,
Expand Down Expand Up @@ -800,22 +805,26 @@ func (m *Manager) execValidate(lastState types.State, header *types.SignedHeader
}

// // Verify that the header's timestamp is strictly greater than the last block's time
// headerTime := header.Time()
// if header.Height() > 1 && lastState.LastBlockTime.After(headerTime) {
// return fmt.Errorf("block time must be strictly increasing: got %v, last block time was %v",
// headerTime.UnixNano(), lastState.LastBlockTime)
// }

// // Validate that the header's AppHash matches the lastState's AppHash
// // Note: Assumes deferred execution
// if !bytes.Equal(header.AppHash, lastState.AppHash) {
// return fmt.Errorf("app hash mismatch: expected %x, got %x", lastState.AppHash, header.AppHash)
// }
headerTime := header.Time()
if header.Height() > 1 && lastState.LastBlockTime.After(headerTime) {
return fmt.Errorf("block time must be strictly increasing: got %v, last block time was %v",
headerTime.UnixNano(), lastState.LastBlockTime)
}

// Validate AppHash based on execution mode
if m.executionMode == coreexecutor.ExecutionModeDelayed {
// In delayed mode, AppHash should match the last state's AppHash
if !bytes.Equal(header.AppHash, lastState.AppHash) {
return fmt.Errorf("appHash mismatch in delayed mode: expected %x, got %x", lastState.AppHash, header.AppHash)
}
Comment thread
tac0turtle marked this conversation as resolved.
}
// Note: For immediate mode, we can't validate AppHash against lastState because
// it represents the state after executing the current block's transactions

return nil
}

func (m *Manager) execCreateBlock(_ context.Context, height uint64, lastSignature *types.Signature, lastHeaderHash types.Hash, _ types.State, batchData *BatchData) (*types.SignedHeader, *types.Data, error) {
func (m *Manager) execCreateBlock(ctx context.Context, height uint64, lastSignature *types.Signature, lastHeaderHash types.Hash, lastState types.State, batchData *BatchData) (*types.SignedHeader, *types.Data, error) {
// Use when batchData is set to data IDs from the DA layer
// batchDataIDs := convertBatchDataToBytes(batchData.Data)

Expand All @@ -840,21 +849,54 @@ func (m *Manager) execCreateBlock(_ context.Context, height uint64, lastSignatur
// Determine if this is an empty block
isEmpty := batchData.Batch == nil || len(batchData.Transactions) == 0

// Create block data with appropriate transactions
blockData := &types.Data{
Txs: make(types.Txs, 0), // Start with empty transaction list
}

// Only add transactions if this is not an empty block
if !isEmpty {
blockData.Txs = make(types.Txs, len(batchData.Transactions))
for i := range batchData.Transactions {
blockData.Txs[i] = types.Tx(batchData.Transactions[i])
}
}

// Determine AppHash based on execution mode
var appHash []byte
if m.executionMode == coreexecutor.ExecutionModeImmediate && !isEmpty {
// For immediate execution, execute transactions now to get the new state root
rawTxs := make([][]byte, len(blockData.Txs))
for i := range blockData.Txs {
rawTxs[i] = blockData.Txs[i]
}
Comment thread
tac0turtle marked this conversation as resolved.
Outdated

// Execute transactions
newStateRoot, _, err := m.exec.ExecuteTxs(ctx, rawTxs, height, batchData.Time, lastState.AppHash)
if err != nil {
return nil, nil, fmt.Errorf("failed to execute transactions for immediate mode: %w", err)
}
appHash = newStateRoot
} else {
// For delayed execution or empty blocks, use the app hash from last state
appHash = lastState.AppHash
}

header := &types.SignedHeader{
Header: types.Header{
Version: types.Version{
Block: m.lastState.Version.Block,
App: m.lastState.Version.App,
Block: lastState.Version.Block,
App: lastState.Version.App,
},
BaseHeader: types.BaseHeader{
ChainID: m.lastState.ChainID,
ChainID: lastState.ChainID,
Height: height,
Time: uint64(batchData.UnixNano()), //nolint:gosec // why is time unix? (tac0turtle)
},
LastHeaderHash: lastHeaderHash,
// DataHash is set at the end of the function
// DataHash is set below
ConsensusHash: make(types.Hash, 32),
AppHash: m.lastState.AppHash,
AppHash: appHash,
ProposerAddress: m.genesis.ProposerAddress,
},
Signature: *lastSignature,
Expand All @@ -864,17 +906,8 @@ func (m *Manager) execCreateBlock(_ context.Context, height uint64, lastSignatur
},
}

// Create block data with appropriate transactions
blockData := &types.Data{
Txs: make(types.Txs, 0), // Start with empty transaction list
}

// Only add transactions if this is not an empty block
// Set DataHash
if !isEmpty {
blockData.Txs = make(types.Txs, len(batchData.Transactions))
for i := range batchData.Transactions {
blockData.Txs[i] = types.Tx(batchData.Transactions[i])
}
header.DataHash = blockData.DACommitment()
} else {
header.DataHash = dataHashForEmptyTxs
Expand All @@ -884,15 +917,61 @@ func (m *Manager) execCreateBlock(_ context.Context, height uint64, lastSignatur
}

func (m *Manager) execApplyBlock(ctx context.Context, lastState types.State, header *types.SignedHeader, data *types.Data) (types.State, error) {
rawTxs := make([][]byte, len(data.Txs))
for i := range data.Txs {
rawTxs[i] = data.Txs[i]
}
var newStateRoot []byte

// Check if we need to execute transactions
if m.executionMode == coreexecutor.ExecutionModeImmediate {
// For immediate mode, the aggregator already executed during block creation
// But syncing nodes need to execute to verify the AppHash
// Check if we're the block creator by verifying we signed this block
isBlockCreator := false
if m.signer != nil {
myAddress, err := m.signer.GetAddress()
if err == nil && bytes.Equal(header.ProposerAddress, myAddress) {
// Also verify this is the current height we're producing
currentHeight, err := m.store.Height(ctx)
if err == nil && header.Height() == currentHeight+1 {
isBlockCreator = true
}
}
}

ctx = context.WithValue(ctx, types.SignedHeaderContextKey, header)
newStateRoot, _, err := m.exec.ExecuteTxs(ctx, rawTxs, header.Height(), header.Time(), lastState.AppHash)
if err != nil {
return types.State{}, fmt.Errorf("failed to execute transactions: %w", err)
if isBlockCreator {
// We created this block, so we already executed transactions in execCreateBlock
// Just use the AppHash from the header
newStateRoot = header.AppHash
} else {
// We're syncing this block, need to execute to verify AppHash
rawTxs := make([][]byte, len(data.Txs))
for i := range data.Txs {
rawTxs[i] = data.Txs[i]
}

ctx = context.WithValue(ctx, types.SignedHeaderContextKey, header)
computedStateRoot, _, err := m.exec.ExecuteTxs(ctx, rawTxs, header.Height(), header.Time(), lastState.AppHash)
if err != nil {
return types.State{}, fmt.Errorf("failed to execute transactions: %w", err)
}

// Verify the AppHash matches
if !bytes.Equal(header.AppHash, computedStateRoot) {
return types.State{}, fmt.Errorf("AppHash mismatch in immediate mode: expected %x, got %x", computedStateRoot, header.AppHash)
}
newStateRoot = computedStateRoot
}
Comment on lines +915 to +952

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Complex branching logic needs careful testing.

The logic for determining whether to execute transactions in immediate mode is complex with multiple conditions. Consider adding unit tests to cover all branches, especially:

  • Block creator path (trusting header's AppHash)
  • Syncing node path (verifying AppHash)
  • Error cases in address/height retrieval

Based on the static analysis, these execution paths lack test coverage. Would you like me to help generate comprehensive unit tests for the immediate execution mode?


🏁 Script executed:

#!/bin/bash
# Search for unit tests covering the immediate execution mode branching in block/manager.go

# 1. Tests mentioning "immediate mode"
rg -n "immediate mode" --glob "*_test.go"

# 2. Tests invoking ExecuteTxs
rg -n "ExecuteTxs" --glob "*_test.go"

# 3. Tests referencing signer.GetAddress
rg -n "GetAddress" --glob "*_test.go"

# 4. Tests referencing store.Height
rg -n "Height(" --glob "*_test.go"

Length of output: 5308


Add unit tests covering immediate‐mode branching in block/manager.go

The complex logic in block/manager.go (lines 924–961) isn’t directly exercised by any existing tests. Please add unit tests around the following paths to ensure full coverage:

• Block creator path (m.signer.GetAddress succeeds, store.Height returns currentHeight, header.Height()==currentHeight+1 → newStateRoot = header.AppHash; ExecuteTxs must not be called)
• Syncing path (any of the signer/height checks fail → ExecuteTxs executes all Txs, matches header.AppHash)
• Error cases:
– signer.GetAddress returns an error
– store.Height returns an error
– exec.ExecuteTxs returns an error (propagate failure)
– AppHash mismatch between header and computedStateRoot

Locations needing tests:

  • block/manager.go:924–961
🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 924-935: block/manager.go#L924-L935
Added lines #L924 - L935 were not covered by tests


[warning] 939-948: block/manager.go#L939-L948
Added lines #L939 - L948 were not covered by tests


[warning] 950-954: block/manager.go#L950-L954
Added lines #L950 - L954 were not covered by tests


[warning] 957-960: block/manager.go#L957-L960
Added lines #L957 - L960 were not covered by tests

🤖 Prompt for AI Agents
In block/manager.go between lines 924 and 961, add unit tests to cover all
branches of the immediate-mode logic. Create tests for the block creator path
where m.signer.GetAddress and store.Height succeed and header.Height equals
currentHeight+1, ensuring newStateRoot is set to header.AppHash and ExecuteTxs
is not called. Add tests for the syncing path where any signer or height check
fails, verifying ExecuteTxs runs all transactions and the computed AppHash
matches header.AppHash. Also, include tests for error cases where
signer.GetAddress returns an error, store.Height returns an error,
exec.ExecuteTxs returns an error (which should propagate), and where there is an
AppHash mismatch between header and computedStateRoot.

} else {
// For delayed mode, always execute transactions
rawTxs := make([][]byte, len(data.Txs))
for i := range data.Txs {
rawTxs[i] = data.Txs[i]
}
Comment on lines +955 to +958

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

This block of code to convert types.Txs to [][]byte is duplicated from lines 945-948 in this same function, and is also present in execCreateBlock. Consider extracting this logic into a helper function to improve maintainability and reduce code duplication.


ctx = context.WithValue(ctx, types.SignedHeaderContextKey, header)
var err error
newStateRoot, _, err = m.exec.ExecuteTxs(ctx, rawTxs, header.Height(), header.Time(), lastState.AppHash)
if err != nil {
return types.State{}, fmt.Errorf("failed to execute transactions: %w", err)
}
}
Comment thread
tac0turtle marked this conversation as resolved.

s, err := lastState.NextState(header, newStateRoot)
Expand Down
42 changes: 20 additions & 22 deletions block/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -575,28 +575,26 @@ func TestManager_execValidate(t *testing.T) {
require.NoError(err)
})

// TODO: https://github.com/rollkit/rollkit/issues/2250

// t.Run("non-monotonic block time with height > 1", func(t *testing.T) {
// state, header, data, privKey := makeValid()
// state.LastBlockTime = time.Now().Add(time.Minute)
// state.LastBlockHeight = 1
// header.BaseHeader.Height = state.LastBlockHeight + 1
// data.Metadata.Height = state.LastBlockHeight + 1
// signer, err := noopsigner.NewNoopSigner(privKey)
// require.NoError(err)
// header.Signature, err = types.GetSignature(header.Header, signer)
// require.NoError(err)
// err = m.execValidate(state, header, data)
// require.ErrorContains(err, "block time must be strictly increasing")
// })

// t.Run("app hash mismatch", func(t *testing.T) {
// state, header, data, _ := makeValid()
// state.AppHash = []byte("different")
// err := m.execValidate(state, header, data)
// require.ErrorContains(err, "app hash mismatch")
// })
t.Run("non-monotonic block time with height > 1", func(t *testing.T) {
state, header, data, privKey := makeValid()
state.LastBlockTime = time.Now().Add(time.Minute)
state.LastBlockHeight = 1
header.BaseHeader.Height = state.LastBlockHeight + 1
data.Metadata.Height = state.LastBlockHeight + 1
signer, err := noopsigner.NewNoopSigner(privKey)
require.NoError(err)
header.Signature, err = types.GetSignature(header.Header, signer)
require.NoError(err)
err = m.execValidate(state, header, data)
require.ErrorContains(err, "block time must be strictly increasing")
})

t.Run("app hash mismatch", func(t *testing.T) {
state, header, data, _ := makeValid()
state.AppHash = []byte("different")
err := m.execValidate(state, header, data)
require.ErrorContains(err, "appHash mismatch in delayed mode")
})
}

// TestGetterMethods tests simple getter methods for the Manager
Expand Down
5 changes: 5 additions & 0 deletions block/publish_block_p2p_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

coreexecution "github.com/rollkit/rollkit/core/execution"
coresequencer "github.com/rollkit/rollkit/core/sequencer"
"github.com/rollkit/rollkit/pkg/config"
genesispkg "github.com/rollkit/rollkit/pkg/genesis"
Expand Down Expand Up @@ -241,6 +242,10 @@ func (m mockExecutor) SetFinal(ctx context.Context, blockHeight uint64) error {
return nil
}

func (m mockExecutor) GetExecutionMode() coreexecution.ExecutionMode {
return coreexecution.ExecutionModeDelayed
}

var rnd = rand.New(rand.NewSource(1)) //nolint:gosec // test code only

func bytesN(n int) []byte {
Expand Down
6 changes: 6 additions & 0 deletions core/execution/dummy.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,12 @@ func (e *DummyExecutor) SetFinal(ctx context.Context, blockHeight uint64) error
return fmt.Errorf("cannot set finalized block at height %d", blockHeight)
}

// GetExecutionMode returns the execution mode for this executor.
// DummyExecutor uses delayed execution mode.
func (e *DummyExecutor) GetExecutionMode() ExecutionMode {
return ExecutionModeDelayed
}

func (e *DummyExecutor) removeExecutedTxs(txs [][]byte) {
e.injectedTxs = slices.DeleteFunc(e.injectedTxs, func(tx []byte) bool {
return slices.ContainsFunc(txs, func(t []byte) bool { return bytes.Equal(tx, t) })
Expand Down
31 changes: 31 additions & 0 deletions core/execution/execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,23 @@ import (
"time"
)

// ExecutionMode defines when state transitions are applied relative to block creation
type ExecutionMode int

const (
// ExecutionModeDelayed means the AppHash in block N represents the state after executing block N-1
// This is the traditional approach used by CometBFT/Tendermint
ExecutionModeDelayed ExecutionMode = iota

// ExecutionModeImmediate means the AppHash in block N represents the state after executing block N
// Transactions are executed during block creation and the resulting state root is included
ExecutionModeImmediate

// ExecutionModeOptimistic means transactions are executed optimistically and finalized later
// This allows for faster block creation but requires a finalization step to confirm state
// ExecutionModeOptimistic
)

// 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.
Expand Down Expand Up @@ -53,6 +70,7 @@ type Executor interface {
// - Must maintain deterministic execution
// - Must respect context cancellation/timeout
// - The rest of the rules are defined by the specific execution layer
// - Must verify state root after execution
//
// Parameters:
// - ctx: Context for timeout/cancellation control
Expand Down Expand Up @@ -82,4 +100,17 @@ type Executor interface {
// Returns:
// - error: Any errors during finalization
SetFinal(ctx context.Context, blockHeight uint64) error

// GetExecutionMode returns the execution mode supported by this executor.
// This determines when state transitions are applied:
// - Delayed: AppHash in block N represents state after executing block N-1
// - Immediate: AppHash in block N represents state after executing block N
//
// Requirements:
// - Must return consistent value across all calls
// - Mode cannot change after initialization
//
// Returns:
// - ExecutionMode: The execution mode supported by this executor
GetExecutionMode() ExecutionMode
}
7 changes: 7 additions & 0 deletions execution/evm/execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,13 @@ func (c *EngineClient) SetFinal(ctx context.Context, blockHeight uint64) error {
return c.setFinal(ctx, blockHash, true)
}

// GetExecutionMode returns the execution mode for this executor.
// EngineClient uses immediate execution mode since it calculates the state root
// during transaction execution via engine_getPayloadV4.
func (c *EngineClient) GetExecutionMode() execution.ExecutionMode {
return execution.ExecutionModeImmediate
}

func (c *EngineClient) derivePrevRandao(blockHeight uint64) common.Hash {
return common.BigToHash(new(big.Int).SetUint64(blockHeight))
}
Expand Down
Loading