Skip to content
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

chore: set empty hash to hash of nothing #20775

Merged
merged 9 commits into from
Jul 2, 2024
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 2 additions & 8 deletions core/header/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,26 +33,20 @@ func (i *Info) Bytes() ([]byte, error) {
binary.LittleEndian.PutUint64(heightBytes, uint64(i.Height))
buf = append(buf, heightBytes...)

// TODO; permit empty hash OK for genesis block?
if len(i.Hash) == 0 {
i.Hash = make([]byte, hashSize)
}
// Encode Hash
if len(i.Hash) != hashSize {
return nil, errors.New("invalid hash size")
}

buf = append(buf, i.Hash...)

// Encode Time
timeBytes := make([]byte, 8)
binary.LittleEndian.PutUint64(timeBytes, uint64(i.Time.Unix()))
buf = append(buf, timeBytes...)

if len(i.AppHash) == 0 {
i.AppHash = make([]byte, hashSize)
}
// Encode AppHash
if len(i.Hash) != hashSize {
if len(i.AppHash) != hashSize {
return nil, errors.New("invalid hash size")
}
buf = append(buf, i.AppHash...)
Expand Down
33 changes: 21 additions & 12 deletions server/v2/cometbft/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package cometbft

import (
"context"
"crypto/sha256"
"errors"
"fmt"
"sync/atomic"
Expand Down Expand Up @@ -103,7 +104,7 @@ func (c *Consensus[T]) CheckTx(ctx context.Context, req *abciproto.CheckTxReques
return nil, err
}

cometResp := &abci.CheckTxResponse{
cometResp := &abciproto.CheckTxResponse{
Code: resp.Code,
GasWanted: uint64ToInt64(resp.GasWanted),
GasUsed: uint64ToInt64(resp.GasUsed),
Expand Down Expand Up @@ -137,7 +138,7 @@ func (c *Consensus[T]) Info(ctx context.Context, _ *abciproto.InfoRequest) (*abc
return nil, err
}

return &abci.InfoResponse{
return &abciproto.InfoResponse{
Data: c.cfg.Name,
Version: c.cfg.Version,
// AppVersion: cp.GetVersion().App,
Expand Down Expand Up @@ -226,11 +227,19 @@ func (c *Consensus[T]) InitChain(ctx context.Context, req *abciproto.InitChainRe
})
}

ci, err := c.store.LastCommitID()
if err != nil {
return nil, err
}

// populate hash with empty byte slice instead of nil
bz := sha256.Sum256([]byte(""))
tac0turtle marked this conversation as resolved.
Show resolved Hide resolved

br := &coreappmgr.BlockRequest[T]{
Height: uint64(req.InitialHeight - 1),
Time: req.Time,
Hash: nil,
AppHash: nil,
Hash: bz[:],
AppHash: ci.Hash,
ChainId: req.ChainId,
ConsensusMessages: consMessages,
IsGenesis: true,
Expand Down Expand Up @@ -271,7 +280,7 @@ func (c *Consensus[T]) InitChain(ctx context.Context, req *abciproto.InitChainRe
return nil, fmt.Errorf("unable to write the changeset: %w", err)
}

return &abci.InitChainResponse{
return &abciproto.InitChainResponse{
ConsensusParams: req.ConsensusParams,
Validators: validatorUpdates,
AppHash: stateRoot,
Expand Down Expand Up @@ -317,7 +326,7 @@ func (c *Consensus[T]) PrepareProposal(
encodedTxs[i] = tx.Bytes()
}

return &abci.PrepareProposalResponse{
return &abciproto.PrepareProposalResponse{
Txs: encodedTxs,
}, nil
}
Expand Down Expand Up @@ -350,13 +359,13 @@ func (c *Consensus[T]) ProcessProposal(
err := c.processProposalHandler(ciCtx, c.app, decodedTxs, req)
if err != nil {
c.logger.Error("failed to process proposal", "height", req.Height, "time", req.Time, "hash", fmt.Sprintf("%X", req.Hash), "err", err)
return &abci.ProcessProposalResponse{
Status: abci.PROCESS_PROPOSAL_STATUS_REJECT,
return &abciproto.ProcessProposalResponse{
Status: abciproto.PROCESS_PROPOSAL_STATUS_REJECT,
}, nil
}

return &abci.ProcessProposalResponse{
Status: abci.PROCESS_PROPOSAL_STATUS_ACCEPT,
return &abciproto.ProcessProposalResponse{
Status: abciproto.PROCESS_PROPOSAL_STATUS_ACCEPT,
}, nil
}

Expand Down Expand Up @@ -529,7 +538,7 @@ func (c *Consensus[T]) VerifyVoteExtension(
resp, err := c.verifyVoteExt(ctx, latestStore, req)
if err != nil {
c.logger.Error("failed to verify vote extension", "height", req.Height, "err", err)
return &abci.VerifyVoteExtensionResponse{Status: abci.VERIFY_VOTE_EXTENSION_STATUS_REJECT}, nil
return &abciproto.VerifyVoteExtensionResponse{Status: abciproto.VERIFY_VOTE_EXTENSION_STATUS_REJECT}, nil
}

return resp, err
Expand Down Expand Up @@ -565,7 +574,7 @@ func (c *Consensus[T]) ExtendVote(ctx context.Context, req *abciproto.ExtendVote
resp, err := c.extendVote(ctx, latestStore, req)
if err != nil {
c.logger.Error("failed to verify vote extension", "height", req.Height, "err", err)
return &abci.ExtendVoteResponse{}, nil
return &abciproto.ExtendVoteResponse{}, nil
}

return resp, err
Expand Down
40 changes: 40 additions & 0 deletions server/v2/stf/core_header_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"

"cosmossdk.io/core/header"
"cosmossdk.io/core/store"
)

var _ header.Service = (*HeaderService)(nil)
Expand All @@ -13,3 +14,42 @@ type HeaderService struct{}
func (h HeaderService) HeaderInfo(ctx context.Context) header.Info {
return ctx.(*executionContext).headerInfo
}

const headerInfoPrefix = 0x37

// setHeaderInfo sets the header info in the state to be used by queries in the future.
func (s STF[T]) setHeaderInfo(state store.WriterMap, headerInfo header.Info) error {
// TODO storing header info is too low level here, stf should be stateless.
// We should have a keeper that does this.
runtimeStore, err := state.GetWriter(Identity)
if err != nil {
return err
}
bz, err := headerInfo.Bytes()
if err != nil {
return err
}
err = runtimeStore.Set([]byte{headerInfoPrefix}, bz)
if err != nil {
return err
}
return nil
}
Comment on lines +20 to +37
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure statelessness in STF design.

The comment on line 22 suggests that storing header info at this level might not be ideal, indicating a potential architectural concern. Consider refactoring to use a dedicated keeper for state management.

- // TODO storing header info is too low level here, stf should be stateless.
- // We should have a keeper that does this.
+ // Refactor to use a dedicated keeper for managing header info.
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// setHeaderInfo sets the header info in the state to be used by queries in the future.
func (s STF[T]) setHeaderInfo(state store.WriterMap, headerInfo header.Info) error {
// TODO storing header info is too low level here, stf should be stateless.
// We should have a keeper that does this.
runtimeStore, err := state.GetWriter(Identity)
if err != nil {
return err
}
bz, err := headerInfo.Bytes()
if err != nil {
return err
}
err = runtimeStore.Set([]byte{headerInfoPrefix}, bz)
if err != nil {
return err
}
return nil
}
// setHeaderInfo sets the header info in the state to be used by queries in the future.
func (s STF[T]) setHeaderInfo(state store.WriterMap, headerInfo header.Info) error {
// Refactor to use a dedicated keeper for managing header info.
runtimeStore, err := state.GetWriter(Identity)
if err != nil {
return err
}
bz, err := headerInfo.Bytes()
if err != nil {
return err
}
err = runtimeStore.Set([]byte{headerInfoPrefix}, bz)
if err != nil {
return err
}
return nil
}


// getHeaderInfo gets the header info from the state. It should only be used for queries
func (s STF[T]) getHeaderInfo(state store.WriterMap) (i header.Info, err error) {
runtimeStore, err := state.GetWriter(Identity)
if err != nil {
return header.Info{}, err
}
v, err := runtimeStore.Get([]byte{headerInfoPrefix})
if err != nil {
return header.Info{}, err
}
if v == nil {
return header.Info{}, nil
}

err = i.FromBytes(v)
return i, err
}
39 changes: 0 additions & 39 deletions server/v2/stf/stf.go
Original file line number Diff line number Diff line change
Expand Up @@ -421,45 +421,6 @@ func (s STF[T]) validatorUpdates(
return ctx.events, valSetUpdates, nil
}

const headerInfoPrefix = 0x37

// setHeaderInfo sets the header info in the state to be used by queries in the future.
func (s STF[T]) setHeaderInfo(state store.WriterMap, headerInfo header.Info) error {
// TODO storing header info is too low level here, stf should be stateless.
// We should have a keeper that does this.
runtimeStore, err := state.GetWriter(Identity)
if err != nil {
return err
}
bz, err := headerInfo.Bytes()
if err != nil {
return err
}
err = runtimeStore.Set([]byte{headerInfoPrefix}, bz)
if err != nil {
return err
}
return nil
}

// getHeaderInfo gets the header info from the state. It should only be used for queries
func (s STF[T]) getHeaderInfo(state store.WriterMap) (i header.Info, err error) {
runtimeStore, err := state.GetWriter(Identity)
if err != nil {
return header.Info{}, err
}
v, err := runtimeStore.Get([]byte{headerInfoPrefix})
if err != nil {
return header.Info{}, err
}
if v == nil {
return header.Info{}, nil
}

err = i.FromBytes(v)
return i, err
}

// Simulate simulates the execution of a tx on the provided state.
func (s STF[T]) Simulate(
ctx context.Context,
Expand Down
5 changes: 4 additions & 1 deletion store/v2/root/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package root

import (
"bytes"
"crypto/sha256"
"errors"
"fmt"
"sync"
Expand Down Expand Up @@ -148,8 +149,10 @@ func (s *Store) LastCommitID() (proof.CommitID, error) {
if err != nil {
return proof.CommitID{}, err
}
// if the latest version is 0, we return a CommitID with version 0 and a hash of an empty byte slice
bz := sha256.Sum256([]byte{})

return proof.CommitID{Version: latestVersion}, nil
return proof.CommitID{Version: latestVersion, Hash: bz[:]}, nil
}

// GetLatestVersion returns the latest version based on the latest internal
Expand Down
Loading