-
Notifications
You must be signed in to change notification settings - Fork 245
feat: Ensure Header integrity on DA #2948
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,6 +34,10 @@ type daRetriever struct { | |
| // on restart, will be refetch as da height is updated by syncer | ||
| pendingHeaders map[uint64]*types.SignedHeader | ||
| pendingData map[uint64]*types.Data | ||
|
|
||
| // strictMode indicates if the node has seen a valid DAHeaderEnvelope | ||
| // and should now reject all legacy/unsigned headers. | ||
| strictMode bool | ||
| } | ||
|
|
||
| // NewDARetriever creates a new DA retriever | ||
|
|
@@ -50,6 +54,7 @@ func NewDARetriever( | |
| logger: logger.With().Str("component", "da_retriever").Logger(), | ||
| pendingHeaders: make(map[uint64]*types.SignedHeader), | ||
| pendingData: make(map[uint64]*types.Data), | ||
| strictMode: false, | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -228,15 +233,58 @@ func (r *daRetriever) processBlobs(ctx context.Context, blobs [][]byte, daHeight | |
| // tryDecodeHeader attempts to decode a blob as a header | ||
| func (r *daRetriever) tryDecodeHeader(bz []byte, daHeight uint64) *types.SignedHeader { | ||
| header := new(types.SignedHeader) | ||
| var headerPb pb.SignedHeader | ||
|
|
||
| if err := proto.Unmarshal(bz, &headerPb); err != nil { | ||
| return nil | ||
| } | ||
| isValidEnvelope := false | ||
|
|
||
| if err := header.FromProto(&headerPb); err != nil { | ||
| // Attempt to unmarshal as DAHeaderEnvelope and get the envelope signature | ||
| if envelopeSignature, err := header.UnmarshalDAEnvelope(bz); err != nil { | ||
| // If in strict mode, we REQUIRE an envelope. | ||
| if r.strictMode { | ||
| r.logger.Warn().Err(err).Msg("strict mode is enabled, rejecting non-envelope blob") | ||
| return nil | ||
| } | ||
|
|
||
| // Fallback for backward compatibility (only if NOT in strict mode) | ||
| r.logger.Debug().Msg("trying legacy decoding") | ||
| var headerPb pb.SignedHeader | ||
| if errLegacy := proto.Unmarshal(bz, &headerPb); errLegacy != nil { | ||
| return nil | ||
| } | ||
| if errLegacy := header.FromProto(&headerPb); errLegacy != nil { | ||
| return nil | ||
| } | ||
| } else { | ||
| // We have a structurally valid envelope (or at least it parsed) | ||
| if len(envelopeSignature) > 0 { | ||
| if header.Signer.PubKey == nil { | ||
| r.logger.Debug().Msg("header signer has no pubkey, cannot verify envelope") | ||
| return nil | ||
| } | ||
| payload, err := header.MarshalBinary() | ||
| if err != nil { | ||
| r.logger.Debug().Err(err).Msg("failed to marshal header for verification") | ||
| return nil | ||
| } | ||
| if valid, err := header.Signer.PubKey.Verify(payload, envelopeSignature); err != nil || !valid { | ||
| r.logger.Info().Err(err).Msg("DA envelope signature verification failed") | ||
| return nil | ||
| } | ||
| r.logger.Debug().Uint64("height", header.Height()).Msg("DA envelope signature verified") | ||
| isValidEnvelope = true | ||
| } | ||
| } | ||
| if r.strictMode && !isValidEnvelope { | ||
| r.logger.Warn().Msg("strict mode: rejecting block that is not a fully valid envelope") | ||
| return nil | ||
| } | ||
| // Mode Switch Logic | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice! Shall we document better why we needed that? Header validation couldn't be done with classic header due to ev-abci. However, necessary because of spam issue. This solves it in a backward compatible way. |
||
| if isValidEnvelope && !r.strictMode { | ||
| r.logger.Info().Uint64("height", header.Height()).Msg("valid DA envelope detected, switching to STRICT MODE") | ||
| r.strictMode = true | ||
| } | ||
|
|
||
| // Legacy blob support implies: strictMode == false AND (!isValidEnvelope). | ||
| // We fall through here. | ||
|
|
||
| // Basic validation | ||
| if err := header.Header.ValidateBasic(); err != nil { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| package syncing | ||
|
|
||
| import ( | ||
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
|
|
||
| "github.com/evstack/ev-node/pkg/config" | ||
| "github.com/evstack/ev-node/pkg/genesis" | ||
| "github.com/evstack/ev-node/types" | ||
| ) | ||
|
|
||
| func TestDARetriever_StrictEnvelopeMode_Switch(t *testing.T) { | ||
| // Setup keys | ||
| addr, pub, signer := buildSyncTestSigner(t) | ||
| gen := genesis.Genesis{ChainID: "tchain", InitialHeight: 1, StartTime: time.Now().Add(-time.Second), ProposerAddress: addr} | ||
|
|
||
| r := newTestDARetriever(t, nil, config.DefaultConfig(), gen) | ||
|
|
||
| // 1. Create a Legacy Header (SignedHeader marshaled directly) | ||
| // This simulates old blobs on the network before the upgrade. | ||
| legacyHeader := &types.SignedHeader{ | ||
| Header: types.Header{ | ||
| BaseHeader: types.BaseHeader{ChainID: gen.ChainID, Height: 1, Time: uint64(time.Now().UnixNano())}, | ||
| ProposerAddress: addr, | ||
| }, | ||
| Signer: types.Signer{PubKey: pub, Address: addr}, | ||
| } | ||
| // Sign it | ||
| bz, err := types.DefaultAggregatorNodeSignatureBytesProvider(&legacyHeader.Header) | ||
| require.NoError(t, err) | ||
| sig, err := signer.Sign(bz) | ||
| require.NoError(t, err) | ||
| legacyHeader.Signature = sig | ||
|
|
||
| legacyBlob, err := legacyHeader.MarshalBinary() | ||
| require.NoError(t, err) | ||
|
|
||
| // 2. Create an Envelope Header (DAHeaderEnvelope) | ||
| // This simulates a new blob after upgrade. | ||
| envelopeHeader := &types.SignedHeader{ | ||
| Header: types.Header{ | ||
| BaseHeader: types.BaseHeader{ChainID: gen.ChainID, Height: 2, Time: uint64(time.Now().UnixNano())}, | ||
| ProposerAddress: addr, | ||
| }, | ||
| Signer: types.Signer{PubKey: pub, Address: addr}, | ||
| } | ||
| // Sign content | ||
| bz2, err := types.DefaultAggregatorNodeSignatureBytesProvider(&envelopeHeader.Header) | ||
| require.NoError(t, err) | ||
| sig2, err := signer.Sign(bz2) | ||
| require.NoError(t, err) | ||
| envelopeHeader.Signature = sig2 | ||
|
|
||
| // Create Envelope | ||
| // We need to sign the envelope itself. | ||
| // The `SubmitHeaders` logic wraps it. We emulate it here using `MarshalDAEnvelope`. | ||
| // First get canonical content bytes (fields 1-3) | ||
| contentBytes, err := envelopeHeader.MarshalBinary() | ||
| require.NoError(t, err) | ||
| // Sign envelope | ||
| envSig, err := signer.Sign(contentBytes) | ||
| require.NoError(t, err) | ||
| // Marshal to envelope | ||
| envelopeBlob, err := envelopeHeader.MarshalDAEnvelope(envSig) | ||
| require.NoError(t, err) | ||
|
|
||
| // --- Test Scenario --- | ||
|
|
||
| // A. Initial State: StrictMode is false. Legacy blob should be accepted. | ||
| assert.False(t, r.strictMode) | ||
|
|
||
| decodedLegacy := r.tryDecodeHeader(legacyBlob, 100) | ||
| require.NotNil(t, decodedLegacy) | ||
| assert.Equal(t, uint64(1), decodedLegacy.Height()) | ||
|
|
||
| // StrictMode should still be false because it was a legacy blob | ||
| assert.False(t, r.strictMode) | ||
|
|
||
| // B. Receiving Envelope: Should be accepted and Switch StrictMode to true. | ||
| decodedEnvelope := r.tryDecodeHeader(envelopeBlob, 101) | ||
| require.NotNil(t, decodedEnvelope) | ||
| assert.Equal(t, uint64(2), decodedEnvelope.Height()) | ||
|
|
||
| assert.True(t, r.strictMode, "retriever should have switched to strict mode") | ||
|
|
||
| // C. Receiving Legacy again: Should be REJECTED now. | ||
| // We reuse the same legacyBlob (or a new one, doesn't matter, structure is legacy). | ||
| decodedLegacyAgain := r.tryDecodeHeader(legacyBlob, 102) | ||
| assert.Nil(t, decodedLegacyAgain, "legacy blob should be rejected in strict mode") | ||
|
|
||
| // D. Receiving Envelope again: Should still be accepted. | ||
| decodedEnvelopeAgain := r.tryDecodeHeader(envelopeBlob, 103) | ||
| require.NotNil(t, decodedEnvelopeAgain) | ||
| } |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This check appears to be redundant. Based on the logic flow, if
r.strictModeistruewhen this function is called,isValidEnvelopemust also betruefor execution to reach this line. Any case wherer.strictModeistrueandisValidEnvelopewould befalse(e.g., failed unmarshal, invalid signature, no signature) results in an early return from the function. Therefore, this conditional block seems to be unreachable and can be removed to simplify the code.