Merge pull request from GHSA-5jgq-x857-p8xw

* fix: only enable claims from authorized channels * changelog * changelog 2 * fix tests * rename to authorized * update forks * upgrade height * check for EVM chains * bonded ration adjustment * update genesis * fixes * swagger
evmos · Mar 6, 2022 · 2887025 · 2887025
1 parent a5a1221
commit 2887025
Show file tree

Hide file tree

Showing 16 changed files with 233 additions and 17 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -37,6 +37,10 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ## [v2.0.0] - 2022-03-06
 
+### State Machine Breaking
+
+- (claims) Restrict claiming to a list of authorized IBC channels.
+
 ### Improvements
 
 - (deps) [\#360](https://github.com/tharsis/evmos/pull/360) Bump Ethermint to [`v0.11.0`](https://github.com/tharsis/ethermint/releases/tag/v0.11.0)

diff --git a/client/docs/statik/statik.go b/client/docs/statik/statik.go
diff --git a/client/docs/swagger-ui/swagger.yaml b/client/docs/swagger-ui/swagger.yaml
@@ -250,6 +250,20 @@ paths:
                   claims_denom:
                     type: string
                     title: denom of claimable coin
+                  authorized_channels:
+                    type: array
+                    items:
+                      type: string
+                    description: >-
+                      list of authorized channel identifiers that can perform
+                      address attestations
+
+                      via IBC.
+                  evm_channels:
+                    type: array
+                    items:
+                      type: string
+                    title: list of channel identifiers from EVM compatible chains
             description: >-
               QueryParamsResponse is the response type for the Query/Params RPC
               method.
@@ -26870,6 +26884,20 @@ definitions:
       claims_denom:
         type: string
         title: denom of claimable coin
+      authorized_channels:
+        type: array
+        items:
+          type: string
+        description: >-
+          list of authorized channel identifiers that can perform address
+          attestations
+
+          via IBC.
+      evm_channels:
+        type: array
+        items:
+          type: string
+        title: list of channel identifiers from EVM compatible chains
     description: Params defines the claims module's parameters.
   evmos.claims.v1.QueryClaimsRecordResponse:
     type: object
@@ -26983,6 +27011,20 @@ definitions:
           claims_denom:
             type: string
             title: denom of claimable coin
+          authorized_channels:
+            type: array
+            items:
+              type: string
+            description: >-
+              list of authorized channel identifiers that can perform address
+              attestations
+
+              via IBC.
+          evm_channels:
+            type: array
+            items:
+              type: string
+            title: list of channel identifiers from EVM compatible chains
     description: QueryParamsResponse is the response type for the Query/Params RPC method.
   evmos.claims.v1.QueryTotalUnclaimedResponse:
     type: object

diff --git a/proto/evmos/claims/v1/genesis.proto b/proto/evmos/claims/v1/genesis.proto
@@ -12,7 +12,6 @@ option go_package = "github.com/tharsis/evmos/v2/x/claims/types";
 message GenesisState {
   // params defines all the parameters of the module.
   Params params = 1 [ (gogoproto.nullable) = false ];
-
   // list of claim records with the corresponding airdrop recipient
   repeated ClaimsRecordAddress claims_records = 2
       [ (gogoproto.nullable) = false ];

diff --git a/types/errors.go b/types/errors.go
@@ -0,0 +1,18 @@
+package types
+
+import (
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+)
+
+// RootCodespace is the codespace for all errors defined in this package
+const RootCodespace = "evmos"
+
+// root error codes for Evmos
+const (
+	codeKeyTypeNotSupported = iota + 2
+)
+
+// errors
+var (
+	ErrKeyTypeNotSupported = sdkerrors.Register(RootCodespace, codeKeyTypeNotSupported, "key type 'secp256k1' not supported")
+)
diff --git a/x/claims/keeper/ibc_callbacks.go b/x/claims/keeper/ibc_callbacks.go
@@ -9,6 +9,7 @@ import (
 	channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types"
 	"github.com/cosmos/ibc-go/v3/modules/core/exported"
 
+	evmos "github.com/tharsis/evmos/v2/types"
 	"github.com/tharsis/evmos/v2/x/claims/types"
 )
 
@@ -21,7 +22,7 @@ func (k Keeper) OnRecvPacket(
 ) exported.Acknowledgement {
 	params := k.GetParams(ctx)
 
-	// short circuit in case claim is not active (no-op)
+	// short (no-op) circuit by returning original ACK in case the claim is not active
 	if !params.IsClaimsActive(ctx.BlockTime()) {
 		return ack
 	}
@@ -60,6 +61,38 @@ func (k Keeper) OnRecvPacket(
 	}
 
 	senderClaimsRecord, senderRecordFound := k.GetClaimsRecord(ctx, sender)
+
+	// NOTE: we know that the connected chains from the authorized IBC channels
+	// don't support ethereum keys (i.e `ethsecp256k1`). Thus, so we return an error,
+	// unless the destination channel from a connection to a chain that is EVM-compatible
+	// or supports ethereum keys (eg: Cronos, Injective).
+	if sender.Equals(recipient) && !params.IsEVMChannel(packet.DestinationChannel) {
+		switch {
+		// case 1: secp256k1 key from sender/recipient has no claimed actions -> error ACK to prevent funds from getting stuck
+		case senderRecordFound && !senderClaimsRecord.HasClaimedAny():
+			return channeltypes.NewErrorAcknowledgement(
+				sdkerrors.Wrapf(
+					evmos.ErrKeyTypeNotSupported, "receiver address %s is not a valid ethereum address", data.Receiver,
+				).Error(),
+			)
+		default:
+			// case 2: sender/recipient has funds stuck -> error acknowledgement to prevent more transferred tokens from
+			// getting stuck while we implement IBC withdrawals
+			return channeltypes.NewErrorAcknowledgement(
+				sdkerrors.Wrapf(
+					evmos.ErrKeyTypeNotSupported,
+					"reverted transfer to unsupported address %s to prevent more funds from getting stuck",
+					data.Receiver,
+				).Error(),
+			)
+		}
+	}
+
+	// return original ACK in case the destination channel is not authorized
+	if !params.IsAuthorizedChannel(packet.DestinationChannel) {
+		return ack
+	}
+
 	recipientClaimsRecord, recipientRecordFound := k.GetClaimsRecord(ctx, recipient)
 
 	// handle the 4 cases for the recipient and sender claim records

diff --git a/x/claims/keeper/ibc_callbacks_test.go b/x/claims/keeper/ibc_callbacks_test.go
@@ -6,14 +6,15 @@ import (
 
 	"github.com/stretchr/testify/suite"
 
+	"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	transfertypes "github.com/cosmos/ibc-go/v3/modules/apps/transfer/types"
 	clienttypes "github.com/cosmos/ibc-go/v3/modules/core/02-client/types"
 	channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types"
 	ibcgotesting "github.com/cosmos/ibc-go/v3/testing"
-
 	ibcmock "github.com/cosmos/ibc-go/v3/testing/mock"
+
 	"github.com/tharsis/evmos/v2/app"
 	"github.com/tharsis/evmos/v2/ibctesting"
 	"github.com/tharsis/evmos/v2/x/claims/types"
@@ -295,12 +296,16 @@ func (suite *IBCTestingSuite) TestOnAckClaim() {
 }
 
 func (suite *KeeperTestSuite) TestReceive() {
+	pk := secp256k1.GenPrivKey()
+	secpAddr := sdk.AccAddress(pk.PubKey().Address())
+	secpAddrEvmos := secpAddr.String()
+	secpAddrCosmos := sdk.MustBech32ifyAddressBytes(sdk.Bech32MainPrefix, secpAddr)
 	sender := "evmos1sv9m0g7ycejwr3s369km58h5qe7xj77hvcxrms"
 	receiver := "evmos1hf0468jjpe6m6vx38s97z2qqe8ldu0njdyf625"
 
 	disabledTimeoutTimestamp := uint64(0)
 	timeoutHeight = clienttypes.NewHeight(0, 100)
-	mockpacket := channeltypes.NewPacket(ibcgotesting.MockPacketData, 1, "port", "channel", "port2", "channel2", timeoutHeight, disabledTimeoutTimestamp)
+	mockpacket := channeltypes.NewPacket(ibcgotesting.MockPacketData, 1, transfertypes.PortID, "channel-0", transfertypes.PortID, "channel-0", timeoutHeight, disabledTimeoutTimestamp)
 	ack := ibcmock.MockAcknowledgement
 
 	testCases := []struct {
@@ -318,6 +323,17 @@ func (suite *KeeperTestSuite) TestReceive() {
 				suite.Require().Equal(ack, resAck)
 			},
 		},
+		{
+			"params, channel not authorized",
+			func() {
+				transfer := transfertypes.NewFungibleTokenPacketData("aevmos", "100", sender, receiver)
+				bz := transfertypes.ModuleCdc.MustMarshalJSON(&transfer)
+				packet := channeltypes.NewPacket(bz, 1, transfertypes.PortID, "channel-0", transfertypes.PortID, "channel-100", timeoutHeight, 0)
+
+				resAck := suite.app.ClaimsKeeper.OnRecvPacket(suite.ctx, packet, ack)
+				suite.Require().Equal(ack, resAck)
+			},
+		},
 		{
 			"non ics20 packet",
 			func() {
@@ -332,18 +348,18 @@ func (suite *KeeperTestSuite) TestReceive() {
 			func() {
 				transfer := transfertypes.NewFungibleTokenPacketData("aevmos", "100", "evmos", receiver)
 				bz := transfertypes.ModuleCdc.MustMarshalJSON(&transfer)
-				packet := channeltypes.NewPacket(bz, 1, "port", "channel", "port2", "channel2", timeoutHeight, 0)
+				packet := channeltypes.NewPacket(bz, 1, transfertypes.PortID, "channel-0", transfertypes.PortID, "channel-0", timeoutHeight, 0)
 
 				resAck := suite.app.ClaimsKeeper.OnRecvPacket(suite.ctx, packet, ack)
 				suite.Require().False(resAck.Success())
 			},
 		},
 		{
-			"invalid sender",
+			"invalid sender 2",
 			func() {
 				transfer := transfertypes.NewFungibleTokenPacketData("aevmos", "100", "badba1sv9m0g7ycejwr3s369km58h5qe7xj77hvcxrms", receiver)
 				bz := transfertypes.ModuleCdc.MustMarshalJSON(&transfer)
-				packet := channeltypes.NewPacket(bz, 1, "port", "channel", "port2", "channel2", timeoutHeight, 0)
+				packet := channeltypes.NewPacket(bz, 1, transfertypes.PortID, "channel-0", transfertypes.PortID, "channel-0", timeoutHeight, 0)
 
 				resAck := suite.app.ClaimsKeeper.OnRecvPacket(suite.ctx, packet, ack)
 				suite.Require().False(resAck.Success())
@@ -354,7 +370,31 @@ func (suite *KeeperTestSuite) TestReceive() {
 			func() {
 				transfer := transfertypes.NewFungibleTokenPacketData("aevmos", "100", receiver, "badbadhf0468jjpe6m6vx38s97z2qqe8ldu0njdyf625")
 				bz := transfertypes.ModuleCdc.MustMarshalJSON(&transfer)
-				packet := channeltypes.NewPacket(bz, 1, "port", "channel", "port2", "channel2", timeoutHeight, 0)
+				packet := channeltypes.NewPacket(bz, 1, transfertypes.PortID, "channel-0", transfertypes.PortID, "channel-0", timeoutHeight, 0)
+
+				resAck := suite.app.ClaimsKeeper.OnRecvPacket(suite.ctx, packet, ack)
+				suite.Require().False(resAck.Success())
+			},
+		},
+		{
+			"fail - sender and receiver address is the same (no claim record)",
+			func() {
+				transfer := transfertypes.NewFungibleTokenPacketData("aevmos", "100", secpAddrCosmos, secpAddrEvmos)
+				bz := transfertypes.ModuleCdc.MustMarshalJSON(&transfer)
+				packet := channeltypes.NewPacket(bz, 1, transfertypes.PortID, "channel-0", transfertypes.PortID, "channel-0", timeoutHeight, 0)
+
+				resAck := suite.app.ClaimsKeeper.OnRecvPacket(suite.ctx, packet, ack)
+				suite.Require().False(resAck.Success())
+			},
+		},
+		{
+			"fail - sender and receiver address is the same (with claim record)",
+			func() {
+				transfer := transfertypes.NewFungibleTokenPacketData("aevmos", "100", secpAddrCosmos, secpAddrEvmos)
+				bz := transfertypes.ModuleCdc.MustMarshalJSON(&transfer)
+				packet := channeltypes.NewPacket(bz, 1, transfertypes.PortID, "channel-0", transfertypes.PortID, "channel-0", timeoutHeight, 0)
+
+				suite.app.ClaimsKeeper.SetClaimsRecord(suite.ctx, secpAddr, types.NewClaimsRecord(sdk.NewInt(100)))
 
 				resAck := suite.app.ClaimsKeeper.OnRecvPacket(suite.ctx, packet, ack)
 				suite.Require().False(resAck.Success())
@@ -365,7 +405,18 @@ func (suite *KeeperTestSuite) TestReceive() {
 			func() {
 				transfer := transfertypes.NewFungibleTokenPacketData("aevmos", "100", sender, receiver)
 				bz := transfertypes.ModuleCdc.MustMarshalJSON(&transfer)
-				packet := channeltypes.NewPacket(bz, 1, "port", "channel", "port2", "channel2", timeoutHeight, 0)
+				packet := channeltypes.NewPacket(bz, 1, transfertypes.PortID, "channel-0", transfertypes.PortID, types.DefaultAuthorizedChannels[0], timeoutHeight, 0)
+
+				resAck := suite.app.ClaimsKeeper.OnRecvPacket(suite.ctx, packet, ack)
+				suite.Require().True(resAck.Success())
+			},
+		},
+		{
+			"correct, same sender with EVM channel",
+			func() {
+				transfer := transfertypes.NewFungibleTokenPacketData("aevmos", "100", secpAddrCosmos, secpAddrEvmos)
+				bz := transfertypes.ModuleCdc.MustMarshalJSON(&transfer)
+				packet := channeltypes.NewPacket(bz, 1, transfertypes.PortID, "channel-0", transfertypes.PortID, types.DefaultEVMChannels[0], timeoutHeight, 0)
 
 				resAck := suite.app.ClaimsKeeper.OnRecvPacket(suite.ctx, packet, ack)
 				suite.Require().True(resAck.Success())
@@ -384,7 +435,7 @@ func (suite *KeeperTestSuite) TestReceive() {
 func (suite *KeeperTestSuite) TestAck() {
 	disabledTimeoutTimestamp := uint64(0)
 	timeoutHeight = clienttypes.NewHeight(0, 100)
-	mockpacket := channeltypes.NewPacket(ibcgotesting.MockPacketData, 1, "port", "channel", "port2", "channel2", timeoutHeight, disabledTimeoutTimestamp)
+	mockpacket := channeltypes.NewPacket(ibcgotesting.MockPacketData, 1, transfertypes.PortID, "channel-0", transfertypes.PortID, "channel-0", timeoutHeight, disabledTimeoutTimestamp)
 	ack := ibcmock.MockAcknowledgement
 
 	testCases := []struct {

diff --git a/x/claims/module.go b/x/claims/module.go
@@ -167,4 +167,4 @@ func (am AppModule) EndBlock(ctx sdk.Context, _ abci.RequestEndBlock) []abci.Val
 }
 
 // ConsensusVersion implements AppModule/ConsensusVersion.
-func (AppModule) ConsensusVersion() uint64 { return 1 }
+func (AppModule) ConsensusVersion() uint64 { return 2 }
diff --git a/x/claims/types/claim_record.go b/x/claims/types/claim_record.go
@@ -57,6 +57,17 @@ func (cr ClaimsRecord) HasClaimedAction(action Action) bool {
 	}
 }
 
+// HasClaimedAny returns true if the user has claimed at least one reward from the
+// available actions
+func (cr ClaimsRecord) HasClaimedAny() bool {
+	for _, completed := range cr.ActionsCompleted {
+		if completed {
+			return true
+		}
+	}
+	return false
+}
+
 // HasClaimedAll returns true if the user has claimed all the rewards from the
 // available actions
 func (cr ClaimsRecord) HasClaimedAll() bool {

diff --git a/x/claims/types/claim_record_test.go b/x/claims/types/claim_record_test.go
@@ -137,6 +137,45 @@ func TestClaimsRecordHasClaimedAll(t *testing.T) {
 	}
 }
 
+func TestClaimsRecordHasAny(t *testing.T) {
+	testCases := []struct {
+		name         string
+		claimsRecord ClaimsRecord
+		expBool      bool
+	}{
+		{
+			"false - empty",
+			ClaimsRecord{},
+			false,
+		},
+		{
+			"false - not claimed",
+			ClaimsRecord{
+				ActionsCompleted: []bool{false, false, false, false},
+			},
+			false,
+		},
+		{
+			"true - single action claimed",
+			ClaimsRecord{
+				ActionsCompleted: []bool{true, false, false, false},
+			},
+			true,
+		},
+		{
+			"true - all claimed",
+			ClaimsRecord{
+				ActionsCompleted: []bool{true, true, true, true},
+			},
+			true,
+		},
+	}
+
+	for _, tc := range testCases {
+		require.True(t, tc.expBool == tc.claimsRecord.HasClaimedAny(), tc.name)
+	}
+}
+
 func TestClaimsRecordAddressValidate(t *testing.T) {
 	addr := sdk.AccAddress(tests.GenerateAddress().Bytes())
 

diff --git a/x/erc20/module.go b/x/erc20/module.go
@@ -42,7 +42,7 @@ func (AppModuleBasic) RegisterLegacyAminoCodec(cdc *codec.LegacyAmino) {}
 
 // ConsensusVersion returns the consensus state-breaking version for the module.
 func (AppModuleBasic) ConsensusVersion() uint64 {
-	return 1
+	return 2
 }
 
 // RegisterInterfaces registers interfaces and implementations of the erc20 module.

diff --git a/x/inflation/genesis.go b/x/inflation/genesis.go
@@ -11,7 +11,7 @@ func InitGenesis(
 	ctx sdk.Context,
 	k keeper.Keeper,
 	ak types.AccountKeeper,
-	sk types.StakingKeeper,
+	_ types.StakingKeeper,
 	data types.GenesisState,
 ) {
 	// Ensure inflation module account is set on genesis
@@ -33,7 +33,7 @@ func InitGenesis(
 	k.SetEpochsPerPeriod(ctx, epochsPerPeriod)
 
 	// Get bondedRatio
-	bondedRatio := sk.BondedRatio(ctx)
+	bondedRatio := k.BondedRatio(ctx)
 
 	// Calculate epoch mint provision
 	epochMintProvision := types.CalculateEpochMintProvision(

diff --git a/x/inflation/keeper/hooks.go b/x/inflation/keeper/hooks.go
@@ -46,7 +46,7 @@ func (k Keeper) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epochNumb
 		period++
 		k.SetPeriod(ctx, period)
 		period = k.GetPeriod(ctx)
-		bondedRatio := k.stakingKeeper.BondedRatio(ctx)
+		bondedRatio := k.BondedRatio(ctx)
 		newProvision = types.CalculateEpochMintProvision(
 			params,
 			period,