From 96b80be8d46e82093418b673ef60bf2906350474 Mon Sep 17 00:00:00 2001 From: Rootul P Date: Sat, 27 Apr 2024 18:58:28 -0400 Subject: [PATCH] feat!: disable x/blobstream in v2 (#3310) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes https://github.com/celestiaorg/celestia-app/issues/3305 ## Testing
Manually verified on a local devnet that a user can register an EVM address if the chain is on app version 1. ```shell $ celestia-appd tx qgb register \ "$(celestia-appd keys show validator --home ${CELESTIA_APP_HOME} --bech val -a)" \ 0x966e6f22781EF6a6A82BBB4DB3df8E225DfD9485 \ --from ${KEY_NAME} \ --home ${CELESTIA_APP_HOME} \ --fees 30000utia \ --broadcast-mode block \ --yes code: 0 codespace: "" data: 12300A2E2F63656C65737469612E7167622E76312E4D7367526567697374657245564D41646472657373526573706F6E7365 events: - attributes: - index: true key: c3BlbmRlcg== value: Y2VsZXN0aWExc3B6YWU4MzNnM2E4dDQwdXN3NTAyZXR4dHo2c2czaG5meDg4OHc= - index: true key: YW1vdW50 value: MzAwMDB1dGlh type: coin_spent - attributes: - index: true key: cmVjZWl2ZXI= value: Y2VsZXN0aWExN3hwZnZha20yYW1nOTYyeWxzNmY4NHoza2VsbDhjNWxwbmpzM3M= - index: true key: YW1vdW50 value: MzAwMDB1dGlh type: coin_received - attributes: - index: true key: cmVjaXBpZW50 value: Y2VsZXN0aWExN3hwZnZha20yYW1nOTYyeWxzNmY4NHoza2VsbDhjNWxwbmpzM3M= - index: true key: c2VuZGVy value: Y2VsZXN0aWExc3B6YWU4MzNnM2E4dDQwdXN3NTAyZXR4dHo2c2czaG5meDg4OHc= - index: true key: YW1vdW50 value: MzAwMDB1dGlh type: transfer - attributes: - index: true key: c2VuZGVy value: Y2VsZXN0aWExc3B6YWU4MzNnM2E4dDQwdXN3NTAyZXR4dHo2c2czaG5meDg4OHc= type: message - attributes: - index: true key: ZmVl value: MzAwMDB1dGlh - index: true key: ZmVlX3BheWVy value: Y2VsZXN0aWExc3B6YWU4MzNnM2E4dDQwdXN3NTAyZXR4dHo2c2czaG5meDg4OHc= type: tx - attributes: - index: true key: YWNjX3NlcQ== value: Y2VsZXN0aWExc3B6YWU4MzNnM2E4dDQwdXN3NTAyZXR4dHo2c2czaG5meDg4OHcvMQ== type: tx - attributes: - index: true key: c2lnbmF0dXJl value: U1lIMjZsRHN4V2JEMzRQdVBMaFl0eXB5NTh6TzhJVHFxc09qYm5Kb0RyVUJSQlFCTEpIdHNuRXY0YXVoaUpCK0ppUExZVVJMc1lESDZ5ZUdKQ3EwaEE9PQ== type: tx - attributes: - index: true key: YWN0aW9u value: L2NlbGVzdGlhLnFnYi52MS5Nc2dSZWdpc3RlckVWTUFkZHJlc3M= type: message gas_used: "66959" gas_wanted: "210000" height: "2" info: "" logs: - events: - attributes: - key: action value: /celestia.qgb.v1.MsgRegisterEVMAddress type: message log: "" msg_index: 0 raw_log: '[{"msg_index":0,"events":[{"type":"message","attributes":[{"key":"action","value":"/celestia.qgb.v1.MsgRegisterEVMAddress"}]}]}]' timestamp: "" tx: null txhash: 60FD4A8507E95637FE97178FBB3F4E202F679D81A59E509C5E72A6815C221085 ```
Manually verify on a local devnet that a user can't register an EVM address if the chain is on app version 2. ```shell $ celestia-appd tx qgb register \ "$(celestia-appd keys show validator --home ${CELESTIA_APP_HOME} --bech val -a)" \ 0x966e6f22781EF6a6A82BBB4DB3df8E225DfD9485 \ --from ${KEY_NAME} \ --home ${CELESTIA_APP_HOME} \ --fees 30000utia \ --broadcast-mode block \ --yes code: 37 codespace: sdk data: "" events: [] gas_used: "8786" gas_wanted: "-1" height: "0" info: "" logs: [] raw_log: 'message type /celestia.qgb.v1.MsgRegisterEVMAddress is not supported in version 2: feature not supported' timestamp: "" tx: null txhash: E378761B61CA6D29C36957DBCAE65E4CED82788735BC3CF21A86B677970BCBD9 ```
Manually verified that tx and query CLI commands don't work. ```shell $ celestia-appd tx qgb Error: unknown command "qgb" for "tx" $ celestia-appd query qgb Error: unknown command "qgb" for "query" ```
Manually verified that gRPC queries don't work ![Screenshot 2024-04-24 at 2 04 23 PM](https://github.com/celestiaorg/celestia-app/assets/3699047/8ff771a7-1f9e-4ffb-9fa1-7f688b816a09) --------- Co-authored-by: CHAMI Rachid --- app/modules.go | 6 +-- app/test/qgb_rpc_test.go | 4 +- specs/src/specs/state_machine_modules.md | 1 - x/blobstream/README.md | 6 +++ x/blobstream/client/suite_test.go | 3 +- x/blobstream/integration_test.go | 4 +- x/blobstream/keeper/hooks.go | 19 +++++--- x/blobstream/keeper/hooks_test.go | 58 ++++++++++++++++++++++++ x/blobstream/module.go | 12 ++--- 9 files changed, 93 insertions(+), 20 deletions(-) create mode 100644 x/blobstream/keeper/hooks_test.go diff --git a/app/modules.go b/app/modules.go index 85dd05049f..8502ba9984 100644 --- a/app/modules.go +++ b/app/modules.go @@ -167,7 +167,7 @@ func (app *App) setupModuleManager(skipGenesisInvariants bool) error { }, { Module: blobstream.NewAppModule(app.appCodec, app.BlobstreamKeeper), - FromVersion: v1, ToVersion: v2, + FromVersion: v1, ToVersion: v1, }, { Module: signal.NewAppModule(app.SignalKeeper), @@ -299,8 +299,7 @@ func allStoreKeys() []string { } } -// versionedStoreKeys returns the store keys for each app version -// ... I wish there was an easier way than this (like using the modules which are already versioned) +// versionedStoreKeys returns the store keys for each app version. func versionedStoreKeys() map[uint64][]string { return map[uint64][]string{ 1: { @@ -325,7 +324,6 @@ func versionedStoreKeys() map[uint64][]string { authtypes.StoreKey, authzkeeper.StoreKey, banktypes.StoreKey, - blobstreamtypes.StoreKey, capabilitytypes.StoreKey, distrtypes.StoreKey, evidencetypes.StoreKey, diff --git a/app/test/qgb_rpc_test.go b/app/test/qgb_rpc_test.go index 7fd42be5dd..33bd9756ad 100644 --- a/app/test/qgb_rpc_test.go +++ b/app/test/qgb_rpc_test.go @@ -19,7 +19,9 @@ func TestBlobstreamRPCQueries(t *testing.T) { t.Skip("skipping blobstream integration test in short mode.") } ecfg := encoding.MakeConfig(app.ModuleEncodingRegisters...) - cfg := testnode.DefaultConfig().WithModifiers(genesis.SetDataCommitmentWindow(ecfg.Codec, 100)) + cfg := testnode.DefaultConfig(). + WithModifiers(genesis.SetDataCommitmentWindow(ecfg.Codec, 100)). + WithConsensusParams(app.DefaultInitialConsensusParams()) cctx, _, _ := testnode.NewNetwork(t, cfg) diff --git a/specs/src/specs/state_machine_modules.md b/specs/src/specs/state_machine_modules.md index ae6551e131..1ac2d47b56 100644 --- a/specs/src/specs/state_machine_modules.md +++ b/specs/src/specs/state_machine_modules.md @@ -5,7 +5,6 @@ Celestia app is built using the cosmos-sdk, and follows standard cosmos-sdk modu ## `celestia-app` Specific Modules - [blob](https://github.com/celestiaorg/celestia-app/blob/main/x/blob/README.md) -- [blobstream](https://github.com/celestiaorg/celestia-app/blob/main/x/blobstream/README.md) - [minfee](https://github.com/celestiaorg/celestia-app/blob/main/x/minfee/README.md) - [mint](https://github.com/celestiaorg/celestia-app/blob/main/x/mint/README.md) - [paramfilter](https://github.com/celestiaorg/celestia-app/blob/main/x/paramfilter/README.md) diff --git a/x/blobstream/README.md b/x/blobstream/README.md index 5042cc214e..0562745406 100644 --- a/x/blobstream/README.md +++ b/x/blobstream/README.md @@ -1,5 +1,8 @@ # `x/blobstream` +> [!NOTE] +> The `x/blobstream` module was enabled for app version 1 and disabled in app version >= 2. + ## Concepts This module contains the [Blobstream](https://blog.celestia.org/celestiums/) state machine implementation. @@ -226,6 +229,9 @@ If all the attestations in store are expired, which is an edge case that should ### Hooks +> [!NOTE] +> Hooks are no-ops for app versions >= 2. + #### Validator unbonding hook When a validator starts unbonding, a [hook](https://github.com/celestiaorg/celestia-app/blob/0629c757ef35a24187a8d7a4c706c7cdc894c8b6/x/qgb/keeper/hooks.go#L23-L34) is executed that [sets](https://github.com/celestiaorg/celestia-app/blob/0629c757ef35a24187a8d7a4c706c7cdc894c8b6/x/qgb/keeper/hooks.go#LL33C2-L33C2) the `LatestUnBondingBlockHeight` to the current block height. This allows creating a new valset that removes that validator from the valset members so that he doesn't need to sign attestations afterwards. diff --git a/x/blobstream/client/suite_test.go b/x/blobstream/client/suite_test.go index a4a7458188..78898ac3f7 100644 --- a/x/blobstream/client/suite_test.go +++ b/x/blobstream/client/suite_test.go @@ -3,6 +3,7 @@ package client_test import ( "testing" + "github.com/celestiaorg/celestia-app/v2/app" "github.com/celestiaorg/celestia-app/v2/test/util/testnode" "github.com/stretchr/testify/suite" tmrand "github.com/tendermint/tendermint/libs/rand" @@ -19,7 +20,7 @@ func (s *CLITestSuite) SetupSuite() { s.T().Skip("skipping Blobstream CLI tests in short mode.") } - cfg := testnode.DefaultConfig() + cfg := testnode.DefaultConfig().WithConsensusParams(app.DefaultInitialConsensusParams()) numAccounts := 120 accounts := make([]string, numAccounts) diff --git a/x/blobstream/integration_test.go b/x/blobstream/integration_test.go index d1f1dbc15d..d49e2728b7 100644 --- a/x/blobstream/integration_test.go +++ b/x/blobstream/integration_test.go @@ -39,7 +39,9 @@ func (s *BlobstreamIntegrationSuite) SetupSuite() { s.accounts = []string{"jimmy"} - cfg := testnode.DefaultConfig().WithFundedAccounts(s.accounts...) + cfg := testnode.DefaultConfig(). + WithFundedAccounts(s.accounts...). + WithConsensusParams(app.DefaultInitialConsensusParams()) cctx, _, _ := testnode.NewNetwork(t, cfg) s.ecfg = encoding.MakeConfig(app.ModuleEncodingRegisters...) s.cctx = cctx diff --git a/x/blobstream/keeper/hooks.go b/x/blobstream/keeper/hooks.go index 603b7171a4..38aaaa691c 100644 --- a/x/blobstream/keeper/hooks.go +++ b/x/blobstream/keeper/hooks.go @@ -22,16 +22,19 @@ func (k Keeper) Hooks() Hooks { } func (h Hooks) AfterValidatorBeginUnbonding(ctx sdk.Context, _ sdk.ConsAddress, _ sdk.ValAddress) error { - // When Validator starts Unbonding, Persist the block height in the store - // Later in endblocker, check if there is at least one validator who started + if ctx.BlockHeader().Version.App > 1 { + // no-op if the app version is greater than 1 because blobstream was disabled in v2. + return nil + } + // When Validator starts Unbonding, Persist the block height in the store. + // Later in EndBlocker, check if there is at least one validator who started // unbonding and create a valset request. The reason for creating valset - // requests in endblock is to create only one valset request per block, if - // multiple validators starts unbonding at same block. + // requests in EndBlock is to create only one valset request per block if + // multiple validators start unbonding in the same block. - // this hook IS called for jailing or unbonding triggered by users but it IS + // This hook is called for jailing or unbonding triggered by users but it is // NOT called for jailing triggered in the endblocker therefore we call the // keeper function ourselves there. - h.k.SetLatestUnBondingBlockHeight(ctx, uint64(ctx.BlockHeight())) return nil } @@ -41,6 +44,10 @@ func (h Hooks) BeforeDelegationCreated(_ sdk.Context, _ sdk.AccAddress, _ sdk.Va } func (h Hooks) AfterValidatorCreated(ctx sdk.Context, addr sdk.ValAddress) error { + if ctx.BlockHeader().Version.App > 1 { + // no-op if the app version is greater than 1 because blobstream was disabled in v2. + return nil + } defaultEvmAddr := types.DefaultEVMAddress(addr) // This should practically never happen that we have a collision. It may be // bad UX to reject the attempt to create a validator and require the user to diff --git a/x/blobstream/keeper/hooks_test.go b/x/blobstream/keeper/hooks_test.go new file mode 100644 index 0000000000..e8e769074d --- /dev/null +++ b/x/blobstream/keeper/hooks_test.go @@ -0,0 +1,58 @@ +package keeper_test + +import ( + "testing" + + "github.com/celestiaorg/celestia-app/v2/test/util" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/ethereum/go-ethereum/common" + "github.com/stretchr/testify/assert" + tmproto "github.com/tendermint/tendermint/proto/tendermint/types" + version "github.com/tendermint/tendermint/proto/tendermint/version" +) + +func TestAfterValidatorBeginUnbonding(t *testing.T) { + testEnv := util.CreateTestEnv(t) + height := int64(1) + + t.Run("should be a no-op if app version is 2", func(t *testing.T) { + ctx := testEnv.Context.WithBlockHeader(tmproto.Header{Version: version.Consensus{App: 2}, Height: height}) + err := testEnv.BlobstreamKeeper.Hooks().AfterValidatorBeginUnbonding(ctx, sdk.ConsAddress{}, sdk.ValAddress{}) + assert.NoError(t, err) + + got := testEnv.BlobstreamKeeper.GetLatestUnBondingBlockHeight(ctx) + assert.Equal(t, uint64(0), got) + }) + t.Run("should set latest unboding height if app version is 1", func(t *testing.T) { + ctx := testEnv.Context.WithBlockHeader(tmproto.Header{Version: version.Consensus{App: 1}, Height: height}) + err := testEnv.BlobstreamKeeper.Hooks().AfterValidatorBeginUnbonding(ctx, sdk.ConsAddress{}, sdk.ValAddress{}) + assert.NoError(t, err) + + got := testEnv.BlobstreamKeeper.GetLatestUnBondingBlockHeight(ctx) + assert.Equal(t, uint64(height), got) + }) +} + +func TestAfterValidatorCreated(t *testing.T) { + testEnv := util.CreateTestEnv(t) + height := int64(1) + valAddress := sdk.ValAddress([]byte("valAddress")) + t.Run("should be a no-op if app version is 2", func(t *testing.T) { + ctx := testEnv.Context.WithBlockHeader(tmproto.Header{Version: version.Consensus{App: 2}, Height: height}) + err := testEnv.BlobstreamKeeper.Hooks().AfterValidatorCreated(ctx, valAddress) + assert.NoError(t, err) + + address, ok := testEnv.BlobstreamKeeper.GetEVMAddress(ctx, valAddress) + assert.False(t, ok) + assert.Empty(t, address) + }) + t.Run("should set EVM address if app version is 1", func(t *testing.T) { + ctx := testEnv.Context.WithBlockHeader(tmproto.Header{Version: version.Consensus{App: 1}, Height: height}) + err := testEnv.BlobstreamKeeper.Hooks().AfterValidatorCreated(ctx, valAddress) + assert.NoError(t, err) + + address, ok := testEnv.BlobstreamKeeper.GetEVMAddress(ctx, valAddress) + assert.True(t, ok) + assert.Equal(t, common.HexToAddress("0x0000000000000000000076616C41646472657373"), address) + }) +} diff --git a/x/blobstream/module.go b/x/blobstream/module.go index cfd7f195a7..a6669ec2f2 100644 --- a/x/blobstream/module.go +++ b/x/blobstream/module.go @@ -5,8 +5,6 @@ import ( "encoding/json" "fmt" - bscmd "github.com/celestiaorg/celestia-app/v2/x/blobstream/client" - "github.com/gorilla/mux" "github.com/grpc-ecosystem/grpc-gateway/runtime" "github.com/spf13/cobra" @@ -79,14 +77,16 @@ func (AppModuleBasic) RegisterGRPCGatewayRoutes(clientCtx client.Context, mux *r } } -// GetTxCmd returns the capability module's root tx command. +// GetTxCmd returns no command because the blobstream module was disabled in app +// version 2. func (a AppModuleBasic) GetTxCmd() *cobra.Command { - return bscmd.GetTxCmd() + return nil } -// GetQueryCmd returns the capability module's root query command. +// GetQueryCmd returns no command because the blobstream module was disabled in +// app version 2. func (AppModuleBasic) GetQueryCmd() *cobra.Command { - return bscmd.GetQueryCmd() + return nil } // ----------------------------------------------------------------------------