Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace hardcoded constants with params #393

Merged
merged 21 commits into from
Oct 20, 2022
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion proto/interchain_security/ccv/consumer/v1/consumer.proto
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,21 @@ message Params {
// provider handshake procedure.
string distribution_transmission_channel = 3;
string provider_fee_pool_addr_str = 4;
// Sent IBC packets will timeout after this duration
// Sent CCV related IBC packets will timeout after this duration
google.protobuf.Duration ccv_timeout_period = 5
[(gogoproto.nullable) = false, (gogoproto.stdduration) = true];

// Sent transfer related IBC packets will timeout after this duration
google.protobuf.Duration transfer_timeout_period = 6
[(gogoproto.nullable) = false, (gogoproto.stdduration) = true];

// The fraction of tokens allocated to the consumer redistribution address
// during distribution events. The fraction is a string representing a
// decimal number. For example "0.75" would represent 75%.
string consumer_redistribution_fraction = 7;

// The number of historical info entries to persist in store
int64 num_historical_entries = 8;
shaspitz marked this conversation as resolved.
Show resolved Hide resolved
}

// LastTransmissionBlockHeight is the last time validator holding
Expand Down
2 changes: 2 additions & 0 deletions proto/interchain_security/ccv/provider/v1/provider.proto
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ message Params {
// Sent IBC packets will timeout after this duration
google.protobuf.Duration ccv_timeout_period = 2
[(gogoproto.nullable) = false, (gogoproto.stdduration) = true];
// TrustingPeriodFraction is used to compute the provider IBC client's TrustingPeriod
shaspitz marked this conversation as resolved.
Show resolved Hide resolved
int64 trusting_period_fraction = 3;
}

message HandshakeMetadata {
Expand Down
3 changes: 3 additions & 0 deletions tests/difference/core/driver/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,9 @@ func (b *Builder) createConsumerGenesis(tmConfig *ibctesting.TendermintConfig) *
"", // ignore distribution
"", // ignore distribution
ccv.DefaultCCVTimeoutPeriod,
consumertypes.DefaultTransferTimeoutPeriod,
consumertypes.DefaultConsumerRedistributeFrac,
consumertypes.DefaultNumHistoricalEntries,
)
return consumertypes.NewInitialGenesisState(providerClient, providerConsState, valUpdates, consumertypes.SlashRequests{}, params)
}
Expand Down
5 changes: 3 additions & 2 deletions tests/e2e/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"

providertypes "github.com/cosmos/interchain-security/x/ccv/provider/types"
ccv "github.com/cosmos/interchain-security/x/ccv/types"
"github.com/cosmos/interchain-security/x/ccv/utils"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -334,7 +335,7 @@ func incrementTimeBy(s *CCVTestSuite, jumpPeriod time.Duration) {
consumerUnbondingPeriod, found := s.consumerChain.App.(*appConsumer.App).ConsumerKeeper.GetUnbondingTime(s.consumerChain.GetContext())
s.Require().True(found)
split := 1
if jumpPeriod > consumerUnbondingPeriod/utils.TrustingPeriodFraction {
if jumpPeriod > consumerUnbondingPeriod/providertypes.DefaultTrustingPeriodFraction {
shaspitz marked this conversation as resolved.
Show resolved Hide resolved
// Make sure the clients do not expire
split = 4
jumpPeriod = jumpPeriod / 4
Expand Down Expand Up @@ -365,7 +366,7 @@ func (suite *CCVTestSuite) CreateCustomClient(endpoint *ibctesting.Endpoint, unb
tmConfig, ok := endpoint.ClientConfig.(*ibctesting.TendermintConfig)
require.True(endpoint.Chain.T, ok)
tmConfig.UnbondingPeriod = unbondingPeriod
tmConfig.TrustingPeriod = unbondingPeriod / utils.TrustingPeriodFraction
tmConfig.TrustingPeriod = unbondingPeriod / providertypes.DefaultTrustingPeriodFraction

height := endpoint.Counterparty.Chain.LastHeader.GetHeight().(clienttypes.Height)
UpgradePath := []string{"upgrade", "upgradedIBCState"}
Expand Down
3 changes: 1 addition & 2 deletions tests/e2e/democracy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,12 @@ import (
proposaltypes "github.com/cosmos/cosmos-sdk/x/params/types/proposal"
appConsumer "github.com/cosmos/interchain-security/app/consumer-democracy"
"github.com/cosmos/interchain-security/testutil/simapp"
consumerkeeper "github.com/cosmos/interchain-security/x/ccv/consumer/keeper"
consumertypes "github.com/cosmos/interchain-security/x/ccv/consumer/types"

"github.com/stretchr/testify/suite"
)

var consumerFraction, _ = sdk.NewDecFromStr(consumerkeeper.ConsumerRedistributeFrac)
var consumerFraction, _ = sdk.NewDecFromStr(consumertypes.DefaultConsumerRedistributeFrac)
shaspitz marked this conversation as resolved.
Show resolved Hide resolved

type ConsumerDemocracyTestSuite struct {
suite.Suite
Expand Down
3 changes: 1 addition & 2 deletions tests/e2e/distribution_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
transfertypes "github.com/cosmos/ibc-go/v3/modules/apps/transfer/types"
app "github.com/cosmos/interchain-security/app/consumer"
providerApp "github.com/cosmos/interchain-security/app/provider"
consumerkeeper "github.com/cosmos/interchain-security/x/ccv/consumer/keeper"
consumertypes "github.com/cosmos/interchain-security/x/ccv/consumer/types"
ccv "github.com/cosmos/interchain-security/x/ccv/types"
)
Expand Down Expand Up @@ -45,7 +44,7 @@ func (s *CCVTestSuite) TestRewardsDistribution() {
s.Require().Equal(sdk.NewInt(100).Add(feePoolTokensOld.AmountOf(sdk.DefaultBondDenom)), feePoolTokens.AmountOf(sdk.DefaultBondDenom))

//calculate the reward for consumer and provider chain. Consumer will receive ConsumerRedistributeFrac, the rest is going to provider
frac, err := sdk.NewDecFromStr(consumerkeeper.ConsumerRedistributeFrac)
frac, err := sdk.NewDecFromStr(consumertypes.DefaultConsumerRedistributeFrac)
shaspitz marked this conversation as resolved.
Show resolved Hide resolved
s.Require().NoError(err)
consumerExpectedRewards, _ := sdk.NewDecCoinsFromCoins(feePoolTokens...).MulDec(frac).TruncateDecimal()
providerExpectedRewards := feePoolTokens.Sub(consumerExpectedRewards)
Expand Down
8 changes: 4 additions & 4 deletions tests/e2e/normal_operations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package e2e_test
import (
"github.com/cosmos/cosmos-sdk/crypto/keys/ed25519"
appConsumer "github.com/cosmos/interchain-security/app/consumer"
"github.com/cosmos/interchain-security/x/ccv/consumer/types"
consumertypes "github.com/cosmos/interchain-security/x/ccv/consumer/types"
tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
)

Expand All @@ -22,7 +22,7 @@ func (k CCVTestSuite) TestTrackHistoricalInfo() {
createVal := func(k CCVTestSuite) {
// add new validator to consumer states
pk := ed25519.GenPrivKey().PubKey()
cVal, err := types.NewCCValidator(pk.Address(), int64(1), pk)
cVal, err := consumertypes.NewCCValidator(pk.Address(), int64(1), pk)
k.Require().NoError(err)

consumerKeeper.SetCCValidator(k.consumerChain.GetContext(), cVal)
Expand All @@ -39,7 +39,7 @@ func (k CCVTestSuite) TestTrackHistoricalInfo() {
createVal,
createVal,
func(k CCVTestSuite) {
newHeight := k.consumerChain.GetContext().BlockHeight() + int64(types.HistoricalEntries)
newHeight := k.consumerChain.GetContext().BlockHeight() + int64(consumertypes.DefaultNumHistoricalEntries)
shaspitz marked this conversation as resolved.
Show resolved Hide resolved
header := tmproto.Header{
ChainID: "HelloChain",
Height: newHeight,
Expand Down Expand Up @@ -71,7 +71,7 @@ func (k CCVTestSuite) TestTrackHistoricalInfo() {
expLen: 0,
},
{
height: initHeight + int64(types.HistoricalEntries) + 2,
height: initHeight + int64(consumertypes.DefaultNumHistoricalEntries) + 2,
mpoke marked this conversation as resolved.
Show resolved Hide resolved
found: true,
expLen: initValsetLen + 2,
},
Expand Down
5 changes: 3 additions & 2 deletions tests/e2e/setup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"bytes"
"testing"

providertypes "github.com/cosmos/interchain-security/x/ccv/provider/types"
ccv "github.com/cosmos/interchain-security/x/ccv/types"
"github.com/cosmos/interchain-security/x/ccv/utils"

Expand Down Expand Up @@ -95,10 +96,10 @@ func (suite *CCVTestSuite) SetupTest() {
// - client config
providerUnbondingPeriod := suite.providerChain.App.(*appProvider.App).GetStakingKeeper().UnbondingTime(suite.providerCtx())
suite.path.EndpointB.ClientConfig.(*ibctesting.TendermintConfig).UnbondingPeriod = providerUnbondingPeriod
suite.path.EndpointB.ClientConfig.(*ibctesting.TendermintConfig).TrustingPeriod = providerUnbondingPeriod / utils.TrustingPeriodFraction
suite.path.EndpointB.ClientConfig.(*ibctesting.TendermintConfig).TrustingPeriod = providerUnbondingPeriod / providertypes.DefaultTrustingPeriodFraction
shaspitz marked this conversation as resolved.
Show resolved Hide resolved
consumerUnbondingPeriod := utils.ComputeConsumerUnbondingPeriod(providerUnbondingPeriod)
suite.path.EndpointA.ClientConfig.(*ibctesting.TendermintConfig).UnbondingPeriod = consumerUnbondingPeriod
suite.path.EndpointA.ClientConfig.(*ibctesting.TendermintConfig).TrustingPeriod = consumerUnbondingPeriod / utils.TrustingPeriodFraction
suite.path.EndpointA.ClientConfig.(*ibctesting.TendermintConfig).TrustingPeriod = consumerUnbondingPeriod / providertypes.DefaultTrustingPeriodFraction
shaspitz marked this conversation as resolved.
Show resolved Hide resolved
// - channel config
suite.path.EndpointA.ChannelConfig.PortID = ccv.ConsumerPortID
suite.path.EndpointB.ChannelConfig.PortID = ccv.ProviderPortID
Expand Down
14 changes: 3 additions & 11 deletions x/ccv/consumer/keeper/distribution.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package keeper

import (
"time"

sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"

Expand All @@ -14,13 +12,6 @@ import (
ccv "github.com/cosmos/interchain-security/x/ccv/types"
)

const TransferTimeDelay = 1 * 7 * 24 * time.Hour // 1 weeks

// The fraction of tokens allocated to the consumer redistribution address
// during distribution events. The fraction is a string representing a
// decimal number. For example "0.75" would represent 75%.
const ConsumerRedistributeFrac = "0.75"

// Simple model, send tokens to the fee pool of the provider validator set
// reference: cosmos/ibc-go/v3/modules/apps/transfer/keeper/msg_server.go
func (k Keeper) DistributeToProviderValidatorSet(ctx sdk.Context) error {
Expand All @@ -34,7 +25,7 @@ func (k Keeper) DistributeToProviderValidatorSet(ctx sdk.Context) error {
fpTokens := k.bankKeeper.GetAllBalances(ctx, consumerFeePoolAddr)

// split the fee pool, send the consumer's fraction to the consumer redistribution address
frac, err := sdk.NewDecFromStr(ConsumerRedistributeFrac)
frac, err := sdk.NewDecFromStr(k.GetConsumerRedistributionFrac(ctx))
if err != nil {
return err
}
Expand Down Expand Up @@ -76,7 +67,8 @@ func (k Keeper) DistributeToProviderValidatorSet(ctx sdk.Context) error {
tstProviderTokens := k.bankKeeper.GetAllBalances(ctx, tstProviderAddr)
providerAddr := k.GetProviderFeePoolAddrStr(ctx)
timeoutHeight := clienttypes.ZeroHeight()
timeoutTimestamp := uint64(ctx.BlockTime().Add(TransferTimeDelay).UnixNano())
transferTimeoutPeriod := k.GetTransferTimeoutPeriod(ctx)
timeoutTimestamp := uint64(ctx.BlockTime().Add(transferTimeoutPeriod).UnixNano())
for _, token := range tstProviderTokens {
err := k.ibcTransferKeeper.SendTransfer(ctx,
transfertypes.PortID,
Expand Down
21 changes: 19 additions & 2 deletions x/ccv/consumer/keeper/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,15 @@ func TestInitGenesis(t *testing.T) {
}

// create parameters for a new chain
params := types.NewParams(true, types.DefaultBlocksPerDistributionTransmission, "", "", ccv.DefaultCCVTimeoutPeriod)
params := types.NewParams(true,
types.DefaultBlocksPerDistributionTransmission,
"",
"",
ccv.DefaultCCVTimeoutPeriod,
consumertypes.DefaultTransferTimeoutPeriod,
consumertypes.DefaultConsumerRedistributeFrac,
consumertypes.DefaultNumHistoricalEntries,
)

testCases := []struct {
name string
Expand Down Expand Up @@ -152,7 +160,16 @@ func TestExportGenesis(t *testing.T) {
MaturityTime: uint64(time.Now().UnixNano()),
}

params := types.NewParams(true, types.DefaultBlocksPerDistributionTransmission, "", "", ccv.DefaultCCVTimeoutPeriod)
params := types.NewParams(
true,
types.DefaultBlocksPerDistributionTransmission,
"",
"",
ccv.DefaultCCVTimeoutPeriod,
consumertypes.DefaultTransferTimeoutPeriod,
consumertypes.DefaultConsumerRedistributeFrac,
consumertypes.DefaultNumHistoricalEntries,
)

// create a single validator
pubKey := ed25519.GenPrivKey().PubKey()
Expand Down
30 changes: 28 additions & 2 deletions x/ccv/consumer/keeper/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,17 @@ import (
ccvtypes "github.com/cosmos/interchain-security/x/ccv/types"
)

// GetParams returns the paramset for the consumer module
// GetParams returns the params for the consumer ccv module
func (k Keeper) GetParams(ctx sdk.Context) types.Params {
return types.NewParams(
k.GetEnabled(ctx),
k.GetBlocksPerDistributionTransmission(ctx),
k.GetDistributionTransmissionChannel(ctx),
k.GetProviderFeePoolAddrStr(ctx),
k.GetCCVTimeoutPeriod(ctx),
k.GetTransferTimeoutPeriod(ctx),
k.GetConsumerRedistributionFrac(ctx),
k.GetNumHistoricalEntries(ctx),
)
}

Expand Down Expand Up @@ -62,9 +65,32 @@ func (k Keeper) SetProviderFeePoolAddrStr(ctx sdk.Context, addr string) {
k.paramStore.Set(ctx, types.KeyProviderFeePoolAddrStr, addr)
}

// GetCCVTimeoutPeriod returns the timeout period for sent ibc packets
// GetCCVTimeoutPeriod returns the timeout period for sent ccv related ibc packets
func (k Keeper) GetCCVTimeoutPeriod(ctx sdk.Context) time.Duration {
var p time.Duration
k.paramStore.Get(ctx, ccvtypes.KeyCCVTimeoutPeriod, &p)
return p
}

// GetTransferTimeoutPeriod returns the timeout period for sent transfer related ibc packets
shaspitz marked this conversation as resolved.
Show resolved Hide resolved
func (k Keeper) GetTransferTimeoutPeriod(ctx sdk.Context) time.Duration {
var p time.Duration
k.paramStore.Get(ctx, types.KeyTransferTimeoutPeriod, &p)
return p
}

// GetConsumerRedistributionFrac returns the fraction of tokens allocated to the consumer redistribution
// address during distribution events. The fraction is a string representing a
// decimal number. For example "0.75" would represent 75%.
func (k Keeper) GetConsumerRedistributionFrac(ctx sdk.Context) string {
var str string
k.paramStore.Get(ctx, types.KeyConsumerRedistributionFrac, &str)
return str
}

// GetNumHistoricalEntries returns the number of historical info entries to persist in store
func (k Keeper) GetNumHistoricalEntries(ctx sdk.Context) int64 {
var n int64
k.paramStore.Get(ctx, types.KeyNumHistoricalEntries, &n)
return n
}
14 changes: 12 additions & 2 deletions x/ccv/consumer/keeper/params_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

testkeeper "github.com/cosmos/interchain-security/testutil/keeper"
"github.com/cosmos/interchain-security/x/ccv/consumer/types"
consumertypes "github.com/cosmos/interchain-security/x/ccv/consumer/types"
ccv "github.com/cosmos/interchain-security/x/ccv/types"
"github.com/stretchr/testify/require"
)
Expand All @@ -16,13 +17,22 @@ func TestParams(t *testing.T) {
defer ctrl.Finish()
consumerKeeper.SetParams(ctx, types.DefaultParams())

expParams := types.NewParams(false, 1000, "", "", ccv.DefaultCCVTimeoutPeriod) // these are the default params, IBC suite independently sets enabled=true
expParams := types.NewParams(
false,
1000,
"",
"",
ccv.DefaultCCVTimeoutPeriod,
consumertypes.DefaultTransferTimeoutPeriod,
consumertypes.DefaultConsumerRedistributeFrac,
consumertypes.DefaultNumHistoricalEntries,
) // these are the default params, IBC suite independently sets enabled=true

params := consumerKeeper.GetParams(ctx)
require.Equal(t, expParams, params)

newParams := types.NewParams(false, 1000,
"channel-2", "cosmos19pe9pg5dv9k5fzgzmsrgnw9rl9asf7ddwhu7lm", 7*24*time.Hour)
"channel-2", "cosmos19pe9pg5dv9k5fzgzmsrgnw9rl9asf7ddwhu7lm", 7*24*time.Hour, 25*time.Hour, "0.5", 500)
consumerKeeper.SetParams(ctx, newParams)
params = consumerKeeper.GetParams(ctx)
require.Equal(t, newParams, params)
Expand Down
6 changes: 3 additions & 3 deletions x/ccv/consumer/keeper/validators.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ func (k Keeper) DeleteHistoricalInfo(ctx sdk.Context, height int64) {
// heights that are below pruning height
func (k Keeper) TrackHistoricalInfo(ctx sdk.Context) {

entryNum := types.HistoricalEntries
numHistoricalEntries := k.GetNumHistoricalEntries(ctx)

// Prune store to ensure we only have parameter-defined historical entries.
// In most cases, this will involve removing a single historical entry.
Expand All @@ -163,7 +163,7 @@ func (k Keeper) TrackHistoricalInfo(ctx sdk.Context) {
// Since the entries to be deleted are always in a continuous range, we can iterate
// over the historical entries starting from the most recent version to be pruned
// and then return at the first empty entry.
for i := ctx.BlockHeight() - int64(entryNum); i >= 0; i-- {
for i := ctx.BlockHeight() - int64(numHistoricalEntries); i >= 0; i-- {
_, found := k.GetHistoricalInfo(ctx, i)
if found {
k.DeleteHistoricalInfo(ctx, i)
Expand All @@ -173,7 +173,7 @@ func (k Keeper) TrackHistoricalInfo(ctx sdk.Context) {
}

// if there is no need to persist historicalInfo, return
if entryNum == 0 {
if numHistoricalEntries == 0 {
return
}

Expand Down
Loading