eth_getBlockByHash and eth_getBlockByNumber JSON RPC endpoints sh…

…ould omit `Committed` field (#1383)

* Filter extra Committed field in jsonRPC endpoints

* UT

* Fix UTs

* Lint fix

* Comments fix

* Comments fix

consensus/consensus.go

@@ -37,6 +37,9 @@ type Consensus interface {
 	// GetBridgeProvider returns an instance of BridgeDataProvider
 	GetBridgeProvider() BridgeDataProvider
 
+	// FilterExtra filters extra data in header that is not a part of block hash
+	FilterExtra(extra []byte) ([]byte, error)
+
 	// Initialize initializes the consensus (e.g. setup data)
 	Initialize() error
 

consensus/dev/dev.go

@@ -246,3 +246,7 @@ func (d *Dev) Close() error {
 func (d *Dev) GetBridgeProvider() consensus.BridgeDataProvider {
 	return nil
 }
+
+func (d *Dev) FilterExtra(extra []byte) ([]byte, error) {
+	return extra, nil
+}

consensus/dummy/dummy.go

@@ -79,6 +79,10 @@ func (d *Dummy) GetBridgeProvider() consensus.BridgeDataProvider {
 	return nil
 }
 
+func (d *Dummy) FilterExtra(extra []byte) ([]byte, error) {
+	return extra, nil
+}
+
 func (d *Dummy) run() {
 	d.logger.Info("started")
 	// do nothing

consensus/ibft/ibft.go

@@ -552,6 +552,11 @@ func (i *backendIBFT) GetBridgeProvider() consensus.BridgeDataProvider {
 	return nil
 }
 
+// FilterExtra is the implementation of Consensus interface
+func (i *backendIBFT) FilterExtra(extra []byte) ([]byte, error) {
+	return extra, nil
+}
+
 // updateCurrentModules updates Signer, Hooks, and Validators
 // that are used at specified height
 // by fetching from ForkManager

consensus/polybft/extra.go

@@ -677,7 +677,7 @@ func GetIbftExtraClean(extraRaw []byte) ([]byte, error) {
 		Committed:  &Signature{},
 	}
 
-	return ibftExtra.MarshalRLPTo(nil), nil
+	return append(make([]byte, ExtraVanity), ibftExtra.MarshalRLPTo(nil)...), nil
 }
 
 // GetIbftExtra returns the istanbul extra data field from the passed in header

consensus/polybft/extra_test.go

@@ -842,6 +842,61 @@ func TestValidatorSetDelta_UnmarshalRLPWith_NegativeCases(t *testing.T) {
 	})
 }
 
+func Test_GetIbftExtraClean(t *testing.T) {
+	t.Parallel()
+
+	key, err := wallet.GenerateAccount()
+	require.NoError(t, err)
+
+	extra := &Extra{
+		Validators: &ValidatorSetDelta{
+			Added: AccountSet{
+				&ValidatorMetadata{
+					Address:     types.BytesToAddress([]byte{11, 22}),
+					BlsKey:      key.Bls.PublicKey(),
+					VotingPower: new(big.Int).SetUint64(1000),
+					IsActive:    true,
+				},
+			},
+		},
+		Seal: []byte{},
+		Committed: &Signature{
+			AggregatedSignature: []byte{23, 24},
+			Bitmap:              []byte{11},
+		},
+		Parent: &Signature{
+			AggregatedSignature: []byte{0, 1},
+			Bitmap:              []byte{1},
+		},
+		Checkpoint: &CheckpointData{
+			BlockRound:            1,
+			EpochNumber:           1,
+			CurrentValidatorsHash: types.BytesToHash([]byte{2, 3}),
+			NextValidatorsHash:    types.BytesToHash([]byte{4, 5}),
+			EventRoot:             types.BytesToHash([]byte{6, 7}),
+		},
+	}
+
+	extraBytes := append(make([]byte, ExtraVanity), extra.MarshalRLPTo(nil)...)
+
+	extraClean, err := GetIbftExtraClean(extraBytes)
+	require.NoError(t, err)
+
+	extraTwo := &Extra{}
+	require.NoError(t, extraTwo.UnmarshalRLP(extraClean[ExtraVanity:]))
+	require.True(t, extra.Validators.Equals(extra.Validators))
+	require.Equal(t, extra.Checkpoint.BlockRound, extraTwo.Checkpoint.BlockRound)
+	require.Equal(t, extra.Checkpoint.EpochNumber, extraTwo.Checkpoint.EpochNumber)
+	require.Equal(t, extra.Checkpoint.CurrentValidatorsHash, extraTwo.Checkpoint.CurrentValidatorsHash)
+	require.Equal(t, extra.Checkpoint.NextValidatorsHash, extraTwo.Checkpoint.NextValidatorsHash)
+	require.Equal(t, extra.Checkpoint.NextValidatorsHash, extraTwo.Checkpoint.NextValidatorsHash)
+	require.Equal(t, extra.Parent.AggregatedSignature, extraTwo.Parent.AggregatedSignature)
+	require.Equal(t, extra.Parent.Bitmap, extraTwo.Parent.Bitmap)
+
+	require.Nil(t, extraTwo.Committed.AggregatedSignature)
+	require.Nil(t, extraTwo.Committed.Bitmap)
+}
+
 func Test_GetIbftExtraClean_Fail(t *testing.T) {
 	t.Parallel()
 

consensus/polybft/polybft.go

@@ -487,7 +487,14 @@ func (p *Polybft) PreCommitState(_ *types.Header, _ *state.Transition) error {
 	return nil
 }
 
-// GetBridgeProvider returns an instance of BridgeDataProvider
+// GetBridgeProvider is an implementation of Consensus interface
+// Returns an instance of BridgeDataProvider
 func (p *Polybft) GetBridgeProvider() consensus.BridgeDataProvider {
 	return p.runtime
 }
+
+// GetBridgeProvider is an implementation of Consensus interface
+// Filters extra data to not contain Committed field
+func (p *Polybft) FilterExtra(extra []byte) ([]byte, error) {
+	return GetIbftExtraClean(extra)
+}

jsonrpc/eth_blockchain_test.go

@@ -525,6 +525,10 @@ func (m *mockBlockStore) SubscribeEvents() blockchain.Subscription {
 	return nil
 }
 
+func (m *mockBlockStore) FilterExtra(extra []byte) ([]byte, error) {
+	return extra, nil
+}
+
 func newTestBlock(number uint64, hash types.Hash) *types.Block {
 	return &types.Block{
 		Header: &types.Header{

jsonrpc/eth_endpoint.go

@@ -68,11 +68,17 @@ type ethBlockchainStore interface {
 	GetSyncProgression() *progress.Progression
 }
 
+type ethFilter interface {
+	// FilterExtra filters extra data from header extra that is not included in block hash
+	FilterExtra(extra []byte) ([]byte, error)
+}
+
 // ethStore provides access to the methods needed by eth endpoint
 type ethStore interface {
 	ethTxPoolStore
 	ethStateStore
 	ethBlockchainStore
+	ethFilter
 }
 
 // Eth is the eth jsonrpc endpoint
@@ -122,6 +128,10 @@ func (e *Eth) GetBlockByNumber(number BlockNumber, fullTx bool) (interface{}, er
 		return nil, nil
 	}
 
+	if err := e.filterExtra(block); err != nil {
+		return nil, err
+	}
+
 	return toBlock(block, fullTx), nil
 }
 
@@ -132,9 +142,30 @@ func (e *Eth) GetBlockByHash(hash types.Hash, fullTx bool) (interface{}, error)
 		return nil, nil
 	}
 
+	if err := e.filterExtra(block); err != nil {
+		return nil, err
+	}
+
 	return toBlock(block, fullTx), nil
 }
 
+func (e *Eth) filterExtra(block *types.Block) error {
+	// we need to copy it because the store returns header from storage directly
+	// and not a copy, so changing it, actually changes it in storage as well
+	headerCopy := block.Header.Copy()
+
+	filteredExtra, err := e.store.FilterExtra(headerCopy.ExtraData)
+	if err != nil {
+		return err
+	}
+
+	headerCopy.ExtraData = filteredExtra
+	// no need to recompute hash (filtered out data is not in the hash in the first place)
+	block.Header = headerCopy
+
+	return nil
+}
+
 func (e *Eth) GetBlockTransactionCountByNumber(number BlockNumber) (interface{}, error) {
 	num, err := GetNumericBlockNumber(number, e.store)
 	if err != nil {

jsonrpc/mocks_test.go

@@ -196,3 +196,7 @@ func (m *mockStore) GetStateSyncProof(stateSyncID uint64) (types.Proof, error) {
 
 	return ssp, nil
 }
+
+func (m *mockStore) FilterExtra(extra []byte) ([]byte, error) {
+	return extra, nil
+}