fix(upgrade): add account number and sequence to migrated IBC escrow …

…accounts (#1252)

* fix(upgrade): add account number and sequence to migrated escrow accounts

* fix(upgrade): small changes

* fix(upgrade): fix linting issues

* fix(upgrade) add changelog entry

* fix(upgrade) apply review comments

* fix(upgrade): add note on docs about escrow accounts

* fix(upgrade): update open channels to 37

* fix(upgrade): fix md linting issues

CHANGELOG.md

@@ -67,6 +67,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 ### Bug Fixes
 
 - (ibc) [#1156](https://github.com/evmos/evmos/pull/1156) Migrate IBC transfer escrow accounts to `ModuleAccount` type.
+- (upgrade) [#1252](https://github.com/evmos/evmos/pull/1252) Add account number and sequence to migrated IBC transfer escrow accounts.
 
 ## [v10.0.1] - 2023-01-03
 

app/upgrades/v11/constants.go

@@ -22,9 +22,9 @@ const (
 	// UpgradeInfo defines the binaries that will be used for the upgrade
 	UpgradeInfo = `'{"binaries":{"darwin/arm64":"https://github.com/evmos/evmos/releases/download/v11.0.0/evmos_11.0.0_Darwin_arm64.tar.gz","darwin/amd64":"https://github.com/evmos/evmos/releases/download/v11.0.0/evmos_11.0.0_Darwin_amd64.tar.gz","linux/arm64":"https://github.com/evmos/evmos/releases/download/v11.0.0/evmos_11.0.0_Linux_arm64.tar.gz","linux/amd64":"https://github.com/evmos/evmos/releases/download/v11.0.0/evmos_11.0.0_Linux_amd64.tar.gz","windows/x86_64":"https://github.com/evmos/evmos/releases/download/v11.0.0/evmos_11.0.0_Windows_x86_64.zip"}}'`
 
-	// at the time of this migration, on mainnet, channels 0 to 36 were open
+	// at the time of this migration, on mainnet, channels 0 to 37 were open
 	// so this migration covers those channels only
-	openChannels = 36
+	OpenChannels = 37
 
 	// FundingAccount is the account which holds the rewards for the incentivized testnet.
 	// It contains ~7.4M tokens, of which ~5.6M are to be distributed.

app/upgrades/v11/upgrades.go

@@ -64,7 +64,7 @@ func CreateUpgradeHandler(
 			HandleRewardDistribution(ctx, logger, bk, sk, dk)
 		}
 
-		MigrateEscrowAccounts(ctx, ak)
+		MigrateEscrowAccounts(ctx, logger, ak)
 
 		// create ICS27 Controller submodule params, with the controller module NOT enabled
 		gs := &genesistypes.GenesisState{
@@ -107,8 +107,8 @@ func CreateUpgradeHandler(
 }
 
 // MigrateEscrowAccounts updates the IBC transfer escrow accounts type to ModuleAccount
-func MigrateEscrowAccounts(ctx sdk.Context, ak authkeeper.AccountKeeper) {
-	for i := 0; i <= openChannels; i++ {
+func MigrateEscrowAccounts(ctx sdk.Context, logger log.Logger, ak authkeeper.AccountKeeper) {
+	for i := 0; i <= OpenChannels; i++ {
 		channelID := fmt.Sprintf("channel-%d", i)
 		address := transfertypes.GetEscrowAddress(transfertypes.PortID, channelID)
 
@@ -131,6 +131,16 @@ func MigrateEscrowAccounts(ctx sdk.Context, ak authkeeper.AccountKeeper) {
 		accountName := fmt.Sprintf("%s\x00%s/%s", transfertypes.Version, transfertypes.PortID, channelID)
 		baseAcc := authtypes.NewBaseAccountWithAddress(address)
 
+		// Set same account number and sequence as the existing account
+		if err := baseAcc.SetAccountNumber(existingAcc.GetAccountNumber()); err != nil {
+			// log error instead of aborting the upgrade
+			logger.Error("failed to set escrow account number for account", accountName, "error", err.Error())
+		}
+		if err := baseAcc.SetSequence(existingAcc.GetSequence()); err != nil {
+			// log error instead of aborting the upgrade
+			logger.Error("failed to set escrow account sequence for account", accountName, "error", err.Error())
+		}
+
 		// no special permissions defined for the module account
 		acc := authtypes.NewModuleAccount(baseAcc, accountName)
 		ak.SetModuleAccount(ctx, acc)

app/upgrades/v11/upgrades_test.go

@@ -82,13 +82,18 @@ func TestUpgradeTestSuite(t *testing.T) {
 	suite.Run(t, s)
 }
 
+var expAccNum = make(map[int]uint64)
+
 func (suite *UpgradeTestSuite) setupEscrowAccounts(accCount int) {
 	for i := 0; i <= accCount; i++ {
 		channelID := fmt.Sprintf("channel-%d", i)
 		addr := ibctypes.GetEscrowAddress(ibctypes.PortID, channelID)
 
 		// set accounts as BaseAccounts
 		baseAcc := authtypes.NewBaseAccountWithAddress(addr)
+		err := baseAcc.SetAccountNumber(suite.app.AccountKeeper.GetNextAccountNumber(suite.ctx))
+		suite.Require().NoError(err)
+		expAccNum[i] = baseAcc.AccountNumber
 		suite.app.AccountKeeper.SetAccount(suite.ctx, baseAcc)
 	}
 }
@@ -122,10 +127,10 @@ func (suite *UpgradeTestSuite) TestMigrateEscrowAcc() {
 	suite.setupEscrowAccounts(existingAccounts)
 
 	// Run migrations
-	v11.MigrateEscrowAccounts(suite.ctx, suite.app.AccountKeeper)
+	v11.MigrateEscrowAccounts(suite.ctx, suite.app.Logger(), suite.app.AccountKeeper)
 
-	// check account types for channels 0 to 36
-	for i := 0; i <= 36; i++ {
+	// check account types for channels 0 to 37
+	for i := 0; i <= v11.OpenChannels; i++ {
 		channelID := fmt.Sprintf("channel-%d", i)
 		addr := ibctypes.GetEscrowAddress(ibctypes.PortID, channelID)
 		acc := suite.app.AccountKeeper.GetAccount(suite.ctx, addr)
@@ -139,6 +144,9 @@ func (suite *UpgradeTestSuite) TestMigrateEscrowAcc() {
 		moduleAcc, isModuleAccount := acc.(*authtypes.ModuleAccount)
 		suite.Require().True(isModuleAccount)
 		suite.Require().NoError(moduleAcc.Validate(), "account validation failed")
+
+		// Check account number
+		suite.Require().Equal(expAccNum[i], moduleAcc.GetAccountNumber())
 	}
 }
 

docs/protocol/moduleaccounts.md

@@ -7,6 +7,32 @@ order: 1
 Some modules have their own module account. Think of this as a wallet that can only be controlled by that module.
 Below is a table of modules, their respective wallet addresses and permissions.
 
+Additionally, there are module accounts associated with IBC transfers.
+For each IBC connection, there's an account of type `ModuleAccount` used to escrow the transferred coins when Evmos is the source chain.
+Their addresses are derived using the first 20 bytes of the SHA256 checksum of the account name and following the format as outlined in [ADR 028](https://github.com/cosmos/cosmos-sdk/blob/master/docs/architecture/adr-028-public-key-addresses.md):
+
+```go
+// accountName is composed by the current version the IBC tranfer module supports (in this case, ics20-1), the portID (transfer) and the channelID
+accountName := Version + "\0" + portID + "/" + channelID
+addr := sha256.Sum256(accountName)[:20]
+
+// example for channel-0
+addr := sha256.Sum256("ics20-1\0transfer/channel-0")[:20]
+```
+
+This can be calculated with the [`GetEscrowAccount` function on IBC-go](https://github.com/cosmos/ibc-go/blob/c56f78905a5d2db01d867381d106c403fa9e5c4b/modules/apps/transfer/types/keys.go#L41-L55).
+
+::: tip
+**Note**: These escrow accounts are not listed when performing the query:
+
+```shell
+evmosd q auth module-accounts
+```
+
+This happens because the [`GetModuleAccount` function](https://github.com/cosmos/cosmos-sdk/blob/74d7a0dfcd9f47d8a507205f82c264a269ef0612/x/auth/keeper/keeper.go#L194-L224) used on the query considers only the accounts on the [`permAddrs` map of the `AccountKeeper`](https://github.com/cosmos/cosmos-sdk/blob/74d7a0dfcd9f47d8a507205f82c264a269ef0612/x/auth/keeper/keeper.go#L54-L68).
+This address map is set at compile time and cannot be changed on runtime.
+:::
+
 ### Account Permisions and their meaning
 
 The `burner` permission means this account has the permission to burn or destroy tokens.