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.

Sign up for GitHub

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

docs: clarify what is available in ctx.BlockHeader() #23394

Merged
merged 2 commits into from
Jan 15, 2025
Merged

docs: clarify what is available in ctx.BlockHeader() #23394

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7ec7bde
docs: clarify what is available in ctx.BlockHeader()
julienrbrt Jan 15, 2025
c4bc3a5
`make mocks`
julienrbrt Jan 15, 2025
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
3 changes: 3 additions & 0 deletions types/context.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ func (c Context) CometInfo() comet.Info { return c.comet
func (c Context) HeaderInfo() header.Info { return c.headerInfo }

// BlockHeader returns the header by value.
// Note, only ChainID, Time and Height are always filled by baseapp.
// In finalize block, the proposer address, validator hash and app hash are also filled.
// Use CometInfo service and/or the Consensus Keeper for getting more information.
func (c Context) BlockHeader() cmtproto.Header {
Comment on lines 92 to 98
Copy link
Contributor

Choose a reason for hiding this comment

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

Change potentially affects state.

Call sequence:

(github.com/cosmos/cosmos-sdk/types.Context).BlockHeader (types/context.go:95)
(github.com/cosmos/cosmos-sdk/types.Context).WithProposer (types/context.go:183)
(*github.com/cosmos/cosmos-sdk/baseapp.BaseApp).PrepareProposal (types/context.go:408)

return c.header
}
Expand Down
1 change: 1 addition & 0 deletions x/auth/ante/expected_keepers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,6 @@ type FeegrantKeeper interface {
}

type ConsensusKeeper interface {
AppVersion(ctx context.Context) (uint64, error)
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
BlockParams(context.Context) (uint64, uint64, error)
}
Comment on lines +31 to 33
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Missing test coverage for ConsensusKeeper interface methods

The ConsensusKeeper interface lacks test coverage:

  • No tests found for the new AppVersion method
  • No actual usage of BlockParams method found in tests
  • Mock implementation in testutil/expected_keepers_mocks.go is outdated and missing the AppVersion method

Please:

  1. Update the mock implementation to include the AppVersion method
  2. Add test coverage for both AppVersion and BlockParams methods
  3. Ensure the mock is properly used in integration tests
🔗 Analysis chain

Add test coverage for the ConsensusKeeper interface.

The pipeline indicates 0% test coverage. Please add tests to verify the behavior of implementations of this interface, particularly the new AppVersion method.

Let me help verify the current test coverage:

Would you like me to help create a test suite for this interface?

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for existing test files and their coverage
echo "Checking for existing test files..."
fd --type f "expected_keepers_test.go" .

echo "Checking test coverage for ConsensusKeeper implementations..."
rg -l "type \w+ struct.*implements.*ConsensusKeeper"

Length of output: 309

Script:

#!/bin/bash
# Find all test files in auth/ante directory
echo "=== Test files in auth/ante ==="
fd -e go -e test.go . x/auth/ante/

# Search for ConsensusKeeper interface implementations
echo -e "\n=== ConsensusKeeper implementations ==="
ast-grep --pattern 'type $_ struct {
  $$$
}'

# Search for ConsensusKeeper usage in test files
echo -e "\n=== ConsensusKeeper usage in tests ==="
rg "ConsensusKeeper" -A 5 -B 5 --type go --glob "*_test.go"

# Look for mock implementations
echo -e "\n=== Mock implementations ==="
rg "mock.*ConsensusKeeper" --type go

Length of output: 68209

Script:

#!/bin/bash
# Search for ConsensusKeeper interface usage and mocks in test files
echo "=== ConsensusKeeper in test files ==="
rg -B 5 -A 5 "ConsensusKeeper" x/auth/ante/testutil/expected_keepers_mocks.go

# Search for AppVersion and BlockParams method usage in tests
echo -e "\n=== AppVersion and BlockParams usage in tests ==="
rg -l "AppVersion|BlockParams" x/auth/ante/**/*_test.go

# Check mock implementations
echo -e "\n=== Mock implementations content ==="
cat x/auth/ante/testutil/expected_keepers_mocks.go

Length of output: 10079

Script:

#!/bin/bash
# Check for actual usage of BlockParams in tests
echo "=== BlockParams usage in tests ==="
rg -B 5 -A 5 "BlockParams" x/auth/ante/unorderedtx/manager_test.go x/auth/ante/unorderedtx/snapshotter_test.go

# Check if AppVersion is used anywhere in tests
echo -e "\n=== AppVersion usage in tests ==="
rg -l "AppVersion" x/auth/ante/**/*_test.go

Length of output: 375

🧰 Tools
🪛 GitHub Actions: Tests / Code Coverage

[warning] No test coverage (0.0% of statements covered)

Loading