Minor fixes: Cork abci/msg_server refactors, test setup improvements (#…

…217)

* Remove rundundant SetOutgoingTx call when creating contract call

* Use setter method in IncrementInvalidationNonce()

* Refactor GetApprovedScheduledCorks

* Refactor ScheduleCork()

* Remove use of ARHIVE_NODE_URL, and speed up orchestrator health check in test setup

* Loop the validator rewards baseline check due to post-startup instability

* Fix cellarfees test bug by loading the gravity denoms dynamically at setup

* Hardcode 2/3 consensus for cork threshold

* Deprecation comment on cork v2 vote threshold param

* Move cork threshold value to static var

* Adjust cork vote threshold var to const

integration_tests/cellarfees_test.go

@@ -16,9 +16,6 @@ import (
 	corktypes "github.com/peggyjv/sommelier/v4/x/cork/types"
 )
 
-const alphaFeeDenom string = "gravity0x4C4a2f8c81640e47606d3fd77B353E87Ba015584"
-const betaFeeDenom string = "gravity0x21dF544947ba3E8b3c32561399E88B52Dc8b2823"
-
 func (s *IntegrationTestSuite) TestCellarFees() {
 	s.Run("Bring up chain, send fees from ethereum, observe auction and fee distribution", func() {
 		val := s.chain.validators[0]

integration_tests/ethereum/Dockerfile

@@ -11,7 +11,6 @@ RUN npm config set user 0
 
 COPY . .
 
-ENV ARCHIVE_NODE_URL=""
 EXPOSE 8545
 
 RUN yarn run compile

integration_tests/ethereum/addresses.ts

@@ -1,9 +1,9 @@
 export const CELLAR = '0x08c0a0B8D2eDB1d040d4f2C00A1d2f9d9b9F2677';
 export const CELLAR_OWNER = '0xB6C951cf962977f123bF37de42945f7ca1cd2A52';
-export const WHALE = '0xd8da6bf26964af9d7eed9e03e53415d37aa96045';
+export const WHALE = '0xf39fd6e51aad88f6f4ce6ab8827279cfffb92266';
 export const VALIDATORS = [
     '0x14fdAC734De10065093C4Ed4a83C41638378005A',
     '0x7bE2a04df4b9C3227928147461e19158eB2B11d1',
     '0xb8c6886FDDa38adaa0F416722dd5554886C43055',
     '0xd312f0f1B39D54Db2829537595fC1167B14d4b34'
-]
+]

integration_tests/ethereum/hardhat.config.ts

@@ -7,13 +7,6 @@ task(
     'integration_test_setup',
     'Sets up contracts for the integration test',
     async (args, hre) => {
-
-        // Take over vitalik.eth
-        await hre.network.provider.request({
-            method: 'hardhat_impersonateAccount',
-            params: [constants.WHALE],
-        });
-
         // Send ETH to needed parties
         const whaleSigner = await hre.ethers.getSigner(constants.WHALE);
 
@@ -92,21 +85,7 @@ task(
         await hre.run('node');
     });
 
-
-/**
- * @type import('hardhat/config').HardhatUserConfig
- */
-const ARCHIVE_NODE_URL = process.env.ARCHIVE_NODE_URL;
-
 module.exports = {
-    networks: {
-        hardhat: {
-            forking: {
-                url: ARCHIVE_NODE_URL,
-                blockNumber: 13405367,
-            },
-        },
-    },
     solidity: {
         compilers: [
             {

integration_tests/incentives_test.go

@@ -53,10 +53,17 @@ func (s *IntegrationTestSuite) TestIncentives() {
 		s.Require().NoError(err)
 		s.Require().True(initialAPY.IsZero())
 
-		beforeAmount := s.queryValidatorRewards(ctx, valOperatorAddress, distQueryClient)
-		time.Sleep(time.Second * 12)
-		afterAmount := s.queryValidatorRewards(ctx, valOperatorAddress, distQueryClient)
-		s.Require().Equal(beforeAmount, afterAmount)
+		// this is looped because for some reason running the test too quickly after the validator
+		// containers are launched results in the validator rewards pool result increasing before
+		// reaching a steady state
+		s.T().Log("verifying validator rewards are not increasing")
+		s.Require().Eventually(func() bool {
+			beforeAmount := s.queryValidatorRewards(ctx, valOperatorAddress, distQueryClient)
+			time.Sleep(time.Second * 12)
+			afterAmount := s.queryValidatorRewards(ctx, valOperatorAddress, distQueryClient)
+
+			return beforeAmount.Equal(afterAmount)
+		}, 120*time.Second, 12*time.Second)
 
 		s.T().Log("submitting proposal to enable incentives")
 		proposer := s.chain.proposer

integration_tests/setup_test.go

@@ -62,6 +62,9 @@ var (
 	betaERC20Contract     = common.HexToAddress("0x0000000000000000000000000000000000000000")
 	unusedGenesisContract = common.HexToAddress("0x0000000000000000000000000000000000000001")
 
+	alphaFeeDenom = ""
+	betaFeeDenom  = ""
+
 	// 67%
 	corkVoteThreshold = sdk.NewDecWithPrec(67, 2)
 
@@ -367,6 +370,8 @@ func (s *IntegrationTestSuite) initGenesis() {
 	s.Require().NoError(cdc.UnmarshalJSON(appGenState[genutiltypes.ModuleName], &genUtilGenState))
 
 	// Add an auction for integration testing of the auction module
+	alphaFeeDenom = fmt.Sprintf("gravity%s", alphaERC20Contract.Hex())
+	betaFeeDenom = fmt.Sprintf("gravity%s", betaERC20Contract.Hex())
 	var auctionGenState auctiontypes.GenesisState
 	s.Require().NoError(cdc.UnmarshalJSON(appGenState[auctiontypes.ModuleName], &auctionGenState))
 	auctionGenState.TokenPrices = append(auctionGenState.TokenPrices, &auctiontypes.TokenPrice{
@@ -534,10 +539,6 @@ func (s *IntegrationTestSuite) initValidatorConfigs() {
 func (s *IntegrationTestSuite) runEthContainer() {
 	s.T().Log("starting Ethereum container...")
 	var err error
-
-	nodeURL := os.Getenv("ARCHIVE_NODE_URL")
-	s.Require().NotEmptyf(nodeURL, "ARCHIVE_NODE_URL env variable must be set")
-
 	runOpts := dockertest.RunOptions{
 		Name:       "ethereum",
 		Repository: "ethereum",
@@ -547,7 +548,6 @@ func (s *IntegrationTestSuite) runEthContainer() {
 			"8545/tcp": {{HostIP: "", HostPort: "8545"}},
 		},
 		ExposedPorts: []string{"8545/tcp"},
-		Env:          []string{fmt.Sprintf("ARCHIVE_NODE_URL=%s", nodeURL)},
 	}
 
 	s.ethResource, err = s.dockerPool.RunWithOptions(
@@ -810,7 +810,7 @@ msg_batch_size = 5
 				return strings.Contains(containerLogsBuf.String(), match)
 			},
 			3*time.Minute,
-			20*time.Second,
+			1*time.Second,
 			"orchestrator %s not healthy",
 			resource.Container.ID,
 		)

proto/cork/v2/genesis.proto

@@ -22,6 +22,7 @@ message GenesisState {
 
 // Params cork parameters
 message Params {
+    // Deprecated
     // VoteThreshold defines the percentage of bonded stake required to vote for a scheduled cork to be approved
     string vote_threshold      = 1 [
         (gogoproto.moretags)   = "yaml:\"vote_threshold\"",

x/cork/keeper/abci.go

@@ -20,15 +20,15 @@ func (k Keeper) submitContractCall(ctx sdk.Context, cork types.Cork) {
 	// increment invalidation nonce
 	invalidationNonce := k.IncrementInvalidationNonce(ctx)
 	// submit contract call to bridge
-	contractCall := k.gravityKeeper.CreateContractCallTx(
+	k.gravityKeeper.CreateContractCallTx(
 		ctx,
 		invalidationNonce,
 		cork.InvalidationScope(),
 		common.HexToAddress(cork.TargetContractAddress),
 		cork.EncodedContractCall,
 		[]gravitytypes.ERC20Token{}, // tokens are always zero
-		[]gravitytypes.ERC20Token{})
-	k.gravityKeeper.SetOutgoingTx(ctx, contractCall)
+		[]gravitytypes.ERC20Token{},
+	)
 }
 
 // EndBlocker defines the oracle logic that executes at the end of every block:
@@ -38,10 +38,8 @@ func (k Keeper) submitContractCall(ctx sdk.Context, cork types.Cork) {
 // 2) Submits all winning votes as contract calls via the gravity bridge
 
 func (k Keeper) EndBlocker(ctx sdk.Context) {
-	params := k.GetParamSet(ctx)
-
 	k.Logger(ctx).Info("tallying scheduled cork votes", "height", fmt.Sprintf("%d", ctx.BlockHeight()))
-	winningScheduledVotes := k.GetApprovedScheduledCorks(ctx, uint64(ctx.BlockHeight()), params.VoteThreshold)
+	winningScheduledVotes := k.GetApprovedScheduledCorks(ctx)
 	if len(winningScheduledVotes) > 0 {
 		k.Logger(ctx).Info("packaging all winning scheduled cork votes into contract calls",
 			"winning votes", winningScheduledVotes)

x/cork/keeper/keeper.go

@@ -11,6 +11,8 @@ import (
 	"github.com/tendermint/tendermint/libs/log"
 )
 
+const corkVoteThresholdStr = "0.67"
+
 // Keeper of the oracle store
 type Keeper struct {
 	storeKey      sdk.StoreKey
@@ -206,9 +208,8 @@ func (k Keeper) SetLatestInvalidationNonce(ctx sdk.Context, invalidationNonce ui
 }
 
 func (k Keeper) IncrementInvalidationNonce(ctx sdk.Context) uint64 {
-	store := ctx.KVStore(k.storeKey)
 	nextNonce := k.GetLatestInvalidationNonce(ctx) + 1
-	store.Set([]byte{types.LatestInvalidationNonceKey}, sdk.Uint64ToBigEndian(nextNonce))
+	k.SetLatestInvalidationNonce(ctx, nextNonce)
 	return nextNonce
 }
 
@@ -271,59 +272,51 @@ func (k Keeper) GetCorkResults(ctx sdk.Context) []*types.CorkResult {
 // Votes //
 ///////////
 
-func (k Keeper) GetApprovedScheduledCorks(ctx sdk.Context, currentBlockHeight uint64, threshold sdk.Dec) (approvedCorks []types.Cork) {
-	corksForBlockHeight := make(map[uint64][]types.Cork)
-	corkPowersForBlockHeight := make(map[uint64][]uint64)
-
+func (k Keeper) GetApprovedScheduledCorks(ctx sdk.Context) (approvedCorks []types.Cork) {
+	currentBlockHeight := uint64(ctx.BlockHeight())
 	totalPower := k.stakingKeeper.GetLastTotalPower(ctx)
-
-	k.IterateScheduledCorks(ctx, func(val sdk.ValAddress, scheduledBlockHeight uint64, id []byte, addr common.Address, cork types.Cork) (stop bool) {
-		if currentBlockHeight != scheduledBlockHeight {
-			// only operate on scheduled corksForBlockHeight that are valid, quit early when a further one is found
-			return true
-		}
-
+	corks := []types.Cork{}
+	powers := []uint64{}
+	k.IterateScheduledCorksByBlockHeight(ctx, currentBlockHeight, func(val sdk.ValAddress, _ uint64, id []byte, addr common.Address, cork types.Cork) (stop bool) {
 		validator := k.stakingKeeper.Validator(ctx, val)
 		validatorPower := uint64(validator.GetConsensusPower(k.stakingKeeper.PowerReduction(ctx)))
-
 		found := false
-		for i, c := range corksForBlockHeight[scheduledBlockHeight] {
+		for i, c := range corks {
 			if c.Equals(cork) {
-				corkPowersForBlockHeight[scheduledBlockHeight][i] += validatorPower
+				powers[i] += validatorPower
 
 				found = true
 				break
 			}
 		}
 
 		if !found {
-			corksForBlockHeight[scheduledBlockHeight] = append(corksForBlockHeight[scheduledBlockHeight], cork)
-			corkPowersForBlockHeight[scheduledBlockHeight] = append(corkPowersForBlockHeight[scheduledBlockHeight], validatorPower)
+			corks = append(corks, cork)
+			powers = append(powers, validatorPower)
 		}
 
-		k.DeleteScheduledCork(ctx, scheduledBlockHeight, id, val, addr)
+		k.DeleteScheduledCork(ctx, currentBlockHeight, id, val, addr)
 
 		return false
 	})
 
-	for blockHeight := range corkPowersForBlockHeight {
-		for i, power := range corkPowersForBlockHeight[blockHeight] {
-			cork := corksForBlockHeight[blockHeight][i]
-			approvalPercentage := sdk.NewIntFromUint64(power).ToDec().Quo(totalPower.ToDec())
-			quorumReached := approvalPercentage.GT(threshold)
-			corkResult := types.CorkResult{
-				Cork:               &cork,
-				BlockHeight:        blockHeight,
-				Approved:           quorumReached,
-				ApprovalPercentage: approvalPercentage.Mul(sdk.NewDec(100)).String(),
-			}
-			corkID := cork.IDHash(blockHeight)
+	threshold := sdk.MustNewDecFromStr(corkVoteThresholdStr)
+	for i, power := range powers {
+		cork := corks[i]
+		approvalPercentage := sdk.NewIntFromUint64(power).ToDec().Quo(totalPower.ToDec())
+		quorumReached := approvalPercentage.GT(threshold)
+		corkResult := types.CorkResult{
+			Cork:               &cork,
+			BlockHeight:        currentBlockHeight,
+			Approved:           quorumReached,
+			ApprovalPercentage: approvalPercentage.Mul(sdk.NewDec(100)).String(),
+		}
+		corkID := cork.IDHash(currentBlockHeight)
 
-			k.SetCorkResult(ctx, corkID, corkResult)
+		k.SetCorkResult(ctx, corkID, corkResult)
 
-			if quorumReached {
-				approvedCorks = append(approvedCorks, cork)
-			}
+		if quorumReached {
+			approvedCorks = append(approvedCorks, cork)
 		}
 	}
 

x/cork/keeper/keeper_test.go

@@ -145,6 +145,9 @@ func (suite *KeeperTestSuite) TestGetWinningVotes() {
 	ctx, corkKeeper := suite.ctx, suite.corkKeeper
 	require := suite.Require()
 	testHeight := uint64(ctx.BlockHeight())
+	params := types.DefaultParams()
+	params.VoteThreshold = sdk.ZeroDec()
+	corkKeeper.setParams(ctx, params)
 	cork := types.Cork{
 		EncodedContractCall:   []byte("testcall"),
 		TargetContractAddress: sampleCellarHex,
@@ -159,7 +162,7 @@ func (suite *KeeperTestSuite) TestGetWinningVotes() {
 	suite.validator.EXPECT().GetConsensusPower(gomock.Any()).Return(int64(100))
 	suite.stakingKeeper.EXPECT().PowerReduction(ctx).Return(sdk.OneInt())
 
-	winningScheduledVotes := corkKeeper.GetApprovedScheduledCorks(ctx, testHeight, sdk.ZeroDec())
+	winningScheduledVotes := corkKeeper.GetApprovedScheduledCorks(ctx)
 	results := corkKeeper.GetCorkResults(ctx)
 	require.Equal(cork, winningScheduledVotes[0])
 	require.Equal(&cork, results[0].Cork)

x/cork/keeper/msg_server.go

@@ -7,30 +7,13 @@ import (
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
-	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
 	"github.com/ethereum/go-ethereum/common"
 
 	"github.com/peggyjv/sommelier/v4/x/cork/types"
 )
 
 var _ types.MsgServer = Keeper{}
 
-func (k Keeper) signerToValAddr(ctx sdk.Context, signer sdk.AccAddress) (sdk.ValAddress, error) {
-	validatorAddr := k.gravityKeeper.GetOrchestratorValidatorAddress(ctx, signer)
-	if validatorAddr == nil {
-		validator := k.stakingKeeper.Validator(ctx, sdk.ValAddress(signer))
-		if validator == nil {
-			return nil, sdkerrors.Wrap(stakingtypes.ErrNoValidatorFound, sdk.ValAddress(signer).String())
-		}
-
-		validatorAddr = validator.GetOperator()
-		// NOTE: we set the validator address so we don't have to call look up for the validator
-		// everytime a validator feeder submits oracle data
-		k.gravityKeeper.SetOrchestratorValidatorAddress(ctx, validatorAddr, signer)
-	}
-	return validatorAddr, nil
-}
-
 // ScheduleCork implements types.MsgServer
 func (k Keeper) ScheduleCork(c context.Context, msg *types.MsgScheduleCorkRequest) (*types.MsgScheduleCorkResponse, error) {
 	ctx := sdk.UnwrapSDKContext(c)
@@ -44,9 +27,9 @@ func (k Keeper) ScheduleCork(c context.Context, msg *types.MsgScheduleCorkReques
 	}
 
 	signer := msg.MustGetSigner()
-	validatorAddr, err := k.signerToValAddr(ctx, signer)
-	if err != nil {
-		return nil, err
+	validatorAddr := k.gravityKeeper.GetOrchestratorValidatorAddress(ctx, signer)
+	if validatorAddr == nil {
+		return nil, sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "signer %s is not a delegate", signer.String())
 	}
 
 	corkID := k.SetScheduledCork(ctx, msg.BlockHeight, validatorAddr, *msg.Cork)

x/cork/keeper/test_common.go

@@ -183,9 +183,8 @@ var (
 	}
 
 	// TestingcorkParams is a set of gravity params for testing
-	threshold, _      = sdk.NewDecFromStr("0.66")
 	TestingcorkParams = types.Params{
-		VoteThreshold: threshold,
+		VoteThreshold: sdk.MustNewDecFromStr(corkVoteThresholdStr),
 	}
 )
 

x/cork/types/params.go

@@ -24,6 +24,7 @@ func ParamKeyTable() paramtypes.KeyTable {
 // DefaultParams returns default oracle parameters
 func DefaultParams() Params {
 	return Params{
+		// Deprecated
 		VoteThreshold: sdk.NewDecWithPrec(67, 2), // 67%
 	}
 }
@@ -53,7 +54,7 @@ func validateVoteThreshold(i interface{}) error {
 		return errors.New("vote threshold cannot be nil")
 	}
 
-	if voteThreshold.LTE(sdk.ZeroDec()) || voteThreshold.GT(sdk.OneDec()) {
+	if voteThreshold.LT(sdk.ZeroDec()) || voteThreshold.GT(sdk.OneDec()) {
 		return fmt.Errorf("vote threshold value must be within the 0% - 100% range, got: %s", voteThreshold)
 	}
 

x/cork/types/params_test.go

@@ -26,7 +26,7 @@ func TestParamsValidate(t *testing.T) {
 		{
 			name: "invalid vote threshold",
 			params: Params{
-				VoteThreshold: sdk.ZeroDec(),
+				VoteThreshold: sdk.NewDec(-1),
 			},
 			expPass: false,
 		},