Merge pull request #23 from queencre/cherry-pick/294

fix: fix gas consumption issue for `ConditionTypeVote`

CHANGELOG.md

@@ -38,6 +38,14 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ## [Unreleased]
 
+## [v1.1.0] - 2022-04-14
+
+### State Machine Breaking
+
+Running a full node will encounter wrong app hash issue if it doesn't upgrade to this version prior to `UpgradeHeight (48000)`. Instead of going through on-chain governance proposal by using `UpgradeProposal`, this upgrade mechanism is chosen as it is security hot fix that is better to be fixed as soon as it can and also it is directly related to governance proposal.
+
+* (x/claim) [\#23](https://github.com/crescent-network/crescent/pull/23) Fix gas consumption issue for `ConditionTypeVote`. `UpgradeHeight` is set as `48000`.
+
 ## [v1.0.0] - 2022-04-12
 
 ### Features
@@ -54,4 +62,5 @@ Ref: https://keepachangelog.com/en/1.0.0/
   * `x/bank` feat: Add dynamic blockedAddrs
 
 [Unreleased]: https://github.com/crescent-network/crescent/compare/v1.0.0...HEAD
-[v1.0.0]: https://github.com/crescent-network/crescent/releases/tag/v1.0.0
+[v1.0.0]: https://github.com/crescent-network/crescent/releases/tag/v1.0.0
+[v1.1.0]: https://github.com/crescent-network/crescent/releases/tag/v1.1.0

README.md

@@ -45,11 +45,11 @@ If you haven't already, install Go by following the [official docs](https://gola
 
 **Step 2. Get Crescent Core source code**
 
-Use `git` to retrieve Crescent Core from the [official repo](https://github.com/crescent-network/crescent/) and checkout the `release/v1.0.x` branch. This branch contains the latest release, which will install the `crescentd` binary.
+Use `git` to retrieve Crescent Core from the [official repo](https://github.com/crescent-network/crescent/) and checkout the `release/v1.1.x` branch. This branch contains the latest release, which will install the `crescentd` binary.
 
 ```bash
 git clone https://github.com/crescent-network/crescent.git
-cd crescent && git checkout release/v1.0.x
+cd crescent && git checkout release/v1.1.x
 make install
 ```
 
@@ -74,6 +74,9 @@ Crescent Core uses a customized Cosmos SDK. Please check the differences on [her
 
 The documentation is available in [docs](docs) directory. If you are a developer interested in technical specification, see inside each `x/{module}`'s `spec` directory.
 
+* [Crescent Official Docs](https://docs.crescent.network/)
+* [Swagger API Docs](https://app.swaggerhub.com/apis-docs/crescent/crescent)
+
 ## Community
 
 * [Official Website](https://crescent.network/)

app/upgrades/mainnet/v1.1.0/upgrade.go

@@ -0,0 +1,4 @@
+package v1_1_0
+
+// UpgradeHeight defines the upgrade height that is used in claim module
+const UpgradeHeight = int64(48000)

x/claim/keeper/claim.go

@@ -7,6 +7,7 @@ import (
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
 
+	v1_1_0 "github.com/crescent-network/crescent/app/upgrades/mainnet/v1.1.0"
 	"github.com/crescent-network/crescent/x/claim/types"
 )
 
@@ -66,11 +67,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,13 +89,29 @@ 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
-			}
-			return false
-		})
+		// IterateAllVotes consumes more gas as a number of votes increase.
+		// To prevent from letting some airdrop recipients experience out of gas issue,
+		// an upgrade is inevitable after the UpgradeHeight.
+		if ctx.BlockHeight() < v1_1_0.UpgradeHeight {
+			k.govKeeper.IterateAllVotes(ctx, func(vote govtypes.Vote) (stop bool) {
+				if vote.Voter == recipient.String() {
+					ok = true
+					return true
+				}
+				return false
+			})
+		} else {
+			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
+			})
+		}
 	}
 
 	if !ok {

x/claim/keeper/claim_test.go

@@ -4,6 +4,7 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
 
+	v1_1_0 "github.com/crescent-network/crescent/app/upgrades/mainnet/v1.1.0"
 	utils "github.com/crescent-network/crescent/types"
 	"github.com/crescent-network/crescent/x/claim"
 	"github.com/crescent-network/crescent/x/claim/types"
@@ -494,3 +495,151 @@ 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)
+	}
+
+	// Set upgrade height
+	s.ctx = s.ctx.WithBlockHeight(v1_1_0.UpgradeHeight)
+
+	// Expected gas threshold
+	expConsumedGasLimit := sdk.Gas(100_000)
+
+	// Vote proposal and claim condition
+	for _, 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.Require().LessOrEqual(gasConsumed, expConsumedGasLimit)
+	}
+}
+
+func (s *KeeperTestSuite) TestGasConsumption_Upgrade_v1_0_0() {
+	// 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 := 10100
+
+	// 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[:10000] {
+		s.vote(recipient, 1, govtypes.OptionYes)
+	}
+
+	// Expected gas threshold
+	expConsumedGasLimit := sdk.Gas(100_000)
+
+	// Vote proposal and claim condition
+	for _, recipient := range recipients[10000:10050] {
+		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.Require().GreaterOrEqual(gasConsumed, expConsumedGasLimit)
+	}
+
+	// Set upgrade height
+	s.ctx = s.ctx.WithBlockHeight(v1_1_0.UpgradeHeight)
+
+	// Vote proposal and claim condition
+	for _, recipient := range recipients[10050:10100] {
+		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.Require().LessOrEqual(gasConsumed, expConsumedGasLimit)
+	}
+}

x/claim/types/expected_keepers.go

@@ -26,6 +26,9 @@ type DistrKeeper interface {
 }
 
 type GovKeeper interface {
+	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)
+	// Note that this function is only used before the UpgradeHeight defined in app/upgrades/v1.1.0
 	IterateAllVotes(ctx sdk.Context, cb func(vote govtypes.Vote) (stop bool))
 }