ci: actually enable v2 system test (#21539)

Co-authored-by: Matt Kocubinski <[email protected]>

.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
@@ -177,7 +176,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 +186,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:

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

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)

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,

core/server/app.go

@@ -39,10 +39,6 @@ type TxResult struct {
 	Resp []transaction.Msg
 	// Error produced by the transaction.
 	Error error
-	// Code produced by the transaction.
-	// A non-zero code is an error that is either define by the module via the cosmossdk.io/errors/v2 package
-	// or injected through the antehandler along the execution of the transaction.
-	Code uint32
 	// GasWanted is the maximum units of work we allow this tx to perform.
 	GasWanted uint64
 	// GasUsed is the amount of gas actually consumed.

server/v2/cometbft/abci.go

@@ -17,7 +17,7 @@ import (
 	"cosmossdk.io/core/server"
 	"cosmossdk.io/core/store"
 	"cosmossdk.io/core/transaction"
-	errorsmod "cosmossdk.io/errors"
+	errorsmod "cosmossdk.io/errors/v2"
 	"cosmossdk.io/log"
 	"cosmossdk.io/server/v2/appmanager"
 	"cosmossdk.io/server/v2/cometbft/client/grpc/cmtservice"
@@ -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
 	}
 
@@ -132,15 +133,18 @@ func (c *Consensus[T]) CheckTx(ctx context.Context, req *abciproto.CheckTxReques
 	}
 
 	cometResp := &abciproto.CheckTxResponse{
-		Code:      resp.Code,
+		Code:      0,
 		GasWanted: uint64ToInt64(resp.GasWanted),
 		GasUsed:   uint64ToInt64(resp.GasUsed),
 		Events:    events,
 	}
 	if resp.Error != nil {
-		cometResp.Code = 1
-		cometResp.Log = resp.Error.Error()
+		space, code, log := errorsmod.ABCIInfo(resp.Error, c.cfg.AppTomlConfig.Trace)
+		cometResp.Code = code
+		cometResp.Codespace = space
+		cometResp.Log = log
 	}
+
 	return cometResp, nil
 }
 
@@ -196,7 +200,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
 
@@ -283,10 +287,10 @@ func (c *Consensus[T]) InitChain(ctx context.Context, req *abciproto.InitChainRe
 		return nil, fmt.Errorf("genesis state init failure: %w", err)
 	}
 
-	// TODO necessary? where should this WARN live if it all. helpful for testing
 	for _, txRes := range blockresponse.TxResults {
-		if txRes.Error != nil {
-			c.logger.Warn("genesis tx failed", "code", txRes.Code, "error", txRes.Error)
+		if err := txRes.Error; err != nil {
+			space, code, log := errorsmod.ABCIInfo(err, c.cfg.AppTomlConfig.Trace)
+			c.logger.Warn("genesis tx failed", "codespace", space, "code", code, "log", log)
 		}
 	}
 
@@ -485,7 +489,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.

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
 		},
 	}

server/v2/cometbft/go.mod

@@ -4,6 +4,7 @@ go 1.23.1
 
 replace (
 	cosmossdk.io/api => ../../../api
+	cosmossdk.io/core => ../../../core
 	cosmossdk.io/server/v2 => ../
 	cosmossdk.io/server/v2/appmanager => ../appmanager
 	cosmossdk.io/server/v2/stf => ../stf
@@ -19,7 +20,7 @@ replace (
 require (
 	cosmossdk.io/api v0.7.6
 	cosmossdk.io/core v1.0.0-alpha.3
-	cosmossdk.io/errors v1.0.1
+	cosmossdk.io/errors/v2 v2.0.0-20240731132947-df72853b3ca5
 	cosmossdk.io/log v1.4.1
 	cosmossdk.io/server/v2 v2.0.0-00010101000000-000000000000
 	cosmossdk.io/server/v2/appmanager v0.0.0-20240802110823-cffeedff643d
@@ -43,7 +44,7 @@ require (
 	cosmossdk.io/collections v0.4.0 // indirect
 	cosmossdk.io/core/testing v0.0.0-20240923163230-04da382a9f29 // indirect
 	cosmossdk.io/depinject v1.0.0 // indirect
-	cosmossdk.io/errors/v2 v2.0.0-20240731132947-df72853b3ca5 // indirect
+	cosmossdk.io/errors v1.0.1 // indirect
 	cosmossdk.io/math v1.3.0 // indirect
 	cosmossdk.io/schema v0.3.0 // indirect
 	cosmossdk.io/store v1.1.1-0.20240418092142-896cdf1971bc // indirect

server/v2/cometbft/go.sum

@@ -6,8 +6,6 @@ cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMT
 cloud.google.com/go v0.34.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw=
 cosmossdk.io/collections v0.4.0 h1:PFmwj2W8szgpD5nOd8GWH6AbYNi1f2J6akWXJ7P5t9s=
 cosmossdk.io/collections v0.4.0/go.mod h1:oa5lUING2dP+gdDquow+QjlF45eL1t4TJDypgGd+tv0=
-cosmossdk.io/core v1.0.0-alpha.3 h1:pnxaYAas7llXgVz1lM7X6De74nWrhNKnB3yMKe4OUUA=
-cosmossdk.io/core v1.0.0-alpha.3/go.mod h1:3u9cWq1FAVtiiCrDPpo4LhR+9V6k/ycSG4/Y/tREWCY=
 cosmossdk.io/core/testing v0.0.0-20240923163230-04da382a9f29 h1:NxxUo0GMJUbIuVg0R70e3cbn9eFTEuMr7ev1AFvypdY=
 cosmossdk.io/core/testing v0.0.0-20240923163230-04da382a9f29/go.mod h1:8s2tPeJtSiQuoyPmr2Ag7meikonISO4Fv4MoO8+ORrs=
 cosmossdk.io/depinject v1.0.0 h1:dQaTu6+O6askNXO06+jyeUAnF2/ssKwrrszP9t5q050=

server/v2/cometbft/query.go

@@ -7,7 +7,7 @@ import (
 	abci "github.com/cometbft/cometbft/api/cometbft/abci/v1"
 	crypto "github.com/cometbft/cometbft/api/cometbft/crypto/v1"
 
-	errorsmod "cosmossdk.io/errors"
+	errorsmod "cosmossdk.io/errors/v2"
 	"cosmossdk.io/server/v2/cometbft/types"
 	cometerrors "cosmossdk.io/server/v2/cometbft/types/errors"
 )

server/v2/cometbft/streaming.go

@@ -6,6 +6,7 @@ import (
 	"cosmossdk.io/core/event"
 	"cosmossdk.io/core/server"
 	"cosmossdk.io/core/store"
+	errorsmod "cosmossdk.io/errors/v2"
 	"cosmossdk.io/server/v2/streaming"
 )
 
@@ -21,12 +22,17 @@ func (c *Consensus[T]) streamDeliverBlockChanges(
 	// convert txresults to streaming txresults
 	streamingTxResults := make([]*streaming.ExecTxResult, len(txResults))
 	for i, txResult := range txResults {
+		space, code, log := errorsmod.ABCIInfo(txResult.Error, c.cfg.AppTomlConfig.Trace)
+
 		events, err := streaming.IntoStreamingEvents(txResult.Events)
 		if err != nil {
 			return err
 		}
+
 		streamingTxResults[i] = &streaming.ExecTxResult{
-			Code:      txResult.Code,
+			Code:      code,
+			Codespace: space,
+			Log:       log,
 			GasWanted: uint64ToInt64(txResult.GasWanted),
 			GasUsed:   uint64ToInt64(txResult.GasUsed),
 			Events:    events,

server/v2/cometbft/types/errors/errors.go

@@ -1,7 +1,7 @@
 package errors
 
 import (
-	errorsmod "cosmossdk.io/errors"
+	errorsmod "cosmossdk.io/errors/v2"
 )
 
 // RootCodespace is the codespace for all errors defined in this package

server/v2/cometbft/utils.go

@@ -18,7 +18,7 @@ import (
 	"cosmossdk.io/core/event"
 	"cosmossdk.io/core/server"
 	"cosmossdk.io/core/transaction"
-	errorsmod "cosmossdk.io/errors"
+	errorsmod "cosmossdk.io/errors/v2"
 	consensus "cosmossdk.io/x/consensus/types"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -70,6 +70,7 @@ func finalizeBlockResponse(
 	cp *cmtproto.ConsensusParams,
 	appHash []byte,
 	indexSet map[string]struct{},
+	debug bool,
 ) (*abci.FinalizeBlockResponse, error) {
 	allEvents := append(in.BeginBlockEvents, in.EndBlockEvents...)
 
@@ -78,7 +79,7 @@ func finalizeBlockResponse(
 		return nil, err
 	}
 
-	txResults, err := intoABCITxResults(in.TxResults, indexSet)
+	txResults, err := intoABCITxResults(in.TxResults, indexSet, debug)
 	if err != nil {
 		return nil, err
 	}
@@ -107,29 +108,20 @@ func intoABCIValidatorUpdates(updates []appmodulev2.ValidatorUpdate) []abci.Vali
 	return valsetUpdates
 }
 
-func intoABCITxResults(results []server.TxResult, indexSet map[string]struct{}) ([]*abci.ExecTxResult, error) {
+func intoABCITxResults(results []server.TxResult, indexSet map[string]struct{}, debug bool) ([]*abci.ExecTxResult, error) {
 	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
-		}
 		events, err := intoABCIEvents(results[i].Events, indexSet)
 		if err != nil {
 			return nil, err
 		}
+
 		res[i] = responseExecTxResultWithEvents(
 			results[i].Error,
 			results[i].GasWanted,
 			results[i].GasUsed,
 			events,
-			false,
+			debug,
 		)
 	}
 
@@ -387,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
 	}
 
@@ -408,14 +400,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,
-	}
-}

server/v2/stf/stf.go

@@ -320,7 +320,7 @@ func (s STF[T]) runTxMsgs(
 		execCtx.sender = txSenders[i]
 		resp, err := s.msgRouter.Invoke(execCtx, msg)
 		if err != nil {
-			return nil, 0, nil, fmt.Errorf("message execution at index %d failed: %w", i, err)
+			return nil, 0, nil, err // do not wrap the error or we lose the original error type
 		}
 		msgResps[i] = resp
 	}

tests/systemtests/testnet_init.go

@@ -162,7 +162,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")
 		})
 	}
 }

tests/systemtests/unordered_tx_test.go

@@ -12,7 +12,8 @@ import (
 )
 
 func TestUnorderedTXDuplicate(t *testing.T) {
-	t.Skip("The unordered tx antehanlder is missing in v2")
+	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

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)