From 532154f0d30e53854452cd3cf222e1955a8ca06d Mon Sep 17 00:00:00 2001 From: Youngtaek Yoon Date: Wed, 26 Oct 2022 07:47:54 +0000 Subject: [PATCH] feat: add invariants to x/foundation (#758) * Add module account invariant * Fix InitGenesis order * Add proposal invariant * Add total weight invariant * Remove reverse dependency on stakingplus * Update CHANGELOG.md --- CHANGELOG.md | 1 + simapp/app.go | 2 +- x/foundation/client/testutil/grpc.go | 2 - x/foundation/client/testutil/query.go | 2 - x/foundation/client/testutil/suite.go | 11 ++- x/foundation/keeper/invariants.go | 77 +++++++++++++++++++++ x/foundation/keeper/invariants_test.go | 96 ++++++++++++++++++++++++++ x/foundation/keeper/msg_server_test.go | 12 +--- x/foundation/module/module.go | 4 +- x/foundation/msgs_test.go | 6 -- 10 files changed, 185 insertions(+), 28 deletions(-) create mode 100644 x/foundation/keeper/invariants.go create mode 100644 x/foundation/keeper/invariants_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 1421501f9e..904731e313 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/foundation) [\#709](https://github.com/line/lbm-sdk/pull/709) add `gov mint` for x/foundation proposal * (iavl) [\#738](https://github.com/line/lbm-sdk/pull/738) bump github.com/cosmos/iavl from v0.17.3 to v0.19.3 * (baseapp) [\#756](https://github.com/line/lbm-sdk/pull/756) Change to create chCheckTx with the value set in app config +* (x/foundation) [\#758](https://github.com/line/lbm-sdk/pull/758) add invariants to x/foundation ### Improvements diff --git a/simapp/app.go b/simapp/app.go index feacad8440..527faa8a0b 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -609,6 +609,7 @@ func NewSimApp( slashingtypes.ModuleName, govtypes.ModuleName, minttypes.ModuleName, + foundation.ModuleName, crisistypes.ModuleName, ibchost.ModuleName, genutiltypes.ModuleName, @@ -621,7 +622,6 @@ func NewSimApp( paramstypes.ModuleName, upgradetypes.ModuleName, vestingtypes.ModuleName, - foundation.ModuleName, token.ModuleName, collection.ModuleName, // wasm after ibc transfer diff --git a/x/foundation/client/testutil/grpc.go b/x/foundation/client/testutil/grpc.go index e461529979..3aba228b17 100644 --- a/x/foundation/client/testutil/grpc.go +++ b/x/foundation/client/testutil/grpc.go @@ -9,7 +9,6 @@ import ( sdk "github.com/line/lbm-sdk/types" "github.com/line/lbm-sdk/x/foundation" - stakingtypes "github.com/line/lbm-sdk/x/staking/types" ) func (s *IntegrationTestSuite) TestGRPCParams() { @@ -31,7 +30,6 @@ func (s *IntegrationTestSuite) TestGRPCParams() { Params: foundation.Params{ FoundationTax: sdk.MustNewDecFromStr("0.2"), CensoredMsgTypeUrls: []string{ - sdk.MsgTypeURL((*stakingtypes.MsgCreateValidator)(nil)), sdk.MsgTypeURL((*foundation.MsgWithdrawFromTreasury)(nil)), }, }, diff --git a/x/foundation/client/testutil/query.go b/x/foundation/client/testutil/query.go index 6f985b3fdf..8c3896d53d 100644 --- a/x/foundation/client/testutil/query.go +++ b/x/foundation/client/testutil/query.go @@ -11,7 +11,6 @@ import ( sdk "github.com/line/lbm-sdk/types" "github.com/line/lbm-sdk/x/foundation" "github.com/line/lbm-sdk/x/foundation/client/cli" - stakingtypes "github.com/line/lbm-sdk/x/staking/types" ) func (s *IntegrationTestSuite) TestNewQueryCmdParams() { @@ -33,7 +32,6 @@ func (s *IntegrationTestSuite) TestNewQueryCmdParams() { Params: foundation.Params{ FoundationTax: sdk.MustNewDecFromStr("0.2"), CensoredMsgTypeUrls: []string{ - sdk.MsgTypeURL((*stakingtypes.MsgCreateValidator)(nil)), sdk.MsgTypeURL((*foundation.MsgWithdrawFromTreasury)(nil)), }, }, diff --git a/x/foundation/client/testutil/suite.go b/x/foundation/client/testutil/suite.go index 838cf50e74..090c2b1bb1 100644 --- a/x/foundation/client/testutil/suite.go +++ b/x/foundation/client/testutil/suite.go @@ -18,7 +18,6 @@ import ( bankcli "github.com/line/lbm-sdk/x/bank/client/cli" "github.com/line/lbm-sdk/x/foundation" "github.com/line/lbm-sdk/x/foundation/client/cli" - stakingtypes "github.com/line/lbm-sdk/x/staking/types" ) type IntegrationTestSuite struct { @@ -61,7 +60,6 @@ func (s *IntegrationTestSuite) SetupSuite() { params := foundation.Params{ FoundationTax: sdk.MustNewDecFromStr("0.2"), CensoredMsgTypeUrls: []string{ - sdk.MsgTypeURL((*stakingtypes.MsgCreateValidator)(nil)), sdk.MsgTypeURL((*foundation.MsgWithdrawFromTreasury)(nil)), }, } @@ -97,14 +95,13 @@ func (s *IntegrationTestSuite) SetupSuite() { }) foundationData.Foundation = info - grantees := []sdk.AccAddress{s.stranger, s.leavingMember} - foundationData.Authorizations = make([]foundation.GrantAuthorization, len(grantees)) - for i, grantee := range grantees { + treasuryReceivers := []sdk.AccAddress{s.stranger, s.leavingMember} + for _, receiver := range treasuryReceivers { ga := foundation.GrantAuthorization{ - Grantee: grantee.String(), + Grantee: receiver.String(), }.WithAuthorization(&foundation.ReceiveFromTreasuryAuthorization{}) s.Require().NotNil(ga) - foundationData.Authorizations[i] = *ga + foundationData.Authorizations = append(foundationData.Authorizations, *ga) } foundationDataBz, err := s.cfg.Codec.MarshalJSON(&foundationData) diff --git a/x/foundation/keeper/invariants.go b/x/foundation/keeper/invariants.go new file mode 100644 index 0000000000..1cd30e45d6 --- /dev/null +++ b/x/foundation/keeper/invariants.go @@ -0,0 +1,77 @@ +package keeper + +import ( + "fmt" + + sdk "github.com/line/lbm-sdk/types" + "github.com/line/lbm-sdk/x/foundation" +) + +const ( + moduleAccountInvariant = "module-accounts" + totalWeightInvariant = "total-weight" + proposalInvariant = "proposals" +) + +func RegisterInvariants(ir sdk.InvariantRegistry, k Keeper) { + ir.RegisterRoute(foundation.ModuleName, moduleAccountInvariant, ModuleAccountInvariant(k)) + ir.RegisterRoute(foundation.ModuleName, totalWeightInvariant, ProposalInvariant(k)) + ir.RegisterRoute(foundation.ModuleName, proposalInvariant, ProposalInvariant(k)) +} + +func ModuleAccountInvariant(k Keeper) sdk.Invariant { + return func(ctx sdk.Context) (string, bool) { + // cache, we don't want to write changes + ctx, _ = ctx.CacheContext() + + treasuryAcc := k.authKeeper.GetModuleAccount(ctx, foundation.TreasuryName) + balance := k.bankKeeper.GetAllBalances(ctx, treasuryAcc.GetAddress()) + + treasury := k.GetTreasury(ctx) + msg := fmt.Sprintf("coins in the treasury; expected %s, got %s\n", treasury, balance) + broken := !treasury.IsEqual(sdk.NewDecCoinsFromCoins(balance...)) + + return sdk.FormatInvariant(foundation.ModuleName, moduleAccountInvariant, msg), broken + } +} + +func TotalWeightInvariant(k Keeper) sdk.Invariant { + return func(ctx sdk.Context) (string, bool) { + // cache, we don't want to write changes + ctx, _ = ctx.CacheContext() + + expected := k.GetFoundationInfo(ctx).TotalWeight + real := sdk.NewDec(int64(len(k.GetMembers(ctx)))) + + msg := fmt.Sprintf("total weight of foundation; expected %s, got %s\n", expected, real) + broken := !real.Equal(expected) + + return sdk.FormatInvariant(foundation.ModuleName, totalWeightInvariant, msg), broken + } +} + +func ProposalInvariant(k Keeper) sdk.Invariant { + return func(ctx sdk.Context) (string, bool) { + // cache, we don't want to write changes + ctx, _ = ctx.CacheContext() + + version := k.GetFoundationInfo(ctx).Version + msg := fmt.Sprintf("current foundation version; %d\n", version) + broken := false + + k.iterateProposals(ctx, func(proposal foundation.Proposal) (stop bool) { + if proposal.FoundationVersion == version { + return true + } + + if proposal.Status == foundation.PROPOSAL_STATUS_SUBMITTED { + msg += fmt.Sprintf("active old proposal %d found; version %d\n", proposal.Id, proposal.FoundationVersion) + broken = true + } + + return false + }) + + return sdk.FormatInvariant(foundation.ModuleName, proposalInvariant, msg), broken + } +} diff --git a/x/foundation/keeper/invariants_test.go b/x/foundation/keeper/invariants_test.go new file mode 100644 index 0000000000..2a43c0302b --- /dev/null +++ b/x/foundation/keeper/invariants_test.go @@ -0,0 +1,96 @@ +package keeper_test + +import ( + sdk "github.com/line/lbm-sdk/types" + "github.com/line/lbm-sdk/x/foundation" + "github.com/line/lbm-sdk/x/foundation/keeper" +) + +func (s *KeeperTestSuite) TestModuleAccountInvariant() { + testCases := map[string]struct { + malleate func(ctx sdk.Context) + valid bool + }{ + "invariant not broken": { + valid: true, + }, + "treasury differs from the balance": { + malleate: func(ctx sdk.Context) { + balance := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, s.balance.Add(sdk.OneInt()))) + s.keeper.SetPool(ctx, foundation.Pool{ + Treasury: sdk.NewDecCoinsFromCoins(balance...), + }) + }, + }, + } + + for name, tc := range testCases { + ctx, _ := s.ctx.CacheContext() + if tc.malleate != nil { + tc.malleate(ctx) + } + + invariant := keeper.ModuleAccountInvariant(s.keeper) + _, broken := invariant(ctx) + s.Require().Equal(!tc.valid, broken, name) + } +} + +func (s *KeeperTestSuite) TestTotalWeightInvariant() { + testCases := map[string]struct { + malleate func(ctx sdk.Context) + valid bool + }{ + "invariant not broken": { + valid: true, + }, + "total weight differs from the number of foundation members": { + malleate: func(ctx sdk.Context) { + info := s.keeper.GetFoundationInfo(ctx) + numMembers := len(s.keeper.GetMembers(ctx)) + info.TotalWeight = sdk.NewDec(int64(numMembers)).Add(sdk.OneDec()) + s.keeper.SetFoundationInfo(ctx, info) + }, + }, + } + + for name, tc := range testCases { + ctx, _ := s.ctx.CacheContext() + if tc.malleate != nil { + tc.malleate(ctx) + } + + invariant := keeper.TotalWeightInvariant(s.keeper) + _, broken := invariant(ctx) + s.Require().Equal(!tc.valid, broken, name) + } +} + +func (s *KeeperTestSuite) TestProposalInvariant() { + testCases := map[string]struct { + malleate func(ctx sdk.Context) + valid bool + }{ + "invariant not broken": { + valid: true, + }, + "active old proposal exists": { + malleate: func(ctx sdk.Context) { + info := s.keeper.GetFoundationInfo(ctx) + info.Version-- + s.keeper.SetFoundationInfo(ctx, info) + }, + }, + } + + for name, tc := range testCases { + ctx, _ := s.ctx.CacheContext() + if tc.malleate != nil { + tc.malleate(ctx) + } + + invariant := keeper.ProposalInvariant(s.keeper) + _, broken := invariant(ctx) + s.Require().Equal(!tc.valid, broken, name) + } +} diff --git a/x/foundation/keeper/msg_server_test.go b/x/foundation/keeper/msg_server_test.go index 0623c758fb..9287001fb1 100644 --- a/x/foundation/keeper/msg_server_test.go +++ b/x/foundation/keeper/msg_server_test.go @@ -6,7 +6,6 @@ import ( "github.com/line/lbm-sdk/testutil/testdata" sdk "github.com/line/lbm-sdk/types" "github.com/line/lbm-sdk/x/foundation" - "github.com/line/lbm-sdk/x/stakingplus" ) func (s *KeeperTestSuite) TestMsgUpdateParams() { @@ -611,20 +610,15 @@ func (s *KeeperTestSuite) TestMsgRevoke() { msgTypeURL: foundation.ReceiveFromTreasuryAuthorization{}.MsgTypeURL(), valid: true, }, - "no grant": { - authority: s.authority, - grantee: s.members[0], - msgTypeURL: foundation.ReceiveFromTreasuryAuthorization{}.MsgTypeURL(), - }, "not authorized": { authority: s.stranger, grantee: s.stranger, msgTypeURL: foundation.ReceiveFromTreasuryAuthorization{}.MsgTypeURL(), }, - "wrong granter": { + "no grant": { authority: s.authority, - grantee: s.stranger, - msgTypeURL: stakingplus.CreateValidatorAuthorization{}.MsgTypeURL(), + grantee: s.members[0], + msgTypeURL: foundation.ReceiveFromTreasuryAuthorization{}.MsgTypeURL(), }, } diff --git a/x/foundation/module/module.go b/x/foundation/module/module.go index 336e515388..dd40d0763a 100644 --- a/x/foundation/module/module.go +++ b/x/foundation/module/module.go @@ -92,7 +92,9 @@ func NewAppModule(cdc codec.Codec, keeper keeper.Keeper) AppModule { } // RegisterInvariants does nothing, there are no invariants to enforce -func (AppModule) RegisterInvariants(_ sdk.InvariantRegistry) {} +func (am AppModule) RegisterInvariants(ir sdk.InvariantRegistry) { + keeper.RegisterInvariants(ir, am.keeper) +} // Route is empty, as we do not handle Messages (just proposals) func (AppModule) Route() sdk.Route { return sdk.Route{} } diff --git a/x/foundation/msgs_test.go b/x/foundation/msgs_test.go index 8a45fa376c..f9e5e3f691 100644 --- a/x/foundation/msgs_test.go +++ b/x/foundation/msgs_test.go @@ -12,7 +12,6 @@ import ( sdk "github.com/line/lbm-sdk/types" "github.com/line/lbm-sdk/x/auth/legacy/legacytx" "github.com/line/lbm-sdk/x/foundation" - "github.com/line/lbm-sdk/x/stakingplus" ) func TestMsgUpdateParams(t *testing.T) { @@ -874,7 +873,6 @@ func TestMsgGrantAminoJson(t *testing.T) { operator = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()) grantee = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()) proposer = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()) - validatorAddr = sdk.ValAddress(secp256k1.GenPrivKey().PubKey().Address()) ) testCases := map[string]struct { @@ -885,10 +883,6 @@ func TestMsgGrantAminoJson(t *testing.T) { &foundation.ReceiveFromTreasuryAuthorization{}, fmt.Sprintf("{\"account_number\":\"1\",\"chain_id\":\"foo\",\"fee\":{\"amount\":[],\"gas\":\"0\"},\"memo\":\"memo\",\"msgs\":[{\"type\":\"lbm-sdk/MsgSubmitProposal\",\"value\":{\"exec\":1,\"messages\":[{\"type\":\"lbm-sdk/MsgGrant\",\"value\":{\"authority\":\"%s\",\"authorization\":{\"type\":\"lbm-sdk/ReceiveFromTreasuryAuthorization\",\"value\":{}},\"grantee\":\"%s\"}}],\"metadata\":\"ReceiveFromTreasuryAuthorization\",\"proposers\":[\"%s\"]}}],\"sequence\":\"1\",\"timeout_height\":\"1\"}", operator.String(), grantee.String(), proposer.String()), }, - "CreateValidatorAuthorization": { - &stakingplus.CreateValidatorAuthorization{ValidatorAddress: validatorAddr.String()}, - fmt.Sprintf("{\"account_number\":\"1\",\"chain_id\":\"foo\",\"fee\":{\"amount\":[],\"gas\":\"0\"},\"memo\":\"memo\",\"msgs\":[{\"type\":\"lbm-sdk/MsgSubmitProposal\",\"value\":{\"exec\":1,\"messages\":[{\"type\":\"lbm-sdk/MsgGrant\",\"value\":{\"authority\":\"%s\",\"authorization\":{\"type\":\"lbm-sdk/CreateValidatorAuthorization\",\"value\":{\"validator_address\":\"%s\"}},\"grantee\":\"%s\"}}],\"metadata\":\"CreateValidatorAuthorization\",\"proposers\":[\"%s\"]}}],\"sequence\":\"1\",\"timeout_height\":\"1\"}", operator.String(), validatorAddr.String(), grantee.String(), proposer.String()), - }, } for name, tc := range testCases {