Skip to content

Commit 4856cd3

Browse files
test: Sign data before submitting and retrieve correctly (#2326)
<!-- Please read and fill out this form before submitting your PR. Please make sure you have reviewed our contributors guide before submitting your first PR. NOTE: PR titles should follow semantic commits: https://www.conventionalcommits.org/en/v1.0.0/ --> ## Overview Closes: #2312 <!-- Please provide an explanation of the PR, including the appropriate context, background, goal, and rationale. If there is an issue with this information, please provide a tl;dr and link the issue. Ex: Closes #<issue number> --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced support for cryptographically signed block data, including a new data structure that bundles data with its signature and signer information. - Added validation to ensure that received block data is properly signed and originates from the expected source. - **Bug Fixes** - Improved robustness of data handling by adding defensive checks and safer serialization/deserialization for block data and related types. - **Refactor** - Updated internal workflows to process and submit signed block data instead of unsigned batches. - Enhanced logging and validation messages to reflect the use of signed data. - Simplified batch submission logic by unifying it around signed data structures. - **Tests** - Updated tests to generate and verify signed block data, ensuring alignment with new data formats and validation logic. - Added comprehensive tests for signature generation and validation. - Improved test synchronization by replacing static delays with dynamic waiting for data availability. - Added extensive serialization and protobuf conversion test coverage for new and existing types. - **Documentation** - Improved comments and documentation to clarify the structure and purpose of signed data and headers. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
1 parent 189b208 commit 4856cd3

15 files changed

Lines changed: 1020 additions & 188 deletions

block/manager.go

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -603,7 +603,7 @@ func (m *Manager) publishBlockInternal(ctx context.Context) error {
603603
}
604604
}
605605

606-
signature, err = m.getSignature(header.Header)
606+
signature, err = m.getHeaderSignature(header.Header)
607607
if err != nil {
608608
return err
609609
}
@@ -906,7 +906,7 @@ func bytesToBatchData(data []byte) ([][]byte, error) {
906906
return result, nil
907907
}
908908

909-
func (m *Manager) getSignature(header types.Header) (types.Signature, error) {
909+
func (m *Manager) getHeaderSignature(header types.Header) (types.Signature, error) {
910910
b, err := header.MarshalBinary()
911911
if err != nil {
912912
return nil, err
@@ -917,6 +917,17 @@ func (m *Manager) getSignature(header types.Header) (types.Signature, error) {
917917
return m.signer.Sign(b)
918918
}
919919

920+
func (m *Manager) getDataSignature(data *types.Data) (types.Signature, error) {
921+
dataBz, err := data.MarshalBinary()
922+
if err != nil {
923+
return nil, err
924+
}
925+
if m.signer == nil {
926+
return nil, fmt.Errorf("signer is nil; cannot sign data")
927+
}
928+
return m.signer.Sign(dataBz)
929+
}
930+
920931
// NotifyNewTransactions signals that new transactions are available for processing
921932
// This method will be called by the Reaper when it receives new transactions
922933
func (m *Manager) NotifyNewTransactions() {
@@ -977,3 +988,20 @@ func (m *Manager) SaveCache() error {
977988
}
978989
return nil
979990
}
991+
992+
// isValidSignedData returns true if the data signature is valid for the expected sequencer.
993+
func (m *Manager) isValidSignedData(signedData *types.SignedData) bool {
994+
if signedData == nil || signedData.Txs == nil {
995+
return false
996+
}
997+
if !bytes.Equal(signedData.Signer.Address, m.genesis.ProposerAddress) {
998+
return false
999+
}
1000+
dataBytes, err := signedData.Data.MarshalBinary()
1001+
if err != nil {
1002+
return false
1003+
}
1004+
1005+
valid, err := signedData.Signer.PubKey.Verify(dataBytes, signedData.Signature)
1006+
return err == nil && valid
1007+
}

block/manager_test.go

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,3 +365,138 @@ func TestBytesToBatchData(t *testing.T) {
365365
assert.Error(err)
366366
assert.Contains(err.Error(), "corrupted data")
367367
}
368+
369+
// TestGetDataSignature_Success ensures a valid signature is returned when the signer is set.
370+
func TestGetDataSignature_Success(t *testing.T) {
371+
require := require.New(t)
372+
mockDAC := mocks.NewDA(t)
373+
m, _ := getManager(t, mockDAC, -1, -1)
374+
375+
privKey, _, err := crypto.GenerateKeyPair(crypto.Ed25519, 256)
376+
require.NoError(err)
377+
signer, err := noopsigner.NewNoopSigner(privKey)
378+
require.NoError(err)
379+
m.signer = signer
380+
_, data := types.GetRandomBlock(1, 2, "TestGetDataSignature")
381+
sig, err := m.getDataSignature(data)
382+
require.NoError(err)
383+
require.NotEmpty(sig)
384+
}
385+
386+
// TestGetDataSignature_NilSigner ensures the correct error is returned when the signer is nil.
387+
func TestGetDataSignature_NilSigner(t *testing.T) {
388+
require := require.New(t)
389+
mockDAC := mocks.NewDA(t)
390+
m, _ := getManager(t, mockDAC, -1, -1)
391+
392+
privKey, _, err := crypto.GenerateKeyPair(crypto.Ed25519, 256)
393+
require.NoError(err)
394+
signer, err := noopsigner.NewNoopSigner(privKey)
395+
require.NoError(err)
396+
m.signer = signer
397+
_, data := types.GetRandomBlock(1, 2, "TestGetDataSignature")
398+
399+
m.signer = nil
400+
_, err = m.getDataSignature(data)
401+
require.ErrorContains(err, "signer is nil; cannot sign data")
402+
}
403+
404+
// TestIsValidSignedData covers valid, nil, wrong proposer, and invalid signature cases for isValidSignedData.
405+
func TestIsValidSignedData(t *testing.T) {
406+
require := require.New(t)
407+
privKey, _, err := crypto.GenerateKeyPair(crypto.Ed25519, 256)
408+
require.NoError(err)
409+
testSigner, err := noopsigner.NewNoopSigner(privKey)
410+
require.NoError(err)
411+
proposerAddr, err := testSigner.GetAddress()
412+
require.NoError(err)
413+
gen := genesispkg.NewGenesis(
414+
"testchain",
415+
1,
416+
time.Now(),
417+
proposerAddr,
418+
)
419+
m := &Manager{
420+
signer: testSigner,
421+
genesis: gen,
422+
}
423+
424+
t.Run("valid signed data", func(t *testing.T) {
425+
batch := &types.Data{
426+
Txs: types.Txs{types.Tx("tx1"), types.Tx("tx2")},
427+
}
428+
sig, err := m.getDataSignature(batch)
429+
require.NoError(err)
430+
pubKey, err := m.signer.GetPublic()
431+
require.NoError(err)
432+
signedData := &types.SignedData{
433+
Data: *batch,
434+
Signature: sig,
435+
Signer: types.Signer{
436+
PubKey: pubKey,
437+
Address: proposerAddr,
438+
},
439+
}
440+
assert.True(t, m.isValidSignedData(signedData))
441+
})
442+
443+
t.Run("nil signed data", func(t *testing.T) {
444+
assert.False(t, m.isValidSignedData(nil))
445+
})
446+
447+
t.Run("nil Txs", func(t *testing.T) {
448+
signedData := &types.SignedData{
449+
Data: types.Data{},
450+
Signer: types.Signer{
451+
Address: proposerAddr,
452+
},
453+
}
454+
signedData.Txs = nil
455+
assert.False(t, m.isValidSignedData(signedData))
456+
})
457+
458+
t.Run("wrong proposer address", func(t *testing.T) {
459+
batch := &types.Data{
460+
Txs: types.Txs{types.Tx("tx1")},
461+
}
462+
sig, err := m.getDataSignature(batch)
463+
require.NoError(err)
464+
pubKey, err := m.signer.GetPublic()
465+
require.NoError(err)
466+
wrongAddr := make([]byte, len(proposerAddr))
467+
copy(wrongAddr, proposerAddr)
468+
wrongAddr[0] ^= 0xFF // flip a bit
469+
signedData := &types.SignedData{
470+
Data: *batch,
471+
Signature: sig,
472+
Signer: types.Signer{
473+
PubKey: pubKey,
474+
Address: wrongAddr,
475+
},
476+
}
477+
assert.False(t, m.isValidSignedData(signedData))
478+
})
479+
480+
t.Run("invalid signature", func(t *testing.T) {
481+
batch := &types.Data{
482+
Txs: types.Txs{types.Tx("tx1")},
483+
}
484+
sig, err := m.getDataSignature(batch)
485+
require.NoError(err)
486+
pubKey, err := m.signer.GetPublic()
487+
require.NoError(err)
488+
// Corrupt the signature
489+
badSig := make([]byte, len(sig))
490+
copy(badSig, sig)
491+
badSig[0] ^= 0xFF
492+
signedData := &types.SignedData{
493+
Data: *batch,
494+
Signature: badSig,
495+
Signer: types.Signer{
496+
PubKey: pubKey,
497+
Address: proposerAddr,
498+
},
499+
}
500+
assert.False(t, m.isValidSignedData(signedData))
501+
})
502+
}

block/retriever.go

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ func (m *Manager) processNextDAHeaderAndData(ctx context.Context) error {
8585
if m.handlePotentialHeader(ctx, bz, daHeight) {
8686
continue
8787
}
88-
m.handlePotentialBatch(ctx, bz, daHeight)
88+
m.handlePotentialData(ctx, bz, daHeight)
8989
}
9090
return nil
9191
}
@@ -117,6 +117,11 @@ func (m *Manager) handlePotentialHeader(ctx context.Context, bz []byte, daHeight
117117
m.logger.Debug("failed to decode unmarshalled header", "error", err)
118118
return true
119119
}
120+
// Stronger validation: check for obviously invalid headers using ValidateBasic
121+
if err := header.ValidateBasic(); err != nil {
122+
m.logger.Debug("blob does not look like a valid header", "daHeight", daHeight, "error", err)
123+
return false
124+
}
120125
// early validation to reject junk headers
121126
if !m.isUsingExpectedSingleSequencer(header) {
122127
m.logger.Debug("skipping header from unexpected sequencer",
@@ -140,36 +145,37 @@ func (m *Manager) handlePotentialHeader(ctx context.Context, bz []byte, daHeight
140145
return true
141146
}
142147

143-
// handlePotentialBatch tries to decode and process a batch. No return value.
144-
func (m *Manager) handlePotentialBatch(ctx context.Context, bz []byte, daHeight uint64) {
145-
var batchPb pb.Batch
146-
err := proto.Unmarshal(bz, &batchPb)
148+
// handlePotentialData tries to decode and process a data. No return value.
149+
func (m *Manager) handlePotentialData(ctx context.Context, bz []byte, daHeight uint64) {
150+
var signedData types.SignedData
151+
err := signedData.UnmarshalBinary(bz)
147152
if err != nil {
148-
m.logger.Debug("failed to unmarshal batch", "error", err)
153+
m.logger.Debug("failed to unmarshal signed data", "error", err)
149154
return
150155
}
151-
if len(batchPb.Txs) == 0 {
152-
m.logger.Debug("ignoring empty batch", "daHeight", daHeight)
156+
if len(signedData.Txs) == 0 {
157+
m.logger.Debug("ignoring empty signed data", "daHeight", daHeight)
153158
return
154159
}
155-
data := &types.Data{
156-
Txs: make(types.Txs, len(batchPb.Txs)),
157-
}
158-
for i, tx := range batchPb.Txs {
159-
data.Txs[i] = types.Tx(tx)
160+
161+
// Early validation to reject junk data
162+
if !m.isValidSignedData(&signedData) {
163+
m.logger.Debug("invalid data signature", "daHeight", daHeight)
164+
return
160165
}
161-
dataHashStr := data.DACommitment().String()
166+
167+
dataHashStr := signedData.Data.DACommitment().String()
162168
m.dataCache.SetDAIncluded(dataHashStr)
163169
m.sendNonBlockingSignalToDAIncluderCh()
164-
m.logger.Info("batch marked as DA included", "batchHash", dataHashStr, "daHeight", daHeight)
170+
m.logger.Info("signed data marked as DA included", "dataHash", dataHashStr, "daHeight", daHeight)
165171
if !m.dataCache.IsSeen(dataHashStr) {
166172
select {
167173
case <-ctx.Done():
168174
return
169175
default:
170-
m.logger.Warn("dataInCh backlog full, dropping batch", "daHeight", daHeight)
176+
m.logger.Warn("dataInCh backlog full, dropping signed data", "daHeight", daHeight)
171177
}
172-
m.dataInCh <- NewDataEvent{data, daHeight}
178+
m.dataInCh <- NewDataEvent{&signedData.Data, daHeight}
173179
}
174180
}
175181

0 commit comments

Comments
 (0)