From 1c4061c4e65b2ef7868405e658a0e7e618e74157 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Wed, 4 Sep 2024 12:35:15 +0200 Subject: [PATCH 01/11] ci: actually enable v2 system test --- .github/workflows/test.yml | 4 ++-- scripts/local-testnet.sh | 17 ----------------- 2 files changed, 2 insertions(+), 19 deletions(-) delete mode 100644 scripts/local-testnet.sh diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 0c3945eb1213..e2caee7cdbb6 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -177,7 +177,7 @@ jobs: - name: system tests v1 if: env.GIT_DIFF run: | - COSMOS_BUILD_OPTIONS=legacy make test-system + make test-system - uses: actions/upload-artifact@v3 if: failure() with: @@ -187,7 +187,7 @@ jobs: - name: system tests v2 if: env.GIT_DIFF run: | - make test-system + COSMOS_BUILD_OPTIONS=v2 make test-system - uses: actions/upload-artifact@v3 if: failure() with: diff --git a/scripts/local-testnet.sh b/scripts/local-testnet.sh deleted file mode 100644 index c73710dbab1e..000000000000 --- a/scripts/local-testnet.sh +++ /dev/null @@ -1,17 +0,0 @@ -#!/usr/bin/env bash - -set -o errexit -set -o nounset -set -x - -ROOT=$PWD - -SIMD="$ROOT/build/simdv2" - -COSMOS_BUILD_OPTIONS=v2 make build - -$SIMD testnet init-files --chain-id=testing --output-dir="$HOME/.testnet" --validator-count=3 --keyring-backend=test --minimum-gas-prices=0.000001stake --commit-timeout=900ms --single-host - -$SIMD start --log_level=info --home "$HOME/.testnet/node0/simdv2" & -$SIMD start --log_level=info --home "$HOME/.testnet/node1/simdv2" & -$SIMD start --log_level=info --home "$HOME/.testnet/node2/simdv2" \ No newline at end of file From 95e845bbdec9525dd6da7d4bafe81be999a84bb7 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Wed, 4 Sep 2024 12:43:42 +0200 Subject: [PATCH 02/11] trigger ci --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 067b5d4f6103..775dc0cd3d7c 100644 --- a/Makefile +++ b/Makefile @@ -10,7 +10,7 @@ include scripts/build/build.mk .DEFAULT_GOAL := help -#? go.sum: Run go mod tidy and ensure dependencies have not been modified +#? go.sum: Run go mod tidy and ensure dependencies have not been modified. go.sum: go.mod echo "Ensure dependencies have not been modified ..." >&2 go mod verify From a0772cbbe791ca3f0f45c40aa0dc671d5307f962 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Wed, 4 Sep 2024 15:56:08 +0200 Subject: [PATCH 03/11] v2 checks --- tests/systemtests/testnet_init.go | 20 ++++++++++++++++++-- tests/systemtests/unordered_tx_test.go | 1 - 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/tests/systemtests/testnet_init.go b/tests/systemtests/testnet_init.go index c3cd9c0f73e7..e69e65a30772 100644 --- a/tests/systemtests/testnet_init.go +++ b/tests/systemtests/testnet_init.go @@ -13,6 +13,12 @@ import ( "github.com/creachadair/tomledit/parser" ) +// isV2 checks if the tests run with simapp v2 +func isV2() bool { + buildOptions := os.Getenv("COSMOS_BUILD_OPTIONS") + return strings.Contains(buildOptions, "v2") +} + // SingleHostTestnetCmdInitializer default testnet cmd that supports the --single-host param type SingleHostTestnetCmdInitializer struct { execBinary string @@ -53,10 +59,16 @@ func (s SingleHostTestnetCmdInitializer) Initialize() { "--output-dir=" + s.outputDir, "--validator-count=" + strconv.Itoa(s.initialNodesCount), "--keyring-backend=test", - "--minimum-gas-prices=" + s.minGasPrice, "--commit-timeout=" + s.commitTimeout.String(), "--single-host", } + + if isV2() { + args = append(args, "--server.minimum-gas-prices="+s.minGasPrice) + } else { + args = append(args, "--minimum-gas-prices="+s.minGasPrice) + } + s.log(fmt.Sprintf("+++ %s %s\n", s.execBinary, strings.Join(args, " "))) out, err := RunShellCmd(s.execBinary, args...) if err != nil { @@ -108,7 +120,11 @@ func (s ModifyConfigYamlInitializer) Initialize() { "--output-dir=" + s.outputDir, "--v=" + strconv.Itoa(s.initialNodesCount), "--keyring-backend=test", - "--minimum-gas-prices=" + s.minGasPrice, + } + if isV2() { + args = append(args, "--server.minimum-gas-prices="+s.minGasPrice) + } else { + args = append(args, "--minimum-gas-prices="+s.minGasPrice) } s.log(fmt.Sprintf("+++ %s %s\n", s.execBinary, strings.Join(args, " "))) diff --git a/tests/systemtests/unordered_tx_test.go b/tests/systemtests/unordered_tx_test.go index 6b1a9c743e7d..4b22df92b0a5 100644 --- a/tests/systemtests/unordered_tx_test.go +++ b/tests/systemtests/unordered_tx_test.go @@ -12,7 +12,6 @@ import ( ) func TestUnorderedTXDuplicate(t *testing.T) { - t.Skip("The unordered tx antehanlder is missing in v2") // scenario: test unordered tx duplicate // given a running chain with a tx in the unordered tx pool // when a new tx with the same hash is broadcasted From e0337c5c74ad3397c35d528442cc0934338b6f0a Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Wed, 4 Sep 2024 15:58:58 +0200 Subject: [PATCH 04/11] unskip tests or add comments --- client/grpc/cmtservice/status_test.go | 2 +- client/v2/autocli/query_test.go | 2 -- x/auth/ante/basic_test.go | 2 +- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/client/grpc/cmtservice/status_test.go b/client/grpc/cmtservice/status_test.go index 9f7ab9567921..d7bfa6bdc192 100644 --- a/client/grpc/cmtservice/status_test.go +++ b/client/grpc/cmtservice/status_test.go @@ -14,7 +14,7 @@ import ( ) func TestStatusCommand(t *testing.T) { - t.Skip() // https://github.com/cosmos/cosmos-sdk/issues/17446 + t.Skip() // Rewrite as system test cfg, err := network.DefaultConfigWithAppConfig(depinject.Configs() /* TODO, test skipped anyway */) require.NoError(t, err) diff --git a/client/v2/autocli/query_test.go b/client/v2/autocli/query_test.go index f5db25c40b36..1cbca64fb951 100644 --- a/client/v2/autocli/query_test.go +++ b/client/v2/autocli/query_test.go @@ -575,8 +575,6 @@ func TestBinaryFlag(t *testing.T) { } func TestAddressValidation(t *testing.T) { - t.Skip() // TODO(@julienrbrt) re-able with better keyring instiantiation - fixture := initFixture(t) _, err := runCmd(fixture, buildModuleQueryCommand, diff --git a/x/auth/ante/basic_test.go b/x/auth/ante/basic_test.go index 389096d3d7c6..2de379e48d64 100644 --- a/x/auth/ante/basic_test.go +++ b/x/auth/ante/basic_test.go @@ -99,7 +99,7 @@ func TestValidateMemo(t *testing.T) { } func TestConsumeGasForTxSize(t *testing.T) { - t.Skip() // TODO(@julienrbrt) Fix after https://github.com/cosmos/cosmos-sdk/pull/20072 + t.Skip() // TO FIX BEFORE 0.52 FINAL. suite := SetupTestSuite(t, true) From d9fe1e4e87c18a91192ed6552d5f7b38c0003ac5 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Wed, 4 Sep 2024 16:39:46 +0200 Subject: [PATCH 05/11] updates --- tests/systemtests/unordered_tx_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/systemtests/unordered_tx_test.go b/tests/systemtests/unordered_tx_test.go index 4b22df92b0a5..579552efe0e7 100644 --- a/tests/systemtests/unordered_tx_test.go +++ b/tests/systemtests/unordered_tx_test.go @@ -12,6 +12,8 @@ import ( ) func TestUnorderedTXDuplicate(t *testing.T) { + t.Skip("The unordered tx handling is not wired in v2") + // scenario: test unordered tx duplicate // given a running chain with a tx in the unordered tx pool // when a new tx with the same hash is broadcasted From 2e5d2bfedabce9d8ee949dfdc78b9581a754965d Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Wed, 4 Sep 2024 18:14:57 +0200 Subject: [PATCH 06/11] remove grpc web + don't wait to run system tess --- .github/workflows/test.yml | 1 - tests/systemtests/testnet_init.go | 1 - 2 files changed, 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index e2caee7cdbb6..ad1a7b8aaefc 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -145,7 +145,6 @@ jobs: path: ./tests/e2e-profile.out test-system: - needs: [tests, test-integration, test-e2e] runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 diff --git a/tests/systemtests/testnet_init.go b/tests/systemtests/testnet_init.go index e69e65a30772..fae6f42ef2de 100644 --- a/tests/systemtests/testnet_init.go +++ b/tests/systemtests/testnet_init.go @@ -160,7 +160,6 @@ func (s ModifyConfigYamlInitializer) Initialize() { EditToml(filepath.Join(nodeDir, "app.toml"), func(doc *tomledit.Document) { UpdatePort(doc, apiPortStart+i, "api", "address") UpdatePort(doc, grpcPortStart+i, "grpc", "address") - SetBool(doc, true, "grpc-web", "enable") }) } } From 88740797804e55b806fa8b31e7834bfa18c2887b Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Wed, 18 Sep 2024 20:39:13 +0200 Subject: [PATCH 07/11] fix checktx --- server/v2/cometbft/abci.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/server/v2/cometbft/abci.go b/server/v2/cometbft/abci.go index dfd113434093..6fb4c8618691 100644 --- a/server/v2/cometbft/abci.go +++ b/server/v2/cometbft/abci.go @@ -122,7 +122,8 @@ func (c *Consensus[T]) CheckTx(ctx context.Context, req *abciproto.CheckTxReques } resp, err := c.app.ValidateTx(ctx, decodedTx) - if err != nil { + // we do not want to return a cometbft error, but a check tx response with the error + if err != nil && err != resp.Error { return nil, err } @@ -136,6 +137,7 @@ func (c *Consensus[T]) CheckTx(ctx context.Context, req *abciproto.CheckTxReques cometResp.Code = 1 cometResp.Log = resp.Error.Error() } + return cometResp, nil } From fce428229f5d3ba86875238122da1d4b5ef2ea48 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Wed, 18 Sep 2024 20:55:18 +0200 Subject: [PATCH 08/11] abci error --- server/v2/cometbft/abci.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/server/v2/cometbft/abci.go b/server/v2/cometbft/abci.go index 6fb4c8618691..60771e0aee7e 100644 --- a/server/v2/cometbft/abci.go +++ b/server/v2/cometbft/abci.go @@ -134,8 +134,10 @@ func (c *Consensus[T]) CheckTx(ctx context.Context, req *abciproto.CheckTxReques Events: intoABCIEvents(resp.Events, c.indexedEvents), } if resp.Error != nil { - cometResp.Code = 1 - cometResp.Log = resp.Error.Error() + space, code, log := errorsmod.ABCIInfo(resp.Error, true) + cometResp.Code = code + cometResp.Codespace = space + cometResp.Log = log } return cometResp, nil From 9b7824c0f82bc798458bac8af67139eab1d8cd86 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Wed, 18 Sep 2024 21:30:37 +0200 Subject: [PATCH 09/11] fix halt height, simplify abci error handling (still not working) --- server/v2/cometbft/abci.go | 2 +- server/v2/cometbft/commands.go | 14 ++++++-------- server/v2/cometbft/utils.go | 15 ++------------- 3 files changed, 9 insertions(+), 22 deletions(-) diff --git a/server/v2/cometbft/abci.go b/server/v2/cometbft/abci.go index 60771e0aee7e..ab510831edf5 100644 --- a/server/v2/cometbft/abci.go +++ b/server/v2/cometbft/abci.go @@ -134,7 +134,7 @@ func (c *Consensus[T]) CheckTx(ctx context.Context, req *abciproto.CheckTxReques Events: intoABCIEvents(resp.Events, c.indexedEvents), } if resp.Error != nil { - space, code, log := errorsmod.ABCIInfo(resp.Error, true) + space, code, log := errorsmod.ABCIInfo(resp.Error, false) cometResp.Code = code cometResp.Codespace = space cometResp.Log = log diff --git a/server/v2/cometbft/commands.go b/server/v2/cometbft/commands.go index 787bd2c7810f..d1bb2f257245 100644 --- a/server/v2/cometbft/commands.go +++ b/server/v2/cometbft/commands.go @@ -106,15 +106,13 @@ func ShowValidatorCmd() *cobra.Command { return err } - cmd.Println(sdkPK) // TODO: figure out if we need the codec here or not, see below - - // clientCtx := client.GetClientContextFromCmd(cmd) - // bz, err := clientCtx.Codec.MarshalInterfaceJSON(sdkPK) - // if err != nil { - // return err - // } + clientCtx := client.GetClientContextFromCmd(cmd) + bz, err := clientCtx.Codec.MarshalInterfaceJSON(sdkPK) + if err != nil { + return err + } - // cmd.Println(string(bz)) + cmd.Println(string(bz)) return nil }, } diff --git a/server/v2/cometbft/utils.go b/server/v2/cometbft/utils.go index 6aaf1b5401d6..267734d68afc 100644 --- a/server/v2/cometbft/utils.go +++ b/server/v2/cometbft/utils.go @@ -100,17 +100,6 @@ func intoABCIValidatorUpdates(updates []appmodulev2.ValidatorUpdate) []abci.Vali func intoABCITxResults(results []server.TxResult, indexSet map[string]struct{}) []*abci.ExecTxResult { res := make([]*abci.ExecTxResult, len(results)) for i := range results { - if results[i].Error != nil { - space, code, log := errorsmod.ABCIInfo(results[i].Error, true) - res[i] = &abci.ExecTxResult{ - Codespace: space, - Code: code, - Log: log, - } - - continue - } - res[i] = responseExecTxResultWithEvents( results[i].Error, results[i].GasWanted, @@ -366,10 +355,10 @@ func (c *Consensus[T]) GetBlockRetentionHeight(cp *cmtproto.ConsensusParams, com func (c *Consensus[T]) checkHalt(height int64, time time.Time) error { var halt bool switch { - case c.cfg.AppTomlConfig.HaltHeight > 0 && uint64(height) > c.cfg.AppTomlConfig.HaltHeight: + case c.cfg.AppTomlConfig.HaltHeight > 0 && uint64(height) >= c.cfg.AppTomlConfig.HaltHeight: halt = true - case c.cfg.AppTomlConfig.HaltTime > 0 && time.Unix() > int64(c.cfg.AppTomlConfig.HaltTime): + case c.cfg.AppTomlConfig.HaltTime > 0 && time.Unix() >= int64(c.cfg.AppTomlConfig.HaltTime): halt = true } From 76820d4274f0f0bd81e391810d08aaaedfb201b0 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Wed, 18 Sep 2024 21:39:48 +0200 Subject: [PATCH 10/11] debug error --- server/v2/cometbft/abci.go | 6 +++--- server/v2/cometbft/utils.go | 18 ++++-------------- 2 files changed, 7 insertions(+), 17 deletions(-) diff --git a/server/v2/cometbft/abci.go b/server/v2/cometbft/abci.go index ab510831edf5..e17478a1a8b3 100644 --- a/server/v2/cometbft/abci.go +++ b/server/v2/cometbft/abci.go @@ -134,7 +134,7 @@ func (c *Consensus[T]) CheckTx(ctx context.Context, req *abciproto.CheckTxReques Events: intoABCIEvents(resp.Events, c.indexedEvents), } if resp.Error != nil { - space, code, log := errorsmod.ABCIInfo(resp.Error, false) + space, code, log := errorsmod.ABCIInfo(resp.Error, c.cfg.AppTomlConfig.Trace) cometResp.Code = code cometResp.Codespace = space cometResp.Log = log @@ -195,7 +195,7 @@ func (c *Consensus[T]) Query(ctx context.Context, req *abciproto.QueryRequest) ( } res, err := c.app.Query(ctx, uint64(req.Height), protoRequest) if err != nil { - resp := queryResult(err) + resp := QueryResult(err, c.cfg.AppTomlConfig.Trace) resp.Height = req.Height return resp, err @@ -484,7 +484,7 @@ func (c *Consensus[T]) FinalizeBlock( return nil, err } - return finalizeBlockResponse(resp, cp, appHash, c.indexedEvents) + return finalizeBlockResponse(resp, cp, appHash, c.indexedEvents, c.cfg.AppTomlConfig.Trace) } // Commit implements types.Application. diff --git a/server/v2/cometbft/utils.go b/server/v2/cometbft/utils.go index 267734d68afc..00add58bbfa9 100644 --- a/server/v2/cometbft/utils.go +++ b/server/v2/cometbft/utils.go @@ -70,12 +70,13 @@ func finalizeBlockResponse( cp *cmtproto.ConsensusParams, appHash []byte, indexSet map[string]struct{}, + debug bool, ) (*abci.FinalizeBlockResponse, error) { allEvents := append(in.BeginBlockEvents, in.EndBlockEvents...) resp := &abci.FinalizeBlockResponse{ Events: intoABCIEvents(allEvents, indexSet), - TxResults: intoABCITxResults(in.TxResults, indexSet), + TxResults: intoABCITxResults(in.TxResults, indexSet, debug), ValidatorUpdates: intoABCIValidatorUpdates(in.ValidatorUpdates), AppHash: appHash, ConsensusParamUpdates: cp, @@ -97,7 +98,7 @@ func intoABCIValidatorUpdates(updates []appmodulev2.ValidatorUpdate) []abci.Vali return valsetUpdates } -func intoABCITxResults(results []server.TxResult, indexSet map[string]struct{}) []*abci.ExecTxResult { +func intoABCITxResults(results []server.TxResult, indexSet map[string]struct{}, debug bool) []*abci.ExecTxResult { res := make([]*abci.ExecTxResult, len(results)) for i := range results { res[i] = responseExecTxResultWithEvents( @@ -105,7 +106,7 @@ func intoABCITxResults(results []server.TxResult, indexSet map[string]struct{}) results[i].GasWanted, results[i].GasUsed, intoABCIEvents(results[i].Events, indexSet), - false, + debug, ) } @@ -376,14 +377,3 @@ func uint64ToInt64(u uint64) int64 { } return int64(u) } - -// queryResult returns a ResponseQuery from an error. It will try to parse ABCI -// info from the error. -func queryResult(err error) *abci.QueryResponse { - space, code, log := errorsmod.ABCIInfo(err, false) - return &abci.QueryResponse{ - Codespace: space, - Code: code, - Log: log, - } -} From 4788bfeb38c71e39fad427682d333a97224bab00 Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Wed, 18 Sep 2024 21:46:28 +0200 Subject: [PATCH 11/11] typo --- server/v2/cometbft/utils.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/v2/cometbft/utils.go b/server/v2/cometbft/utils.go index 9703e83ddb24..d4d715bd23dc 100644 --- a/server/v2/cometbft/utils.go +++ b/server/v2/cometbft/utils.go @@ -379,10 +379,10 @@ func (c *Consensus[T]) GetBlockRetentionHeight(cp *cmtproto.ConsensusParams, com func (c *Consensus[T]) checkHalt(height int64, time time.Time) error { var halt bool switch { - case c.cfg.AppTomlConfig.HaltHeight >= 0 && uint64(height) > c.cfg.AppTomlConfig.HaltHeight: + case c.cfg.AppTomlConfig.HaltHeight > 0 && uint64(height) >= c.cfg.AppTomlConfig.HaltHeight: halt = true - case c.cfg.AppTomlConfig.HaltTime >= 0 && time.Unix() > int64(c.cfg.AppTomlConfig.HaltTime): + case c.cfg.AppTomlConfig.HaltTime > 0 && time.Unix() >= int64(c.cfg.AppTomlConfig.HaltTime): halt = true }