From 1afc2b671041c81ef0f8bb9f7e218780dac051b0 Mon Sep 17 00:00:00 2001 From: kogisin Date: Thu, 14 Apr 2022 13:59:08 +0900 Subject: [PATCH 1/2] fix: fix gas consumption issue for vote condition type --- x/claim/keeper/claim.go | 15 ++++++-- x/claim/keeper/claim_test.go | 61 +++++++++++++++++++++++++++++++ x/claim/types/expected_keepers.go | 3 +- 3 files changed, 74 insertions(+), 5 deletions(-) diff --git a/x/claim/keeper/claim.go b/x/claim/keeper/claim.go index d5f79812..d4bd8d44 100644 --- a/x/claim/keeper/claim.go +++ b/x/claim/keeper/claim.go @@ -66,11 +66,15 @@ func (k Keeper) ValidateCondition(ctx sdk.Context, recipient sdk.AccAddress, ct switch ct { case types.ConditionTypeDeposit: + // Expect this condition to be executed with multiple messages (MsgDeposit + MsgClaim) + // in a single transaction because deposit request gets deleted at the begin blocker if len(k.liquidityKeeper.GetDepositRequestsByDepositor(ctx, recipient)) != 0 { ok = true } case types.ConditionTypeSwap: + // Expect this condition to be executed with multiple messages (MsgLimitOrder or MsgMarketOrder + MsgClaim) + // in a single transaction because order request gets deleted at the begin blocker after order lifespan if len(k.liquidityKeeper.GetOrdersByOrderer(ctx, recipient)) != 0 { ok = true } @@ -84,10 +88,13 @@ func (k Keeper) ValidateCondition(ctx sdk.Context, recipient sdk.AccAddress, ct } case types.ConditionTypeVote: - k.govKeeper.IterateAllVotes(ctx, func(vote govtypes.Vote) (stop bool) { - if vote.Voter == recipient.String() { - ok = true - return true + k.govKeeper.IterateProposals(ctx, func(proposal govtypes.Proposal) (stop bool) { + if proposal.Status == govtypes.StatusVotingPeriod { + _, found := k.govKeeper.GetVote(ctx, proposal.ProposalId, recipient) + if found { + ok = true + return true + } } return false }) diff --git a/x/claim/keeper/claim_test.go b/x/claim/keeper/claim_test.go index b43efa52..b175f6be 100644 --- a/x/claim/keeper/claim_test.go +++ b/x/claim/keeper/claim_test.go @@ -494,3 +494,64 @@ func (s *KeeperTestSuite) TestClaim_Partial_TerminatAirdrop() { feePool := s.app.DistrKeeper.GetFeePool(s.ctx) s.Require().False(feePool.CommunityPool.IsZero()) } + +func (s *KeeperTestSuite) TestSimulateGasUsage_VoteCondition() { + // Create an airdrop + sourceAddr := s.addr(0) + airdrop := s.createAirdrop( + 1, + sourceAddr, + utils.ParseCoins("100000000000denom1"), + []types.ConditionType{ + types.ConditionTypeDeposit, + types.ConditionTypeSwap, + types.ConditionTypeLiquidStake, + types.ConditionTypeVote, + }, + s.ctx.BlockTime(), + s.ctx.BlockTime().AddDate(0, 6, 0), + true, + ) + + // Submit governance proposals + s.createTextProposal(sourceAddr, "Text1", "Description") + s.createTextProposal(sourceAddr, "Text2", "Description") + + recipients := []sdk.AccAddress{} + numRecipients := 10000 + + // Claim records for all recipients + for i := 1; i <= numRecipients; i++ { + recipient := s.addr(i) + recipients = append(recipients, recipient) + + s.createClaimRecord( + airdrop.Id, + recipient, + utils.ParseCoins("1000000denom1"), + utils.ParseCoins("1000000denom1"), + []types.ConditionType{}, + ) + + _, found := s.keeper.GetClaimRecordByRecipient(s.ctx, airdrop.Id, recipient) + s.Require().True(found) + } + + for _, recipient := range recipients[:5000] { + s.vote(recipient, 1, govtypes.OptionYes) + } + + // Vote proposal and claim condition + for i, recipient := range recipients[5000:] { + gasConsumedBefore := s.ctx.GasMeter().GasConsumed() + + s.vote(recipient, 2, govtypes.OptionYes) + + _, err := s.keeper.Claim(s.ctx, types.NewMsgClaim(airdrop.Id, recipient, types.ConditionTypeVote)) + s.Require().NoError(err) + + gasConsumed := s.ctx.GasMeter().GasConsumed() + gasConsumed = gasConsumed - gasConsumedBefore + s.T().Logf("[%d] GasConsumed: %d\n", i+1, gasConsumed) + } +} diff --git a/x/claim/types/expected_keepers.go b/x/claim/types/expected_keepers.go index aabe470e..311d45e6 100644 --- a/x/claim/types/expected_keepers.go +++ b/x/claim/types/expected_keepers.go @@ -26,7 +26,8 @@ type DistrKeeper interface { } type GovKeeper interface { - IterateAllVotes(ctx sdk.Context, cb func(vote govtypes.Vote) (stop bool)) + IterateProposals(ctx sdk.Context, cb func(proposal govtypes.Proposal) (stop bool)) + GetVote(ctx sdk.Context, proposalID uint64, voterAddr sdk.AccAddress) (vote govtypes.Vote, found bool) } // LiquidityKeeper defines the expected interface needed to check the condition. From 43c51ccdcea4bda1da5a9a984e3f798f63266126 Mon Sep 17 00:00:00 2001 From: kogisin Date: Thu, 14 Apr 2022 16:03:00 +0900 Subject: [PATCH 2/2] test: remove print --- x/claim/keeper/claim_test.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/x/claim/keeper/claim_test.go b/x/claim/keeper/claim_test.go index b175f6be..ed4f7e4c 100644 --- a/x/claim/keeper/claim_test.go +++ b/x/claim/keeper/claim_test.go @@ -541,8 +541,11 @@ func (s *KeeperTestSuite) TestSimulateGasUsage_VoteCondition() { s.vote(recipient, 1, govtypes.OptionYes) } + // Expected gas threshold + expConsumedGasLimit := sdk.Gas(100_000) + // Vote proposal and claim condition - for i, recipient := range recipients[5000:] { + for _, recipient := range recipients[5000:] { gasConsumedBefore := s.ctx.GasMeter().GasConsumed() s.vote(recipient, 2, govtypes.OptionYes) @@ -552,6 +555,6 @@ func (s *KeeperTestSuite) TestSimulateGasUsage_VoteCondition() { gasConsumed := s.ctx.GasMeter().GasConsumed() gasConsumed = gasConsumed - gasConsumedBefore - s.T().Logf("[%d] GasConsumed: %d\n", i+1, gasConsumed) + s.Require().LessOrEqual(gasConsumed, expConsumedGasLimit) } }