Skip to content

Commit

Permalink
impr(claims): move claimsRecords deletion out of iteration (#514)
Browse files Browse the repository at this point in the history 
* impr(claims): move claimsRecords deletion out of iteration

* refactor with testutil.FundModuleAccount()

* get balance instead of balances

* add test for claiming back multiple empty accounts
  • Loading branch information
@danburck
danburck committed Apr 20, 2022
1 parent a722946 commit 72ec2c8
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 86 deletions.
14 changes: 4 additions & 10 deletions x/claims/genesis_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ import (
feemarkettypes "github.com/tharsis/ethermint/x/feemarket/types"

"github.com/tharsis/evmos/v3/app"
"github.com/tharsis/evmos/v3/testutil"
"github.com/tharsis/evmos/v3/x/claims"
"github.com/tharsis/evmos/v3/x/claims/types"
inflationtypes "github.com/tharsis/evmos/v3/x/inflation/types"
)

type GenesisTestSuite struct {
Expand Down Expand Up @@ -114,9 +114,7 @@ func (suite *GenesisTestSuite) TestClaimInitGenesis() {
},
func() {
coins := sdk.NewCoins(sdk.NewCoin("aevmos", sdk.NewInt(2_800)))
err := suite.app.BankKeeper.MintCoins(suite.ctx, inflationtypes.ModuleName, coins)
suite.Require().NoError(err)
err = suite.app.BankKeeper.SendCoinsFromModuleToModule(suite.ctx, inflationtypes.ModuleName, types.ModuleName, coins)
err := testutil.FundModuleAccount(suite.app.BankKeeper, suite.ctx, types.ModuleName, coins)
suite.Require().NoError(err)
},
false,
Expand All @@ -140,9 +138,7 @@ func (suite *GenesisTestSuite) TestClaimInitGenesis() {
},
func() {
coins := sdk.NewCoins(sdk.NewCoin("aevmos", sdk.NewInt(400)))
err := suite.app.BankKeeper.MintCoins(suite.ctx, inflationtypes.ModuleName, coins)
suite.Require().NoError(err)
err = suite.app.BankKeeper.SendCoinsFromModuleToModule(suite.ctx, inflationtypes.ModuleName, types.ModuleName, coins)
err := testutil.FundModuleAccount(suite.app.BankKeeper, suite.ctx, types.ModuleName, coins)
suite.Require().NoError(err)
},
false,
Expand Down Expand Up @@ -189,9 +185,7 @@ func (suite *GenesisTestSuite) TestClaimExportGenesis() {
}

coins := sdk.NewCoins(sdk.NewCoin("aevmos", sdk.NewInt(400)))
err := suite.app.BankKeeper.MintCoins(suite.ctx, inflationtypes.ModuleName, coins)
suite.Require().NoError(err)
err = suite.app.BankKeeper.SendCoinsFromModuleToModule(suite.ctx, inflationtypes.ModuleName, types.ModuleName, coins)
err := testutil.FundModuleAccount(suite.app.BankKeeper, suite.ctx, types.ModuleName, coins)
suite.Require().NoError(err)

claims.InitGenesis(suite.ctx, *suite.app.ClaimsKeeper, suite.genesis)
Expand Down
31 changes: 18 additions & 13 deletions x/claims/keeper/abci.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,20 +78,25 @@ func (k Keeper) ClawbackEscrowedTokens(ctx sdk.Context) error {
return nil
}

// ClawbackEmptyAccounts performs the clawback of all allocated tokens
// from airdrop recipient accounts with a sequence number of 0 (i.e the account
// hasn't performed a single tx during the claim window).
// Once the account is clawbacked, the claims record is deleted from state.
// ClawbackEmptyAccounts performs the clawback of all allocated tokens from
// airdrop recipient accounts with a sequence number of 0 (i.e the account
// hasn't performed a single tx during the claim window). Once the account is
// clawbacked, the claims record is deleted from state.
func (k Keeper) ClawbackEmptyAccounts(ctx sdk.Context, claimsDenom string) {
totalClawback := sdk.Coins{}
logger := k.Logger(ctx)

accPruned := int64(0)
accClawbacked := int64(0)

var addresses []sdk.AccAddress

k.IterateClaimsRecords(ctx, func(addr sdk.AccAddress, _ types.ClaimsRecord) (stop bool) {
// delete claims record once the account balance is clawed back
defer k.DeleteClaimsRecord(ctx, addr)
// NOTE: we cannot delete the record while iterating over it
// Ref: https://github.com/cosmos/cosmos-sdk/blob/c2fd51b4c5f41efc56c9aec1f44b4ce9e963dfc3/store/types/store.go#L215-L221
defer func() {
addresses = append(addresses, addr)
}()

acc := k.accountKeeper.GetAccount(ctx, addr)
if acc == nil {
Expand Down Expand Up @@ -120,20 +125,15 @@ func (k Keeper) ClawbackEmptyAccounts(ctx sdk.Context, claimsDenom string) {
return false
}

balances := k.bankKeeper.GetAllBalances(ctx, addr)
clawbackCoin := k.bankKeeper.GetBalance(ctx, addr, claimsDenom)

// prune empty accounts from the airdrop
if balances == nil || balances.IsZero() {
if clawbackCoin.IsZero() {
k.accountKeeper.RemoveAccount(ctx, acc)
accPruned++
return false
}

clawbackCoin := sdk.Coin{Denom: claimsDenom, Amount: balances.AmountOfNoDenomValidation(claimsDenom)}
if !clawbackCoin.IsPositive() {
return false
}

// Send all unclaimed airdropped coins back to the community pool
// and prune those inactive wallets from current state.
// "Unclaimed" tokens are defined as being in wallets which have a sequence
Expand All @@ -154,6 +154,11 @@ func (k Keeper) ClawbackEmptyAccounts(ctx sdk.Context, claimsDenom string) {
return false
})

// delete claims record once the account balance is clawed back
for _, addr := range addresses {
k.DeleteClaimsRecord(ctx, addr)
}

logger.Info(
"clawed back funds into community pool",
"total", totalClawback.String(),
Expand Down
38 changes: 29 additions & 9 deletions x/claims/keeper/abci_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (

"github.com/tharsis/evmos/v3/testutil"
"github.com/tharsis/evmos/v3/x/claims/types"
inflationtypes "github.com/tharsis/evmos/v3/x/inflation/types"
vestingtypes "github.com/tharsis/evmos/v3/x/vesting/types"
)

Expand Down Expand Up @@ -61,6 +60,8 @@ func (suite *KeeperTestSuite) TestEndBlock() {

func (suite *KeeperTestSuite) TestClawbackEmptyAccounts() {
addr := sdk.AccAddress(tests.GenerateAddress().Bytes())
addr2 := sdk.AccAddress(tests.GenerateAddress().Bytes())
addr3 := sdk.AccAddress(tests.GenerateAddress().Bytes())

var amount int64 = 10000

Expand Down Expand Up @@ -109,9 +110,7 @@ func (suite *KeeperTestSuite) TestClawbackEmptyAccounts() {
vestingAcc := vestingtypes.NewClawbackVestingAccount(bAcc, funder, coins, time.Now().UTC(), nil, nil)
suite.app.AccountKeeper.SetAccount(suite.ctx, vestingAcc)

err := suite.app.BankKeeper.MintCoins(suite.ctx, inflationtypes.ModuleName, coins)
suite.Require().NoError(err)
err = suite.app.BankKeeper.SendCoinsFromModuleToAccount(suite.ctx, inflationtypes.ModuleName, addr, coins)
err := testutil.FundModuleAccount(suite.app.BankKeeper, suite.ctx, types.ModuleName, coins)
suite.Require().NoError(err)
suite.app.ClaimsKeeper.SetClaimsRecord(suite.ctx, addr, types.ClaimsRecord{})
},
Expand All @@ -135,11 +134,30 @@ func (suite *KeeperTestSuite) TestClawbackEmptyAccounts() {
suite.app.AccountKeeper.SetAccount(suite.ctx, authtypes.NewBaseAccount(addr, nil, 0, 0))

coins := sdk.NewCoins(sdk.NewCoin("testcoin", sdk.NewInt(amount)))
err := suite.app.BankKeeper.MintCoins(suite.ctx, inflationtypes.ModuleName, coins)
err := testutil.FundAccount(suite.app.BankKeeper, suite.ctx, addr, coins)
suite.Require().NoError(err)
suite.app.ClaimsKeeper.SetClaimsRecord(suite.ctx, addr, types.ClaimsRecord{})
},
},
{
"multiple accounts, all clawed back",
amount * 3,
func() {
suite.app.AccountKeeper.SetAccount(suite.ctx, authtypes.NewBaseAccount(addr, nil, 0, 0))
suite.app.AccountKeeper.SetAccount(suite.ctx, authtypes.NewBaseAccount(addr2, nil, 0, 0))
suite.app.AccountKeeper.SetAccount(suite.ctx, authtypes.NewBaseAccount(addr3, nil, 0, 0))

coins := sdk.NewCoins(sdk.NewCoin("aevmos", sdk.NewInt(amount)))
err := testutil.FundAccount(suite.app.BankKeeper, suite.ctx, addr, coins)
suite.Require().NoError(err)
err = suite.app.BankKeeper.SendCoinsFromModuleToAccount(suite.ctx, inflationtypes.ModuleName, addr, coins)
err = testutil.FundAccount(suite.app.BankKeeper, suite.ctx, addr2, coins)
suite.Require().NoError(err)
err = testutil.FundAccount(suite.app.BankKeeper, suite.ctx, addr3, coins)
suite.Require().NoError(err)

suite.app.ClaimsKeeper.SetClaimsRecord(suite.ctx, addr, types.ClaimsRecord{})
suite.app.ClaimsKeeper.SetClaimsRecord(suite.ctx, addr2, types.ClaimsRecord{})
suite.app.ClaimsKeeper.SetClaimsRecord(suite.ctx, addr3, types.ClaimsRecord{})
},
},
}
Expand All @@ -154,6 +172,10 @@ func (suite *KeeperTestSuite) TestClawbackEmptyAccounts() {
moduleAcc := suite.app.AccountKeeper.GetModuleAccount(suite.ctx, distrtypes.ModuleName)
balance := suite.app.BankKeeper.GetBalance(suite.ctx, moduleAcc.GetAddress(), "aevmos")
suite.Require().Equal(tc.expBalance, balance.Amount.Int64())

// test that all claims records are deleted
claimsRecords := suite.app.ClaimsKeeper.GetClaimsRecords(suite.ctx)
suite.Require().Len(claimsRecords, 0)
})
}
}
Expand All @@ -177,9 +199,7 @@ func (suite *KeeperTestSuite) TestClawbackEscrowedTokensABCI() {
amount,
func() {
coins := sdk.NewCoins(sdk.NewCoin("aevmos", sdk.NewInt(amount)))
err := suite.app.BankKeeper.MintCoins(suite.ctx, inflationtypes.ModuleName, coins)
suite.Require().NoError(err)
err = suite.app.BankKeeper.SendCoinsFromModuleToModule(suite.ctx, inflationtypes.ModuleName, types.ModuleName, coins)
err := testutil.FundModuleAccount(suite.app.BankKeeper, suite.ctx, types.ModuleName, coins)
suite.Require().NoError(err)
},
},
Expand Down
32 changes: 8 additions & 24 deletions x/claims/keeper/claim_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -280,9 +280,7 @@ func (suite *KeeperTestSuite) TestClaimCoinsForAction() {
"failed - error during community pool fund",
func() {
coins := sdk.NewCoins(sdk.NewCoin(types.DefaultClaimsDenom, sdk.NewInt(25)))
err := suite.app.BankKeeper.MintCoins(suite.ctx, inflationtypes.ModuleName, coins)
suite.Require().NoError(err)
err = suite.app.BankKeeper.SendCoinsFromModuleToModule(suite.ctx, inflationtypes.ModuleName, types.ModuleName, coins)
err := testutil.FundModuleAccount(suite.app.BankKeeper, suite.ctx, types.ModuleName, coins)
suite.Require().NoError(err)
},
types.NewClaimsRecord(sdk.NewInt(200)),
Expand All @@ -303,9 +301,7 @@ func (suite *KeeperTestSuite) TestClaimCoinsForAction() {
"success - claim single action",
func() {
coins := sdk.NewCoins(sdk.NewCoin(types.DefaultClaimsDenom, sdk.NewInt(50)))
err := suite.app.BankKeeper.MintCoins(suite.ctx, inflationtypes.ModuleName, coins)
suite.Require().NoError(err)
err = suite.app.BankKeeper.SendCoinsFromModuleToModule(suite.ctx, inflationtypes.ModuleName, types.ModuleName, coins)
err := testutil.FundModuleAccount(suite.app.BankKeeper, suite.ctx, types.ModuleName, coins)
suite.Require().NoError(err)
},
types.NewClaimsRecord(sdk.NewInt(200)),
Expand All @@ -326,9 +322,7 @@ func (suite *KeeperTestSuite) TestClaimCoinsForAction() {
"success - claim single action during decay",
func() {
coins := sdk.NewCoins(sdk.NewCoin(types.DefaultClaimsDenom, sdk.NewInt(50)))
err := suite.app.BankKeeper.MintCoins(suite.ctx, inflationtypes.ModuleName, coins)
suite.Require().NoError(err)
err = suite.app.BankKeeper.SendCoinsFromModuleToModule(suite.ctx, inflationtypes.ModuleName, types.ModuleName, coins)
err := testutil.FundModuleAccount(suite.app.BankKeeper, suite.ctx, types.ModuleName, coins)
suite.Require().NoError(err)
},
types.NewClaimsRecord(sdk.NewInt(200)),
Expand All @@ -349,9 +343,7 @@ func (suite *KeeperTestSuite) TestClaimCoinsForAction() {
"success - claimed all actions",
func() {
coins := sdk.NewCoins(sdk.NewCoin(types.DefaultClaimsDenom, sdk.NewInt(50)))
err := suite.app.BankKeeper.MintCoins(suite.ctx, inflationtypes.ModuleName, coins)
suite.Require().NoError(err)
err = suite.app.BankKeeper.SendCoinsFromModuleToModule(suite.ctx, inflationtypes.ModuleName, types.ModuleName, coins)
err := testutil.FundModuleAccount(suite.app.BankKeeper, suite.ctx, types.ModuleName, coins)
suite.Require().NoError(err)
},
types.ClaimsRecord{
Expand Down Expand Up @@ -441,9 +433,7 @@ func (suite *KeeperTestSuite) TestMergeClaimRecords() {
initialCommunityPoolCoins := suite.app.DistrKeeper.GetFeePoolCommunityCoins(suite.ctx)

coins := sdk.Coins{sdk.NewCoin(params.ClaimsDenom, sdk.NewInt(100))}
err := suite.app.BankKeeper.MintCoins(suite.ctx, inflationtypes.ModuleName, coins)
suite.Require().NoError(err)
err = suite.app.BankKeeper.SendCoinsFromModuleToModule(suite.ctx, inflationtypes.ModuleName, types.ModuleName, coins)
err := testutil.FundModuleAccount(suite.app.BankKeeper, suite.ctx, types.ModuleName, coins)
suite.Require().NoError(err)

params := types.Params{
Expand Down Expand Up @@ -482,9 +472,7 @@ func (suite *KeeperTestSuite) TestMergeClaimRecords() {
expBalance := suite.app.BankKeeper.GetBalance(suite.ctx, recipient, params.ClaimsDenom)

coins := sdk.Coins{sdk.NewCoin(params.ClaimsDenom, sdk.NewInt(100))}
err := suite.app.BankKeeper.MintCoins(suite.ctx, inflationtypes.ModuleName, coins)
suite.Require().NoError(err)
err = suite.app.BankKeeper.SendCoinsFromModuleToModule(suite.ctx, inflationtypes.ModuleName, types.ModuleName, coins)
err := testutil.FundModuleAccount(suite.app.BankKeeper, suite.ctx, types.ModuleName, coins)
suite.Require().NoError(err)

mergedRecord, err := suite.app.ClaimsKeeper.MergeClaimsRecords(suite.ctx, recipient, senderClaimsRecord, recipientClaimsRecord, params)
Expand Down Expand Up @@ -516,9 +504,7 @@ func (suite *KeeperTestSuite) TestMergeClaimRecords() {
expBalance := suite.app.BankKeeper.GetBalance(suite.ctx, recipient, params.ClaimsDenom)

coins := sdk.Coins{sdk.NewCoin(params.ClaimsDenom, sdk.NewInt(250))}
err := suite.app.BankKeeper.MintCoins(suite.ctx, inflationtypes.ModuleName, coins)
suite.Require().NoError(err)
err = suite.app.BankKeeper.SendCoinsFromModuleToModule(suite.ctx, inflationtypes.ModuleName, types.ModuleName, coins)
err := testutil.FundModuleAccount(suite.app.BankKeeper, suite.ctx, types.ModuleName, coins)
suite.Require().NoError(err)

mergedRecord, err := suite.app.ClaimsKeeper.MergeClaimsRecords(suite.ctx, recipient, senderClaimsRecord, recipientClaimsRecord, params)
Expand Down Expand Up @@ -579,9 +565,7 @@ func (suite *KeeperTestSuite) TestMergeClaimRecords() {
suite.app.ClaimsKeeper.SetParams(suite.ctx, params)

coins := sdk.Coins{sdk.NewCoin(params.ClaimsDenom, sdk.NewInt(50))}
err := suite.app.BankKeeper.MintCoins(suite.ctx, inflationtypes.ModuleName, coins)
suite.Require().NoError(err)
err = suite.app.BankKeeper.SendCoinsFromModuleToModule(suite.ctx, inflationtypes.ModuleName, types.ModuleName, coins)
err := testutil.FundModuleAccount(suite.app.BankKeeper, suite.ctx, types.ModuleName, coins)
suite.Require().NoError(err)

_, err = suite.app.ClaimsKeeper.MergeClaimsRecords(suite.ctx, recipient, senderClaimsRecord, recipientClaimsRecord, params)
Expand Down
6 changes: 2 additions & 4 deletions x/claims/keeper/grpc_query_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/tharsis/ethermint/tests"
"github.com/tharsis/evmos/v3/testutil"
"github.com/tharsis/evmos/v3/x/claims/types"
inflationtypes "github.com/tharsis/evmos/v3/x/inflation/types"
)

func (suite *KeeperTestSuite) TestTotalUnclaimed() {
Expand All @@ -25,9 +25,7 @@ func (suite *KeeperTestSuite) TestTotalUnclaimed() {
{
"non-empty balance",
func() {
err := suite.app.BankKeeper.MintCoins(suite.ctx, inflationtypes.ModuleName, coins)
suite.Require().NoError(err)
err = suite.app.BankKeeper.SendCoinsFromModuleToModule(suite.ctx, inflationtypes.ModuleName, types.ModuleName, coins)
err := testutil.FundModuleAccount(suite.app.BankKeeper, suite.ctx, types.ModuleName, coins)
suite.Require().NoError(err)
}, coins,
},
Expand Down
14 changes: 4 additions & 10 deletions x/claims/keeper/hooks_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import (
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
ethtypes "github.com/ethereum/go-ethereum/core/types"
"github.com/tharsis/ethermint/tests"
"github.com/tharsis/evmos/v3/testutil"
"github.com/tharsis/evmos/v3/x/claims/types"
inflationtypes "github.com/tharsis/evmos/v3/x/inflation/types"
)

func (suite *KeeperTestSuite) TestAfterProposalVote() {
Expand Down Expand Up @@ -82,9 +82,7 @@ func (suite *KeeperTestSuite) TestAfterProposalVote() {
expBalance := suite.app.BankKeeper.GetBalance(suite.ctx, addr, params.ClaimsDenom)

coins := sdk.Coins{sdk.NewCoin(params.ClaimsDenom, sdk.NewInt(250))}
err := suite.app.BankKeeper.MintCoins(suite.ctx, inflationtypes.ModuleName, coins)
suite.Require().NoError(err)
err = suite.app.BankKeeper.SendCoinsFromModuleToModule(suite.ctx, inflationtypes.ModuleName, types.ModuleName, coins)
err := testutil.FundModuleAccount(suite.app.BankKeeper, suite.ctx, types.ModuleName, coins)
suite.Require().NoError(err)

suite.app.ClaimsKeeper.AfterProposalVote(suite.ctx, 1, addr)
Expand Down Expand Up @@ -207,9 +205,7 @@ func (suite *KeeperTestSuite) TestAfterDelegation() {
expBalance := suite.app.BankKeeper.GetBalance(suite.ctx, addr, params.ClaimsDenom)

coins := sdk.Coins{sdk.NewCoin(params.ClaimsDenom, sdk.NewInt(250))}
err := suite.app.BankKeeper.MintCoins(suite.ctx, inflationtypes.ModuleName, coins)
suite.Require().NoError(err)
err = suite.app.BankKeeper.SendCoinsFromModuleToModule(suite.ctx, inflationtypes.ModuleName, types.ModuleName, coins)
err := testutil.FundModuleAccount(suite.app.BankKeeper, suite.ctx, types.ModuleName, coins)
suite.Require().NoError(err)

suite.app.ClaimsKeeper.AfterDelegationModified(suite.ctx, addr, addr2)
Expand Down Expand Up @@ -341,9 +337,7 @@ func (suite *KeeperTestSuite) TestAfterEVMStateTransition() {
expBalance := suite.app.BankKeeper.GetBalance(suite.ctx, addr, params.ClaimsDenom)

coins := sdk.Coins{sdk.NewCoin(params.ClaimsDenom, sdk.NewInt(250))}
err := suite.app.BankKeeper.MintCoins(suite.ctx, inflationtypes.ModuleName, coins)
suite.Require().NoError(err)
err = suite.app.BankKeeper.SendCoinsFromModuleToModule(suite.ctx, inflationtypes.ModuleName, types.ModuleName, coins)
err := testutil.FundModuleAccount(suite.app.BankKeeper, suite.ctx, types.ModuleName, coins)
suite.Require().NoError(err)

err = suite.app.ClaimsKeeper.AfterEVMStateTransition(suite.ctx, msg, &receipt)
Expand Down
Loading

0 comments on commit 72ec2c8

Please sign in to comment.