[TA-1797] KEK-03: Insufficient Check When Adding And Removing Validator In poa Module

Dockerfile

@@ -22,8 +22,7 @@ RUN golangci-lint run
 # Unit tests
 RUN go test $(go list ./... | grep -v github.com/Peersyst/exrp/tests/e2e/poa)
 # End to end tests
-WORKDIR /go/src/github.com/Peersyst/exrp/tests/e2e/poa
-RUN go test
+RUN go test -p 1 -v ./tests/e2e/...
 RUN touch /test.lock
 
 FROM golang:1.20 AS release

app/app.go

@@ -2,12 +2,13 @@ package app
 
 import (
 	"encoding/json"
+	poaante "github.com/Peersyst/exrp/x/poa/ante"
+	ethante "github.com/evmos/evmos/v15/app/ante/evm"
 	"io"
 	"os"
 	"path/filepath"
 
 	"cosmossdk.io/math"
-	ethante "github.com/evmos/evmos/v15/app/ante/evm"
 
 	"github.com/ethereum/go-ethereum/core/vm"
 	"golang.org/x/exp/maps"
@@ -423,6 +424,7 @@ func New(
 	app.PoaKeeper = *poakeeper.NewKeeper(
 		appCodec,
 		app.GetSubspace(poatypes.ModuleName),
+		app.MsgServiceRouter(),
 		app.BankKeeper,
 		*app.StakingKeeper,
 		app.SlashingKeeper,
@@ -732,6 +734,7 @@ func (app *App) setAnteHandler(txConfig client.TxConfig, maxGasWanted uint64) {
 		SigGasConsumer:         ante.SigVerificationGasConsumer,
 		MaxTxGasWanted:         maxGasWanted,
 		TxFeeChecker:           ethante.NewDynamicFeeChecker(app.EvmKeeper),
+		ExtraDecorator:         poaante.NewPoaDecorator(),
 	}
 
 	if err := options.Validate(); err != nil {

go.mod

@@ -226,7 +226,7 @@ replace (
 	// use Evmos geth fork
 	github.com/ethereum/go-ethereum => github.com/evmos/go-ethereum v1.10.26-evmos-rc2
 	// use exrp Evmos fork
-	github.com/evmos/evmos/v15 => github.com/Peersyst/evmos/v15 v15.0.0-exrp.0
+	github.com/evmos/evmos/v15 => github.com/Peersyst/evmos/v15 v15.0.0-exrp.1
 	// Security Advisory https://github.com/advisories/GHSA-h395-qcrw-5vmq
 	github.com/gin-gonic/gin => github.com/gin-gonic/gin v1.9.1
 	// replace broken goleveldb

go.sum

@@ -232,8 +232,8 @@ github.com/Microsoft/go-winio v0.6.0 h1:slsWYD/zyx7lCXoZVlvQrj0hPTM1HI4+v1sIda2y
 github.com/Nvveen/Gotty v0.0.0-20120604004816-cd527374f1e5 h1:TngWCqHvy9oXAN6lEVMRuU21PR1EtLVZJmdB18Gu3Rw=
 github.com/OneOfOne/xxhash v1.2.2 h1:KMrpdQIwFcEqXDklaen+P1axHaj9BSKzvpUUfnHldSE=
 github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU=
-github.com/Peersyst/evmos/v15 v15.0.0-exrp.0 h1:uRzoVCOVtlw1buGn1QqWPdd8soXbY13mW2GIxSlnZg0=
-github.com/Peersyst/evmos/v15 v15.0.0-exrp.0/go.mod h1:15ZOo7jqFRe5elw2ipTb3oHq3x7rebUjwq4y2vJEj4Q=
+github.com/Peersyst/evmos/v15 v15.0.0-exrp.1 h1:hZo97kG5OIKacTd3cTfWXwZAX3bXOEnfZP0bgVdjVAw=
+github.com/Peersyst/evmos/v15 v15.0.0-exrp.1/go.mod h1:15ZOo7jqFRe5elw2ipTb3oHq3x7rebUjwq4y2vJEj4Q=
 github.com/Shopify/sarama v1.19.0/go.mod h1:FVkBWblsNy7DGZRfXLU0O9RCGt5g3g3yEuWXgklEdEo=
 github.com/Shopify/toxiproxy v2.1.4+incompatible/go.mod h1:OXgGpZ6Cli1/URJOF1DMxUHB2q5Ap20/P/eIdh4G0pI=
 github.com/StackExchange/wmi v0.0.0-20180116203802-5d049714c4a6/go.mod h1:3eOhrUMpNV+6aFIbp5/iudMxNCF27Vw2OZgy4xEx0Fg=

proto/packages/blockchain/poa/tx.proto

@@ -1,29 +1,36 @@
 syntax = "proto3";
-
 package packages.blockchain.poa;
 
+import "gogoproto/gogo.proto";
 import "cosmos_proto/cosmos.proto";
 import "cosmos/msg/v1/msg.proto";
+import "cosmos/staking/v1beta1/staking.proto";
+import "google/protobuf/any.proto";
+import "amino/amino.proto";
 
 option go_package = "github.com/Peersyst/exrp/x/poa/types";
 
 // Msg defines the Msg service.
 service Msg {
-  rpc AddValidator           (MsgAddValidator          ) returns (MsgAddValidatorResponse          );
-  rpc RemoveValidator        (MsgRemoveValidator       ) returns (MsgRemoveValidatorResponse       );
+  rpc AddValidator           (MsgAddValidator) returns (MsgAddValidatorResponse);
+  rpc RemoveValidator        (MsgRemoveValidator) returns (MsgRemoveValidatorResponse);
 }
 
 message MsgAddValidator {
   option (cosmos.msg.v1.signer) = "authority";
-  string authority         = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"];
+  option (cosmos.msg.v1.signer) = "validator_address";
+
+  string authority = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"];
   string validator_address = 2 [(cosmos_proto.scalar) = "cosmos.AddressString"];
+  cosmos.staking.v1beta1.Description description = 3 [(gogoproto.nullable) = false, (amino.dont_omitempty) = true];
+  google.protobuf.Any pubkey = 4 [(cosmos_proto.accepts_interface) = "cosmos.crypto.PubKey"];
 }
 
 message MsgAddValidatorResponse {}
 
 message MsgRemoveValidator {
   option (cosmos.msg.v1.signer) = "authority";
-  string authority         = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"];
+  string authority = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"];
   string validator_address = 2 [(cosmos_proto.scalar) = "cosmos.AddressString"];
 }
 

tests/e2e/poa/add_validator_test.go

@@ -1,103 +1,76 @@
 package poa_test
 
 import (
-	"fmt"
+	"github.com/Peersyst/exrp/tests/e2e"
+	sdk "github.com/cosmos/cosmos-sdk/types"
 	govtypesv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1"
 )
 
-func (s *IntegrationTestSuite) Test_AddNewValidator() {
-	fmt.Println("==== Test_AddNewValidator")
+func (s *TestSuite) Test_AddNewValidator() {
+	s.T().Logf("==== Test_AddNewValidator")
 
-	address := "evmos1ycvhcxthjju0466d4ga0j7du7wt8kmaep28zqv"
-	validators := s.network.Validators
+	address, _ := sdk.AccAddressFromBech32("evmos1ycvhcxthjju0466d4ga0j7du7wt8kmaep28zqv")
+	validators := s.Network.Validators
+	pubKey := e2e.GenPubKey()
 
 	// PRE:
-	// Validator has no balance at the beginning
-	s.RequireBondBalance(address, zero)
+	// Validator should not be in the validator set
+	s.RequireValidatorSet().NotContains(pubKey)
 
 	// EXEC:
 	// Make a PoA change to add validator
-	ChangeValidator(s, AddValidatorAction, address, validators, govtypesv1.StatusPassed)
+	e2e.ChangeValidator(&s.IntegrationTestSuite, e2e.AddValidatorAction, address, pubKey, validators, govtypesv1.StatusPassed)
+	// Wait enough to be sure that the validator is in the validator set
+	s.Network.MustWaitForNextBlock()
 
 	// POST:
-	// Validator should have bonded tokens in bank
-	s.RequireBondBalance(address, DefaultBondedTokens)
+	// Validator should be in the validator set
+	s.RequireValidatorSet().Contains(pubKey)
 
-	fmt.Println("==== [V] Test_AddNewValidator")
+	s.T().Logf("==== [V] Test_AddNewValidator")
 }
 
-func (s *IntegrationTestSuite) Test_AddValidatorWithUnboundedTokens() {
-	fmt.Println("==== Test_AddValidatorWithUnboundedTokens")
+func (s *TestSuite) Test_AddValidatorWithUnboundedTokens() {
+	s.T().Logf("==== Test_AddValidatorWithUnboundedTokens")
 
-	address := "evmos1vnaenttkyalgvjus34xxt8h8k0fpuuurdlukaq"
-	validators := s.network.Validators
+	validators := s.Network.Validators
+	validator := s.Network.Validators[s.Cfg.NumBondedValidators]
 
 	// PRE:
 	// Validator has no initial balance
-	s.RequireBondBalance(address, zero)
+	s.RequireBondBalance(validator.Address.String(), e2e.DefaultBondedTokens)
 
 	// EXEC:
 	// Add validator through PoA change twice
-	ChangeValidator(s, AddValidatorAction, address, validators, govtypesv1.StatusPassed)
-	s.RequireBondBalance(address, DefaultBondedTokens)
-	ChangeValidator(s, AddValidatorAction, address, validators, govtypesv1.StatusFailed)
+	e2e.ChangeValidator(&s.IntegrationTestSuite, e2e.AddValidatorAction, validator.Address, validator.PubKey, validators, govtypesv1.StatusFailed)
 
 	// POST:
 	// Only one of the proposals should have passed, validator should have the default power tokens
-	s.RequireBondBalance(address, DefaultBondedTokens)
+	s.RequireBondBalance(validator.Address.String(), e2e.DefaultBondedTokens)
 
-	fmt.Println("==== [V] Test_AddValidatorWithUnboundedTokens")
+	s.T().Logf("==== [V] Test_AddValidatorWithUnboundedTokens")
 }
 
-func (s *IntegrationTestSuite) Test_AddValidatorWithBondedTokens() {
-	fmt.Println("==== Test_AddValidatorWithBondedTokens")
+func (s *TestSuite) Test_AddValidatorWithBondedTokens() {
+	s.T().Logf("==== Test_AddValidatorWithBondedTokens")
 
-	address := s.network.Validators[0].Address.String()
-	validators := s.network.Validators
+	validator := s.Network.Validators[0]
+	address := validator.Address.String()
+	validators := s.Network.Validators
 
 	// PRE:
 	// Validator has no balance in bank and bonded balance in staking
-	s.RequireBondBalance(address, zero)
-	s.RequireValidator(address, &bondedStatus, &DefaultBondedTokens)
+	s.RequireBondBalance(address, e2e.Zero)
+	s.RequireValidator(address, &e2e.BondedStatus, &e2e.DefaultBondedTokens)
 
 	// EXEC:
 	// Add validator through PoA Change
-	ChangeValidator(s, AddValidatorAction, address, validators, govtypesv1.StatusFailed)
+	e2e.ChangeValidator(&s.IntegrationTestSuite, e2e.AddValidatorAction, validator.Address, validator.PubKey, validators, govtypesv1.StatusFailed)
 
 	// POST:
 	// Validator should not have extra balance in bank
-	s.RequireBondBalance(address, zero)
-	s.RequireValidator(address, &bondedStatus, &DefaultBondedTokens)
+	s.RequireBondBalance(address, e2e.Zero)
+	s.RequireValidator(address, &e2e.BondedStatus, &e2e.DefaultBondedTokens)
 
-	fmt.Println("==== [V] Test_AddValidatorWithBondedTokens")
-}
-
-func (s *IntegrationTestSuite) Test_AddUnbondingValidator() {
-	fmt.Println("==== Test_AddUnbondingValidator")
-
-	validator := s.network.Validators[1]
-	validatorAddress := validator.Address.String()
-
-	// PRE:
-	// Validator is bonded and has no balance in bank
-	s.RequireValidator(validatorAddress, &bondedStatus, &DefaultBondedTokens)
-	s.RequireBondBalance(validatorAddress, zero)
-	s.RequireValidatorSet().Contains(validator)
-
-	// EXEC:
-	// Add validator from a poa change but don't wait to be finished
-	ChangeValidator(s, AddValidatorAction, validatorAddress, s.network.Validators, govtypesv1.StatusNil)
-	// Execute unbond tokens so at the moment of the proposal execution the status is unbonding
-	if err := s.network.WaitForNextBlock(); err != nil {
-		panic(err)
-	}
-	UnBondTokens(s, validator, DefaultBondedTokens, true)
-
-	// POST:
-	// Validator should not have any tokens in staking and bonded
-	s.RequireValidator(validatorAddress, nil, nil)
-	s.RequireBondBalance(validatorAddress, DefaultBondedTokens)
-	s.RequireValidatorSet().NotContains(validator)
-
-	fmt.Println("==== [V] Test_AddUnbondingValidator")
+	s.T().Logf("==== [V] Test_AddValidatorWithBondedTokens")
 }

tests/e2e/poa/ante_handler_test.go

@@ -0,0 +1,19 @@
+package poa_test
+
+import (
+	"github.com/Peersyst/exrp/tests/e2e"
+	"strings"
+)
+
+func (s *TestSuite) Test_AnteHandlerForbiddenTransactions() {
+	s.T().Logf("==== Test_AnteHandlerForbiddenTransactions")
+	validator := s.Network.Validators[0]
+	dst := s.Network.Validators[1]
+
+	res := e2e.UnBondTokens(&s.IntegrationTestSuite, validator, e2e.DefaultBondedTokens, false)
+	s.Require().True(strings.Contains(res, "tx type not allowed"))
+	res = e2e.Redelegate(&s.IntegrationTestSuite, validator, dst, e2e.DefaultBondedTokens)
+	s.Require().True(strings.Contains(res, "tx type not allowed"))
+
+	s.T().Logf("==== [V] Test_AnteHandlerForbiddenTransactions")
+}

tests/e2e/poa/delegation_test.go

@@ -1,56 +1,56 @@
 package poa_test
 
 import (
-	"fmt"
+	"github.com/Peersyst/exrp/tests/e2e"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 )
 
-func (s *IntegrationTestSuite) Test_AddDelegationIsNotAllowedToOtherValidators() {
-	fmt.Println("==== Test_AddDelegationIsNotAllowedToOtherValidators")
-	validator := s.network.Validators[0]
+func (s *TestSuite) Test_AddDelegationIsNotAllowedToOtherValidators() {
+	s.T().Logf("==== Test_AddDelegationIsNotAllowedToOtherValidators")
+	validator := s.Network.Validators[0]
 	validatorAddress := validator.Address.String()
-	delegator := s.network.Validators[s.cfg.NumBondedValidators]
+	delegator := s.Network.Validators[s.Cfg.NumBondedValidators]
 	delegatorAddress := delegator.Address.String()
 
 	// Validator should be bonded and have default bonded tokens power
-	s.RequireValidator(validatorAddress, &bondedStatus, &DefaultBondedTokens)
-	s.RequireBondBalance(validatorAddress, zero)
+	s.RequireValidator(validatorAddress, &e2e.BondedStatus, &e2e.DefaultBondedTokens)
+	s.RequireBondBalance(validatorAddress, e2e.Zero)
 	// Delegator should not have any shares and should have default bonded tokens in bank
 	s.RequireDelegation(validatorAddress, delegatorAddress, sdk.ZeroDec())
-	s.RequireBondBalance(delegatorAddress, DefaultBondedTokens)
+	s.RequireBondBalance(delegatorAddress, e2e.DefaultBondedTokens)
 
-	Delegate(s, delegator, validator, DefaultBondedTokens)
+	e2e.Delegate(&s.IntegrationTestSuite, delegator, validator, e2e.DefaultBondedTokens)
 
 	// Delegator should not have any shares and should have default bonded tokens in bank
 	s.RequireDelegation(validatorAddress, delegatorAddress, sdk.ZeroDec())
-	s.RequireBondBalance(delegatorAddress, DefaultBondedTokens)
-	fmt.Println("==== [V] Test_AddDelegationIsNotAllowedToOtherValidators")
+	s.RequireBondBalance(delegatorAddress, e2e.DefaultBondedTokens)
+	s.T().Logf("==== [V] Test_AddDelegationIsNotAllowedToOtherValidators")
 }
 
-func (s *IntegrationTestSuite) Test_AddDelegationIsAllowedToSelfValidator() {
-	fmt.Println("==== Test_AddDelegationIsAllowedToSelfValidator")
-	validator := s.network.Validators[s.cfg.NumBondedValidators]
+func (s *TestSuite) Test_AddDelegationIsAllowedToSelfValidator() {
+	s.T().Logf("==== Test_AddDelegationIsAllowedToSelfValidator")
+	validator := s.Network.Validators[s.Cfg.NumBondedValidators]
 	validatorAddress := validator.Address.String()
 
 	// PRE:
 	// Validator should not be bonded and have default bonded tokens power
 	s.RequireValidator(validatorAddress, nil, nil)
-	s.RequireBondBalance(validatorAddress, DefaultBondedTokens)
+	s.RequireBondBalance(validatorAddress, e2e.DefaultBondedTokens)
 
 	// EXEC:
-	halfTokens := sdk.NewDec(DefaultBondedTokens.Int64()).Quo(sdk.NewDec(2)).RoundInt()
+	halfTokens := sdk.NewDec(e2e.DefaultBondedTokens.Int64()).Quo(sdk.NewDec(2)).RoundInt()
 
-	BondTokens(s, validator, halfTokens)
+	e2e.BondTokens(&s.IntegrationTestSuite, validator, halfTokens)
 
 	// Check validator is active and there are pending bonded tokens in bank
-	s.RequireValidator(validatorAddress, &unbondedStatus, &halfTokens)
+	s.RequireValidator(validatorAddress, &e2e.UnbondedStatus, &halfTokens)
 	s.RequireBondBalance(validatorAddress, halfTokens)
 
-	Delegate(s, validator, validator, halfTokens)
+	e2e.Delegate(&s.IntegrationTestSuite, validator, validator, halfTokens)
 
 	// POST:
 	// Delegator should have all the tokens bonded and delegation should have happened
-	s.RequireValidator(validatorAddress, &bondedStatus, &DefaultBondedTokens)
+	s.RequireValidator(validatorAddress, &e2e.BondedStatus, &e2e.DefaultBondedTokens)
 	s.RequireBondBalance(validatorAddress, sdk.ZeroInt())
-	fmt.Println("==== [V] Test_AddDelegationIsAllowedToSelfValidator")
+	s.T().Logf("==== [V] Test_AddDelegationIsAllowedToSelfValidator")
 }

tests/e2e/poa/poa_suite_test.go

@@ -1,51 +1,20 @@
 package poa_test
 
 import (
-	"fmt"
+	"github.com/Peersyst/exrp/tests/e2e"
 	"github.com/stretchr/testify/suite"
 	"testing"
-
-	"github.com/Peersyst/exrp/testutil/network"
-	"github.com/ethereum/go-ethereum/ethclient"
+	"time"
 )
 
-type IntegrationTestSuite struct {
-	suite.Suite
-
-	proposalCount int
-
-	cfg     network.Config
-	network *network.Network
-}
-
-func (s *IntegrationTestSuite) SetupTest() {
-	s.T().Log("setting up network test suite")
-
-	var err error
-	cfg := network.DefaultConfig(3, 2)
-
-	s.network, err = network.New(s.T(), s.T().TempDir(), cfg)
-	s.cfg = cfg
-	s.proposalCount = 0
-
-	s.Require().NoError(err)
-	s.Require().NotNil(s.network)
-
-	_, err = s.network.WaitForHeight(2)
-	s.Require().NoError(err)
-
-	if s.network.Validators[0].JSONRPCClient == nil {
-		address := fmt.Sprintf("http://%s", s.network.Validators[0].AppConfig.JSONRPC.Address)
-		s.network.Validators[0].JSONRPCClient, err = ethclient.Dial(address)
-		s.Require().NoError(err)
-	}
+type TestSuite struct {
+	e2e.IntegrationTestSuite
 }
 
-func (s *IntegrationTestSuite) TearDownTest() {
-	s.T().Log("tearing down network test suite")
-	s.network.Cleanup()
+func (s *TestSuite) SetupTest() {
+	s.SetupNetwork(3, 2, time.Second, 3)
 }
 
-func TestIntegrationTestSuite(t *testing.T) {
-	suite.Run(t, new(IntegrationTestSuite))
+func Test_TestSuite(t *testing.T) {
+	suite.Run(t, new(TestSuite))
 }