Merge pull request #311 from pushchain/feat/read-state-upgrade-handler

feat: add read-state upgrade handler

Author: 0xNilesh

app/coininfo.go

@@ -0,0 +1,24 @@
+//go:build !test
+// +build !test
+
+package app
+
+import (
+	evmtypes "github.com/cosmos/evm/x/vm/types"
+)
+
+// seedDefaultEvmCoinInfo populates the EVM coin-info fallback at app construction.
+//
+// The authoritative value is set by x/vm's PreBlock, but x/upgrade's PreBlocker
+// runs before it. An upgrade handler that touches EVM state — reading an account,
+// or writing one, which sets a balance — resolves the coin denom through a global
+// that is still nil at that point, and the node dies on a nil dereference partway
+// through the upgrade. That is a chain halt, not a failed tx.
+//
+// Upstream provides this fallback for exactly that window (and for RPC served
+// before the first PreBlock); nothing in this app was populating it. It is a
+// different variable from the one x/vm sets, so there is no double-set: the getter
+// prefers the PreBlock value and falls back to this only while that is nil.
+func seedDefaultEvmCoinInfo(coinInfo evmtypes.EvmCoinInfo) {
+	evmtypes.SetDefaultEvmCoinInfo(coinInfo)
+}

app/coininfo_test_build.go

@@ -0,0 +1,15 @@
+//go:build test
+// +build test
+
+package app
+
+import (
+	evmtypes "github.com/cosmos/evm/x/vm/types"
+)
+
+// seedDefaultEvmCoinInfo is a no-op under the test build tag.
+//
+// There, SetDefaultEvmCoinInfo writes the single testing coin-info variable rather
+// than a separate fallback, so seeding it here would make x/vm's InitGenesis panic
+// with "EVM coin info already set". Test apps configure coin info themselves.
+func seedDefaultEvmCoinInfo(_ evmtypes.EvmCoinInfo) {}

app/config.go

@@ -59,6 +59,8 @@ func EVMAppOptions(chainID string) error {
 		return err
 	}
 
+	seedDefaultEvmCoinInfo(coinInfo)
+
 	sealed = true
 	return nil
 }

app/upgrades.go

@@ -16,12 +16,12 @@ import (
 	ethhashfix "github.com/pushchain/push-chain-node/app/upgrades/eth-hash-fix"
 	evmblockscoutfix "github.com/pushchain/push-chain-node/app/upgrades/evm-blockscout-fix"
 	evmchainidffix "github.com/pushchain/push-chain-node/app/upgrades/evm-chainid-fix"
+	evmderivedgasprice "github.com/pushchain/push-chain-node/app/upgrades/evm-derived-gas-price"
 	evmparamsmigration "github.com/pushchain/push-chain-node/app/upgrades/evm-params-migration"
 	evmpreinstalls "github.com/pushchain/push-chain-node/app/upgrades/evm-preinstalls"
 	evmrpcfix "github.com/pushchain/push-chain-node/app/upgrades/evm-rpc-fix"
 	evmv040 "github.com/pushchain/push-chain-node/app/upgrades/evm-v0-4-0"
 	evmv050 "github.com/pushchain/push-chain-node/app/upgrades/evm-v0-5-0"
-	evmderivedgasprice "github.com/pushchain/push-chain-node/app/upgrades/evm-derived-gas-price"
 	evmv060 "github.com/pushchain/push-chain-node/app/upgrades/evm-v0-6-0"
 	feeabs "github.com/pushchain/push-chain-node/app/upgrades/fee-abs"
 	gasoracle "github.com/pushchain/push-chain-node/app/upgrades/gas-oracle"
@@ -31,6 +31,7 @@ import (
 	pc20 "github.com/pushchain/push-chain-node/app/upgrades/pc20"
 	proxybytecodefix "github.com/pushchain/push-chain-node/app/upgrades/proxy-bytecode-fix"
 	purgeexpiredoutbounds "github.com/pushchain/push-chain-node/app/upgrades/purge-expired-outbounds"
+	readstate "github.com/pushchain/push-chain-node/app/upgrades/read-state"
 	removefeeabsv1 "github.com/pushchain/push-chain-node/app/upgrades/remove-fee-abs-v1"
 	removeutxverifier "github.com/pushchain/push-chain-node/app/upgrades/remove-utxverifier"
 	sdkv053 "github.com/pushchain/push-chain-node/app/upgrades/sdk-v0-53"
@@ -94,6 +95,9 @@ var Upgrades = []upgrades.Upgrade{
 	// evm-derived-gas-price — cosmos/evm bump for the derived-tx gas price fix;
 	// no-op (JSON-RPC reporting only, no module ConsensusVersion changes)
 	evmderivedgasprice.NewUpgrade(),
+	// read-state — adds the x/ucallback store and reserves every system-contract
+	// address still unclaimed in the A/B/C ranges (41 of 47 on donut, incl. 0xC2)
+	readstate.NewUpgrade(),
 }
 
 // RegisterUpgradeHandlers registers the chain upgrade handlers

app/upgrades/read-state/upgrade.go

@@ -0,0 +1,68 @@
+package readstate
+
+import (
+	"context"
+
+	storetypes "cosmossdk.io/store/types"
+	upgradetypes "cosmossdk.io/x/upgrade/types"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/types/module"
+
+	"github.com/pushchain/push-chain-node/app/upgrades"
+	ucallbacktypes "github.com/pushchain/push-chain-node/x/ucallback/types"
+)
+
+const UpgradeName = "read-state"
+
+// NewUpgrade constructs the upgrade definition.
+//
+// x/ucallback is a new module, so its store has to be added here — RunMigrations
+// alone will register the module's consensus version but cannot create a store that
+// the multistore was never told about, and the node fails to load at the upgrade
+// height without it.
+func NewUpgrade() upgrades.Upgrade {
+	return upgrades.Upgrade{
+		UpgradeName:          UpgradeName,
+		CreateUpgradeHandler: CreateUpgradeHandler,
+		StoreUpgrades: storetypes.StoreUpgrades{
+			Added:   []string{ucallbacktypes.StoreKey},
+			Deleted: []string{},
+		},
+	}
+}
+
+func CreateUpgradeHandler(
+	mm upgrades.ModuleManager,
+	configurator module.Configurator,
+	ak *upgrades.AppKeepers,
+) upgradetypes.UpgradeHandler {
+	return func(ctx context.Context, plan upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) {
+		sdkCtx := sdk.UnwrapSDKContext(ctx)
+		sdkCtx.Logger().Info("🔧 Running upgrade:", "name", UpgradeName)
+
+		// Reserve every system-contract address that is still empty.
+		//
+		// SYSTEM_CONTRACTS is not just the named contracts: constants.go's init()
+		// fills the 0xA0-0xAF, 0xB0-0xBF and 0xC0-0xCF ranges with a full
+		// proxy+admin+impl triple per slot, 47 entries in total. The genesis loop
+		// runs only at InitGenesis, so every slot added after a chain launched is
+		// unreserved on it — and an unreserved slot can be squatted by an ordinary
+		// account, which is the squatting defence those ranges exist for.
+		//
+		// On donut that is 41 of the 47, all three addresses bare, including
+		// UNIVERSAL_CALLBACK (0x…C2). x/ucallback only accepts ReadRequested logs
+		// from that exact address, so until it holds the proxy the module is inert.
+		// The remaining 6 are already deployed and the already-deployed guard skips
+		// them, leaving their code untouched.
+		if err := ak.URegistryKeeper.DeployMissingSystemContracts(sdkCtx); err != nil {
+			return nil, err
+		}
+		sdkCtx.Logger().Info("Reserved any missing system contract addresses")
+
+		// RunMigrations registers x/ucallback at its current consensus version and
+		// runs its InitGenesis, which seeds Params. There is no state to migrate:
+		// the module is new, so it starts empty.
+		return mm.RunMigrations(ctx, configurator, fromVM)
+	}
+}

test/integration/upgrades/read_state_test.go

@@ -0,0 +1,81 @@
+package integrationtest
+
+import (
+	"testing"
+
+	evmtypes "github.com/cosmos/evm/x/vm/types"
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/stretchr/testify/require"
+
+	utils "github.com/pushchain/push-chain-node/test/utils"
+	uregistrytypes "github.com/pushchain/push-chain-node/x/uregistry/types"
+)
+
+// The read-state upgrade must reserve every system-contract slot that is still
+// empty, and leave the ones already deployed exactly as they are.
+//
+// UNIVERSAL_CALLBACK (0x…C2) was added to SYSTEM_CONTRACTS after donut launched, so
+// on that chain its proxy, admin and implementation are all bare — verified against
+// the live testnet. x/ucallback only accepts ReadRequested logs from that exact
+// address, so if the upgrade does not reserve it the module is inert.
+func TestReadStateUpgrade_ReservesMissingSystemContracts(t *testing.T) {
+	chainApp, ctx, _ := utils.SetAppWithValidators(t)
+	k := chainApp.UregistryKeeper
+
+	code := func(addr string) []byte {
+		a := common.HexToAddress(addr)
+		acct := chainApp.EVMKeeper.GetAccountOrEmpty(ctx, a)
+		return chainApp.EVMKeeper.GetCode(ctx, common.BytesToHash(acct.CodeHash))
+	}
+
+	// Simulate a chain that launched before the slot existed: clear the triple.
+	cb := uregistrytypes.SYSTEM_CONTRACTS["UNIVERSAL_CALLBACK"]
+	for _, a := range []string{cb.Address, cb.ProxyAdmin, cb.Implementation} {
+		// Point the account back at the empty-code sentinel; that is exactly the
+		// state the reservation guard treats as "not deployed".
+		chainApp.EVMKeeper.SetCodeHash(ctx,
+			common.HexToAddress(a).Bytes(), evmtypes.EmptyCodeHash)
+	}
+	require.Empty(t, code(cb.Address), "precondition: the slot must start bare")
+
+	// A slot that IS deployed must be left untouched.
+	core := uregistrytypes.SYSTEM_CONTRACTS["UNIVERSAL_CORE"]
+	coreBefore := code(core.Address)
+	require.NotEmpty(t, coreBefore, "precondition: UNIVERSAL_CORE is deployed at genesis")
+
+	require.NoError(t, k.DeployMissingSystemContracts(ctx))
+
+	require.NotEmpty(t, code(cb.Address), "the callback proxy must be reserved")
+	require.NotEmpty(t, code(cb.ProxyAdmin), "its ProxyAdmin must be reserved")
+	require.NotEmpty(t, code(cb.Implementation), "its implementation must be reserved")
+	require.Equal(t, coreBefore, code(core.Address),
+		"an already-deployed contract must not be redeployed or overwritten")
+}
+
+// Running the reservation twice must change nothing. The upgrade runs once, but the
+// same guard protects chains where the slot is already present, and a second pass is
+// the cheapest way to prove the guard actually holds.
+func TestReadStateUpgrade_ReservationIsIdempotent(t *testing.T) {
+	chainApp, ctx, _ := utils.SetAppWithValidators(t)
+	k := chainApp.UregistryKeeper
+
+	code := func(addr string) []byte {
+		a := common.HexToAddress(addr)
+		acct := chainApp.EVMKeeper.GetAccountOrEmpty(ctx, a)
+		return chainApp.EVMKeeper.GetCode(ctx, common.BytesToHash(acct.CodeHash))
+	}
+
+	require.NoError(t, k.DeployMissingSystemContracts(ctx))
+
+	before := map[string][]byte{}
+	for name, c := range uregistrytypes.SYSTEM_CONTRACTS {
+		before[name] = code(c.Address)
+		require.NotEmpty(t, before[name], "%s must be reserved after the first pass", name)
+	}
+
+	require.NoError(t, k.DeployMissingSystemContracts(ctx))
+
+	for name, c := range uregistrytypes.SYSTEM_CONTRACTS {
+		require.Equal(t, before[name], code(c.Address), "%s changed on the second pass", name)
+	}
+}

x/uregistry/keeper/genesis.go

@@ -12,6 +12,7 @@ import (
 	"github.com/ethereum/go-ethereum/crypto"
 
 	"github.com/cosmos/evm/x/vm/statedb"
+	evmtypes "github.com/cosmos/evm/x/vm/types"
 	"github.com/pushchain/push-chain-node/x/uregistry/types"
 )
 
@@ -53,7 +54,7 @@ func deployImplementationContract(ctx context.Context, evmKeeper types.EVMKeeper
 
 	// Create the EVM account object
 	evmAccount := statedb.Account{
-		Nonce:    1,             // prevent tx nonce=0 conflicts
+		Nonce:    1,                // prevent tx nonce=0 conflicts
 		Balance:  new(uint256.Int), // zero balance by default
 		CodeHash: codeHash,
 	}
@@ -77,9 +78,9 @@ func deployProxyAdminContract(ctx context.Context, evmKeeper types.EVMKeeper, pr
 
 	// Create the EVM account object
 	evmAccount := statedb.Account{
-		Nonce:    1,             // to prevent tx nonce=0 conflicts
+		Nonce:    1,                // to prevent tx nonce=0 conflicts
 		Balance:  new(uint256.Int), // zero balance by default
-		CodeHash: codeHash,      // link to deployed code
+		CodeHash: codeHash,         // link to deployed code
 	}
 
 	// Set the EVM account with the proxy admin contract
@@ -138,15 +139,18 @@ func deploySystemContracts(ctx context.Context, evmKeeper types.EVMKeeper, syste
 // EOAs in cosmos/evm carry the keccak256-of-empty-bytes sentinel, so a
 // length-only check would treat any touched EOA as a deployed contract and
 // silently skip the deploy sequence for that slot (F-2026-17025). Compare
-// against the empty-code-hash sentinel via Account.HasCodeHash instead.
+// against the empty-code-hash sentinel instead.
+//
+// GetCodeHash, not GetAccount: GetAccount reads the balance, which resolves the
+// EVM coin denom from a global that x/vm only populates in its own PreBlock.
+// x/upgrade's PreBlocker runs first, so an upgrade handler calling this through
+// GetAccount dereferences a nil coin config and halts the chain. Reading the
+// code-hash store directly needs none of that, and a code check has no business
+// loading balances anyway.
 func isContractDeployed(
 	ctx sdk.Context,
 	evmKeeper types.EVMKeeper,
 	addr common.Address,
 ) bool {
-	acc := evmKeeper.GetAccount(ctx, addr)
-	if acc == nil || len(acc.CodeHash) == 0 {
-		return false
-	}
-	return acc.HasCodeHash()
+	return evmKeeper.GetCodeHash(ctx, addr) != common.BytesToHash(evmtypes.EmptyCodeHash)
 }

x/uregistry/keeper/genesis_internal_test.go

@@ -27,6 +27,16 @@ func (s stubEVMKeeper) GetAccount(_ sdk.Context, addr common.Address) *statedb.A
 	return s.accounts[addr]
 }
 
+// GetCodeHash mirrors the real keeper: absent or code-less accounts report the
+// empty-code-hash sentinel rather than a zero hash.
+func (s stubEVMKeeper) GetCodeHash(_ sdk.Context, addr common.Address) common.Hash {
+	acc := s.accounts[addr]
+	if acc == nil || len(acc.CodeHash) == 0 {
+		return common.BytesToHash(evmtypes.EmptyCodeHash)
+	}
+	return common.BytesToHash(acc.CodeHash)
+}
+
 func (stubEVMKeeper) SetAccount(_ sdk.Context, _ common.Address, _ statedb.Account) error {
 	panic("not used in test")
 }
@@ -121,6 +131,14 @@ func (t *trackerEVMKeeper) GetAccount(_ sdk.Context, addr common.Address) *state
 	return nil
 }
 
+func (t *trackerEVMKeeper) GetCodeHash(_ sdk.Context, addr common.Address) common.Hash {
+	acc, ok := t.accounts[addr]
+	if !ok || len(acc.CodeHash) == 0 {
+		return common.BytesToHash(evmtypes.EmptyCodeHash)
+	}
+	return common.BytesToHash(acc.CodeHash)
+}
+
 func (t *trackerEVMKeeper) SetAccount(_ sdk.Context, addr common.Address, account statedb.Account) error {
 	t.accounts[addr] = account
 	return nil

x/uregistry/keeper/keeper.go

@@ -360,3 +360,17 @@ func (k Keeper) FixReservedBytecode(ctx context.Context) error {
 
 	return nil
 }
+
+// DeployMissingSystemContracts reserves every address in types.SYSTEM_CONTRACTS
+// that does not already hold code.
+//
+// The genesis loop only runs at InitGenesis, so a slot added to SYSTEM_CONTRACTS
+// after a chain launched is never reserved on that chain — an ordinary account can
+// take the address, and the module that expects a system contract there finds an
+// EOA. This is the upgrade-time counterpart: same map, same bytecode, same
+// deterministic order, and the same already-deployed guard, so it deploys only what
+// is genuinely missing and is safe to run on a chain where everything is present.
+func (k Keeper) DeployMissingSystemContracts(ctx context.Context) error {
+	deploySystemContracts(ctx, k.evmKeeper, types.SYSTEM_CONTRACTS)
+	return nil
+}

x/uregistry/types/expected_keepers.go

@@ -9,6 +9,11 @@ import (
 // EVMKeeper defines the expected interface for the EVM module.
 type EVMKeeper interface {
 	GetAccount(_ sdk.Context, addr common.Address) *statedb.Account
+
+	// GetCodeHash reads the code-hash store directly. Unlike GetAccount it does
+	// not touch balances, so it works before x/vm's PreBlock has configured the
+	// EVM coin info — which is exactly the situation an upgrade handler runs in.
+	GetCodeHash(ctx sdk.Context, addr common.Address) common.Hash
 	SetAccount(ctx sdk.Context, addr common.Address, account statedb.Account) error
 	SetState(ctx sdk.Context, addr common.Address, key common.Hash, value []byte)
 	GetCode(_ sdk.Context, codeHash common.Hash) []byte