From 21b72f806912fd36913fb9cace13b2ceb9fbce16 Mon Sep 17 00:00:00 2001 From: Nilesh Gupta Date: Wed, 26 Aug 2026 08:35:53 +0530 Subject: [PATCH 1/2] fix: reject MsgVoteChainMeta for unregistered chains (F-2026-18803) Gate Keeper.VoteChainMeta on uregistry before any state read/write, and cap observed_chain_id length + CAIP-2 shape in ValidateBasic. --- x/uexecutor/keeper/chain_meta.go | 17 +++++++++++++++++ x/uexecutor/types/msg_vote_chain_meta.go | 23 +++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/x/uexecutor/keeper/chain_meta.go b/x/uexecutor/keeper/chain_meta.go index 179644685..c5777cf71 100644 --- a/x/uexecutor/keeper/chain_meta.go +++ b/x/uexecutor/keeper/chain_meta.go @@ -46,6 +46,8 @@ func (k Keeper) SetChainMeta(ctx context.Context, chainID string, chainMeta type // VoteChainMeta processes a universal validator's vote on chain metadata (gas price + chain height). // // Rules: +// 0. The observed chain must be registered in x/uregistry. Unregistered chains are +// rejected before any state is touched (F-2026-18803). // 1. Each vote is stamped with the current block time (storedAt) when it is recorded // and either inserted (new validator) or updated in place (existing validator). // 2. The oracle is bootstrapped on the first EVM write only after at least @@ -60,6 +62,21 @@ func (k Keeper) SetChainMeta(ctx context.Context, chainID string, chainMeta type // 5. Price median and chain-height median are computed independently (upper median = len/2). // 6. After a successful EVM call, LastAppliedChainHeight is updated. func (k Keeper) VoteChainMeta(ctx context.Context, universalValidator sdk.ValAddress, observedChainId string, price, blockNumber uint64) error { + // F-2026-18803: check the chain is registered before any state read/write. + // A GetChainMeta miss below *creates* the row on the cold-start path, so a + // vote for an arbitrary chain id would otherwise mint an unbounded number of + // ChainMetas keys (the raw id is the IAVL key) that every node then walks in + // AfterValidatorRemoved. Gate on *registered*, not IsChainInboundEnabled: + // chain meta also feeds gas-price quoting for outbounds, so an inbound-only + // check would starve outbound-enabled chains. + if _, err := k.uregistryKeeper.GetChainConfig(ctx, observedChainId); err != nil { + k.Logger().Warn("chain meta vote rejected: chain not registered", + "chain_id", observedChainId, + "validator", universalValidator.String(), + ) + return sdkerrors.Wrapf(err, "chain %s is not registered", observedChainId) + } + sdkCtx := sdk.UnwrapSDKContext(ctx) now := uint64(sdkCtx.BlockTime().Unix()) diff --git a/x/uexecutor/types/msg_vote_chain_meta.go b/x/uexecutor/types/msg_vote_chain_meta.go index f4052aa6a..fc3dec952 100644 --- a/x/uexecutor/types/msg_vote_chain_meta.go +++ b/x/uexecutor/types/msg_vote_chain_meta.go @@ -10,6 +10,16 @@ var ( _ sdk.Msg = &MsgVoteChainMeta{} ) +// MaxObservedChainIdLen caps the CAIP-2 chain id carried by a chain-meta vote. +// +// F-2026-18803: the id is used verbatim as the ChainMetas map key +// (collections.StringKey), so an uncapped id is an attacker-controlled IAVL key +// of arbitrary size. CAIP-2 itself allows at most 8 (namespace) + 1 + 32 +// (reference) = 41 characters, and the longest id we actually register is +// "solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1" (41). 128 leaves generous headroom +// for future namespaces while keeping the key bounded. +const MaxObservedChainIdLen = 128 + // NewMsgVoteChainMeta creates new instance of MsgVoteChainMeta func NewMsgVoteChainMeta( sender sdk.Address, @@ -49,6 +59,19 @@ func (msg *MsgVoteChainMeta) ValidateBasic() error { if msg.ObservedChainId == "" { return errors.Wrap(sdkerrors.ErrInvalidRequest, "observed_chain_id cannot be empty") } + // F-2026-18803 (stateless half): ValidateBasic has no keeper, so it cannot + // ask whether the chain is registered — Keeper.VoteChainMeta does that. What + // it can do for free at CheckTx time is bound the id's size and shape, so an + // absurd id is dropped at mempool admission rather than after a block + // commits it as a ChainMetas key. + if len(msg.ObservedChainId) > MaxObservedChainIdLen { + return errors.Wrapf(sdkerrors.ErrInvalidRequest, + "observed_chain_id exceeds %d characters (got %d)", MaxObservedChainIdLen, len(msg.ObservedChainId)) + } + if _, _, err := ParseCAIP2(msg.ObservedChainId); err != nil { + return errors.Wrap(sdkerrors.ErrInvalidRequest, + "observed_chain_id must be in CAIP-2 format :") + } if msg.Price == 0 { return errors.Wrap(sdkerrors.ErrInvalidRequest, "price must be greater than 0") } From 888af5c6ed67bc304c50b35273024918e0a624f7 Mon Sep 17 00:00:00 2001 From: Nilesh Gupta Date: Wed, 26 Aug 2026 08:35:53 +0530 Subject: [PATCH 2/2] test: cover the unregistered-chain chain-meta gate (F-2026-18803) Assert no ChainMetas row is written, at keeper and integration level. --- .../uexecutor/vote_chain_meta_test.go | 43 +++++ x/uexecutor/keeper/chain_meta_test.go | 176 ++++++++++++++++++ x/uexecutor/types/msg_vote_chain_meta_test.go | 101 ++++++++++ 3 files changed, 320 insertions(+) create mode 100644 x/uexecutor/keeper/chain_meta_test.go create mode 100644 x/uexecutor/types/msg_vote_chain_meta_test.go diff --git a/test/integration/uexecutor/vote_chain_meta_test.go b/test/integration/uexecutor/vote_chain_meta_test.go index 9bc81a9b7..5c72966bc 100644 --- a/test/integration/uexecutor/vote_chain_meta_test.go +++ b/test/integration/uexecutor/vote_chain_meta_test.go @@ -53,6 +53,18 @@ func setupVoteChainMetaTest(t *testing.T, numVals int) (*app.ChainApp, sdk.Conte return testApp, ctx, universalVals, validators } +// chainMetaKeys returns every key currently present in the ChainMetas map. +func chainMetaKeys(t *testing.T, ctx sdk.Context, testApp *app.ChainApp) []string { + t.Helper() + var keys []string + require.NoError(t, testApp.UexecutorKeeper.ChainMetas.Walk(ctx, nil, + func(chainID string, _ uexecutortypes.ChainMeta) (bool, error) { + keys = append(keys, chainID) + return false, nil + })) + return keys +} + func TestVoteChainMetaIntegration(t *testing.T) { t.Parallel() chainId := "eip155:11155111" @@ -84,6 +96,37 @@ func TestVoteChainMetaIntegration(t *testing.T) { require.Equal(t, uint64(0), stored.LastAppliedChainHeight, "two votes should still not bootstrap the oracle") }) + t.Run("vote for an unregistered chain is rejected and writes no ChainMetas row", func(t *testing.T) { + // F-2026-18803: only eip155:11155111 is registered by the fixture. A + // bonded universal validator voting on any other chain id used to mint a + // ChainMetas row keyed by that raw id (collections.StringKey). + const unregistered = "eip155:999999999" + + testApp, ctx, uvals, vals := setupVoteChainMetaTest(t, 1) + + coreVal, err := sdk.ValAddressFromBech32(vals[0].OperatorAddress) + require.NoError(t, err) + coreAcc := sdk.AccAddress(coreVal).String() + + before := chainMetaKeys(t, ctx, testApp) + require.Empty(t, before) + + voteErr := utils.ExecVoteChainMeta(t, ctx, testApp, uvals[0], coreAcc, unregistered, 100_000_000_000, 12345) + + // Store first, deliberately: the finding is the row being written. + _, found, err := testApp.UexecutorKeeper.GetChainMeta(ctx, unregistered) + require.NoError(t, err) + require.False(t, found, "unregistered chain must not create a ChainMetas row") + require.Equal(t, before, chainMetaKeys(t, ctx, testApp), "ChainMetas must be unchanged") + + require.Error(t, voteErr) + require.Contains(t, voteErr.Error(), "is not registered") + + // The registered chain still works from the same validator. + require.NoError(t, utils.ExecVoteChainMeta(t, ctx, testApp, uvals[0], coreAcc, chainId, 100_000_000_000, 12345)) + require.Equal(t, []string{chainId}, chainMetaKeys(t, ctx, testApp)) + }) + t.Run("third fresh vote bootstraps the oracle and sets LastAppliedChainHeight to median", func(t *testing.T) { testApp, ctx, uvals, vals := setupVoteChainMetaTest(t, 3) diff --git a/x/uexecutor/keeper/chain_meta_test.go b/x/uexecutor/keeper/chain_meta_test.go new file mode 100644 index 000000000..a51ac98a5 --- /dev/null +++ b/x/uexecutor/keeper/chain_meta_test.go @@ -0,0 +1,176 @@ +package keeper_test + +import ( + "testing" + "time" + + "cosmossdk.io/collections" + "github.com/golang/mock/gomock" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/stretchr/testify/require" + + "github.com/pushchain/push-chain-node/x/uexecutor/types" + uregistrytypes "github.com/pushchain/push-chain-node/x/uregistry/types" +) + +const ( + registeredChainID = "eip155:11155111" + unregisteredChainID = "eip155:999999999" +) + +// setupChainMetaFixture builds the keeper fixture with a deterministic block +// time so storedAt/staleness arithmetic is stable. +func setupChainMetaFixture(t *testing.T) *testFixture { + t.Helper() + f := SetupTest(t) + f.ctx = f.ctx.WithBlockTime(time.Unix(1_700_000_000, 0)) + return f +} + +// registerChain makes the uregistry mock answer GetChainConfig for chain. +func registerChain(f *testFixture, chain string) { + f.mockUregistryKeeper.EXPECT(). + GetChainConfig(gomock.Any(), chain). + Return(uregistrytypes.ChainConfig{ + Chain: chain, + VmType: uregistrytypes.VmType_EVM, + Enabled: &uregistrytypes.ChainEnabled{ + IsInboundEnabled: true, + IsOutboundEnabled: true, + }, + }, nil). + AnyTimes() +} + +// unregisterChain makes the uregistry mock report chain as absent, exactly as +// the real keeper does (collections.ErrNotFound out of ChainConfigs.Get). +func unregisterChain(f *testFixture, chain string) { + f.mockUregistryKeeper.EXPECT(). + GetChainConfig(gomock.Any(), chain). + Return(uregistrytypes.ChainConfig{}, collections.ErrNotFound). + AnyTimes() +} + +// countChainMetas returns every key currently present in the ChainMetas map. +func countChainMetas(t *testing.T, f *testFixture) []string { + t.Helper() + var keys []string + require.NoError(t, f.k.ChainMetas.Walk(f.ctx, nil, func(chainID string, _ types.ChainMeta) (bool, error) { + keys = append(keys, chainID) + return false, nil + })) + return keys +} + +// F-2026-18803: a vote for a chain that is not in x/uregistry must be rejected +// *before* the keeper writes anything. The finding is not "an error is missing" +// — it is that the GetChainMeta miss creates the row on the cold-start path, so +// the store assertion is the one that matters. +func TestVoteChainMeta_UnregisteredChain_RejectedAndStoreUnchanged(t *testing.T) { + f := setupChainMetaFixture(t) + require := require.New(t) + + unregisterChain(f, unregisteredChainID) + + before := countChainMetas(t, f) + require.Empty(before) + + err := f.k.VoteChainMeta(f.ctx, sdk.ValAddress(f.addrs[0]), unregisteredChainID, 100_000_000_000, 12345) + + // Store first, deliberately: the finding is the *row being written*, not a + // missing error. Removing the registry gate must break this assertion. + has, hasErr := f.k.ChainMetas.Has(f.ctx, unregisteredChainID) + require.NoError(hasErr) + require.False(has, "unregistered chain must not create a ChainMetas row") + require.Equal(before, countChainMetas(t, f), "ChainMetas must be unchanged") + + require.Error(err) + require.Contains(err.Error(), "is not registered") +} + +// An attacker-shaped id (long, arbitrary) must not become an IAVL key either. +func TestVoteChainMeta_UnregisteredLongChainId_WritesNoKey(t *testing.T) { + f := setupChainMetaFixture(t) + require := require.New(t) + + longID := "eip155:" + for i := 0; i < 200; i++ { + longID += "9" + } + unregisterChain(f, longID) + + err := f.k.VoteChainMeta(f.ctx, sdk.ValAddress(f.addrs[0]), longID, 1, 1) + + require.Empty(countChainMetas(t, f), "no ChainMetas key may be minted for an unregistered id") + require.Error(err) +} + +// A registered chain keeps working: the first vote is recorded and creates the +// row (this is required — bootstrap quorum can never be reached otherwise). +func TestVoteChainMeta_RegisteredChain_CreatesRow(t *testing.T) { + f := setupChainMetaFixture(t) + require := require.New(t) + + registerChain(f, registeredChainID) + + valAddr := sdk.ValAddress(f.addrs[0]) + require.NoError(f.k.VoteChainMeta(f.ctx, valAddr, registeredChainID, 100_000_000_000, 12345)) + + stored, found, err := f.k.GetChainMeta(f.ctx, registeredChainID) + require.NoError(err) + require.True(found) + require.Equal(registeredChainID, stored.ObservedChainId) + require.Equal([]string{valAddr.String()}, stored.Signers) + require.Equal([]uint64{100_000_000_000}, stored.Prices) + require.Equal([]uint64{12345}, stored.ChainHeights) + require.Equal([]uint64{uint64(f.ctx.BlockTime().Unix())}, stored.StoredAts) + // Below the bootstrap quorum the oracle is not written. + require.Equal(uint64(0), stored.LastAppliedChainHeight) + + require.Equal([]string{registeredChainID}, countChainMetas(t, f)) +} + +// Existing pre-bootstrap accumulation behaviour is unchanged for a registered +// chain: votes below chainMetaMinVotesForFirstWrite are stored, not applied. +func TestVoteChainMeta_RegisteredChain_BootstrapAccumulationUnchanged(t *testing.T) { + f := setupChainMetaFixture(t) + require := require.New(t) + + registerChain(f, registeredChainID) + + val0 := sdk.ValAddress(f.addrs[0]) + val1 := sdk.ValAddress(f.addrs[1]) + + require.NoError(f.k.VoteChainMeta(f.ctx, val0, registeredChainID, 100_000_000_000, 12345)) + require.NoError(f.k.VoteChainMeta(f.ctx, val1, registeredChainID, 200_000_000_000, 12346)) + + stored, found, err := f.k.GetChainMeta(f.ctx, registeredChainID) + require.NoError(err) + require.True(found) + require.Len(stored.Signers, 2) + require.Equal([]uint64{100_000_000_000, 200_000_000_000}, stored.Prices) + require.Equal(uint64(0), stored.LastAppliedChainHeight, "two votes must not bootstrap the oracle") + + // A re-vote from the same validator still updates in place, not appends. + require.NoError(f.k.VoteChainMeta(f.ctx, val0, registeredChainID, 400_000_000_000, 12350)) + stored, _, err = f.k.GetChainMeta(f.ctx, registeredChainID) + require.NoError(err) + require.Len(stored.Signers, 2) + require.Equal(uint64(400_000_000_000), stored.Prices[0]) + require.Equal(uint64(12350), stored.ChainHeights[0]) +} + +// Registering one chain must not implicitly admit its neighbours. +func TestVoteChainMeta_OnlyRegisteredChainAdmitted(t *testing.T) { + f := setupChainMetaFixture(t) + require := require.New(t) + + registerChain(f, registeredChainID) + unregisterChain(f, unregisteredChainID) + + valAddr := sdk.ValAddress(f.addrs[0]) + require.NoError(f.k.VoteChainMeta(f.ctx, valAddr, registeredChainID, 1, 1)) + require.Error(f.k.VoteChainMeta(f.ctx, valAddr, unregisteredChainID, 1, 1)) + + require.Equal([]string{registeredChainID}, countChainMetas(t, f)) +} diff --git a/x/uexecutor/types/msg_vote_chain_meta_test.go b/x/uexecutor/types/msg_vote_chain_meta_test.go new file mode 100644 index 000000000..8c234c346 --- /dev/null +++ b/x/uexecutor/types/msg_vote_chain_meta_test.go @@ -0,0 +1,101 @@ +package types_test + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/pushchain/push-chain-node/x/uexecutor/types" +) + +// F-2026-18803 (stateless half): observed_chain_id becomes the ChainMetas map +// key verbatim, so ValidateBasic bounds its size and shape at CheckTx time. +func TestMsgVoteChainMeta_ValidateBasic(t *testing.T) { + const validSigner = "push1fgaewhyd9fkwtqaj9c233letwcuey6dgly9gv9" + + newMsg := func(chainID string) *types.MsgVoteChainMeta { + return &types.MsgVoteChainMeta{ + Signer: validSigner, + ObservedChainId: chainID, + Price: 100_000_000_000, + ChainHeight: 12345, + } + } + + tests := []struct { + name string + msg *types.MsgVoteChainMeta + expectErr string + }{ + { + name: "valid evm chain id", + msg: newMsg("eip155:11155111"), + }, + { + name: "valid solana chain id", + msg: newMsg("solana:EtWTRABZaYq6iMfeYKouRu166VU2xqa1"), + }, + { + name: "chain id exactly at the cap is accepted", + msg: newMsg("eip155:" + strings.Repeat("9", types.MaxObservedChainIdLen-len("eip155:"))), + }, + { + name: "chain id one byte over the cap is rejected", + msg: newMsg("eip155:" + strings.Repeat("9", types.MaxObservedChainIdLen-len("eip155:")+1)), + expectErr: "exceeds 128 characters", + }, + { + name: "oversized chain id is rejected", + msg: newMsg("eip155:" + strings.Repeat("9", 100_000)), + expectErr: "exceeds 128 characters", + }, + { + name: "non-CAIP-2 chain id is rejected", + msg: newMsg("ethereum"), + expectErr: "CAIP-2 format", + }, + { + name: "empty namespace is rejected", + msg: newMsg(":11155111"), + expectErr: "CAIP-2 format", + }, + { + name: "empty reference is rejected", + msg: newMsg("eip155:"), + expectErr: "CAIP-2 format", + }, + { + name: "empty chain id is rejected", + msg: newMsg(""), + expectErr: "observed_chain_id cannot be empty", + }, + { + name: "invalid signer is rejected", + msg: &types.MsgVoteChainMeta{Signer: "not-bech32", ObservedChainId: "eip155:1", Price: 1, ChainHeight: 1}, + expectErr: "invalid signer address", + }, + { + name: "zero price is rejected", + msg: &types.MsgVoteChainMeta{Signer: validSigner, ObservedChainId: "eip155:1", Price: 0, ChainHeight: 1}, + expectErr: "price must be greater than 0", + }, + { + name: "zero chain height is rejected", + msg: &types.MsgVoteChainMeta{Signer: validSigner, ObservedChainId: "eip155:1", Price: 1, ChainHeight: 0}, + expectErr: "chain_height must be greater than 0", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := tc.msg.ValidateBasic() + if tc.expectErr == "" { + require.NoError(t, err) + return + } + require.Error(t, err) + require.Contains(t, err.Error(), tc.expectErr) + }) + } +}