refactor: override the upgrade height for chain-id test to 3 (celes…

…tiaorg#4065)

Closes celestiaorg#4053

## Description
1. Override the upgrade height for chain-id `test` to 3 so that there
are still a few blocks of delay in tests
1. Remove `OverrideUpgradeHeightDelayStr` because it was confusing to
have multiple ways to override the upgrade height (1) chain-id and (2)
via ENV variables

Makefile

@@ -28,16 +28,13 @@ PACKAGE_NAME          := github.com/celestiaorg/celestia-app/v3
 GOLANG_CROSS_VERSION  ?= v1.23.1
 # 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)
+		  -X github.com/celestiaorg/celestia-app/v3/pkg/appconsts.OverrideSquareSizeUpperBoundStr=$(OVERRIDE_MAX_SQUARE_SIZE)
 
 BUILD_FLAGS := -tags "ledger" -ldflags '$(ldflags)'
 

app/test/upgrade_test.go

@@ -38,9 +38,6 @@ func TestAppUpgradeV3(t *testing.T) {
 		t.Skip("skipping TestAppUpgradeV3 in short mode")
 	}
 
-	appconsts.OverrideUpgradeHeightDelayStr = "1"
-	defer func() { appconsts.OverrideUpgradeHeightDelayStr = "" }()
-
 	testApp, genesis := SetupTestAppWithUpgradeHeight(t, 3)
 	upgradeFromV1ToV2(t, testApp)
 
@@ -104,7 +101,6 @@ func TestAppUpgradeV3(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, v3.Version, getUpgradeResp.Upgrade.AppVersion)
 
-	// brace yourselfs, this part may take a while
 	initialHeight := int64(4)
 	for height := initialHeight; height < initialHeight+appconsts.UpgradeHeightDelay(testApp.GetChainID(), v2.Version); height++ {
 		appVersion := v2.Version
@@ -259,6 +255,7 @@ func SetupTestAppWithUpgradeHeight(t *testing.T, upgradeHeight int64) (*app.App,
 	encCfg := encoding.MakeConfig(app.ModuleEncodingRegisters...)
 	testApp := app.New(log.NewNopLogger(), db, nil, 0, encCfg, upgradeHeight, util.EmptyAppOptions{})
 	genesis := genesis.NewDefaultGenesis().
+		WithChainID("test").
 		WithValidators(genesis.NewDefaultValidator(testnode.DefaultValidatorAccountName)).
 		WithConsensusParams(app.DefaultInitialConsensusParams())
 	genDoc, err := genesis.Export()

docs/architecture/adr-015-namespace-id-size.md

@@ -50,7 +50,7 @@ Users will specify a version (1 byte) and a ID (28 bytes) in their PFB. Addition
 
 ### Criteria 1
 
-The namespace ID must provide at least 72 bits of randomness [^2] to satisfy criteria 1. Since an 8 byte namespace ID can only provide 64 bits of randomness, it fail to meet this criteria.
+The namespace ID must provide at least 72 bits of randomness to satisfy criteria 1. Since an 8 byte namespace ID can only provide 64 bits of randomness, it fail to meet this criteria.
 
 | Namespace ID size (bytes) | Criteria 1 |
 |---------------------------|------------|
@@ -229,7 +229,6 @@ Note: to verify the number of SHA256 compression invocations, we analyzed the nu
 - <https://github.com/celestiaorg/celestia-app/issues/1308>
 
 [^1]: This assumes a user uses sufficient entropy to generate the namespace ID and isn't front-run by an adversary prior to actually using the namespace.
-[^2]: <https://eager.io/blog/how-long-does-an-id-need-to-be/>
 [^3]: <https://kevingal.com/apps/collision.html>
 [^4]: <https://www.johndcook.com/blog/2017/01/10/probability-of-secure-hash-collisions/>
 <!-- markdown-link-check-disable -->

pkg/appconsts/overrides.go

@@ -6,5 +6,4 @@ package appconsts
 // Look at the Makefile to see how these are set.
 var (
 	OverrideSquareSizeUpperBoundStr string
-	OverrideUpgradeHeightDelayStr   string
 )

pkg/appconsts/versioned_consts.go

@@ -80,12 +80,8 @@ func GetTimeoutCommit(v uint64) time.Duration {
 
 // UpgradeHeightDelay returns the delay in blocks after a quorum has been reached that the chain should upgrade to the new version.
 func UpgradeHeightDelay(chainID string, v uint64) int64 {
-	if OverrideUpgradeHeightDelayStr != "" {
-		parsedValue, err := strconv.ParseInt(OverrideUpgradeHeightDelayStr, 10, 64)
-		if err != nil {
-			panic("Invalid OverrideUpgradeHeightDelayStr value")
-		}
-		return parsedValue
+	if chainID == "test" {
+		return 3
 	}
 	switch v {
 	case v1.Version:
@@ -100,6 +96,5 @@ func UpgradeHeightDelay(chainID string, v uint64) int64 {
 		return v2.UpgradeHeightDelay
 	default:
 		return v3.UpgradeHeightDelay
-
 	}
 }

pkg/appconsts/versioned_consts_test.go

@@ -118,6 +118,18 @@ func TestUpgradeHeightDelay(t *testing.T) {
 			version:                    3,
 			expectedUpgradeHeightDelay: v3.UpgradeHeightDelay,
 		},
+		{
+			name:                       "the upgrade delay for chainID 'test' should be 3 regardless of the version",
+			chainID:                    "test",
+			version:                    3,
+			expectedUpgradeHeightDelay: 3,
+		},
+		{
+			name:                       "the upgrade delay for chainID 'test' should be 3 regardless of the version",
+			chainID:                    "test",
+			version:                    4,
+			expectedUpgradeHeightDelay: 3,
+		},
 	}
 
 	for _, tc := range tests {