Skip to content

Commit

Permalink
fix: resolve TODO comments
Browse files Browse the repository at this point in the history
Merge pull request #209 from hallazzang/resolve-todos-1
  • Loading branch information
hallazzang authored Nov 9, 2021
2 parents f137b35 + d64f044 commit 494de4f
Show file tree
Hide file tree
Showing 9 changed files with 116 additions and 29 deletions.
1 change: 0 additions & 1 deletion x/farming/keeper/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ func (k Keeper) InitGenesis(ctx sdk.Context, genState types.GenesisState) {
ctx, writeCache := ctx.CacheContext()

k.SetParams(ctx, genState.Params)
// TODO: what if CurrentEpochDays field was empty?
k.SetCurrentEpochDays(ctx, genState.CurrentEpochDays)
moduleAcc := k.accountKeeper.GetModuleAccount(ctx, types.ModuleName)
k.accountKeeper.SetModuleAccount(ctx, moduleAcc)
Expand Down
7 changes: 7 additions & 0 deletions x/farming/keeper/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,13 @@ func (suite *KeeperTestSuite) TestInitGenesisPanics() {
},
true,
},
{
"invalid current epoch days",
func(genState *types.GenesisState) {
genState.CurrentEpochDays = 0
},
true,
},
} {
suite.Run(tc.name, func() {
genState := suite.keeper.ExportGenesis(cacheCtx)
Expand Down
2 changes: 1 addition & 1 deletion x/farming/keeper/reward.go
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ func (k Keeper) ValidateOutstandingRewardsAmount(ctx sdk.Context) error {
rewardsReservePoolBalances := sdk.NewDecCoinsFromCoins(k.bankKeeper.GetAllBalances(ctx, k.GetRewardsReservePoolAcc(ctx))...)
_, hasNeg := rewardsReservePoolBalances.SafeSub(totalOutstandingRewards)
if hasNeg {
return types.ErrInvalidRemainingRewardsAmount // TODO: use different error type
return types.ErrInvalidOutstandingRewardsAmount
}

return nil
Expand Down
18 changes: 17 additions & 1 deletion x/farming/keeper/reward_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,23 @@ func (suite *KeeperTestSuite) TestHarvest() {
}

func (suite *KeeperTestSuite) TestMultipleHarvest() {
// TODO: implement
suite.SetFixedAmountPlan(1, suite.addrs[4], map[string]string{denom1: "1"}, map[string]int64{denom3: 1000000})

suite.Stake(suite.addrs[0], sdk.NewCoins(sdk.NewInt64Coin(denom1, 1000000)))

suite.AdvanceEpoch()
suite.AdvanceEpoch()

balancesBefore := suite.app.BankKeeper.GetAllBalances(suite.ctx, suite.addrs[0])
suite.Harvest(suite.addrs[0], []string{denom1})
balancesAfter := suite.app.BankKeeper.GetAllBalances(suite.ctx, suite.addrs[0])
delta := balancesAfter.Sub(balancesBefore)
suite.Require().True(coinsEq(sdk.NewCoins(sdk.NewInt64Coin(denom3, 1000000)), delta))

balancesBefore = balancesAfter
suite.Harvest(suite.addrs[0], []string{denom1})
balancesAfter = suite.app.BankKeeper.GetAllBalances(suite.ctx, suite.addrs[0])
suite.Require().True(coinsEq(balancesBefore, balancesAfter))
}

func (suite *KeeperTestSuite) TestHistoricalRewards() {
Expand Down
2 changes: 0 additions & 2 deletions x/farming/keeper/staking.go
Original file line number Diff line number Diff line change
Expand Up @@ -314,8 +314,6 @@ func (k Keeper) Stake(ctx sdk.Context, farmerAcc sdk.AccAddress, amount sdk.Coin
// Unstake unstakes an amount of staking coins from the staking reserve account.
// It causes accumulated rewards to be withdrawn to the farmer.
func (k Keeper) Unstake(ctx sdk.Context, farmerAcc sdk.AccAddress, amount sdk.Coins) error {
// TODO: send coins at once, not in every WithdrawRewards

for _, coin := range amount {
staking, found := k.GetStaking(ctx, coin.Denom, farmerAcc)
if !found {
Expand Down
36 changes: 34 additions & 2 deletions x/farming/keeper/staking_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,28 @@ func (suite *KeeperTestSuite) TestStake() {
}

func (suite *KeeperTestSuite) TestMultipleStake() {
// TODO: implement
suite.SetFixedAmountPlan(1, suite.addrs[4], map[string]string{denom1: "1"}, map[string]int64{denom3: 1000000})

suite.Stake(suite.addrs[0], sdk.NewCoins(sdk.NewInt64Coin(denom1, 1000000)))
suite.Stake(suite.addrs[0], sdk.NewCoins(sdk.NewInt64Coin(denom1, 1000000)))

suite.Require().True(coinsEq(
sdk.NewCoins(sdk.NewInt64Coin(denom1, 2000000)),
suite.keeper.GetAllQueuedCoinsByFarmer(suite.ctx, suite.addrs[0])))
suite.Require().True(coinsEq(sdk.Coins{}, suite.keeper.GetAllStakedCoinsByFarmer(suite.ctx, suite.addrs[0])))

suite.AdvanceEpoch()

suite.Require().True(coinsEq(
sdk.NewCoins(sdk.NewInt64Coin(denom1, 2000000)),
suite.keeper.GetAllStakedCoinsByFarmer(suite.ctx, suite.addrs[0])))
suite.Require().True(coinsEq(sdk.Coins{}, suite.keeper.GetAllQueuedCoinsByFarmer(suite.ctx, suite.addrs[0])))

suite.Stake(suite.addrs[0], sdk.NewCoins(sdk.NewInt64Coin(denom1, 1000000)))
balanceBefore := suite.app.BankKeeper.GetBalance(suite.ctx, suite.addrs[0], denom3)
suite.Stake(suite.addrs[0], sdk.NewCoins(sdk.NewInt64Coin(denom1, 1000000)))
balanceAfter := suite.app.BankKeeper.GetBalance(suite.ctx, suite.addrs[0], denom3)
suite.Require().True(intEq(balanceBefore.Amount, balanceAfter.Amount))
}

func (suite *KeeperTestSuite) TestStakeInAdvance() {
Expand Down Expand Up @@ -203,7 +224,18 @@ func (suite *KeeperTestSuite) TestUnstakeNotAlwaysWithdraw() {
}

func (suite *KeeperTestSuite) TestMultipleUnstake() {
// TODO: implement
suite.SetFixedAmountPlan(1, suite.addrs[4], map[string]string{denom1: "1"}, map[string]int64{denom3: 1000000})

suite.Stake(suite.addrs[0], sdk.NewCoins(sdk.NewInt64Coin(denom1, 1000000)))

suite.AdvanceEpoch()
suite.AdvanceEpoch()

suite.Unstake(suite.addrs[0], sdk.NewCoins(sdk.NewInt64Coin(denom1, 250000)))
balanceBefore := suite.app.BankKeeper.GetBalance(suite.ctx, suite.addrs[0], denom3)
suite.Unstake(suite.addrs[0], sdk.NewCoins(sdk.NewInt64Coin(denom1, 250000)))
balanceAfter := suite.app.BankKeeper.GetBalance(suite.ctx, suite.addrs[0], denom3)
suite.Require().True(intEq(balanceBefore.Amount, balanceAfter.Amount))
}

func (suite *KeeperTestSuite) TestTotalStakings() {
Expand Down
19 changes: 10 additions & 9 deletions x/farming/types/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@ import (

// farming module sentinel errors
var (
ErrInvalidPlanType = sdkerrors.Register(ModuleName, 2, "invalid plan type")
ErrInvalidPlanName = sdkerrors.Register(ModuleName, 3, "invalid plan name")
ErrInvalidPlanEndTime = sdkerrors.Register(ModuleName, 4, "invalid plan end time")
ErrInvalidStakingCoinWeights = sdkerrors.Register(ModuleName, 5, "invalid staking coin weights")
ErrInvalidTotalEpochRatio = sdkerrors.Register(ModuleName, 6, "invalid total epoch ratio")
ErrStakingNotExists = sdkerrors.Register(ModuleName, 7, "staking not exists")
ErrConflictPrivatePlanFarmingPool = sdkerrors.Register(ModuleName, 8, "the address is already in use, please use a different plan name")
ErrInvalidStakingReservedAmount = sdkerrors.Register(ModuleName, 9, "staking reserved amount invariant broken")
ErrInvalidRemainingRewardsAmount = sdkerrors.Register(ModuleName, 10, "remaining rewards amount invariant broken")
ErrInvalidPlanType = sdkerrors.Register(ModuleName, 2, "invalid plan type")
ErrInvalidPlanName = sdkerrors.Register(ModuleName, 3, "invalid plan name")
ErrInvalidPlanEndTime = sdkerrors.Register(ModuleName, 4, "invalid plan end time")
ErrInvalidStakingCoinWeights = sdkerrors.Register(ModuleName, 5, "invalid staking coin weights")
ErrInvalidTotalEpochRatio = sdkerrors.Register(ModuleName, 6, "invalid total epoch ratio")
ErrStakingNotExists = sdkerrors.Register(ModuleName, 7, "staking not exists")
ErrConflictPrivatePlanFarmingPool = sdkerrors.Register(ModuleName, 8, "the address is already in use, please use a different plan name")
ErrInvalidStakingReservedAmount = sdkerrors.Register(ModuleName, 9, "staking reserved amount invariant broken")
ErrInvalidRemainingRewardsAmount = sdkerrors.Register(ModuleName, 10, "remaining rewards amount invariant broken")
ErrInvalidOutstandingRewardsAmount = sdkerrors.Register(ModuleName, 11, "outstanding rewards amount invariant broken")
)
14 changes: 7 additions & 7 deletions x/farming/types/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -377,13 +377,13 @@ func TestValidateGenesis(t *testing.T) {
},
"coin 0denom1 amount is not positive",
},
//{
// "invalid current epoch days",
// func(genState *types.GenesisState) {
// genState.CurrentEpochDays = 0
// },
// "not implemented",
//},
{
"invalid current epoch days",
func(genState *types.GenesisState) {
genState.CurrentEpochDays = 0
},
"current epoch days must be positive",
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
Expand Down
46 changes: 40 additions & 6 deletions x/farming/types/plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"testing"
"time"

"github.com/cosmos/cosmos-sdk/codec"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
"github.com/stretchr/testify/require"
"github.com/tendermint/tendermint/crypto"

Expand Down Expand Up @@ -493,8 +495,6 @@ func TestPrivatePlanFarmingPoolAddress(t *testing.T) {
require.Equal(t, "cosmos172yhzhxwgwul3s8m6qpgw2ww3auedq4k3dt224543d0sd44fgx4spcjthr", testAcc2.String())
}

// TODO: needs to cover more cases
// https://github.com/tendermint/farming/issues/90
func TestUnpackPlan(t *testing.T) {
plan := []types.PlanI{
types.NewRatioPlan(
Expand All @@ -518,19 +518,53 @@ func TestUnpackPlan(t *testing.T) {
marshaled, err := any.Marshal()
require.NoError(t, err)

any.Value = []byte{}
err = any.Unmarshal(marshaled)
var any2 codectypes.Any
err = any2.Unmarshal(marshaled)
require.NoError(t, err)

reMarshal, err := any.Marshal()
reMarshal, err := any2.Marshal()
require.NoError(t, err)
require.Equal(t, marshaled, reMarshal)

planRecord := types.PlanRecord{
Plan: *any,
Plan: any2,
FarmingPoolCoins: sdk.NewCoins(),
}

_, err = types.UnpackPlan(&planRecord.Plan)
require.NoError(t, err)
}

func TestUnpackPlanJSON(t *testing.T) {
plan := types.NewRatioPlan(
types.NewBasePlan(
1,
"testPlan1",
types.PlanTypePrivate,
types.PrivatePlanFarmingPoolAddress("farmingPoolAddr1", 1).String(),
sdk.AccAddress("terminationAddr1").String(),
sdk.NewDecCoins(sdk.DecCoin{Denom: "testFarmStakingCoinDenom", Amount: sdk.MustNewDecFromStr("1.0")}),
types.ParseTime("2021-08-03T00:00:00Z"),
types.ParseTime("2021-08-07T00:00:00Z"),
),
sdk.NewDec(1),
)

any, err := types.PackPlan(plan)
require.NoError(t, err)

registry := codectypes.NewInterfaceRegistry()
types.RegisterInterfaces(registry)
cdc := codec.NewProtoCodec(registry)

bz := cdc.MustMarshalJSON(any)

var any2 codectypes.Any
err = cdc.UnmarshalJSON(bz, &any2)
require.NoError(t, err)

plan2, err := types.UnpackPlan(&any2)
require.NoError(t, err)

require.Equal(t, uint64(1), plan2.GetId())
}

0 comments on commit 494de4f

Please sign in to comment.