diff --git a/.github/workflows/ci_release.yml b/.github/workflows/ci_release.yml index 915b24e2..434d772b 100644 --- a/.github/workflows/ci_release.yml +++ b/.github/workflows/ci_release.yml @@ -23,6 +23,10 @@ on: - minor - major +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + jobs: lint: uses: ./.github/workflows/lint.yml diff --git a/.github/workflows/integration_test.yml b/.github/workflows/integration_test.yml index a18bd930..82717f21 100644 --- a/.github/workflows/integration_test.yml +++ b/.github/workflows/integration_test.yml @@ -7,6 +7,10 @@ on: branches: ["main"] workflow_dispatch: +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + jobs: liveness: name: Test with Rollkit Chain @@ -345,7 +349,7 @@ jobs: ROLLKIT_ADDR=$(./gmd/go/bin/gmd keys show carol -a --home ./gmd/.gm) CELESTIA_ADDR=$(celestia-appd keys show validator -a --keyring-backend test) ./gmd/go/bin/gmd tx ibc-transfer transfer transfer channel-0 $CELESTIA_ADDR 100stake --from carol -y --home ./gmd/.gm - sleep 20 + sleep 15 BALANCE=$(celestia-appd query bank balances $CELESTIA_ADDR --output json --node http://localhost:26654 | jq '.balances') echo "Celestia balance after IBC transfer: $BALANCE" # TODO: check that the balance is correct @@ -356,7 +360,7 @@ jobs: ROLLKIT_ADDR=$(./gmd/go/bin/gmd keys show carol -a --home ./gmd/.gm) CELESTIA_ADDR=$(celestia-appd keys show validator -a --keyring-backend test) celestia-appd tx ibc-transfer transfer transfer channel-0 $ROLLKIT_ADDR 100utia --from validator --node http://localhost:26654 --fees 400utia --keyring-backend test -y - sleep 20 + sleep 15 BALANCE=$(./gmd/go/bin/gmd query bank balances $ROLLKIT_ADDR --output json --home ./gmd/.gm | jq '.balances') echo "Gm balance after IBC transfer: $BALANCE" # fail if no denom starts with ibc/ diff --git a/pkg/adapter/adapter.go b/pkg/adapter/adapter.go index 80902c5a..cd80f1cc 100644 --- a/pkg/adapter/adapter.go +++ b/pkg/adapter/adapter.go @@ -8,7 +8,7 @@ import ( "cosmossdk.io/log" abci "github.com/cometbft/cometbft/abci/types" - "github.com/cometbft/cometbft/config" + cmtcfg "github.com/cometbft/cometbft/config" "github.com/cometbft/cometbft/libs/bytes" "github.com/cometbft/cometbft/mempool" corep2p "github.com/cometbft/cometbft/p2p" @@ -44,7 +44,7 @@ type P2PClientInfo interface { } // LoadGenesisDoc returns the genesis document from the provided config file. -func LoadGenesisDoc(cfg *config.Config) (*cmttypes.GenesisDoc, error) { +func LoadGenesisDoc(cfg *cmtcfg.Config) (*cmttypes.GenesisDoc, error) { genesisFile := cfg.GenesisFile() doc, err := cmttypes.GenesisDocFromFile(genesisFile) if err != nil { @@ -80,7 +80,7 @@ func NewABCIExecutor( p2pClient *rollkitp2p.Client, p2pMetrics *rollkitp2p.Metrics, logger log.Logger, - cfg *config.Config, + cfg *cmtcfg.Config, appGenesis *genutiltypes.AppGenesis, metrics *Metrics, ) *Adapter { @@ -93,8 +93,8 @@ func NewABCIExecutor( Prefix: ds.NewKey(rollnode.RollkitPrefix), }) rollkitStore := rstore.New(rollkitPrefixStore) - // Create a new Store with ABCI prefix - abciStore := NewStore(store) + + abciStore := NewExecABCIStore(store) a := &Adapter{ App: app, @@ -302,40 +302,9 @@ func (a *Adapter) ExecuteTxs( return nil, 0, fmt.Errorf("rollkit header not found in context") } - var proposedLastCommit abci.CommitInfo - var lastCommit *cmttypes.Commit - - if blockHeight > 1 { - header, data, err := a.RollkitStore.GetBlockData(ctx, blockHeight-1) - if err != nil { - return nil, 0, fmt.Errorf("failed to get previous block data: %w", err) - } - - commitForPrevBlock := &cmttypes.Commit{ - Height: int64(header.Height()), - Round: 0, - BlockID: cmttypes.BlockID{Hash: bytes.HexBytes(header.Hash()), PartSetHeader: cmttypes.PartSetHeader{Total: 1, Hash: bytes.HexBytes(data.Hash())}}, - Signatures: []cmttypes.CommitSig{ - { - BlockIDFlag: cmttypes.BlockIDFlagCommit, - ValidatorAddress: cmttypes.Address(header.ProposerAddress), - Timestamp: header.Time(), - Signature: header.Signature, - }, - }, - } - - lastCommit = commitForPrevBlock - proposedLastCommit = cometCommitToABCICommitInfo(commitForPrevBlock) - } else { - // For the first block, ProposedLastCommit is empty - proposedLastCommit = abci.CommitInfo{Round: 0, Votes: []abci.VoteInfo{}} - lastCommit = &cmttypes.Commit{ - Height: int64(blockHeight), - Round: 0, - BlockID: cmttypes.BlockID{}, - Signatures: []cmttypes.CommitSig{}, - } + lastCommit, err := a.getLastCommit(ctx, blockHeight) + if err != nil { + return nil, 0, fmt.Errorf("failed to get last commit: %w", err) } emptyBlock, err := cometcompat.ToABCIBlock(header, &types.Data{}, lastCommit) @@ -348,7 +317,7 @@ func (a *Adapter) ExecuteTxs( Height: int64(blockHeight), Time: timestamp, Txs: txs, - ProposedLastCommit: proposedLastCommit, + ProposedLastCommit: cometCommitToABCICommitInfo(lastCommit), Misbehavior: []abci.Misbehavior{}, ProposerAddress: s.Validators.Proposer.Address, NextValidatorsHash: s.NextValidators.Hash(), @@ -464,70 +433,57 @@ func (a *Adapter) ExecuteTxs( cmtTxs[i] = txs[i] } - commit := &cmttypes.Commit{ - Height: int64(blockHeight), - Round: 0, - Signatures: []cmttypes.CommitSig{ + // if blockheight is 0, we create a signed last commit. + if blockHeight == 0 { + lastCommit.Signatures = []cmttypes.CommitSig{ { BlockIDFlag: cmttypes.BlockIDFlagCommit, ValidatorAddress: s.Validators.Proposer.Address, Timestamp: time.Now().UTC(), Signature: []byte{}, }, - }, + } } - if blockHeight > 1 { - header, data, err := a.RollkitStore.GetBlockData(ctx, blockHeight-1) - if err != nil { - return nil, 0, fmt.Errorf("failed to get previous block data: %w", err) - } + block := s.MakeBlock(int64(blockHeight), cmtTxs, lastCommit, nil, s.Validators.Proposer.Address) - commit = &cmttypes.Commit{ - Height: int64(header.Height()), - Round: 0, - BlockID: cmttypes.BlockID{Hash: bytes.HexBytes(header.Hash()), PartSetHeader: cmttypes.PartSetHeader{Total: 1, Hash: bytes.HexBytes(data.Hash())}}, - Signatures: []cmttypes.CommitSig{ - { - BlockIDFlag: cmttypes.BlockIDFlagCommit, - ValidatorAddress: cmttypes.Address(header.ProposerAddress), - Timestamp: header.Time(), - Signature: header.Signature, - }, - }, - } + currentBlockID := cmttypes.BlockID{ + Hash: block.Hash(), + PartSetHeader: cmttypes.PartSetHeader{Total: 1, Hash: block.DataHash}, } - block := s.MakeBlock(int64(blockHeight), cmtTxs, commit, nil, s.Validators.Proposer.Address) - - currentBlockID := cmttypes.BlockID{Hash: block.Hash(), PartSetHeader: cmttypes.PartSetHeader{Total: 1, Hash: block.DataHash}} + if err := fireEvents(a.EventBus, block, currentBlockID, fbResp, validatorUpdates); err != nil { + a.Logger.Error("failed to fire events", "err", err) + } - fireEvents(a.Logger, a.EventBus, block, currentBlockID, fbResp, validatorUpdates) + // save the finalized block response + if err := a.Store.SaveBlockResponse(ctx, blockHeight, fbResp); err != nil { + return nil, 0, fmt.Errorf("failed to save block response: %w", err) + } a.Logger.Info("block executed successfully", "height", blockHeight, "appHash", fmt.Sprintf("%X", fbResp.AppHash)) return fbResp.AppHash, uint64(s.ConsensusParams.Block.MaxBytes), nil } func fireEvents( - logger log.Logger, eventBus cmttypes.BlockEventPublisher, block *cmttypes.Block, blockID cmttypes.BlockID, abciResponse *abci.ResponseFinalizeBlock, validatorUpdates []*cmttypes.Validator, -) { +) error { if err := eventBus.PublishEventNewBlock(cmttypes.EventDataNewBlock{ Block: block, BlockID: blockID, ResultFinalizeBlock: *abciResponse, }); err != nil { - logger.Error("failed publishing new block", "err", err) + return fmt.Errorf("failed publishing new block: %w", err) } if err := eventBus.PublishEventNewBlockHeader(cmttypes.EventDataNewBlockHeader{ Header: block.Header, }); err != nil { - logger.Error("failed publishing new block header", "err", err) + return fmt.Errorf("failed publishing new block header: %w", err) } if err := eventBus.PublishEventNewBlockEvents(cmttypes.EventDataNewBlockEvents{ @@ -535,7 +491,7 @@ func fireEvents( Events: abciResponse.Events, NumTxs: int64(len(block.Txs)), }); err != nil { - logger.Error("failed publishing new block events", "err", err) + return fmt.Errorf("failed publishing new block events: %w", err) } if len(block.Evidence.Evidence) != 0 { @@ -544,7 +500,7 @@ func fireEvents( Evidence: ev, Height: block.Height, }); err != nil { - logger.Error("failed publishing new evidence", "err", err) + return fmt.Errorf("failed publishing new evidence: %w", err) } } } @@ -556,15 +512,74 @@ func fireEvents( Tx: tx, Result: *(abciResponse.TxResults[i]), }}); err != nil { - logger.Error("failed publishing event TX", "err", err) + return fmt.Errorf("failed publishing event TX: %w", err) } } if len(validatorUpdates) > 0 { if err := eventBus.PublishEventValidatorSetUpdates( cmttypes.EventDataValidatorSetUpdates{ValidatorUpdates: validatorUpdates}); err != nil { - logger.Error("failed publishing event", "err", err) + return fmt.Errorf("failed publishing event: %w", err) + } + } + + return nil +} + +func (a *Adapter) getLastCommit(ctx context.Context, blockHeight uint64) (*cmttypes.Commit, error) { + if blockHeight > 1 { + header, data, err := a.RollkitStore.GetBlockData(ctx, blockHeight-1) + if err != nil { + return nil, fmt.Errorf("failed to get previous block data: %w", err) + } + + commitForPrevBlock := &cmttypes.Commit{ + Height: int64(header.Height()), + Round: 0, + BlockID: cmttypes.BlockID{Hash: bytes.HexBytes(header.Hash()), PartSetHeader: cmttypes.PartSetHeader{Total: 1, Hash: bytes.HexBytes(data.Hash())}}, + Signatures: []cmttypes.CommitSig{ + { + BlockIDFlag: cmttypes.BlockIDFlagCommit, + ValidatorAddress: cmttypes.Address(header.ProposerAddress), + Timestamp: header.Time(), + Signature: header.Signature, + }, + }, } + + return commitForPrevBlock, nil + } + + return &cmttypes.Commit{ + Height: int64(blockHeight), + Round: 0, + BlockID: cmttypes.BlockID{}, + Signatures: []cmttypes.CommitSig{}, + }, nil +} + +func cometCommitToABCICommitInfo(commit *cmttypes.Commit) abci.CommitInfo { + if len(commit.Signatures) == 0 { + return abci.CommitInfo{ + Round: commit.Round, + Votes: []abci.VoteInfo{}, + } + } + + votes := make([]abci.VoteInfo, len(commit.Signatures)) + for i, sig := range commit.Signatures { + votes[i] = abci.VoteInfo{ + Validator: abci.Validator{ + Address: sig.ValidatorAddress, + Power: 0, + }, + BlockIdFlag: cmtprototypes.BlockIDFlag(sig.BlockIDFlag), + } + } + + return abci.CommitInfo{ + Round: commit.Round, + Votes: votes, } } @@ -618,34 +633,3 @@ func (a *Adapter) GetTxs(ctx context.Context) ([][]byte, error) { func (a *Adapter) SetFinal(ctx context.Context, blockHeight uint64) error { return nil } - -func cometCommitToABCICommitInfo(commit *cmttypes.Commit) abci.CommitInfo { - if commit == nil { - return abci.CommitInfo{ - Round: 0, - Votes: []abci.VoteInfo{}, - } - } - - if len(commit.Signatures) == 0 { - return abci.CommitInfo{ - Round: commit.Round, - Votes: []abci.VoteInfo{}, - } - } - - votes := make([]abci.VoteInfo, len(commit.Signatures)) - for i, sig := range commit.Signatures { - votes[i] = abci.VoteInfo{ - Validator: abci.Validator{ - Address: sig.ValidatorAddress, - Power: 0, - }, - BlockIdFlag: cmtprototypes.BlockIDFlag(sig.BlockIDFlag), - } - } - return abci.CommitInfo{ - Round: commit.Round, - Votes: votes, - } -} diff --git a/pkg/adapter/store.go b/pkg/adapter/store.go index 5b8740f4..700f3604 100644 --- a/pkg/adapter/store.go +++ b/pkg/adapter/store.go @@ -4,6 +4,7 @@ import ( "context" "fmt" + abci "github.com/cometbft/cometbft/abci/types" cmtstateproto "github.com/cometbft/cometbft/proto/tendermint/state" cmtstate "github.com/cometbft/cometbft/state" proto "github.com/cosmos/gogoproto/proto" @@ -16,6 +17,8 @@ const ( keyPrefix = "abci" // stateKey is the key used for storing state stateKey = "s" + // blockResponseKey is the key used for storing block responses + blockResponseKey = "br" ) // Store wraps a datastore with ABCI-specific functionality @@ -23,8 +26,9 @@ type Store struct { prefixedStore ds.Batching } -// NewStore creates a new Store with the ABCI prefix -func NewStore(store ds.Batching) *Store { +// NewABCIStore creates a new Store with the ABCI prefix. +// The data is stored under rollkit database and not in the app's database. +func NewExecABCIStore(store ds.Batching) *Store { return &Store{ prefixedStore: kt.Wrap(store, &kt.PrefixTransform{ Prefix: ds.NewKey(keyPrefix), @@ -64,3 +68,36 @@ func (s *Store) SaveState(ctx context.Context, state *cmtstate.State) error { return s.prefixedStore.Put(ctx, ds.NewKey(stateKey), data) } + +// SaveBlockResponse saves the block response to disk per height +// This is used to store the results of the block execution +// so that they can be retrieved later, e.g., for querying transaction results. +func (s *Store) SaveBlockResponse(ctx context.Context, height uint64, resp *abci.ResponseFinalizeBlock) error { + data, err := proto.Marshal(resp) + if err != nil { + return fmt.Errorf("failed to marshal block response: %w", err) + } + + key := fmt.Sprintf("%s/%d", blockResponseKey, height) + return s.prefixedStore.Put(ctx, ds.NewKey(key), data) +} + +// GetBlockResponse loads the block response from disk for a specific height +func (s *Store) GetBlockResponse(ctx context.Context, height uint64) (*abci.ResponseFinalizeBlock, error) { + key := fmt.Sprintf("%s/%d", blockResponseKey, height) + data, err := s.prefixedStore.Get(ctx, ds.NewKey(key)) + if err != nil { + return nil, fmt.Errorf("failed to get block response: %w", err) + } + + if data == nil { + return nil, fmt.Errorf("block response not found for height %d", height) + } + + resp := &abci.ResponseFinalizeBlock{} + if err := proto.Unmarshal(data, resp); err != nil { + return nil, fmt.Errorf("failed to unmarshal block response: %w", err) + } + + return resp, nil +} diff --git a/pkg/adapter/store_test.go b/pkg/adapter/store_test.go index 4261bf44..ef0020d9 100644 --- a/pkg/adapter/store_test.go +++ b/pkg/adapter/store_test.go @@ -14,7 +14,7 @@ import ( func TestStateIO(t *testing.T) { db := ds.NewMapDatastore() - abciStore := NewStore(db) + abciStore := NewExecABCIStore(db) myState := stateFixture() require.NoError(t, abciStore.SaveState(t.Context(), myState)) gotState, gotErr := abciStore.LoadState(t.Context()) diff --git a/pkg/rpc/core/blocks.go b/pkg/rpc/core/blocks.go index 688f5569..d2b9dd82 100644 --- a/pkg/rpc/core/blocks.go +++ b/pkg/rpc/core/blocks.go @@ -98,7 +98,10 @@ func BlockSearch( // If no height is provided, it will fetch the latest block. // More: https://docs.cometbft.com/v0.37/rpc/#/Info/block func Block(ctx *rpctypes.Context, heightPtr *int64) (*ctypes.ResultBlock, error) { - var heightValue uint64 + var ( + heightValue uint64 + err error + ) switch { case heightPtr != nil && *heightPtr == -1: @@ -116,7 +119,10 @@ func Block(ctx *rpctypes.Context, heightPtr *int64) (*ctypes.ResultBlock, error) heightValue = binary.LittleEndian.Uint64(rawVal) default: - heightValue = normalizeHeight(heightPtr) + heightValue, err = normalizeHeight(ctx.Context(), heightPtr) + if err != nil { + return nil, err + } } header, data, err := env.Adapter.RollkitStore.GetBlockData(ctx.Context(), heightValue) @@ -204,9 +210,12 @@ func BlockByHash(ctx *rpctypes.Context, hash []byte) (*ctypes.ResultBlock, error // If no height is provided, it will fetch the commit for the latest block. // More: https://docs.cometbft.com/main/rpc/#/Info/commit func Commit(ctx *rpctypes.Context, heightPtr *int64) (*ctypes.ResultCommit, error) { - wrappedCtx := ctx.Context() - heightValue := normalizeHeight(heightPtr) - header, rollkitData, err := env.Adapter.RollkitStore.GetBlockData(wrappedCtx, heightValue) + height, err := normalizeHeight(ctx.Context(), heightPtr) + if err != nil { + return nil, err + } + + header, rollkitData, err := env.Adapter.RollkitStore.GetBlockData(ctx.Context(), height) if err != nil { return nil, err } @@ -272,44 +281,38 @@ func Commit(ctx *rpctypes.Context, heightPtr *int64) (*ctypes.ResultCommit, erro }, nil } -// BlockResults is not fully implemented as in FullClient because -// env.Adapter.RollkitStore (pkg/store.Store) does not provide GetBlockResponses method. +// BlockResults gets block results at a given height. +// If no height is provided, it will fetch the results for the latest block. func BlockResults(ctx *rpctypes.Context, heightPtr *int64) (*ctypes.ResultBlockResults, error) { - // var h uint64 - // var err error - // if heightPtr == nil { - // h, err = env.Adapter.RollkitStore.Height(ctx.Context()) - // if err != nil { - // return nil, err - // } - // } else { - // h = uint64(*heightPtr) - // } - // header, _, err := env.Adapter.RollkitStore.GetBlockData(ctx.Context(), h) - // if err != nil { - // return nil, err - // } - // resp, err := env.Adapter.Store.GetBlockResponses(ctx.Context(), h) - // if err != nil { - // return nil, err - // } - - // return &ctypes.ResultBlockResults{ - // Height: int64(h), //nolint:gosec - // TxsResults: resp.TxResults, - // FinalizeBlockEvents: resp.Events, - // ValidatorUpdates: resp.ValidatorUpdates, - // ConsensusParamUpdates: resp.ConsensusParamUpdates, - // AppHash: header.Header.AppHash, - // }, nil - return nil, errors.New("BlockResults not implemented") + height, err := normalizeHeight(ctx.Context(), heightPtr) + if err != nil { + return nil, err + } + + resp, err := env.Adapter.Store.GetBlockResponse(ctx.Context(), height) + if err != nil { + return nil, err + } + + return &ctypes.ResultBlockResults{ + Height: int64(height), + TxsResults: resp.TxResults, + FinalizeBlockEvents: resp.Events, + ValidatorUpdates: resp.ValidatorUpdates, + ConsensusParamUpdates: resp.ConsensusParamUpdates, + AppHash: resp.AppHash, + }, nil } // Header gets block header at a given height. // If no height is provided, it will fetch the latest header. // More: https://docs.cometbft.com/v0.37/rpc/#/Info/header func Header(ctx *rpctypes.Context, heightPtr *int64) (*ctypes.ResultHeader, error) { - height := normalizeHeight(heightPtr) + height, err := normalizeHeight(ctx.Context(), heightPtr) + if err != nil { + return nil, err + } + blockMeta := getBlockMeta(ctx.Context(), height) if blockMeta == nil { return nil, fmt.Errorf("block at height %d not found", height) diff --git a/pkg/rpc/core/consensus.go b/pkg/rpc/core/consensus.go index 131eab5d..a6750d6d 100644 --- a/pkg/rpc/core/consensus.go +++ b/pkg/rpc/core/consensus.go @@ -16,7 +16,11 @@ import ( // // More: https://docs.cometbft.com/v0.37/rpc/#/Info/validators func Validators(ctx *rpctypes.Context, heightPtr *int64, pagePtr, perPagePtr *int) (*coretypes.ResultValidators, error) { - height := normalizeHeight(heightPtr) + height, err := normalizeHeight(ctx.Context(), heightPtr) + if err != nil { + return nil, fmt.Errorf("failed to normalize height: %w", err) + } + genesisValidators := env.Adapter.AppGenesis.Consensus.Validators if len(genesisValidators) != 1 { @@ -62,13 +66,19 @@ func ConsensusState(ctx *rpctypes.Context) (*coretypes.ResultConsensusState, err // If no height is provided, it will fetch the latest consensus params. // More: https://docs.cometbft.com/v0.37/rpc/#/Info/consensus_params func ConsensusParams(ctx *rpctypes.Context, heightPtr *int64) (*coretypes.ResultConsensusParams, error) { + height, err := normalizeHeight(ctx.Context(), heightPtr) + if err != nil { + return nil, fmt.Errorf("failed to normalize height: %w", err) + } + state, err := env.Adapter.Store.LoadState(ctx.Context()) if err != nil { return nil, err } + params := state.ConsensusParams return &coretypes.ResultConsensusParams{ - BlockHeight: int64(normalizeHeight(heightPtr)), //nolint:gosec + BlockHeight: int64(height), //nolint:gosec ConsensusParams: cmttypes.ConsensusParams{ Block: cmttypes.BlockParams{ MaxBytes: params.Block.MaxBytes, diff --git a/pkg/rpc/core/consensus_test.go b/pkg/rpc/core/consensus_test.go index 49971607..93a89397 100644 --- a/pkg/rpc/core/consensus_test.go +++ b/pkg/rpc/core/consensus_test.go @@ -113,7 +113,7 @@ func setupTestConsensusParamsEnv(t *testing.T, useMockRollkitStore bool, stateTo } dsStore := ds.NewMapDatastore() - abciStore := adapter.NewStore(dsStore) + abciStore := adapter.NewExecABCIStore(dsStore) if stateToSave != nil { err := abciStore.SaveState(context.Background(), stateToSave) @@ -195,9 +195,9 @@ func TestValidators(t *testing.T) { mockStore.AssertExpectations(t) }) - t.Run("Success_HeightNormalizationReturnsZeroOnError", func(t *testing.T) { + t.Run("Success_NilHeight", func(t *testing.T) { mockStore := setupTestValidatorsEnv(t, []cmttypes.GenesisValidator{testGenesisValidator}, testSampleConsensusParams) - mockStore.On("Height", testifymock.Anything).Return(uint64(0), errors.New("failed to get height")).Once() + mockStore.On("Height", testifymock.Anything).Return(uint64(0), nil).Once() result, err := Validators(ctx, nil, nil, nil) require.NoError(err) @@ -206,6 +206,17 @@ func TestValidators(t *testing.T) { assert.Len(result.Validators, 1) // Still expect validator details mockStore.AssertExpectations(t) }) + + t.Run("Error_NilHeightAndStoreError", func(t *testing.T) { + mockStore := setupTestValidatorsEnv(t, []cmttypes.GenesisValidator{testGenesisValidator}, testSampleConsensusParams) + mockStore.On("Height", testifymock.Anything).Return(uint64(0), errors.New("failed to get height")).Once() + + result, err := Validators(ctx, nil, nil, nil) + require.Error(err) + assert.Nil(result) + assert.Contains(err.Error(), "failed to get height") + mockStore.AssertExpectations(t) + }) } func TestDumpConsensusState(t *testing.T) { @@ -287,12 +298,18 @@ func TestConsensusParams(t *testing.T) { mockRollkitStore.AssertExpectations(t) }) - t.Run("Success_HeightNormalizationReturnsZeroOnError", func(t *testing.T) { + t.Run("Error_NilHeightAndStoreError", func(t *testing.T) { mockRollkitStore, _ := setupTestConsensusParamsEnv(t, true, &testMockStateWithConsensusParams) mockRollkitStore.On("Height", testifymock.Anything).Return(uint64(0), errors.New("failed to get height")).Once() - // err := abciStore.SaveState(context.Background(), &testMockStateWithConsensusParams) // Moved to helper - // require.NoError(err) // Moved to helper + _, err := ConsensusParams(ctx, nil) + require.Error(err) + mockRollkitStore.AssertExpectations(t) + }) + + t.Run("Success_NilHeight", func(t *testing.T) { + mockRollkitStore, _ := setupTestConsensusParamsEnv(t, true, &testMockStateWithConsensusParams) + mockRollkitStore.On("Height", testifymock.Anything).Return(uint64(0), nil).Once() result, err := ConsensusParams(ctx, nil) require.NoError(err) diff --git a/pkg/rpc/core/utils.go b/pkg/rpc/core/utils.go index 210011d5..88821cca 100644 --- a/pkg/rpc/core/utils.go +++ b/pkg/rpc/core/utils.go @@ -19,32 +19,24 @@ import ( const NodeIDByteLength = 20 -func normalizeHeight(height *int64) uint64 { - var heightValue uint64 - if height == nil { - var err error - // TODO: Decide how to handle context here. Using background for now. - heightValue, err = env.Adapter.RollkitStore.Height(context.Background()) +func normalizeHeight(ctx context.Context, height *int64) (uint64, error) { + var ( + heightValue uint64 + err error + ) + + if height == nil || *height < 0 { + // Handle negative heights if they have special meaning + // (e.g., -1 for latest) + heightValue, err = env.Adapter.RollkitStore.Height(ctx) if err != nil { - // TODO: Consider logging or returning error - env.Logger.Error("Failed to get current height in normalizeHeight", "err", err) - return 0 - } - } else if *height < 0 { - // Handle negative heights if they have special meaning (e.g., -1 for latest) - // Currently, just treat them as 0 or latest, adjust as needed. - // For now, let's assume negative height means latest valid height. - var err error - heightValue, err = env.Adapter.RollkitStore.Height(context.Background()) - if err != nil { - env.Logger.Error("Failed to get current height for negative height in normalizeHeight", "err", err) - return 0 + return 0, fmt.Errorf("failed to get current height: %w", err) } } else { heightValue = uint64(*height) } - return heightValue + return heightValue, nil } func getLastCommit(ctx context.Context, blockHeight uint64) (*cmttypes.Commit, error) {