Fixed #38, Check state root after processing block

[meyer9]

Tests aren't passing. Can you make sure proposers are proposing the correct state root? I think it might need to be the previous state root actually.

[wqking]

I fixed the test, but seems there are conflict of how StateRoot is calculated. In MineBlockWithSpecialsAndAttestations in util.go

    lastBlock, err := b.LastBlock()
    if err != nil {
        return nil, err
    }

    parentRoot, err := ssz.TreeHash(lastBlock)
    if err != nil {
        return nil, err
    }

    stateRoot, err := ssz.TreeHash(lastBlock)

State root is the hash of the block.

In GetStateRoot in beacon/rpc/rpc.go

func (s *server) GetStateRoot(ctx context.Context, in *empty.Empty) (*pb.GetStateRootResponse, error) {
    state := s.chain.GetState()

    root, err := ssz.TreeHash(state)
    if err != nil {
        return nil, err
    }

    return &pb.GetStateRootResponse{StateRoot: root[:]}, nil
}

State root is the hash of the state.

I think something wrong?

[codecov-io]

Codecov Report

Merging #41 into testnet will decrease coverage by 0.02%. The diff coverage is 34.48%.

@@             Coverage Diff             @@
##           testnet      #41      +/-   ##
===========================================
- Coverage     38.7%   38.68%   -0.03%     
===========================================
  Files           20       20              
  Lines         3126     3151      +25     
===========================================
+ Hits          1210     1219       +9     
- Misses        1783     1794      +11     
- Partials       133      138       +5

Impacted Files	Coverage Δ
primitives/state.go	17.04% <0%> (-0.17%)	:arrow_down:
beacon/blockchain.go	46% <55.55%> (+0.85%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update a240a89...a05c647. Read the comment docs.

[meyer9]

Yeah, the state root calculation in util is wrong. It should get it from the blockchain, not the hash of the last block.

[meyer9]

Move this to state.processBlock. There's a line starting with TODO:

[wqking]

How to access Blockchain instance in primitives.State.ProcessBlock? Do we need to pass Blockchain to the function?

[meyer9]

There should be a BlockView interface available for accessing the blockchain. Although, you might have to add a function to that interface for accessing state.

On May 5, 2019, at 7:28 PM, Wang Qi notifications@github.com wrote:

How to access Blockchain instance in primitives.State.ProcessBlock? Do we need to pass Blockchain to the function?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

[wqking]

BlockView doesn't provide enough functions to verify the state root, such as GetBlockByHash, or access the state manager. Indeed I wonder why to move the state root verification to primitives.State. primitives.State should not depend on the block chain, while the verification needs to be done by the blockchain.

[meyer9]

Hmm... I'd like to keep it all in the primitives package. You should add a function to the BlockView interface to access the state root for the previous slot.

[meyer9]

LGTM! I'll merge it once the tests pass.