Skip to content

Commit a76f88c

Browse files
julienrbrtjulienrbrt
committed
refactor(block): log error on data verification failed
1 parent 6d93f12 commit a76f88c

4 files changed

Lines changed: 131 additions & 121 deletions

File tree

‎block/manager.go‎

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1127,22 +1127,3 @@ func (m *Manager) SaveCache() error {
11271127

11281128
return nil
11291129
}
1130-
1131-
// isValidSignedData returns true if the data signature is valid for the expected sequencer.
1132-
func (m *Manager) isValidSignedData(signedData *types.SignedData) bool {
1133-
if signedData == nil || signedData.Txs == nil {
1134-
return false
1135-
}
1136-
1137-
if err := m.assertUsingExpectedSingleSequencer(signedData.Signer.Address); err != nil {
1138-
return false
1139-
}
1140-
1141-
dataBytes, err := signedData.Data.MarshalBinary()
1142-
if err != nil {
1143-
return false
1144-
}
1145-
1146-
valid, err := signedData.Signer.PubKey.Verify(dataBytes, signedData.Signature)
1147-
return err == nil && valid
1148-
}

‎block/manager_test.go‎

Lines changed: 0 additions & 100 deletions
Original file line numberDiff line numberDiff line change
@@ -410,106 +410,6 @@ func TestGetDataSignature_NilSigner(t *testing.T) {
410410
require.ErrorContains(err, "signer is nil; cannot sign data")
411411
}
412412

413-
// TestIsValidSignedData covers valid, nil, wrong proposer, and invalid signature cases for isValidSignedData.
414-
func TestIsValidSignedData(t *testing.T) {
415-
require := require.New(t)
416-
privKey, _, err := crypto.GenerateKeyPair(crypto.Ed25519, 256)
417-
require.NoError(err)
418-
testSigner, err := noopsigner.NewNoopSigner(privKey)
419-
require.NoError(err)
420-
proposerAddr, err := testSigner.GetAddress()
421-
require.NoError(err)
422-
gen := genesispkg.NewGenesis(
423-
"testchain",
424-
1,
425-
time.Now(),
426-
proposerAddr,
427-
)
428-
m := &Manager{
429-
signer: testSigner,
430-
genesis: gen,
431-
}
432-
433-
t.Run("valid signed data", func(t *testing.T) {
434-
batch := &types.Data{
435-
Txs: types.Txs{types.Tx("tx1"), types.Tx("tx2")},
436-
}
437-
sig, err := m.getDataSignature(batch)
438-
require.NoError(err)
439-
pubKey, err := m.signer.GetPublic()
440-
require.NoError(err)
441-
signedData := &types.SignedData{
442-
Data: *batch,
443-
Signature: sig,
444-
Signer: types.Signer{
445-
PubKey: pubKey,
446-
Address: proposerAddr,
447-
},
448-
}
449-
assert.True(t, m.isValidSignedData(signedData))
450-
})
451-
452-
t.Run("nil signed data", func(t *testing.T) {
453-
assert.False(t, m.isValidSignedData(nil))
454-
})
455-
456-
t.Run("nil Txs", func(t *testing.T) {
457-
signedData := &types.SignedData{
458-
Data: types.Data{},
459-
Signer: types.Signer{
460-
Address: proposerAddr,
461-
},
462-
}
463-
signedData.Txs = nil
464-
assert.False(t, m.isValidSignedData(signedData))
465-
})
466-
467-
t.Run("wrong proposer address", func(t *testing.T) {
468-
batch := &types.Data{
469-
Txs: types.Txs{types.Tx("tx1")},
470-
}
471-
sig, err := m.getDataSignature(batch)
472-
require.NoError(err)
473-
pubKey, err := m.signer.GetPublic()
474-
require.NoError(err)
475-
wrongAddr := make([]byte, len(proposerAddr))
476-
copy(wrongAddr, proposerAddr)
477-
wrongAddr[0] ^= 0xFF // flip a bit
478-
signedData := &types.SignedData{
479-
Data: *batch,
480-
Signature: sig,
481-
Signer: types.Signer{
482-
PubKey: pubKey,
483-
Address: wrongAddr,
484-
},
485-
}
486-
assert.False(t, m.isValidSignedData(signedData))
487-
})
488-
489-
t.Run("invalid signature", func(t *testing.T) {
490-
batch := &types.Data{
491-
Txs: types.Txs{types.Tx("tx1")},
492-
}
493-
sig, err := m.getDataSignature(batch)
494-
require.NoError(err)
495-
pubKey, err := m.signer.GetPublic()
496-
require.NoError(err)
497-
// Corrupt the signature
498-
badSig := make([]byte, len(sig))
499-
copy(badSig, sig)
500-
badSig[0] ^= 0xFF
501-
signedData := &types.SignedData{
502-
Data: *batch,
503-
Signature: badSig,
504-
Signer: types.Signer{
505-
PubKey: pubKey,
506-
Address: proposerAddr,
507-
},
508-
}
509-
assert.False(t, m.isValidSignedData(signedData))
510-
})
511-
}
512-
513413
// TestManager_execValidate tests the execValidate method for various header/data/state conditions.
514414
func TestManager_execValidate(t *testing.T) {
515415
require := require.New(t)

‎block/retriever_da.go‎

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -255,8 +255,8 @@ func (m *Manager) tryDecodeData(bz []byte, daHeight uint64) *types.Data {
255255
}
256256

257257
// Early validation to reject junk data
258-
if !m.isValidSignedData(&signedData) {
259-
m.logger.Debug().Uint64("daHeight", daHeight).Msg("invalid data signature")
258+
if err := m.isValidSignedData(&signedData); err != nil {
259+
m.logger.Debug().Uint64("daHeight", daHeight).Err(err).Msg("invalid data signature")
260260
return nil
261261
}
262262

@@ -268,6 +268,33 @@ func (m *Manager) tryDecodeData(bz []byte, daHeight uint64) *types.Data {
268268
return &signedData.Data
269269
}
270270

271+
// isValidSignedData returns true if the data signature is valid for the expected sequencer.
272+
func (m *Manager) isValidSignedData(signedData *types.SignedData) error {
273+
if signedData == nil || signedData.Txs == nil {
274+
return errors.New("empty signed data")
275+
}
276+
277+
if err := m.assertUsingExpectedSingleSequencer(signedData.Signer.Address); err != nil {
278+
return err
279+
}
280+
281+
dataBytes, err := signedData.Data.MarshalBinary()
282+
if err != nil {
283+
return fmt.Errorf("failed to marshal data: %w", err)
284+
}
285+
286+
valid, err := signedData.Signer.PubKey.Verify(dataBytes, signedData.Signature)
287+
if err != nil {
288+
return fmt.Errorf("failed to verify signature: %w", err)
289+
}
290+
291+
if !valid {
292+
return fmt.Errorf("invalid signature")
293+
}
294+
295+
return nil
296+
}
297+
271298
// isAtHeight checks if a height is available.
272299
func (m *Manager) isAtHeight(ctx context.Context, height uint64) error {
273300
currentHeight, err := m.GetStoreHeight(ctx)

‎block/retriever_da_test.go‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@ import (
2323
"github.com/evstack/ev-node/pkg/cache"
2424
"github.com/evstack/ev-node/pkg/config"
2525
"github.com/evstack/ev-node/pkg/genesis"
26+
genesispkg "github.com/evstack/ev-node/pkg/genesis"
2627
"github.com/evstack/ev-node/pkg/signer/noop"
28+
noopsigner "github.com/evstack/ev-node/pkg/signer/noop"
2729
storepkg "github.com/evstack/ev-node/pkg/store"
2830
rollmocks "github.com/evstack/ev-node/test/mocks"
2931
"github.com/evstack/ev-node/types"
@@ -732,3 +734,103 @@ func TestRetrieveLoop_DAHeightIncrementsOnlyOnSuccess(t *testing.T) {
732734

733735
mockDAClient.AssertExpectations(t)
734736
}
737+
738+
// TestIsValidSignedData covers valid, nil, wrong proposer, and invalid signature cases for isValidSignedData.
739+
func TestIsValidSignedData(t *testing.T) {
740+
require := require.New(t)
741+
privKey, _, err := crypto.GenerateKeyPair(crypto.Ed25519, 256)
742+
require.NoError(err)
743+
testSigner, err := noopsigner.NewNoopSigner(privKey)
744+
require.NoError(err)
745+
proposerAddr, err := testSigner.GetAddress()
746+
require.NoError(err)
747+
gen := genesispkg.NewGenesis(
748+
"testchain",
749+
1,
750+
time.Now(),
751+
proposerAddr,
752+
)
753+
m := &Manager{
754+
signer: testSigner,
755+
genesis: gen,
756+
}
757+
758+
t.Run("valid signed data", func(t *testing.T) {
759+
batch := &types.Data{
760+
Txs: types.Txs{types.Tx("tx1"), types.Tx("tx2")},
761+
}
762+
sig, err := m.getDataSignature(batch)
763+
require.NoError(err)
764+
pubKey, err := m.signer.GetPublic()
765+
require.NoError(err)
766+
signedData := &types.SignedData{
767+
Data: *batch,
768+
Signature: sig,
769+
Signer: types.Signer{
770+
PubKey: pubKey,
771+
Address: proposerAddr,
772+
},
773+
}
774+
assert.NoError(t, m.isValidSignedData(signedData))
775+
})
776+
777+
t.Run("nil signed data", func(t *testing.T) {
778+
assert.Error(t, m.isValidSignedData(nil))
779+
})
780+
781+
t.Run("nil Txs", func(t *testing.T) {
782+
signedData := &types.SignedData{
783+
Data: types.Data{},
784+
Signer: types.Signer{
785+
Address: proposerAddr,
786+
},
787+
}
788+
signedData.Txs = nil
789+
assert.Error(t, m.isValidSignedData(signedData))
790+
})
791+
792+
t.Run("wrong proposer address", func(t *testing.T) {
793+
batch := &types.Data{
794+
Txs: types.Txs{types.Tx("tx1")},
795+
}
796+
sig, err := m.getDataSignature(batch)
797+
require.NoError(err)
798+
pubKey, err := m.signer.GetPublic()
799+
require.NoError(err)
800+
wrongAddr := make([]byte, len(proposerAddr))
801+
copy(wrongAddr, proposerAddr)
802+
wrongAddr[0] ^= 0xFF // flip a bit
803+
signedData := &types.SignedData{
804+
Data: *batch,
805+
Signature: sig,
806+
Signer: types.Signer{
807+
PubKey: pubKey,
808+
Address: wrongAddr,
809+
},
810+
}
811+
assert.Error(t, m.isValidSignedData(signedData))
812+
})
813+
814+
t.Run("invalid signature", func(t *testing.T) {
815+
batch := &types.Data{
816+
Txs: types.Txs{types.Tx("tx1")},
817+
}
818+
sig, err := m.getDataSignature(batch)
819+
require.NoError(err)
820+
pubKey, err := m.signer.GetPublic()
821+
require.NoError(err)
822+
// Corrupt the signature
823+
badSig := make([]byte, len(sig))
824+
copy(badSig, sig)
825+
badSig[0] ^= 0xFF
826+
signedData := &types.SignedData{
827+
Data: *batch,
828+
Signature: badSig,
829+
Signer: types.Signer{
830+
PubKey: pubKey,
831+
Address: proposerAddr,
832+
},
833+
}
834+
assert.Error(t, m.isValidSignedData(signedData))
835+
})
836+
}

0 commit comments

Comments
 (0)