feat: fix payload provider (#2368)

Required for evstack/ev-abci#101

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **New Features**
- Introduced a customizable mechanism for generating signature payloads
for headers, enabling more flexible signature verification.
- **Bug Fixes**
- Improved error handling when setting custom signature verifiers on
headers.
- **Chores**
- Updated function signatures and related calls throughout the codebase
to support the new signature payload provider.
- Minor improvements to code comments and permission literal formatting
in tests.
- Added new optional parameters to node startup commands and tests for
enhanced configuration flexibility.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Julien Robert <julien@rbrt.fr>

Author: randygrok

apps/evm/based/cmd/run.go

@@ -96,7 +96,6 @@ func NewExtendedRunNodeCmd(ctx context.Context) *cobra.Command {
 			return nil
 		},
 		RunE: func(cmd *cobra.Command, args []string) error {
-
 			nodeConfig, err := rollcmd.ParseConfig(cmd)
 			if err != nil {
 				return fmt.Errorf("failed to parse config: %w", err)
@@ -173,7 +172,7 @@ func NewExtendedRunNodeCmd(ctx context.Context) *cobra.Command {
 			// StartNode might need adjustment if it strictly requires coreda.Client methods.
 			// For now, assume it can work with coreda.DA or will be adjusted later.
 			// We also need to pass the namespace config for rollDA.
-			return rollcmd.StartNode(logger, cmd, executor, sequencer, rollDA, p2pClient, datastore, nodeConfig)
+			return rollcmd.StartNode(logger, cmd, executor, sequencer, rollDA, p2pClient, datastore, nodeConfig, nil)
 		},
 	}
 

apps/evm/single/cmd/run.go

@@ -89,7 +89,7 @@ var RunCmd = &cobra.Command{
 			return err
 		}
 
-		return rollcmd.StartNode(logger, cmd, executor, sequencer, &daJrpc.DA, p2pClient, datastore, nodeConfig)
+		return rollcmd.StartNode(logger, cmd, executor, sequencer, &daJrpc.DA, p2pClient, datastore, nodeConfig, nil)
 	},
 }
 

apps/testapp/cmd/run.go

@@ -106,6 +106,6 @@ var RunCmd = &cobra.Command{
 			return err
 		}
 
-		return rollcmd.StartNode(logger, cmd, executor, sequencer, &daJrpc.DA, p2pClient, datastore, nodeConfig)
+		return rollcmd.StartNode(logger, cmd, executor, sequencer, &daJrpc.DA, p2pClient, datastore, nodeConfig, nil)
 	},
 }

block/manager.go

@@ -69,6 +69,10 @@ var (
 // This allows for overriding the behavior in tests.
 type publishBlockFunc func(ctx context.Context) error
 
+func defaultSignaturePayloadProvider(header *types.Header) ([]byte, error) {
+	return header.MarshalBinary()
+}
+
 // NewHeaderEvent is used to pass header and DA height to headerInCh
 type NewHeaderEvent struct {
 	Header   *types.SignedHeader
@@ -165,10 +169,18 @@ type Manager struct {
 	// dataCommitmentToHeight tracks the height a data commitment (data hash) has been seen on.
 	// Key: data commitment (string), Value: uint64 (height)
 	dataCommitmentToHeight sync.Map
+
+	// signaturePayloadProvider is used to provide a signature payload for the header.
+	// It is used to sign the header with the provided signer.
+	signaturePayloadProvider types.SignaturePayloadProvider
 }
 
 // getInitialState tries to load lastState from Store, and if it's not available it reads genesis.
-func getInitialState(ctx context.Context, genesis genesis.Genesis, signer signer.Signer, store store.Store, exec coreexecutor.Executor, logger log.Logger) (types.State, error) {
+func getInitialState(ctx context.Context, genesis genesis.Genesis, signer signer.Signer, store store.Store, exec coreexecutor.Executor, logger log.Logger, signaturePayloadProvider types.SignaturePayloadProvider) (types.State, error) {
+	if signaturePayloadProvider == nil {
+		signaturePayloadProvider = defaultSignaturePayloadProvider
+	}
+
 	// Load the state from store.
 	s, err := store.GetState(ctx)
 
@@ -206,28 +218,33 @@ func getInitialState(ctx context.Context, genesis genesis.Genesis, signer signer
 				return types.State{}, fmt.Errorf("failed to get public key: %w", err)
 			}
 
-			b, err := header.MarshalBinary()
+			b, err := signaturePayloadProvider(&header)
 			if err != nil {
-				return types.State{}, err
+				return types.State{}, fmt.Errorf("failed to get signature payload: %w", err)
 			}
 			signature, err = signer.Sign(b)
 			if err != nil {
 				return types.State{}, fmt.Errorf("failed to get header signature: %w", err)
 			}
 		}
 
-		err = store.SaveBlockData(ctx,
-			&types.SignedHeader{
-				Header: header,
-				Signer: types.Signer{
-					PubKey:  pubKey,
-					Address: genesis.ProposerAddress,
-				},
-				Signature: signature,
+		genesisHeader := &types.SignedHeader{
+			Header: header,
+			Signer: types.Signer{
+				PubKey:  pubKey,
+				Address: genesis.ProposerAddress,
 			},
-			&types.Data{},
-			&signature,
-		)
+			Signature: signature,
+		}
+
+		// Set the same custom verifier used during normal block validation
+		if err := genesisHeader.SetCustomVerifier(func(h *types.Header) ([]byte, error) {
+			return signaturePayloadProvider(h)
+		}); err != nil {
+			return types.State{}, fmt.Errorf("failed to set custom verifier for genesis header: %w", err)
+		}
+
+		err = store.SaveBlockData(ctx, genesisHeader, &types.Data{}, &signature)
 		if err != nil {
 			return types.State{}, fmt.Errorf("failed to save genesis block: %w", err)
 		}
@@ -275,8 +292,13 @@ func NewManager(
 	seqMetrics *Metrics,
 	gasPrice float64,
 	gasMultiplier float64,
+	signaturePayloadProvider types.SignaturePayloadProvider,
 ) (*Manager, error) {
-	s, err := getInitialState(ctx, genesis, signer, store, exec, logger)
+	if signaturePayloadProvider == nil {
+		signaturePayloadProvider = defaultSignaturePayloadProvider
+	}
+
+	s, err := getInitialState(ctx, genesis, signer, store, exec, logger, signaturePayloadProvider)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get initial state: %w", err)
 	}
@@ -339,29 +361,30 @@ func NewManager(
 		headerBroadcaster: headerBroadcaster,
 		dataBroadcaster:   dataBroadcaster,
 		// channels are buffered to avoid blocking on input/output operations, buffer sizes are arbitrary
-		headerInCh:          make(chan NewHeaderEvent, eventInChLength),
-		dataInCh:            make(chan NewDataEvent, eventInChLength),
-		headerStoreCh:       make(chan struct{}, 1),
-		dataStoreCh:         make(chan struct{}, 1),
-		headerStore:         headerStore,
-		dataStore:           dataStore,
-		lastStateMtx:        new(sync.RWMutex),
-		lastBatchData:       lastBatchData,
-		headerCache:         cache.NewCache[types.SignedHeader](),
-		dataCache:           cache.NewCache[types.Data](),
-		retrieveCh:          make(chan struct{}, 1),
-		daIncluderCh:        make(chan struct{}, 1),
-		logger:              logger,
-		txsAvailable:        false,
-		pendingHeaders:      pendingHeaders,
-		metrics:             seqMetrics,
-		sequencer:           sequencer,
-		exec:                exec,
-		da:                  da,
-		gasPrice:            gasPrice,
-		gasMultiplier:       gasMultiplier,
-		txNotifyCh:          make(chan struct{}, 1), // Non-blocking channel
-		batchSubmissionChan: make(chan coresequencer.Batch, eventInChLength),
+		headerInCh:               make(chan NewHeaderEvent, eventInChLength),
+		dataInCh:                 make(chan NewDataEvent, eventInChLength),
+		headerStoreCh:            make(chan struct{}, 1),
+		dataStoreCh:              make(chan struct{}, 1),
+		headerStore:              headerStore,
+		dataStore:                dataStore,
+		lastStateMtx:             new(sync.RWMutex),
+		lastBatchData:            lastBatchData,
+		headerCache:              cache.NewCache[types.SignedHeader](),
+		dataCache:                cache.NewCache[types.Data](),
+		retrieveCh:               make(chan struct{}, 1),
+		daIncluderCh:             make(chan struct{}, 1),
+		logger:                   logger,
+		txsAvailable:             false,
+		pendingHeaders:           pendingHeaders,
+		metrics:                  seqMetrics,
+		sequencer:                sequencer,
+		exec:                     exec,
+		da:                       da,
+		gasPrice:                 gasPrice,
+		gasMultiplier:            gasMultiplier,
+		txNotifyCh:               make(chan struct{}, 1), // Non-blocking channel
+		batchSubmissionChan:      make(chan coresequencer.Batch, eventInChLength),
+		signaturePayloadProvider: signaturePayloadProvider,
 	}
 
 	// initialize da included height
@@ -611,6 +634,14 @@ func (m *Manager) publishBlockInternal(ctx context.Context) error {
 	// set the signature to current block's signed header
 	header.Signature = signature
 
+	// Set the custom verifier to ensure proper signature validation (if not already set by executor)
+	// Note: The executor may have already set a custom verifier during transaction execution
+	if err := header.SetCustomVerifier(func(h *types.Header) ([]byte, error) {
+		return m.signaturePayloadProvider(h)
+	}); err != nil {
+		return fmt.Errorf("failed to set custom verifier: %w", err)
+	}
+
 	if err := header.ValidateBasic(); err != nil {
 		// If this ever happens, for recovery, check for a mismatch between the configured signing key and the proposer address in the genesis file
 		return fmt.Errorf("header validation error: %w", err)
@@ -914,7 +945,7 @@ func bytesToBatchData(data []byte) ([][]byte, error) {
 }
 
 func (m *Manager) getHeaderSignature(header types.Header) (types.Signature, error) {
-	b, err := header.MarshalBinary()
+	b, err := m.signaturePayloadProvider(&header)
 	if err != nil {
 		return nil, err
 	}

block/manager_test.go

@@ -74,7 +74,7 @@ func TestInitialStateClean(t *testing.T) {
 	mockExecutor.On("InitChain", ctx, genesisData.GenesisDAStartTime, genesisData.InitialHeight, genesisData.ChainID).
 		Return([]byte("mockAppHash"), uint64(1000), nil).Once()
 
-	s, err := getInitialState(ctx, genesisData, nil, emptyStore, mockExecutor, logger)
+	s, err := getInitialState(ctx, genesisData, nil, emptyStore, mockExecutor, logger, nil /* uses default signature verification */)
 	require.NoError(err)
 	initialHeight := genesisData.InitialHeight
 	require.Equal(initialHeight-1, s.LastBlockHeight)
@@ -105,7 +105,7 @@ func TestInitialStateStored(t *testing.T) {
 	mockExecutor := mocks.NewExecutor(t)
 
 	// getInitialState should not call InitChain if state exists
-	s, err := getInitialState(ctx, genesisData, nil, store, mockExecutor, logger)
+	s, err := getInitialState(ctx, genesisData, nil, store, mockExecutor, logger, nil /* uses default signature verification */)
 	require.NoError(err)
 	require.Equal(s.LastBlockHeight, uint64(100))
 	require.Equal(s.InitialHeight, uint64(1))
@@ -140,7 +140,7 @@ func TestInitialStateUnexpectedHigherGenesis(t *testing.T) {
 	require.NoError(err)
 	mockExecutor := mocks.NewExecutor(t)
 
-	_, err = getInitialState(ctx, genesis, nil, store, mockExecutor, logger)
+	_, err = getInitialState(ctx, genesis, nil, store, mockExecutor, logger, nil /* uses default signature verification */)
 	require.EqualError(err, "genesis.InitialHeight (2) is greater than last stored state's LastBlockHeight (0)")
 
 	// Assert mock expectations (InitChain should not have been called)

block/publish_block2_test.go

@@ -212,6 +212,7 @@ func setupBlockManager(t *testing.T, ctx context.Context, workDir string, mainKV
 		NopMetrics(),
 		1.,
 		1.,
+		nil, // using default signature verification
 	)
 	require.NoError(t, err)
 	return result, headerSyncService, dataSyncService

block/publish_block_test.go

@@ -78,14 +78,15 @@ func setupManagerForPublishBlockTest(
 		dataBroadcaster: broadcasterFn[*types.Data](func(ctx context.Context, payload *types.Data) error {
 			return nil
 		}),
-		headerStore:    headerStore,
-		daHeight:       &atomic.Uint64{},
-		dataStore:      dataStore,
-		headerCache:    cache.NewCache[types.SignedHeader](),
-		dataCache:      cache.NewCache[types.Data](),
-		lastStateMtx:   &sync.RWMutex{},
-		metrics:        NopMetrics(),
-		pendingHeaders: nil,
+		headerStore:              headerStore,
+		daHeight:                 &atomic.Uint64{},
+		dataStore:                dataStore,
+		headerCache:              cache.NewCache[types.SignedHeader](),
+		dataCache:                cache.NewCache[types.Data](),
+		lastStateMtx:             &sync.RWMutex{},
+		metrics:                  NopMetrics(),
+		pendingHeaders:           nil,
+		signaturePayloadProvider: defaultSignaturePayloadProvider,
 	}
 	manager.publishBlock = manager.publishBlockInternal
 
@@ -168,8 +169,9 @@ func Test_publishBlock_NoBatch(t *testing.T) {
 			store:  mockStore,
 			logger: logger,
 		},
-		lastStateMtx: &sync.RWMutex{},
-		metrics:      NopMetrics(),
+		lastStateMtx:             &sync.RWMutex{},
+		metrics:                  NopMetrics(),
+		signaturePayloadProvider: defaultSignaturePayloadProvider,
 	}
 
 	m.publishBlock = m.publishBlockInternal
@@ -267,7 +269,8 @@ func Test_publishBlock_EmptyBatch(t *testing.T) {
 		dataBroadcaster: broadcasterFn[*types.Data](func(ctx context.Context, payload *types.Data) error {
 			return nil
 		}),
-		daHeight: &daH,
+		daHeight:                 &daH,
+		signaturePayloadProvider: defaultSignaturePayloadProvider,
 	}
 
 	m.publishBlock = m.publishBlockInternal

node/full.go

@@ -29,6 +29,7 @@ import (
 	"github.com/rollkit/rollkit/pkg/signer"
 	"github.com/rollkit/rollkit/pkg/store"
 	rollkitsync "github.com/rollkit/rollkit/pkg/sync"
+	"github.com/rollkit/rollkit/types"
 )
 
 // prefixes used in KV store to separate rollkit data from execution environment data (if the same data base is reused)
@@ -80,6 +81,7 @@ func newFullNode(
 	da coreda.DA,
 	metricsProvider MetricsProvider,
 	logger log.Logger,
+	signaturePayloadProvider types.SignaturePayloadProvider,
 ) (fn *FullNode, err error) {
 	seqMetrics, _ := metricsProvider(genesis.ChainID)
 
@@ -111,6 +113,7 @@ func newFullNode(
 		seqMetrics,
 		nodeConfig.DA.GasPrice,
 		nodeConfig.DA.GasMultiplier,
+		signaturePayloadProvider,
 	)
 	if err != nil {
 		return nil, err
@@ -197,6 +200,7 @@ func initBlockManager(
 	seqMetrics *block.Metrics,
 	gasPrice float64,
 	gasMultiplier float64,
+	signaturePayloadProvider types.SignaturePayloadProvider,
 ) (*block.Manager, error) {
 	logger.Debug("Proposer address", "address", genesis.ProposerAddress)
 
@@ -217,6 +221,7 @@ func initBlockManager(
 		seqMetrics,
 		gasPrice,
 		gasMultiplier,
+		signaturePayloadProvider,
 	)
 	if err != nil {
 		return nil, fmt.Errorf("error while initializing BlockManager: %w", err)

node/helpers_test.go

@@ -123,6 +123,7 @@ func newTestNode(
 		ds,
 		DefaultMetricsProvider(rollkitconfig.DefaultInstrumentationConfig()),
 		log.NewTestLogger(t),
+		nil,
 	)
 	require.NoError(t, err)
 
@@ -187,6 +188,7 @@ func createNodesWithCleanup(t *testing.T, num int, config rollkitconfig.Config)
 		ds,
 		DefaultMetricsProvider(rollkitconfig.DefaultInstrumentationConfig()),
 		log.NewTestLogger(t),
+		nil,
 	)
 	require.NoError(err)
 
@@ -224,6 +226,7 @@ func createNodesWithCleanup(t *testing.T, num int, config rollkitconfig.Config)
 			dssync.MutexWrap(datastore.NewMapDatastore()),
 			DefaultMetricsProvider(rollkitconfig.DefaultInstrumentationConfig()),
 			log.NewTestLogger(t),
+			nil,
 		)
 		require.NoError(err)
 		// Update cleanup to cancel the context instead of calling Stop

node/node.go

@@ -14,6 +14,7 @@ import (
 	"github.com/rollkit/rollkit/pkg/p2p"
 	"github.com/rollkit/rollkit/pkg/service"
 	"github.com/rollkit/rollkit/pkg/signer"
+	"github.com/rollkit/rollkit/types"
 )
 
 // Node is the interface for an application node
@@ -38,6 +39,7 @@ func NewNode(
 	database ds.Batching,
 	metricsProvider MetricsProvider,
 	logger log.Logger,
+	signaturePayloadProvider types.SignaturePayloadProvider,
 ) (Node, error) {
 	if conf.Node.Light {
 		return newLightNode(conf, genesis, p2pClient, database, logger)
@@ -55,5 +57,6 @@ func NewNode(
 		da,
 		metricsProvider,
 		logger,
+		signaturePayloadProvider,
 	)
 }