Skip to content

Commit

Permalink
Merge pull request #110 from tendermint/109-imporve-budget-validation
Browse files Browse the repository at this point in the history 
fix: improve budget validation logic and tests, spec doc
  • Loading branch information
@dongsam
dongsam authored Mar 11, 2022
2 parents f83d023 + c2f3fab commit 63734a6
Show file tree
Hide file tree
Showing 22 changed files with 644 additions and 184 deletions.
2 changes: 2 additions & 0 deletions app/params/params.go
View file
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
View file
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
View file
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
View file
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

0 comments on commit 63734a6

Please sign in to comment.