From 7b337ed0832e6e5f461345bf0a2601b8c9597857 Mon Sep 17 00:00:00 2001 From: BigBoss Date: Sat, 5 Aug 2023 21:34:20 -0400 Subject: [PATCH] add upgrade validations, update e2e tests --- e2e/README.md | 2 +- e2e/tests/account.feature | 2 +- e2e/tests/node.go | 18 ++++-- e2e/tests/query.feature | 8 +-- e2e/tests/root.feature | 4 +- e2e/tests/steps_init_test.go | 44 ++++++++++++-- e2e/tests/steps_upgrade_test.go | 71 ---------------------- e2e/tests/upgrade.feature | 54 ++++++++++------ e2e/tests/upgrade_cancel.feature | 4 +- e2e/tests/validator.feature | 2 +- utility/unit_of_work/gov.go | 8 +++ utility/unit_of_work/transaction.go | 4 ++ utility/unit_of_work/tx_message_handler.go | 36 ++++++++--- utility/unit_of_work/tx_message_signers.go | 4 -- 14 files changed, 138 insertions(+), 123 deletions(-) delete mode 100644 e2e/tests/steps_upgrade_test.go diff --git a/e2e/README.md b/e2e/README.md index 5e3ee41c9..5f001ff27 100644 --- a/e2e/README.md +++ b/e2e/README.md @@ -38,7 +38,7 @@ Feature: Example Namespace Scenario: User Needs Example Given the user has a node - When the user runs the command "example" + When the user runs the command with no error "example" Then the user should be able to see standard output containing "Example Output" And the pocket client should have exited without error ``` diff --git a/e2e/tests/account.feature b/e2e/tests/account.feature index 8e793dcd9..2b20cc944 100644 --- a/e2e/tests/account.feature +++ b/e2e/tests/account.feature @@ -2,7 +2,7 @@ Feature: Node Namespace Scenario: User Wants Help Using The Node Command Given the user has a node - When the user runs the command "Validator help" + When the user runs the command with no error "Validator help" Then the user should be able to see standard output containing "Available Commands" And the node should have exited without error diff --git a/e2e/tests/node.go b/e2e/tests/node.go index d9db0014f..555d0e2a0 100644 --- a/e2e/tests/node.go +++ b/e2e/tests/node.go @@ -63,18 +63,24 @@ func (n *nodePod) RunCommandOnHost(rpcUrl string, args ...string) (*commandResul "--remote_cli_url=" + rpcUrl, } args = append(base, args...) - - e2eLogger.Debug().Msgf("Running command: kubectl %s", strings.Join(args, " ")) + cmdStr := strings.Join(args, " ") + e2eLogger.Debug().Msgf("Running command: kubectl %s", cmdStr) cmd := exec.Command("kubectl", args...) r := &commandResult{} out, err := cmd.Output() - if err != nil { - return nil, err - } r.Stdout = string(out) n.result = r // IMPROVE: make targetPodName configurable n.targetPodName = targetDevClientPod - return r, nil + + if err != nil { + // unpack ExitError to get stderr + if exitErr, ok := err.(*exec.ExitError); ok { + r.Stderr = string(exitErr.Stderr) + } + return r, err + } + e2eLogger.Debug().Str("stderr", r.Stderr).Err(err).Msgf("command result: %s", string(out)) + return r, err } diff --git a/e2e/tests/query.feature b/e2e/tests/query.feature index 74cc60180..15aa5d899 100644 --- a/e2e/tests/query.feature +++ b/e2e/tests/query.feature @@ -3,12 +3,12 @@ Feature: Query Namespace Scenario: User Wants Help Using The Query Command Given the user has a node - When the user runs the command "Query help" + When the user runs the command with no error "Query help" Then the user should be able to see standard output containing "Available Commands" And the node should have exited without error - Scenario: User Wants To See The Block At Current Height + Scenario: User Wants To See The Block At Current Height Given the user has a node - When the user runs the command "Query Block" + When the user runs the command with no error "Query Block" Then the user should be able to see standard output containing "state_hash" - And the node should have exited without error \ No newline at end of file + And the node should have exited without error diff --git a/e2e/tests/root.feature b/e2e/tests/root.feature index b9d6225d4..2d02f6a9e 100644 --- a/e2e/tests/root.feature +++ b/e2e/tests/root.feature @@ -2,6 +2,6 @@ Feature: Root Namespace Scenario: User Needs Help Given the user has a node - When the user runs the command "help" + When the user runs the command with no error "help" Then the user should be able to see standard output containing "Available Commands" - And the node should have exited without error \ No newline at end of file + And the node should have exited without error diff --git a/e2e/tests/steps_init_test.go b/e2e/tests/steps_init_test.go index 34b190a91..522c3557b 100644 --- a/e2e/tests/steps_init_test.go +++ b/e2e/tests/steps_init_test.go @@ -76,7 +76,7 @@ func TestFeatures(t *testing.T) { func (s *rootSuite) TheUserHasANode() { res, err := s.node.RunCommand("help") - require.NoErrorf(s, err, res.Stderr) + require.NoErrorf(s, err, res.Stderr, "failed to run command: 'help' due to error: %s", err) s.node.result = res } @@ -84,13 +84,44 @@ func (s *rootSuite) TheNodeShouldHaveExitedWithoutError() { require.NoError(s, s.node.result.Err) } -func (s *rootSuite) TheUserRunsTheCommand(cmd string) { +func (s *rootSuite) TheUserRunsTheCommandWithNoError(cmd string) { cmds := strings.Split(cmd, " ") res, err := s.node.RunCommand(cmds...) - require.NoError(s, err) + require.NoError(s, err, fmt.Sprintf("failed to run command: '%s' due to error: %s", cmd, err)) + s.node.result = res +} + +func (s *rootSuite) TheUserRunsTheCommandWithError(cmd string) { + cmds := strings.Split(cmd, " ") + res, err := s.node.RunCommand(cmds...) + require.Error(s, err, fmt.Sprintf("expected error running command: '%s'", cmd)) s.node.result = res } +func (s *rootSuite) TheUserSubmitsTheTransaction(cmd string) { + s.TheUserRunsTheCommandWithNoError(cmd) + // TECHBDEBT: cli outputs debug logs last non-blank line is our answer + var lines = strings.Split(s.node.result.Stdout, "\n") + var answer string + for i := len(lines) - 1; i >= 0; i-- { + if lines[i] != "" { + answer = lines[i] + break + } + } + // ensure it is a valid sha256 hash + require.Regexp(s, "^([a-f0-9]{64})$", answer, "invalid tx hash") + s.pendingTxs = append(s.pendingTxs, answer) +} + +func (s *rootSuite) TheUserQueriesTheTransaction(cmd string) { + if len(s.pendingTxs) == 0 { + e2eLogger.Info().Msg("no pending transactions to query") + require.FailNow(s, "no pending transactions to query") + } + s.TheUserRunsTheCommandWithNoError("query tx " + s.pendingTxs[len(s.pendingTxs)-1]) +} + // TheDeveloperRunsTheCommand is similar to TheUserRunsTheCommand but exclusive to `Debug` commands func (s *rootSuite) TheDeveloperRunsTheCommand(cmd string) { cmds := strings.Split(cmd, " ") @@ -199,8 +230,7 @@ func (s *rootSuite) ShouldBeAtHeight(pod string, height int64) { res := getResponseFromStdout[expectedResponse](s, resRaw.Stdout, validate) require.NotNil(s, res) - - require.Equal(s, height, *res.Height) + require.Equal(s, height, *res.Height, "height mismatch") } // TheNetworkCommitsTheTransactions is a step that triggers the next view and verifies that all pending transactions are committed @@ -263,6 +293,10 @@ func (s *rootSuite) TheUserShouldBeAbleToSeeStandardOutputContaining(arg1 string require.Contains(s, s.node.result.Stdout, arg1) } +func (s *rootSuite) TheUserShouldBeAbleToSeeStandardErrorContaining(arg1 string) { + require.Contains(s, s.node.result.Stderr, arg1) +} + func (s *rootSuite) TheUserStakesTheirValidatorWithAmountUpokt(amount int64) { privKey := s.getPrivateKey(validatorA) s.stakeValidator(privKey, fmt.Sprintf("%d", amount)) diff --git a/e2e/tests/steps_upgrade_test.go b/e2e/tests/steps_upgrade_test.go deleted file mode 100644 index 6a1c96bb2..000000000 --- a/e2e/tests/steps_upgrade_test.go +++ /dev/null @@ -1,71 +0,0 @@ -//go:build e2e - -package e2e - -import ( - "fmt" - "strings" - - "github.com/blang/semver/v4" - "github.com/pokt-network/pocket/rpc" - "github.com/pokt-network/pocket/runtime/test_artifacts" - "github.com/stretchr/testify/require" - "k8s.io/apimachinery/pkg/util/json" -) - -func (s *rootSuite) TheUserSubmitsAMajorProtocolUpgrade() { - res, err := s.node.RunCommand("query", "upgrade") - require.NoError(s, err) - - var qur rpc.QueryUpgradeResponse - // TECHDEBT: cli outputs debug logs so scan for our answer - for _, line := range strings.Split(res.Stdout, "\n") { - // parse into QueryUpgradeResponse - err = json.Unmarshal([]byte(line), &qur) - if err == nil && qur.Version != "" { - break - } - } - require.NoError(s, err) - - // submit a major protocol upgrade - newVersion, err := semver.Parse(qur.Version) - require.NoError(s, err) - newVersion.Major++ - res, err = s.node.RunCommand("gov", "upgrade", test_artifacts.DefaultParams().AclOwner, newVersion.String(), fmt.Sprint(qur.Height+1)) - require.NoError(s, err) - - // TECHBDEBT: cli outputs debug logs last non-blank line is our answer - var lines = strings.Split(res.Stdout, "\n") - var answer string - for i := len(lines) - 1; i >= 0; i-- { - if lines[i] != "" { - answer = lines[i] - break - } - } - // ensure it is a valid sha256 hash - require.Regexp(s, "^([a-f0-9]{64})$", answer, "invalid tx hash") - s.pendingTxs = append(s.pendingTxs, answer) - s.node.result = res -} - -func (s *rootSuite) TheSystemReachesTheUpgradeHeight() { - for { - res, err := s.node.RunCommand("query", "upgrade") - require.NoError(s, err) - - // parse into QueryUpgradeResponse - var qur rpc.QueryUpgradeResponse - err = json.Unmarshal([]byte(res.Stdout), &qur) - require.NoError(s, err) - - if qur.Height > 0 { - break - } - } -} - -func (s *rootSuite) TheUserShouldBeAbleToSeeTheNewVersion() { - panic("PENDING") -} diff --git a/e2e/tests/upgrade.feature b/e2e/tests/upgrade.feature index 3380de047..df9c8b197 100644 --- a/e2e/tests/upgrade.feature +++ b/e2e/tests/upgrade.feature @@ -4,7 +4,7 @@ Feature: Upgrade Protocol Given the network is at genesis And the network has "4" actors of type "Validator" And "validator-001" should be at height "0" - And the user runs the command "query upgrade" + And the user runs the command with no error "query upgrade" Then the user should be able to see standard output containing "" Examples: @@ -16,33 +16,49 @@ Feature: Upgrade Protocol And the network has "4" actors of type "Validator" And "validator-001" should be at height "0" And the user is an ACL Owner - When the user submits a major protocol upgrade - And the network commits the transactions - When the user runs the command "query upgrade" - Then the user should be able to see the new version + When the user runs the command with no error "gov upgrade da034209758b78eaea06dd99c07909ab54c99b45 2.0.0 1" + And the developer runs the command "TriggerView" + And the developer waits for "1000" milliseconds + And "validator-001" should be at height "1" + And the user runs the command with no error "query upgrade" + Then the user should be able to see standard output containing "2.0.0" - Scenario: ACL Owner Submits an Invalid Protocol Upgrade Using CLI + Scenario: ACL Owner Fails Basic Validation Submitting a Protocol Upgrade Using CLI Given the network is at genesis And the network has "4" actors of type "Validator" And "validator-001" should be at height "0" And the user is an ACL Owner - And the user has an invalid upgrade protocol command - When the user runs the command "gov upgrade" - Then the system should validate the command - And the system should reject the command due to invalid input + When the user runs the command with error "" + Then the user should be able to see standard error containing "" - Scenario: ACL Owner Submits a Protocol Upgrade with Too Many Versions Ahead Using CLI + Examples: + | cmd | error | + | gov upgrade da034209758b78eaea06dd99c07909ab54c99b45 2.0.zxcv 1 | CODE: 149, ERROR: the protocol version is invalid | + | gov upgrade da034209758b78eaea06dd99c07909ab54c99b45 new 1 | CODE: 149, ERROR: the protocol version is invalid | + + Scenario: ACL Owner Fails Consensus Validation Submitting a Protocol Upgrade Using CLI Given the network is at genesis And the network has "4" actors of type "Validator" + And "validator-001" should be at height "0" And the user is an ACL Owner - And the user has a upgrade protocol command with too many versions jump - When the user runs the command "gov upgrade" - Then the system should validate the command - And the system should reject the command due to too many versions ahead + When the user runs the command with no error "" + And the developer runs the command "TriggerView" + And the developer waits for "1000" milliseconds + And "validator-001" should be at height "1" + And the user queries the transaction + Then the user should be able to see standard output containing "" + + Examples: + | cmd | error | + | gov upgrade da034209758b78eaea06dd99c07909ab54c99b45 3.0.0 1 | CODE: 149, ERROR: the protocol version is invalid | + | gov upgrade da034209758b78eaea06dd99c07909ab54c99b45 2.0.0 0 | CODE: 149, ERROR: the protocol version is invalid | - Scenario: Regular User Submits an Upgrade Using CLI + Scenario: Regular User Fails Consensus Validation Submits an Upgrade Using CLI Given the network is at genesis And the network has "4" actors of type "Validator" - When the user submits a major protocol upgrade - When the user runs the command "gov upgrade 100.0.0 100000" - Then the user should be able to see standard output containing "invalid upgrade proposal: sender is not authorized to submit upgrade proposals" + When the user submits the transaction "gov upgrade 00101f2ff54811e84df2d767c661f57a06349b7e 2.0.0 1" + And the developer runs the command "TriggerView" + And the developer waits for "1000" milliseconds + And "validator-001" should be at height "1" + And the user queries the transaction + Then the user should be able to see standard output containing "CODE: 3, ERROR: the signer of the message is not a proper candidate: da034209758b78eaea06dd99c07909ab54c99b45" diff --git a/e2e/tests/upgrade_cancel.feature b/e2e/tests/upgrade_cancel.feature index 5fbb80a38..62aa92d0f 100644 --- a/e2e/tests/upgrade_cancel.feature +++ b/e2e/tests/upgrade_cancel.feature @@ -3,7 +3,7 @@ # Scenario: User Successfully Cancels a Scheduled Upgrade using CLI # Given the user is an ACL Owner # And the specified upgrade is scheduled and not yet activated -# When the user runs the command "gov cancel_upgrade" +# When the user runs the command with no error "gov cancel_upgrade" # Then the system should cancel the scheduled upgrade # When user runs the command "gov query upgrade" # Then the system should return the successful cancellation status @@ -11,6 +11,6 @@ # Scenario: User Attempts to Cancel a Past Upgrade using CLI # Given the user is an ACL Owner # And the user has a cancel upgrade command for a past version -# When the user runs the command "gov cancel_upgrade" +# When the user runs the command with no error "gov cancel_upgrade" # Then the system should validate the command # And the system should reject the command as it cannot cancel a past upgrade diff --git a/e2e/tests/validator.feature b/e2e/tests/validator.feature index e1bd22c4f..6caa38ae7 100644 --- a/e2e/tests/validator.feature +++ b/e2e/tests/validator.feature @@ -2,7 +2,7 @@ Feature: Validator Namespace Scenario: User Wants Help Using The Validator Command Given the user has a node - When the user runs the command "Validator help" + When the user runs the command with no error "Validator help" Then the user should be able to see standard output containing "Available Commands" And the node should have exited without error diff --git a/utility/unit_of_work/gov.go b/utility/unit_of_work/gov.go index 85f646874..9923514ab 100644 --- a/utility/unit_of_work/gov.go +++ b/utility/unit_of_work/gov.go @@ -270,3 +270,11 @@ func (u *baseUtilityUnitOfWork) getStringParam(paramName string) (string, coreTy } return value, nil } + +func (u *baseUtilityUnitOfWork) getMessageUpgradeSignerCandidates(msg *typesUtil.MessageUpgrade) ([][]byte, coreTypes.Error) { + owner, err := u.getStringParam(typesUtil.AclOwner) + if err != nil { + return nil, coreTypes.ErrGetParam(typesUtil.AclOwner, err) + } + return [][]byte{[]byte(owner)}, nil +} diff --git a/utility/unit_of_work/transaction.go b/utility/unit_of_work/transaction.go index 12ee09538..53251ee3e 100644 --- a/utility/unit_of_work/transaction.go +++ b/utility/unit_of_work/transaction.go @@ -77,6 +77,10 @@ func (u *baseUtilityUnitOfWork) validateTxMessage(tx *coreTypes.Transaction) (ty if !ok { return nil, coreTypes.ErrDecodeMessage(fmt.Errorf("not a supported message type")) } + // Validate the message the same as the client is expected to + if err := msg.ValidateBasic(); err != nil { + return nil, err + } return msg, nil } diff --git a/utility/unit_of_work/tx_message_handler.go b/utility/unit_of_work/tx_message_handler.go index 2d620b971..e0ff5ef9e 100644 --- a/utility/unit_of_work/tx_message_handler.go +++ b/utility/unit_of_work/tx_message_handler.go @@ -2,8 +2,10 @@ package unit_of_work import ( "encoding/hex" + "errors" "math/big" + "github.com/blang/semver/v4" ibcTypes "github.com/pokt-network/pocket/ibc/types" "github.com/pokt-network/pocket/shared/codec" coreTypes "github.com/pokt-network/pocket/shared/core/types" @@ -242,13 +244,33 @@ func (u *baseUtilityUnitOfWork) handlePruneIBCStore(message *ibcTypes.PruneIBCSt return nil } -func (u *baseUtilityUnitOfWork) handleMessageUpgrade(message *typesUtil.MessageUpgrade) coreTypes.Error { - u.logger.Debug().Str("version", message.Version).Int64("height", message.Height).Msg("setting upgrade") - - // validate new upgrade - - if err := u.persistenceRWContext.SetUpgrade(hex.EncodeToString(message.Signer), message.Version, message.Height); err != nil { - return coreTypes.ErrSettingUpgrade(err) +func (u *baseUtilityUnitOfWork) handleMessageUpgrade(msg *typesUtil.MessageUpgrade) coreTypes.Error { + u.logger.Debug().Str("version", msg.Version).Int64("height", msg.Height).Msg("setting upgrade") + if msg.Height <= u.height { + return coreTypes.ErrSettingUpgrade(errors.New("upgrade height must be greater than current height")) + } + ver, err := semver.Parse(msg.Version) + if err != nil { + return coreTypes.ErrInvalidProtocolVersion(msg.Version) + } + curStr, err := u.persistenceRWContext.GetVersionAtHeight(msg.Height) + if err != nil { + return coreTypes.ErrSettingUpgrade(errors.Join(err, errors.New("getting current version"))) + } + cur, err := semver.Parse(curStr) + if err != nil { + return coreTypes.ErrSettingUpgrade(errors.Join(err, errors.New("parsing current version"))) + } + // ensure version is not too large of a jump + if ver.Major-cur.Major > 1 { + return coreTypes.ErrSettingUpgrade(errors.Join(err, errors.New("major version jump too large"))) + } + // ensure version is greater than current + if ver.LTE(cur) { + return coreTypes.ErrSettingUpgrade(errors.Join(err, errors.New("version must be greater than current"))) + } + if err := u.persistenceRWContext.SetUpgrade(hex.EncodeToString(msg.Signer), msg.Version, msg.Height); err != nil { + return coreTypes.ErrSettingUpgrade(errors.Join(err, errors.New("setting upgrade"))) } return nil } diff --git a/utility/unit_of_work/tx_message_signers.go b/utility/unit_of_work/tx_message_signers.go index 6b81a3d0d..103a3fc0c 100644 --- a/utility/unit_of_work/tx_message_signers.go +++ b/utility/unit_of_work/tx_message_signers.go @@ -89,8 +89,4 @@ func (u *baseUtilityUnitOfWork) getPruneIBCStoreSingerCandidates(msg *ibcTypes.P return [][]byte{msg.Signer}, nil } -func (u *baseUtilityUnitOfWork) getMessageUpgradeSignerCandidates(msg *typesUtil.MessageUpgrade) ([][]byte, coreTypes.Error) { - return [][]byte{msg.Signer}, nil -} - // TODO: 0xbigboss MessageCancelUpgrade