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

Jump to bottom

feat: add custom build tags to override certain global variables #3897

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
+76 −22
Open

feat: add custom build tags to override certain global variables #3897

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e02bb43
feat: add custom build tags to override certain global variables
cmwaters Sep 23, 2024
73f4574
Merge branch 'main' into cal/add-build-tags
cmwaters Sep 23, 2024
94334d2
update the Dockerfile
cmwaters Sep 23, 2024
47e3407
use strings instead of integers
cmwaters Sep 23, 2024
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
8 changes: 8 additions & 0 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@ ARG BUILDER_IMAGE=docker.io/golang:1.22.6-alpine3.19
ARG RUNTIME_IMAGE=docker.io/alpine:3.19
ARG TARGETOS
ARG TARGETARCH
# Use build args to override the maxuimum square size of the docker image i.e.
# docker build --build-arg MAX_SQUARE_SIZE=64 -t celestia-app:latest .
ARG MAX_SQUARE_SIZE
# Use build args to override the upgrade height delay of the docker image i.e.
# docker build --build-arg UPGRADE_HEIGHT_DELAY=1000 -t celestia-app:latest .
ARG UPGRADE_HEIGHT_DELAY

# Stage 1: Build the celestia-appd binary inside a builder image that will be discarded later.
# Ignore hadolint rule because hadolint can't parse the variable.
Expand All @@ -28,6 +34,8 @@ COPY . /celestia-app
WORKDIR /celestia-app
RUN uname -a &&\
CGO_ENABLED=${CGO_ENABLED} GOOS=${TARGETOS} GOARCH=${TARGETARCH} \
OVERRIDE_MAX_SQUARE_SIZE=${MAX_SQUARE_SIZE} \
OVERRIDE_UPGRADE_HEIGHT_DELAY=${UPGRADE_HEIGHT_DELAY} \
make build

# Stage 2: Create a minimal image to run the celestia-appd binary
Expand Down
8 changes: 7 additions & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,18 @@ PROJECTNAME=$(shell basename "$(PWD)")
HTTPS_GIT := https://github.com/celestiaorg/celestia-app.git
PACKAGE_NAME := github.com/celestiaorg/celestia-app/v3
GOLANG_CROSS_VERSION ?= v1.22.6
# Set this to override the max square size of the binary
OVERRIDE_MAX_SQUARE_SIZE ?=
# Set this to override the upgrade height delay of the binary
OVERRIDE_UPGRADE_HEIGHT_DELAY ?=

# process linker flags
ldflags = -X github.com/cosmos/cosmos-sdk/version.Name=celestia-app \
-X github.com/cosmos/cosmos-sdk/version.AppName=celestia-appd \
-X github.com/cosmos/cosmos-sdk/version.Version=$(VERSION) \
-X github.com/cosmos/cosmos-sdk/version.Commit=$(COMMIT) \
-X github.com/celestiaorg/celestia-app/v3/pkg/appconsts.OverrideSquareSizeUpperBoundStr=$(OVERRIDE_MAX_SQUARE_SIZE) \
-X github.com/celestiaorg/celestia-app/v3/pkg/appconsts.OverrideUpgradeHeightDelayStr=$(OVERRIDE_UPGRADE_HEIGHT_DELAY)

BUILD_FLAGS := -tags "ledger" -ldflags '$(ldflags)'

Expand All @@ -31,7 +37,7 @@ build: mod
.PHONY: build

## install: Build and install the celestia-appd binary into the $GOPATH/bin directory.
install: go.sum check-bbr
install: check-bbr
@echo "--> Installing celestia-appd"
@go install $(BUILD_FLAGS) ./cmd/celestia-appd
.PHONY: install
Expand Down
3 changes: 2 additions & 1 deletion app/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
celestiatx "github.com/celestiaorg/celestia-app/v3/app/grpc/tx"
"github.com/celestiaorg/celestia-app/v3/app/module"
"github.com/celestiaorg/celestia-app/v3/app/posthandler"
"github.com/celestiaorg/celestia-app/v3/pkg/appconsts"
appv1 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v1"
appv2 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v2"
appv3 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v3"
Expand Down Expand Up @@ -279,7 +280,7 @@ func New(
),
)

app.SignalKeeper = signal.NewKeeper(appCodec, keys[signaltypes.StoreKey], app.StakingKeeper)
app.SignalKeeper = signal.NewKeeper(appCodec, keys[signaltypes.StoreKey], app.StakingKeeper, appconsts.UpgradeHeightDelay())

app.IBCKeeper = ibckeeper.NewKeeper(
appCodec,
Expand Down
2 changes: 1 addition & 1 deletion app/module/configurator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func TestConfigurator(t *testing.T) {
stateStore.MountStoreWithDB(storeKey, storetypes.StoreTypeIAVL, db)
require.NoError(t, stateStore.LoadLatestVersion())

keeper := signal.NewKeeper(config.Codec, storeKey, nil)
keeper := signal.NewKeeper(config.Codec, storeKey, nil, 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Tip

Codebase Verification

Inconsistent NewKeeper Function Usage Detected

The recent addition of a new parameter to the NewKeeper function in configurator_test.go is not reflected consistently across the codebase. Several instances of NewKeeper are still being called without the new parameter, which could lead to potential issues.

  • Affected Files:
    • x/tokenfilter/keeper.go
    • x/signal/keeper_test.go
    • x/signal/keeper.go
    • x/mint/keeper/keeper.go
    • x/blobstream/keeper/keeper.go
    • test/pfm/simapp.go
    • test/util/common.go
    • x/blob/keeper/keeper_test.go
    • x/blob/keeper/keeper.go
    • app/app.go
    • app/ante/min_fee_test.go

Please ensure all NewKeeper function calls are updated accordingly to maintain consistency.

Analysis chain

LGTM, but consider adding more test cases.

The addition of the new parameter to NewKeeper aligns with the changes mentioned in the AI summary. This change likely relates to the PR objectives of adding customizable upper bounds.

Consider adding more test cases that use different values for this new parameter to ensure the Keeper behaves correctly with various inputs.

Let's verify if this change is consistent across the codebase:

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for all NewKeeper function calls in the codebase
rg --type go 'NewKeeper\(' -A 3

Length of output: 12876

require.NotNil(t, keeper)
upgradeModule := signal.NewAppModule(keeper)
manager, err := module.NewManager([]module.VersionedModule{
Expand Down
7 changes: 3 additions & 4 deletions app/test/upgrade_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
"github.com/celestiaorg/celestia-app/v3/test/util/testnode"
blobstreamtypes "github.com/celestiaorg/celestia-app/v3/x/blobstream/types"
"github.com/celestiaorg/celestia-app/v3/x/minfee"
"github.com/celestiaorg/celestia-app/v3/x/signal"
signaltypes "github.com/celestiaorg/celestia-app/v3/x/signal/types"
"github.com/celestiaorg/go-square/v2/share"
"github.com/celestiaorg/go-square/v2/tx"
Expand Down Expand Up @@ -99,7 +98,7 @@ func TestAppUpgradeV3(t *testing.T) {

// brace yourselfs, this part may take a while
initialHeight := int64(4)
for height := initialHeight; height < initialHeight+signal.DefaultUpgradeHeightDelay; height++ {
for height := initialHeight; height < initialHeight+appconsts.DefaultUpgradeHeightDelay; height++ {
_ = testApp.BeginBlock(abci.RequestBeginBlock{
Header: tmproto.Header{
Height: height,
Expand All @@ -108,7 +107,7 @@ func TestAppUpgradeV3(t *testing.T) {
})

endBlockResp = testApp.EndBlock(abci.RequestEndBlock{
Height: 3 + signal.DefaultUpgradeHeightDelay,
Height: 3 + appconsts.DefaultUpgradeHeightDelay,
})

_ = testApp.Commit()
Expand All @@ -130,7 +129,7 @@ func TestAppUpgradeV3(t *testing.T) {
_ = testApp.BeginBlock(abci.RequestBeginBlock{
Header: tmproto.Header{
ChainID: genesis.ChainID,
Height: initialHeight + signal.DefaultUpgradeHeightDelay,
Height: initialHeight + appconsts.DefaultUpgradeHeightDelay,
Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM: Consistent use of new constant in BeginBlock request.

The change to use appconsts.DefaultUpgradeHeightDelay in the BeginBlock request header is consistent with the previous modifications. This ensures that the test uses the same constant throughout for the upgrade height.

Consider refactoring the upgrade height calculation into a separate variable to improve code maintainability. For example:

+upgradeHeight := initialHeight + appconsts.DefaultUpgradeHeightDelay
 _ = testApp.BeginBlock(abci.RequestBeginBlock{
 	Header: tmproto.Header{
 		ChainID: genesis.ChainID,
-		Height:  initialHeight + appconsts.DefaultUpgradeHeightDelay,
+		Height:  upgradeHeight,
 		Version: tmversion.Consensus{App: 3},
 	},
 })

This refactoring would make it easier to update the upgrade height calculation in the future if needed.

Committable suggestion was skipped due to low confidence.

Version: tmversion.Consensus{App: 3},
},
})
Expand Down
19 changes: 19 additions & 0 deletions pkg/appconsts/global_consts.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package appconsts

import (
"strconv"

"github.com/celestiaorg/go-square/v2/share"
"github.com/celestiaorg/rsmt2d"
"github.com/tendermint/tendermint/pkg/consts"
Expand All @@ -24,6 +26,11 @@ const (

// BondDenom defines the native staking denomination
BondDenom = "utia"

// DefaultUpgradeHeightDelay is the number of blocks after a quorum has been
// reached that the chain should upgrade to the new version. Assuming a block
// interval of 12 seconds, this is 7 days.
DefaultUpgradeHeightDelay = int64(7 * 24 * 60 * 60 / 12) // 7 days * 24 hours * 60 minutes * 60 seconds / 12 seconds per block = 50,400 blocks.
)

var (
Expand All @@ -44,6 +51,18 @@ var (
SupportedShareVersions = share.SupportedShareVersions
)

// returns the delay in blocks after a quorum has been reached that the chain should upgrade to the new version.
func UpgradeHeightDelay() int64 {
if OverrideUpgradeHeightDelayStr != "" {
parsedValue, err := strconv.ParseInt(OverrideUpgradeHeightDelayStr, 10, 64)
if err != nil {
panic("Invalid OverrideUpgradeHeightDelayStr value")
}
return parsedValue
}
return DefaultUpgradeHeightDelay
}
Comment on lines +55 to +64
Copy link
Contributor

Choose a reason for hiding this comment

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

Tip

Codebase Verification

Function implementation needs improvement.

The UpgradeHeightDelay() function aligns with the PR objectives, and it has been confirmed that OverrideUpgradeHeightDelayStr is properly defined in pkg/appconsts/overrides.go. However, there are still some concerns:

  1. Panicking on invalid input might not be the best approach for error handling.
  2. There's no validation for negative values, which wouldn't make sense for a delay.

Consider the following improvements:

  1. Use error returning instead of panicking.
  2. Add validation for non-negative values.
Analysis chain

Function implementation needs improvement.

The UpgradeHeightDelay() function aligns with the PR objectives, but there are some concerns:

  1. The OverrideUpgradeHeightDelayStr variable is not defined in this file. Ensure it's properly declared and documented.
  2. Panicking on invalid input might not be the best approach for error handling.
  3. There's no validation for negative values, which wouldn't make sense for a delay.

Consider the following improvements:

  1. Add a comment explaining where OverrideUpgradeHeightDelayStr is defined.
  2. Use error returning instead of panicking.
  3. Add validation for non-negative values.

Here's a suggested implementation:

var ErrInvalidUpgradeHeightDelay = errors.New("invalid UpgradeHeightDelay value")

func UpgradeHeightDelay() (int64, error) {
	if OverrideUpgradeHeightDelayStr != "" {
		parsedValue, err := strconv.ParseInt(OverrideUpgradeHeightDelayStr, 10, 64)
		if err != nil {
			return 0, fmt.Errorf("%w: %s", ErrInvalidUpgradeHeightDelay, err)
		}
		if parsedValue < 0 {
			return 0, fmt.Errorf("%w: value must be non-negative", ErrInvalidUpgradeHeightDelay)
		}
		return parsedValue, nil
	}
	return DefaultUpgradeHeightDelay, nil
}

This approach provides better error handling and ensures the returned value is always non-negative.

To ensure OverrideUpgradeHeightDelayStr is properly defined, run:


// HashLength returns the length of a hash in bytes.
func HashLength() int {
return hashLength
Expand Down
10 changes: 10 additions & 0 deletions pkg/appconsts/overrides.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package appconsts

// Set of values that can be overridden at compile time to modify the behavior of the app.
// WARNING: This should only be modified for testing purposes. All nodes in a network
// must have the same values for these constants.
// Look at the Makefile to see how these are set.
var (
OverrideSquareSizeUpperBoundStr string
OverrideUpgradeHeightDelayStr string
)
9 changes: 9 additions & 0 deletions pkg/appconsts/versioned_consts.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package appconsts

import (
"strconv"

v3 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v3"
)

Expand All @@ -22,6 +24,13 @@ func SubtreeRootThreshold(_ uint64) int {

// SquareSizeUpperBound imposes an upper bound on the max effective square size.
func SquareSizeUpperBound(_ uint64) int {
if OverrideSquareSizeUpperBoundStr != "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

Tip

Codebase Verification

Issue Found: OverrideSquareSizeUpperBoundStr is Defined as a Struct Field but Used as a Global Variable

The variable OverrideSquareSizeUpperBoundStr is defined as a field within a struct in pkg/appconsts/overrides.go but is incorrectly used as a standalone global variable in pkg/appconsts/versioned_consts.go. This misuse can lead to undefined behavior or runtime errors.

  • Location of Misuse:
    • pkg/appconsts/versioned_consts.go
      • Line: 27
      • Usage: if OverrideSquareSizeUpperBoundStr != "" {

Recommended Actions:

  • Access OverrideSquareSizeUpperBoundStr through its parent struct instance in versioned_consts.go.
  • If a global variable is intended, properly define and document OverrideSquareSizeUpperBoundStr as a global variable or constant.
Analysis chain

Verify the definition of OverrideSquareSizeUpperBoundStr

The global variable OverrideSquareSizeUpperBoundStr is used here but not defined in this file. Ensure it's properly defined and documented in the appropriate location.

Run the following script to check for the definition of OverrideSquareSizeUpperBoundStr:

If the variable is not found, it needs to be defined and documented properly.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Search for the definition of OverrideSquareSizeUpperBoundStr

# Test: Look for the variable definition
rg --type go -A 5 $'var OverrideSquareSizeUpperBoundStr'

# If not found as a var, check for const definition
rg --type go -A 5 $'const OverrideSquareSizeUpperBoundStr'

# If still not found, search for any occurrence to see if it's defined differently
rg --type go $'OverrideSquareSizeUpperBoundStr'

Length of output: 503

parsedValue, err := strconv.Atoi(OverrideSquareSizeUpperBoundStr)
if err != nil {
panic("Invalid OverrideSquareSizeUpperBoundStr value")
}
return parsedValue
}
Comment on lines +27 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.

Consider updating the function signature

The function still takes an unused uint64 parameter. Consider updating the function signature to reflect that this parameter is no longer used, or document why it's kept for backward compatibility.

If the parameter is no longer needed, update the function signature:

-func SquareSizeUpperBound(_ uint64) int {
+func SquareSizeUpperBound() int {

If it's kept for backward compatibility, add a comment explaining this:

// SquareSizeUpperBound imposes an upper bound on the max effective square size.
// The uint64 parameter is kept for backward compatibility but is no longer used.
func SquareSizeUpperBound(_ uint64) int {
    // ... (rest of the function)
}

Implement additional validation for the override value

While the implementation allows for customizable upper bounds as intended, consider adding validation to ensure the parsed value is positive and within a reasonable range. This will prevent potential issues with extremely large or negative values.

Consider modifying the function as follows:

 if OverrideSquareSizeUpperBoundStr != "" {
 	parsedValue, err := strconv.Atoi(OverrideSquareSizeUpperBoundStr)
-	if err != nil {
-		panic("Invalid OverrideSquareSizeUpperBoundStr value")
+	if err != nil || parsedValue <= 0 {
+		panic(fmt.Sprintf("Invalid OverrideSquareSizeUpperBoundStr value: %s. Must be a positive integer.", OverrideSquareSizeUpperBoundStr))
 	}
+	// Add an upper limit check if necessary, e.g.:
+	// if parsedValue > someMaxAllowedValue {
+	//     panic(fmt.Sprintf("OverrideSquareSizeUpperBoundStr value too large: %d. Maximum allowed: %d", parsedValue, someMaxAllowedValue))
+	// }
 	return parsedValue
 }

This change will provide more informative error messages and ensure the override value is valid.

Committable suggestion was skipped due to low confidence.

return v3.SquareSizeUpperBound
}

Expand Down
4 changes: 2 additions & 2 deletions x/signal/integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ import (
"testing"

"github.com/celestiaorg/celestia-app/v3/app"
"github.com/celestiaorg/celestia-app/v3/pkg/appconsts"
v2 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v2"
testutil "github.com/celestiaorg/celestia-app/v3/test/util"
"github.com/celestiaorg/celestia-app/v3/x/signal"
"github.com/celestiaorg/celestia-app/v3/x/signal/types"
"github.com/stretchr/testify/require"

Expand Down Expand Up @@ -77,7 +77,7 @@ func TestUpgradeIntegration(t *testing.T) {
require.False(t, shouldUpgrade)
require.EqualValues(t, 0, version)

ctx = ctx.WithBlockHeight(ctx.BlockHeight() + signal.DefaultUpgradeHeightDelay)
ctx = ctx.WithBlockHeight(ctx.BlockHeight() + appconsts.DefaultUpgradeHeightDelay)

shouldUpgrade, version = app.SignalKeeper.ShouldUpgrade(ctx)
require.True(t, shouldUpgrade)
Expand Down
19 changes: 10 additions & 9 deletions x/signal/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,6 @@ import (
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
)

// DefaultUpgradeHeightDelay is the number of blocks after a quorum has been
// reached that the chain should upgrade to the new version. Assuming a block
// interval of 12 seconds, this is 7 days.
const DefaultUpgradeHeightDelay = int64(7 * 24 * 60 * 60 / 12) // 7 days * 24 hours * 60 minutes * 60 seconds / 12 seconds per block = 50,400 blocks.

// Keeper implements the MsgServer and QueryServer interfaces
var (
_ types.MsgServer = &Keeper{}
Expand Down Expand Up @@ -46,18 +41,24 @@ type Keeper struct {
// stakingKeeper is used to fetch validators to calculate the total power
// signalled to a version.
stakingKeeper StakingKeeper

// upgradeHeightDelayBlocks is the number of blocks after a quorum has been
// reached that the chain should upgrade to the new version
upgradeHeightDelayBlocks int64
}

// NewKeeper returns a signal keeper.
func NewKeeper(
binaryCodec codec.BinaryCodec,
storeKey storetypes.StoreKey,
stakingKeeper StakingKeeper,
upgradeHeightDelayBlocks int64,
Copy link
Contributor

Choose a reason for hiding this comment

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

Tip

Codebase Verification

Missing upgradeHeightDelayBlocks Parameter in NewKeeper Calls

Several calls to NewKeeper do not include the required upgradeHeightDelayBlocks parameter. Please update the following instances to include this new parameter:

  • x/tokenfilter/keeper.go:func NewKeeper(wrapper porttypes.ICS4Wrapper) Keeper { ... }
  • x/blobstream/keeper.go:func NewKeeper(cdc codec.BinaryCodec, storeKey storetypes.StoreKey, paramSpace paramtypes.Subspace, stakingKeeper StakingKeeper) *Keeper { ... }
Analysis chain

LGTM! Verify impact on keeper initialization.

The changes to the NewKeeper function correctly incorporate the new upgradeHeightDelayBlocks parameter. This modification allows for flexible configuration of the upgrade delay at the time of keeper creation, which aligns well with the PR objectives.

To ensure that these changes don't break existing code, please verify all calls to NewKeeper in the codebase. Run the following script to check for any instances that need to be updated:

Make sure to update all instances of NewKeeper calls with the appropriate upgradeHeightDelayBlocks value.

Also applies to: 61-61

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Find all calls to NewKeeper and verify they include the new parameter

# Search for NewKeeper calls
rg --type go "NewKeeper\s*\(" -A 10

Length of output: 23733

) Keeper {
return Keeper{
binaryCodec: binaryCodec,
storeKey: storeKey,
stakingKeeper: stakingKeeper,
binaryCodec: binaryCodec,
storeKey: storeKey,
stakingKeeper: stakingKeeper,
upgradeHeightDelayBlocks: upgradeHeightDelayBlocks,
}
}

Expand Down Expand Up @@ -108,7 +109,7 @@ func (k *Keeper) TryUpgrade(ctx context.Context, _ *types.MsgTryUpgrade) (*types
}
upgrade := types.Upgrade{
AppVersion: version,
UpgradeHeight: sdkCtx.BlockHeader().Height + DefaultUpgradeHeightDelay,
UpgradeHeight: sdkCtx.BlockHeader().Height + k.upgradeHeightDelayBlocks,
}
k.setUpgrade(sdkCtx, upgrade)
}
Expand Down
9 changes: 5 additions & 4 deletions x/signal/keeper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

"github.com/celestiaorg/celestia-app/v3/app"
"github.com/celestiaorg/celestia-app/v3/app/encoding"
"github.com/celestiaorg/celestia-app/v3/pkg/appconsts"
v1 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v1"
v2 "github.com/celestiaorg/celestia-app/v3/pkg/appconsts/v2"
"github.com/celestiaorg/celestia-app/v3/x/signal"
Expand Down Expand Up @@ -68,7 +69,7 @@ func TestGetVotingPowerThreshold(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
config := encoding.MakeConfig(app.ModuleEncodingRegisters...)
stakingKeeper := newMockStakingKeeper(tc.validators)
k := signal.NewKeeper(config.Codec, nil, stakingKeeper)
k := signal.NewKeeper(config.Codec, nil, stakingKeeper, appconsts.DefaultUpgradeHeightDelay)
got := k.GetVotingPowerThreshold(sdk.Context{})
assert.Equal(t, tc.want, got, fmt.Sprintf("want %v, got %v", tc.want.String(), got.String()))
})
Expand Down Expand Up @@ -182,7 +183,7 @@ func TestTallyingLogic(t *testing.T) {
require.False(t, shouldUpgrade) // should be false because upgrade height hasn't been reached.
require.Equal(t, uint64(0), version)

ctx = ctx.WithBlockHeight(ctx.BlockHeight() + signal.DefaultUpgradeHeightDelay)
ctx = ctx.WithBlockHeight(ctx.BlockHeight() + appconsts.DefaultUpgradeHeightDelay)

shouldUpgrade, version = upgradeKeeper.ShouldUpgrade(ctx)
require.True(t, shouldUpgrade) // should be true because upgrade height has been reached.
Expand Down Expand Up @@ -425,7 +426,7 @@ func TestGetUpgrade(t *testing.T) {
got, err := upgradeKeeper.GetUpgrade(ctx, &types.QueryGetUpgradeRequest{})
require.NoError(t, err)
assert.Equal(t, v2.Version, got.Upgrade.AppVersion)
assert.Equal(t, signal.DefaultUpgradeHeightDelay, got.Upgrade.UpgradeHeight)
assert.Equal(t, appconsts.DefaultUpgradeHeightDelay, got.Upgrade.UpgradeHeight)
})
}

Expand All @@ -451,7 +452,7 @@ func setup(t *testing.T) (signal.Keeper, sdk.Context, *mockStakingKeeper) {
)

config := encoding.MakeConfig(app.ModuleEncodingRegisters...)
upgradeKeeper := signal.NewKeeper(config.Codec, signalStore, mockStakingKeeper)
upgradeKeeper := signal.NewKeeper(config.Codec, signalStore, mockStakingKeeper, appconsts.DefaultUpgradeHeightDelay)
return upgradeKeeper, mockCtx, mockStakingKeeper
}

Expand Down
Loading