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

fix: improve budget validation logic and tests, spec doc #110

Merged
merged 6 commits into from
Mar 11, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 2 additions & 0 deletions app/params/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,6 @@ package params
const (
StakePerAccount = "stake_per_account"
InitiallyBondedValidators = "initially_bonded_validators"

DefaultWeightUpdateBudgetPlans int = 5
)
14 changes: 6 additions & 8 deletions x/budget/keeper/budget.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package keeper
import (
sdk "github.com/cosmos/cosmos-sdk/types"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"

"github.com/tendermint/budget/x/budget/types"
)

Expand All @@ -27,7 +26,7 @@ func (k Keeper) CollectBudgets(ctx sdk.Context) error {
if err != nil {
return err
}
sourceBalances := sdk.NewDecCoinsFromCoins(k.bankKeeper.GetAllBalances(ctx, sourceAcc)...)
sourceBalances := sdk.NewDecCoinsFromCoins(k.bankKeeper.SpendableCoins(ctx, sourceAcc)...)
if sourceBalances.IsZero() {
continue
}
Expand All @@ -40,9 +39,8 @@ func (k Keeper) CollectBudgets(ctx sdk.Context) error {
if err != nil {
return err
}

collectionCoins, _ := sourceBalances.MulDecTruncate(budget.Rate).TruncateDecimal()
if collectionCoins.Empty() || !collectionCoins.IsValid() {
if collectionCoins.Empty() || collectionCoins.IsZero() || !collectionCoins.IsValid() {
continue
}

Expand All @@ -51,19 +49,19 @@ func (k Keeper) CollectBudgets(ctx sdk.Context) error {
budgetsBySource.CollectionCoins[i] = collectionCoins
}

if err := k.bankKeeper.InputOutputCoins(ctx, inputs, outputs); err != nil {
if err = k.bankKeeper.InputOutputCoins(ctx, inputs, outputs); err != nil {
return err
}

for i, budget := range budgetsBySource.Budgets {
if budgetsBySource.CollectionCoins[i].Empty() || budgetsBySource.CollectionCoins[i].IsZero() || !budgetsBySource.CollectionCoins[i].IsValid() {
continue
}
k.AddTotalCollectedCoins(ctx, budget.Name, budgetsBySource.CollectionCoins[i])
ctx.EventManager().EmitEvents(sdk.Events{
sdk.NewEvent(
types.EventTypeBudgetCollected,
sdk.NewAttribute(types.AttributeValueName, budget.Name),
sdk.NewAttribute(types.AttributeValueDestinationAddress, budget.DestinationAddress),
sdk.NewAttribute(types.AttributeValueSourceAddress, budget.SourceAddress),
sdk.NewAttribute(types.AttributeValueRate, budget.Rate.String()),
sdk.NewAttribute(types.AttributeValueAmount, budgetsBySource.CollectionCoins[i].String()),
),
})
Expand Down
92 changes: 64 additions & 28 deletions x/budget/keeper/budget_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func (suite *KeeperTestSuite) TestCollectBudgets() {
epochBlocks uint32
accAsserts []sdk.AccAddress
balanceAsserts []sdk.Coins
expectErr bool
expectErr error
}{
{
"basic budgets case",
Expand All @@ -44,7 +44,7 @@ func (suite *KeeperTestSuite) TestCollectBudgets() {
{},
mustParseCoinsNormalized("1000000000denom1,1000000000denom2,1000000000denom3,1000000000stake"),
},
false,
nil,
},
{
"only expired budget case",
Expand All @@ -58,7 +58,7 @@ func (suite *KeeperTestSuite) TestCollectBudgets() {
{},
mustParseCoinsNormalized("1000000000denom1,1000000000denom2,1000000000denom3,1000000000stake"),
},
false,
nil,
},
{
"source has small balances case",
Expand All @@ -74,7 +74,35 @@ func (suite *KeeperTestSuite) TestCollectBudgets() {
mustParseCoinsNormalized("1denom2,1denom3,500000000stake"),
mustParseCoinsNormalized("1denom1,1denom3"),
},
false,
nil,
},
{
"source has small balances case 2",
[]types.Budget{suite.budgets[6]},
types.DefaultEpochBlocks,
[]sdk.AccAddress{
suite.destinationAddrs[4],
suite.sourceAddrs[4],
},
[]sdk.Coins{
mustParseCoinsNormalized(""),
mustParseCoinsNormalized("1denom1,2denom2"),
},
nil,
},
{
"empty source case",
[]types.Budget{suite.budgets[7]},
types.DefaultEpochBlocks,
[]sdk.AccAddress{
suite.destinationAddrs[5],
suite.sourceAddrs[5],
},
[]sdk.Coins{
{},
{},
},
nil,
},
{
"none budgets case",
Expand All @@ -100,7 +128,7 @@ func (suite *KeeperTestSuite) TestCollectBudgets() {
mustParseCoinsNormalized("1000000000denom1,1000000000denom2,1000000000denom3,1000000000stake"),
mustParseCoinsNormalized("1denom1,2denom2,3denom3,1000000000stake"),
},
false,
nil,
},
{
"disabled budget epoch",
Expand All @@ -126,7 +154,7 @@ func (suite *KeeperTestSuite) TestCollectBudgets() {
mustParseCoinsNormalized("1000000000denom1,1000000000denom2,1000000000denom3,1000000000stake"),
mustParseCoinsNormalized("1denom1,2denom2,3denom3,1000000000stake"),
},
false,
nil,
},
{
"disabled budget epoch with budgets",
Expand All @@ -152,7 +180,7 @@ func (suite *KeeperTestSuite) TestCollectBudgets() {
mustParseCoinsNormalized("1000000000denom1,1000000000denom2,1000000000denom3,1000000000stake"),
mustParseCoinsNormalized("1denom1,2denom2,3denom3,1000000000stake"),
},
false,
nil,
},
} {
suite.Run(tc.name, func() {
Expand All @@ -163,7 +191,7 @@ func (suite *KeeperTestSuite) TestCollectBudgets() {
suite.keeper.SetParams(suite.ctx, params)

err := suite.keeper.CollectBudgets(suite.ctx)
if tc.expectErr {
if tc.expectErr != nil {
suite.Error(err)
} else {
suite.NoError(err)
Expand Down Expand Up @@ -211,7 +239,7 @@ func (suite *KeeperTestSuite) TestBudgetChangeSituation() {
"source_address": "cosmos10wy60v3zuks7rkwnqxs3e878zqfhus6m98l77q6rppz40kxwgllsruc0az",
"destination_address": "cosmos1qceyjmnrl6hapntjq3z25vn38nh68u7yxvufs2thptxvqm7huxeqj7zyrq",
"start_time": "2021-09-01T00:00:00Z",
"end_time": "2031-09-30T00:00:00Z"
"end_time": "2021-12-31T00:00:00Z"
}
]`,
}),
Expand Down Expand Up @@ -240,7 +268,7 @@ func (suite *KeeperTestSuite) TestBudgetChangeSituation() {
"source_address": "cosmos10wy60v3zuks7rkwnqxs3e878zqfhus6m98l77q6rppz40kxwgllsruc0az",
"destination_address": "cosmos1qceyjmnrl6hapntjq3z25vn38nh68u7yxvufs2thptxvqm7huxeqj7zyrq",
"start_time": "2021-09-01T00:00:00Z",
"end_time": "2031-09-30T00:00:00Z"
"end_time": "2021-12-31T00:00:00Z"
},
{
"name": "gravity-dex-farming-2",
Expand All @@ -266,7 +294,7 @@ func (suite *KeeperTestSuite) TestBudgetChangeSituation() {
},
},
{
"add budget 3 with invalid total rate case 1",
"add budget 3 overlapped with 2, invalid total rate, 1.5",
testProposal(proposal.ParamChange{
Subspace: types.ModuleName,
Key: string(types.KeyBudgets),
Expand All @@ -277,7 +305,7 @@ func (suite *KeeperTestSuite) TestBudgetChangeSituation() {
"source_address": "cosmos10wy60v3zuks7rkwnqxs3e878zqfhus6m98l77q6rppz40kxwgllsruc0az",
"destination_address": "cosmos1qceyjmnrl6hapntjq3z25vn38nh68u7yxvufs2thptxvqm7huxeqj7zyrq",
"start_time": "2021-09-01T00:00:00Z",
"end_time": "2031-09-30T00:00:00Z"
"end_time": "2021-12-31T00:00:00Z"
},
{
"name": "gravity-dex-farming-2",
Expand All @@ -292,7 +320,7 @@ func (suite *KeeperTestSuite) TestBudgetChangeSituation() {
"rate": "0.500000000000000000",
"source_address": "cosmos10wy60v3zuks7rkwnqxs3e878zqfhus6m98l77q6rppz40kxwgllsruc0az",
"destination_address": "cosmos1e0n8jmeg4u8q3es2tmhz5zlte8a4q8687ndns8pj4q8grdl74a0sw3045s",
"start_time": "2021-09-30T00:00:00Z",
"start_time": "2021-09-29T00:00:00Z",
"end_time": "2021-10-10T00:00:00Z"
}
]`,
Expand All @@ -301,7 +329,7 @@ func (suite *KeeperTestSuite) TestBudgetChangeSituation() {
1, // left last budgets of 2nd tc
types.MustParseRFC3339("2021-09-29T00:00:00Z"),
types.MustParseRFC3339("2021-09-30T00:00:00Z"),
types.ErrInvalidTotalBudgetRate,
types.ErrInvalidTotalBudgetRate, // 1.5 ( 0.5 + 0.5 + 0.5 )
[]sdk.AccAddress{budgetSource, suite.destinationAddrs[0], suite.destinationAddrs[1], suite.destinationAddrs[2]},
[]sdk.Coins{
mustParseCoinsNormalized("500000000denom1,500000000denom2,500000000denom3,500000000stake"),
Expand All @@ -311,7 +339,7 @@ func (suite *KeeperTestSuite) TestBudgetChangeSituation() {
},
},
{
"add budget 3 with invalid total rate case 2",
"add budget 3 with invalid total rate, 1.01",
testProposal(proposal.ParamChange{
Subspace: types.ModuleName,
Key: string(types.KeyBudgets),
Expand All @@ -322,7 +350,7 @@ func (suite *KeeperTestSuite) TestBudgetChangeSituation() {
"source_address": "cosmos10wy60v3zuks7rkwnqxs3e878zqfhus6m98l77q6rppz40kxwgllsruc0az",
"destination_address": "cosmos1qceyjmnrl6hapntjq3z25vn38nh68u7yxvufs2thptxvqm7huxeqj7zyrq",
"start_time": "2021-09-01T00:00:00Z",
"end_time": "2031-09-30T00:00:00Z"
"end_time": "2021-12-31T00:00:00Z"
},
{
"name": "gravity-dex-farming-2",
Expand All @@ -334,7 +362,7 @@ func (suite *KeeperTestSuite) TestBudgetChangeSituation() {
},
{
"name": "gravity-dex-farming-3",
"rate": "0.500000000000000000",
"rate": "0.510000000000000000",
"source_address": "cosmos10wy60v3zuks7rkwnqxs3e878zqfhus6m98l77q6rppz40kxwgllsruc0az",
"destination_address": "cosmos1e0n8jmeg4u8q3es2tmhz5zlte8a4q8687ndns8pj4q8grdl74a0sw3045s",
"start_time": "2021-09-30T00:00:00Z",
Expand All @@ -346,7 +374,7 @@ func (suite *KeeperTestSuite) TestBudgetChangeSituation() {
1, // left last budgets of 2nd tc
types.MustParseRFC3339("2021-10-01T00:00:00Z"),
types.MustParseRFC3339("2021-10-01T00:00:00Z"),
types.ErrInvalidTotalBudgetRate,
types.ErrInvalidTotalBudgetRate, // 1.01 ( 0.5 + 0.0 + 0.51 )
[]sdk.AccAddress{budgetSource, suite.destinationAddrs[0], suite.destinationAddrs[1], suite.destinationAddrs[2]},
[]sdk.Coins{
mustParseCoinsNormalized("750000000denom1,750000000denom2,750000000denom3,750000000stake"),
Expand All @@ -367,7 +395,15 @@ func (suite *KeeperTestSuite) TestBudgetChangeSituation() {
"source_address": "cosmos10wy60v3zuks7rkwnqxs3e878zqfhus6m98l77q6rppz40kxwgllsruc0az",
"destination_address": "cosmos1qceyjmnrl6hapntjq3z25vn38nh68u7yxvufs2thptxvqm7huxeqj7zyrq",
"start_time": "2021-09-01T00:00:00Z",
"end_time": "2031-09-30T00:00:00Z"
"end_time": "2021-12-31T00:00:00Z"
},
{
"name": "gravity-dex-farming-2",
"rate": "0.500000000000000000",
"source_address": "cosmos10wy60v3zuks7rkwnqxs3e878zqfhus6m98l77q6rppz40kxwgllsruc0az",
"destination_address": "cosmos1czyx0dj2yd26gv3stpxzv23ddy8pld4j6p90a683mdcg8vzy72jqa8tm6p",
"start_time": "2021-09-01T00:00:00Z",
"end_time": "2021-09-30T00:00:00Z"
},
{
"name": "gravity-dex-farming-3",
Expand All @@ -379,7 +415,7 @@ func (suite *KeeperTestSuite) TestBudgetChangeSituation() {
}
]`,
}),
2,
3,
2,
types.MustParseRFC3339("2021-10-01T00:00:00Z"),
types.MustParseRFC3339("2021-10-01T00:00:00Z"),
Expand All @@ -393,7 +429,7 @@ func (suite *KeeperTestSuite) TestBudgetChangeSituation() {
},
},
{
"add budget 4 without date range overlap",
"add budget 4 without date range overlap, remove 2, 3",
testProposal(proposal.ParamChange{
Subspace: types.ModuleName,
Key: string(types.KeyBudgets),
Expand All @@ -404,22 +440,22 @@ func (suite *KeeperTestSuite) TestBudgetChangeSituation() {
"source_address": "cosmos10wy60v3zuks7rkwnqxs3e878zqfhus6m98l77q6rppz40kxwgllsruc0az",
"destination_address": "cosmos1qceyjmnrl6hapntjq3z25vn38nh68u7yxvufs2thptxvqm7huxeqj7zyrq",
"start_time": "2021-09-01T00:00:00Z",
"end_time": "2031-09-30T00:00:00Z"
"end_time": "2021-12-31T00:00:00Z"
},
{
"name": "gravity-dex-farming-4",
"rate": "1.000000000000000000",
"source_address": "cosmos10wy60v3zuks7rkwnqxs3e878zqfhus6m98l77q6rppz40kxwgllsruc0az",
"destination_address": "cosmos1e0n8jmeg4u8q3es2tmhz5zlte8a4q8687ndns8pj4q8grdl74a0sw3045s",
"start_time": "2031-09-30T00:00:01Z",
"end_time": "2031-12-10T00:00:00Z"
"start_time": "2021-12-31T00:00:00Z",
"end_time": "2022-12-31T00:00:00Z"
}
]`,
}),
2,
1,
types.MustParseRFC3339("2021-09-29T00:00:00Z"),
types.MustParseRFC3339("2021-09-30T00:00:00Z"),
types.MustParseRFC3339("2021-10-02T00:00:00Z"),
types.MustParseRFC3339("2021-10-14T00:00:00Z"),
nil,
[]sdk.AccAddress{budgetSource, suite.destinationAddrs[0], suite.destinationAddrs[1], suite.destinationAddrs[2]},
[]sdk.Coins{
Expand Down Expand Up @@ -475,10 +511,10 @@ func (suite *KeeperTestSuite) TestBudgetChangeSituation() {
suite.ctx = suite.ctx.WithBlockHeight(int64(height))
suite.ctx = suite.ctx.WithBlockTime(tc.nextBlockTime)

params := suite.keeper.GetParams(suite.ctx)
params = suite.keeper.GetParams(suite.ctx)
suite.Require().Len(params.Budgets, tc.budgetCount)
for _, budget := range params.Budgets {
err := budget.Validate()
err = budget.Validate()
suite.Require().NoError(err)
}

Expand Down
28 changes: 19 additions & 9 deletions x/budget/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package keeper_test
import (
"testing"

authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
"github.com/cosmos/cosmos-sdk/x/params"
"github.com/cosmos/cosmos-sdk/x/params/types/proposal"
Expand All @@ -29,7 +28,8 @@ var (
sdk.NewInt64Coin(denom1, 1_000_000_000),
sdk.NewInt64Coin(denom2, 1_000_000_000),
sdk.NewInt64Coin(denom3, 1_000_000_000))
smallBalances = mustParseCoinsNormalized("1denom1,2denom2,3denom3,1000000000stake")
smallBalances = mustParseCoinsNormalized("1denom1,2denom2,3denom3,1000000000stake")
smallBalances2 = mustParseCoinsNormalized("1denom1,2denom2")
)

type KeeperTestSuite struct {
Expand Down Expand Up @@ -67,21 +67,23 @@ func (suite *KeeperTestSuite) SetupTest() {
dAddr3 := types.DeriveAddress(types.AddressType32Bytes, types.ModuleName, "destinationAddr3")
dAddr4 := types.DeriveAddress(types.AddressType32Bytes, types.ModuleName, "destinationAddr4")
dAddr5 := types.DeriveAddress(types.AddressType32Bytes, types.ModuleName, "destinationAddr5")
dAddr6 := types.DeriveAddress(types.AddressType32Bytes, "farming", "GravityDEXFarmingBudget")
dAddr6 := types.DeriveAddress(types.AddressType32Bytes, types.ModuleName, "destinationAddr6")
sAddr1 := types.DeriveAddress(types.AddressType32Bytes, types.ModuleName, "sourceAddr1")
sAddr2 := types.DeriveAddress(types.AddressType32Bytes, types.ModuleName, "sourceAddr2")
sAddr3 := types.DeriveAddress(types.AddressType32Bytes, types.ModuleName, "sourceAddr3")
sAddr4 := types.DeriveAddress(types.AddressType32Bytes, types.ModuleName, "sourceAddr4")
sAddr5 := types.DeriveAddress(types.AddressType32Bytes, types.ModuleName, "sourceAddr5")
sAddr6 := suite.app.AccountKeeper.GetModuleAccount(suite.ctx, authtypes.FeeCollectorName).GetAddress()
sAddr6 := types.DeriveAddress(types.AddressType32Bytes, types.ModuleName, "sourceAddr6")
suite.destinationAddrs = []sdk.AccAddress{dAddr1, dAddr2, dAddr3, dAddr4, dAddr5, dAddr6}
suite.sourceAddrs = []sdk.AccAddress{sAddr1, sAddr2, sAddr3, sAddr4, sAddr5, sAddr6}
suite.sourceAddrs = []sdk.AccAddress{sAddr1, sAddr2, sAddr3, sAddr4, sAddr5, sAddr6, sAddr6}
for _, addr := range append(suite.addrs, suite.sourceAddrs[:3]...) {
err := simapp.FundAccount(suite.app.BankKeeper, suite.ctx, addr, initialBalances)
suite.Require().NoError(err)
}
err := simapp.FundAccount(suite.app.BankKeeper, suite.ctx, suite.sourceAddrs[3], smallBalances)
suite.Require().NoError(err)
err = simapp.FundAccount(suite.app.BankKeeper, suite.ctx, suite.sourceAddrs[4], smallBalances2)
suite.Require().NoError(err)

suite.budgets = []types.Budget{
{
Expand Down Expand Up @@ -133,12 +135,20 @@ func (suite *KeeperTestSuite) SetupTest() {
EndTime: types.MustParseRFC3339("9999-12-31T00:00:00Z"),
},
{
Name: "gravity-dex-farming-20213Q-20313Q",
Rate: sdk.MustNewDecFromStr("0.5"),
Name: "small2-source-budget",
Rate: sdk.MustNewDecFromStr("0.1"),
SourceAddress: suite.sourceAddrs[4].String(),
DestinationAddress: suite.destinationAddrs[4].String(),
StartTime: types.MustParseRFC3339("0000-01-01T00:00:00Z"),
EndTime: types.MustParseRFC3339("9999-12-31T00:00:00Z"),
},
{
Name: "empty-source-budget",
Rate: sdk.MustNewDecFromStr("0.1"),
SourceAddress: suite.sourceAddrs[5].String(),
DestinationAddress: suite.destinationAddrs[5].String(),
StartTime: types.MustParseRFC3339("2021-09-01T00:00:00Z"),
EndTime: types.MustParseRFC3339("2031-09-30T00:00:00Z"),
StartTime: types.MustParseRFC3339("0000-01-01T00:00:00Z"),
EndTime: types.MustParseRFC3339("9999-12-31T00:00:00Z"),
},
}
}
Expand Down
Loading