Skip to content

Commit

Permalink
Add Data Hash Validation to Block's ValidateBasic (cosmos#1200)
Browse files Browse the repository at this point in the history
<!--
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.
-->

## Overview
Closes: cosmos#627 
<!-- 
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. 
-->

## Checklist

<!-- 
Please complete the checklist to ensure that the PR is ready to be
reviewed.

IMPORTANT:
PRs should be left in Draft until the below checklist is completed.
-->

- [x] New and updated code has appropriate documentation
- [x] New and updated code has new and/or updated testing
- [x] Required CI checks are passing
- [ ] Visual proof for any user facing features like CLI or
documentation updates
- [x] Linked issues closed with keywords
  • Loading branch information
Manav-Aggarwal authored Sep 25, 2023
1 parent 68bd58f commit 44b680c
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 11 deletions.
34 changes: 26 additions & 8 deletions block/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -603,6 +603,10 @@ func (m *Manager) publishBlock(ctx context.Context) error {
block = m.createBlock(newHeight, lastCommit, lastHeaderHash)
m.logger.Debug("block info", "num_tx", len(block.Data.Txs))

block.SignedHeader.DataHash, err = block.Data.Hash()
if err != nil {
return nil
}
commit, err = m.getCommit(block.SignedHeader.Header)
if err != nil {
return err
Expand All @@ -626,20 +630,34 @@ func (m *Manager) publishBlock(ctx context.Context) error {
return err
}

blockHeight := uint64(block.Height())
// Before taking the hash, we need updated ISRs, hence after ApplyBlock
block.SignedHeader.Header.DataHash, err = block.Data.Hash()
if err != nil {
return err
}

commit, err = m.getCommit(block.SignedHeader.Header)
if err != nil {
return err
}

// set the commit to current block's signed header
block.SignedHeader.Commit = *commit

block.SignedHeader.Validators = m.getLastStateValidators()

// Validate the created block before storing
if err := m.executor.Validate(m.lastState, block); err != nil {
return fmt.Errorf("failed to validate block: %w", err)
}

blockHeight := block.Height()
// Update the stored height before submitting to the DA layer and committing to the DB
m.store.SetHeight(blockHeight)

blockHash := block.Hash().String()
m.blockCache.setSeen(blockHash)

if commit == nil {
commit, err = m.getCommit(block.SignedHeader.Header)
if err != nil {
return err
}
}

// SaveBlock commits the DB tx
err = m.store.SaveBlock(block, commit)
if err != nil {
Expand Down
5 changes: 2 additions & 3 deletions state/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,10 @@ func (e *BlockExecutor) CreateBlock(height uint64, lastCommit *types.Commit, las

// ApplyBlock validates and executes the block.
func (e *BlockExecutor) ApplyBlock(ctx context.Context, state types.State, block *types.Block) (types.State, *cmstate.ABCIResponses, error) {
err := e.validate(state, block)
err := e.Validate(state, block)
if err != nil {
return types.State{}, nil, err
}

// This makes calls to the AppClient
resp, err := e.execute(ctx, state, block)
if err != nil {
Expand Down Expand Up @@ -261,7 +260,7 @@ func (e *BlockExecutor) commit(ctx context.Context, state types.State, block *ty
return resp.Data, uint64(resp.RetainHeight), err
}

func (e *BlockExecutor) validate(state types.State, block *types.Block) error {
func (e *BlockExecutor) Validate(state types.State, block *types.Block) error {
err := block.ValidateBasic()
if err != nil {
return err
Expand Down
6 changes: 6 additions & 0 deletions state/executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,9 @@ func doTestApplyBlock(t *testing.T) {
require.NotNil(block)
assert.Equal(uint64(1), block.Height())
assert.Len(block.Data.Txs, 1)
dataHash, err := block.Data.Hash()
assert.NoError(err)
block.SignedHeader.DataHash = dataHash

// Update the signature on the block to current from last
headerBytes, _ := block.SignedHeader.Header.MarshalBinary()
Expand All @@ -186,6 +189,9 @@ func doTestApplyBlock(t *testing.T) {
require.NotNil(block)
assert.Equal(uint64(2), block.Height())
assert.Len(block.Data.Txs, 3)
dataHash, err = block.Data.Hash()
assert.NoError(err)
block.SignedHeader.DataHash = dataHash

headerBytes, _ = block.SignedHeader.Header.MarshalBinary()
sig, _ = vKey.Sign(headerBytes)
Expand Down
8 changes: 8 additions & 0 deletions types/block.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package types

import (
"bytes"
"encoding"
"errors"
"time"
Expand Down Expand Up @@ -113,6 +114,13 @@ func (b *Block) ValidateBasic() error {
if err := b.Data.ValidateBasic(); err != nil {
return err
}
dataHash, err := b.Data.Hash()
if err != nil {
return err
}
if !bytes.Equal(dataHash[:], b.SignedHeader.DataHash[:]) {
return errors.New("dataHash from the header does not match with hash of the block's data")
}
return nil
}

Expand Down
12 changes: 12 additions & 0 deletions types/hashing.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package types

import (
"github.com/cometbft/cometbft/crypto/merkle"
cmbytes "github.com/cometbft/cometbft/libs/bytes"
cmversion "github.com/cometbft/cometbft/proto/tendermint/version"
cmtypes "github.com/cometbft/cometbft/types"
Expand Down Expand Up @@ -40,3 +41,14 @@ func (h *Header) Hash() Hash {
func (b *Block) Hash() Hash {
return b.SignedHeader.Hash()
}

// Hash returns hash of the Data
func (d *Data) Hash() (Hash, error) {
dBytes, err := d.MarshalBinary()
if err != nil {
return nil, err
}
return merkle.HashFromByteSlices([][]byte{
dBytes,
}), nil
}

0 comments on commit 44b680c

Please sign in to comment.