Skip to content

Commit

Permalink
Make bech32 prefix configurable in address conversion
Browse files Browse the repository at this point in the history 
The previous method of converting EVM to Cosmos addresses used a global variable in the Cosmos SDK that may not be set in e2e tests.
This led to difficult-to-debug errors.
  • Loading branch information
@joshklop
joshklop committed Oct 25, 2024
1 parent 2f8bb43 commit 5c5511b
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 33 deletions.
10 changes: 6 additions & 4 deletions builder/builder_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -314,14 +314,16 @@ func TestBuildRollupTxs(t *testing.T) {
depositTxETH := ethTxs[1]
require.NotNil(t, depositTxETH.Mint())
require.NotNil(t, depositTxETH.To(), "Deposit transaction must have a 'to' address")
recipientAddr := utils.EvmToCosmosAddress(*depositTxETH.To())
recipientAddr, err := utils.EvmToCosmosAddress("cosmos", *depositTxETH.To())
require.NoError(t, err)

from, err := gethtypes.NewCancunSigner(depositTxETH.ChainId()).Sender(depositTxETH)
require.NoError(t, err)
mintAddr := utils.EvmToCosmosAddress(from)
mintAddr, err := utils.EvmToCosmosAddress("cosmos", from)
require.NoError(t, err)

withdrawalTx := testapp.ToTx(t, &types.MsgInitiateWithdrawal{
Sender: recipientAddr.String(),
Sender: recipientAddr,
Target: common.HexToAddress("0x12345abcde").String(),
Value: math.NewIntFromBigInt(depositTxETH.Value()),
GasLimit: big.NewInt(100_000).Bytes(),
Expand Down Expand Up @@ -351,7 +353,7 @@ func TestBuildRollupTxs(t *testing.T) {
builtBlock, preBuildInfo, postBuildInfo := buildBlock(t, b, env.app, payload)

// Test deposit was received
checkDepositTxResult(t, env.txStore, depositTxs, depositTxETH.Mint(), depositTxETH.Value(), mintAddr.String(), recipientAddr.String())
checkDepositTxResult(t, env.txStore, depositTxs, depositTxETH.Mint(), depositTxETH.Value(), mintAddr, recipientAddr)

withdrawalTxResult, err := env.txStore.Get(bfttypes.Tx(withdrawalTx).Hash())
require.NoError(t, err)
Expand Down
11 changes: 8 additions & 3 deletions e2e/stack_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,8 @@ func ethRollupFlow(t *testing.T, stack *e2e.StackConfig) {
// check that the user's balance has been updated on L1
require.Equal(t, expectedBalance, balanceAfterDeposit)

userCosmosAddr := utils.EvmToCosmosAddress(userAddress).String()
userCosmosAddr, err := utils.EvmToCosmosAddress("cosmos", userAddress)
require.NoError(t, err)
depositValueHex := hexutil.Encode(depositAmount.Bytes())
requireEthIsMinted(t, stack, userCosmosAddr, depositValueHex)

Expand All @@ -330,10 +331,12 @@ func ethRollupFlow(t *testing.T, stack *e2e.StackConfig) {
withdrawalTx := e2e.NewWithdrawalTx(0, userAddress, userAddress, depositAmount, new(big.Int).SetUint64(params.TxGas))

// initiate the withdrawal of the deposited amount on L2
senderAddr, err := utils.EvmToCosmosAddress("cosmos", *withdrawalTx.Sender)
require.NoError(t, err)
withdrawalTxResult, err := stack.L2Client.BroadcastTxAsync(
stack.Ctx,
testapp.ToTx(t, &rolluptypes.MsgInitiateWithdrawal{
Sender: utils.EvmToCosmosAddress(*withdrawalTx.Sender).String(),
Sender: senderAddr,
Target: withdrawalTx.Target.String(),
Value: math.NewIntFromBigInt(withdrawalTx.Value),
GasLimit: withdrawalTx.GasLimit.Bytes(),
Expand Down Expand Up @@ -530,7 +533,9 @@ func erc20RollupFlow(t *testing.T, stack *e2e.StackConfig) {
require.NoError(t, stack.WaitL2(1))

// assert the user's bridged WETH is on L2
requireERC20IsMinted(t, stack, utils.EvmToCosmosAddress(userAddress).String(), weth9Address.String(), hexutil.Encode(wethL2Amount.Bytes()))
userAddr, err := utils.EvmToCosmosAddress("cosmos", userAddress)
require.NoError(t, err)
requireERC20IsMinted(t, stack, userAddr, weth9Address.String(), hexutil.Encode(wethL2Amount.Bytes()))

t.Log("Monomer can ingest ERC-20 deposit txs from L1 and mint ERC-20 tokens on L2")
}
Expand Down
12 changes: 8 additions & 4 deletions utils/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"fmt"
"io"

sdktypes "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/bech32"
"github.com/ethereum/go-ethereum/common"
"github.com/hashicorp/go-multierror"
)
Expand All @@ -24,7 +24,11 @@ func WrapCloseErr(err error, closer io.Closer) error {
return nil
}

// EvmToCosmosAddress converts an EVM address to a sdktypes.AccAddress
func EvmToCosmosAddress(addr common.Address) sdktypes.AccAddress {
return addr.Bytes()
// EvmToCosmosAddress converts an EVM address to a string
func EvmToCosmosAddress(prefix string, ethAddr common.Address) (string, error) {
addr, err := bech32.ConvertAndEncode(prefix, ethAddr.Bytes())
if err != nil {
return "", fmt.Errorf("convert and encode: %v", err)
}
return addr, nil
}
49 changes: 37 additions & 12 deletions x/rollup/keeper/deposits.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,18 @@ func (k *Keeper) processL1UserDepositTxs(
ctx.Logger().Error("Failed to get sender address", "evmAddress", from, "err", err)
return nil, types.WrapError(types.ErrInvalidL1Txs, "failed to get sender address: %v", err)
}
mintAddr := utils.EvmToCosmosAddress(from)
addrPrefix := sdk.GetConfig().GetBech32AccountAddrPrefix()
mintAddr, err := utils.EvmToCosmosAddress(addrPrefix, from)
if err != nil {
ctx.Logger().Error("Failed to convert EVM to Cosmos address", "err", err)
return nil, fmt.Errorf("evm to cosmos address: %v", err)
}
mintAmount := sdkmath.NewIntFromBigInt(tx.Mint())
recipientAddr := utils.EvmToCosmosAddress(*tx.To())
recipientAddr, err := utils.EvmToCosmosAddress(addrPrefix, *tx.To())
if err != nil {
ctx.Logger().Error("Failed to convert EVM to Cosmos address", "err", err)
return nil, fmt.Errorf("evm to cosmos address: %v", err)
}
transferAmount := sdkmath.NewIntFromBigInt(tx.Value())

mintEvent, err := k.mintETH(ctx, mintAddr, recipientAddr, mintAmount, transferAmount)
Expand Down Expand Up @@ -183,10 +192,14 @@ func (k *Keeper) parseAndExecuteCrossDomainMessage(ctx sdk.Context, txData []byt
return nil, fmt.Errorf("failed to unpack relay message into finalizeBridgeERC20 interface: %v", err)
}

toAddr, err := utils.EvmToCosmosAddress(sdk.GetConfig().GetBech32AccountAddrPrefix(), finalizeBridgeERC20.To)
if err != nil {
return nil, fmt.Errorf("evm to cosmos address: %v", err)
}
// Mint the ERC-20 token to the specified Cosmos address
mintEvent, err := k.mintERC20(
ctx,
utils.EvmToCosmosAddress(finalizeBridgeERC20.To),
toAddr,
finalizeBridgeERC20.RemoteToken.String(),
sdkmath.NewIntFromBigInt(finalizeBridgeERC20.Amount),
)
Expand All @@ -203,7 +216,7 @@ func (k *Keeper) parseAndExecuteCrossDomainMessage(ctx sdk.Context, txData []byt
// mintETH mints ETH to an account where the amount is in wei and returns the associated event.
func (k *Keeper) mintETH(
ctx sdk.Context, //nolint:gocritic // hugeParam
mintAddr, recipientAddr sdk.AccAddress,
mintAddr, recipientAddr string,
mintAmount, transferAmount sdkmath.Int,
) (*sdk.Event, error) {
// Mint the deposit amount to the rollup module
Expand All @@ -215,25 +228,33 @@ func (k *Keeper) mintETH(
return nil, fmt.Errorf("transfer amount %v is greater than mint amount %v", transferAmount, mintAmount)
}

recipientSDKAddr, err := sdk.AccAddressFromBech32(recipientAddr)
if err != nil {
return nil, fmt.Errorf("account address from bech32: %v", err)
}
if !transferAmount.IsZero() {
// Send the transfer amount to the recipient address
if err := k.bankkeeper.SendCoinsFromModuleToAccount(
ctx,
types.ModuleName,
recipientAddr,
recipientSDKAddr,
sdk.NewCoins(sdk.NewCoin(types.WEI, transferAmount)),
); err != nil {
return nil, fmt.Errorf("failed to send ETH deposit coins from rollup module to user account %v: %v", recipientAddr, err)
return nil, fmt.Errorf("failed to send ETH deposit coins from rollup module to user account %v: %v", recipientSDKAddr, err)
}
}

mintSDKAddr, err := sdk.AccAddressFromBech32(mintAddr)
if err != nil {
return nil, fmt.Errorf("account address from bech32: %v", err)
}
remainingCoins := mintAmount.Sub(transferAmount)
if remainingCoins.IsPositive() {
// Send the remaining mint amount to the deposit tx sender address
if err := k.bankkeeper.SendCoinsFromModuleToAccount(
ctx,
types.ModuleName,
mintAddr,
mintSDKAddr,
sdk.NewCoins(sdk.NewCoin(types.WEI, remainingCoins)),
); err != nil {
return nil, fmt.Errorf("failed to send ETH deposit coins from rollup module to user account %v: %v", mintAddr, err)
Expand All @@ -243,9 +264,9 @@ func (k *Keeper) mintETH(
mintEvent := sdk.NewEvent(
types.EventTypeMintETH,
sdk.NewAttribute(types.AttributeKeyL1DepositTxType, types.L1UserDepositTxType),
sdk.NewAttribute(types.AttributeKeyMintCosmosAddress, mintAddr.String()),
sdk.NewAttribute(types.AttributeKeyMintCosmosAddress, mintAddr),
sdk.NewAttribute(types.AttributeKeyMint, hexutil.Encode(remainingCoins.BigInt().Bytes())),
sdk.NewAttribute(types.AttributeKeyToCosmosAddress, recipientAddr.String()),
sdk.NewAttribute(types.AttributeKeyToCosmosAddress, recipientAddr),
sdk.NewAttribute(types.AttributeKeyValue, hexutil.Encode(transferAmount.BigInt().Bytes())),
)

Expand All @@ -255,7 +276,7 @@ func (k *Keeper) mintETH(
// mintERC20 mints a bridged ERC-20 token to an account and returns the associated event.
func (k *Keeper) mintERC20(
ctx sdk.Context, //nolint:gocritic // hugeParam
userAddr sdk.AccAddress,
userAddr string,
erc20addr string,
amount sdkmath.Int,
) (*sdk.Event, error) {
Expand All @@ -264,14 +285,18 @@ func (k *Keeper) mintERC20(
if err := k.bankkeeper.MintCoins(ctx, types.ModuleName, sdk.NewCoins(coin)); err != nil {
return nil, fmt.Errorf("failed to mint ERC-20 deposit coins to the rollup module: %v", err)
}
if err := k.bankkeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, userAddr, sdk.NewCoins(coin)); err != nil {
userSDKAddr, err := sdk.AccAddressFromBech32(userAddr)
if err != nil {
return nil, fmt.Errorf("account address from bech32: %v", err)
}
if err := k.bankkeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, userSDKAddr, sdk.NewCoins(coin)); err != nil {
return nil, fmt.Errorf("failed to send ERC-20 deposit coins from rollup module to user account %v: %v", userAddr, err)
}

mintEvent := sdk.NewEvent(
types.EventTypeMintERC20,
sdk.NewAttribute(types.AttributeKeyL1DepositTxType, types.L1UserDepositTxType),
sdk.NewAttribute(types.AttributeKeyToCosmosAddress, userAddr.String()),
sdk.NewAttribute(types.AttributeKeyToCosmosAddress, userAddr),
sdk.NewAttribute(types.AttributeKeyERC20Address, erc20addr),
sdk.NewAttribute(types.AttributeKeyValue, hexutil.Encode(amount.BigInt().Bytes())),
)
Expand Down
25 changes: 15 additions & 10 deletions x/rollup/tests/integration/rollup_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,11 @@ func TestRollup(t *testing.T) {
transferAmount := depositTx.Value()
from, err := gethtypes.NewCancunSigner(depositTx.ChainId()).Sender(depositTx)
require.NoError(t, err)
mintAddr := utils.EvmToCosmosAddress(from)
recipientAddr := utils.EvmToCosmosAddress(*depositTx.To())

mintAddr, err := utils.EvmToCosmosAddress(sdk.GetConfig().GetBech32AccountAddrPrefix(), from)
require.NoError(t, err)
recipientAddr, err := utils.EvmToCosmosAddress(sdk.GetConfig().GetBech32AccountAddrPrefix(), *depositTx.To())
require.NoError(t, err)

// query the mint address ETH balance and assert it's zero
require.Equal(t, math.ZeroInt(), queryUserETHBalance(t, queryClient, mintAddr, integrationApp))
Expand All @@ -61,7 +64,9 @@ func TestRollup(t *testing.T) {
require.Equal(t, math.ZeroInt(), queryUserETHBalance(t, queryClient, recipientAddr, integrationApp))

// query the user's ERC20 balance and assert it's zero
require.Equal(t, math.ZeroInt(), queryUserERC20Balance(t, queryClient, utils.EvmToCosmosAddress(erc20userAddr), erc20tokenAddr, integrationApp))
erc20userCosmosAddr, err := utils.EvmToCosmosAddress(sdk.GetConfig().GetBech32AccountAddrPrefix(), erc20userAddr)
require.NoError(t, err)
require.Equal(t, math.ZeroInt(), queryUserERC20Balance(t, queryClient, erc20userCosmosAddr, erc20tokenAddr, integrationApp))

// send an invalid MsgApplyL1Txs and assert error
_, err = integrationApp.RunMsg(&rolluptypes.MsgApplyL1Txs{
Expand All @@ -82,11 +87,11 @@ func TestRollup(t *testing.T) {
require.Equal(t, transferAmount, queryUserETHBalance(t, queryClient, recipientAddr, integrationApp).BigInt())

// query the user's ERC20 balance and assert it's equal to the deposit amount
require.Equal(t, erc20depositAmount, queryUserERC20Balance(t, queryClient, utils.EvmToCosmosAddress(erc20userAddr), erc20tokenAddr, integrationApp).BigInt())
require.Equal(t, erc20depositAmount, queryUserERC20Balance(t, queryClient, erc20userCosmosAddr, erc20tokenAddr, integrationApp).BigInt())

// try to withdraw more than deposited and assert error
_, err = integrationApp.RunMsg(&rolluptypes.MsgInitiateWithdrawal{
Sender: mintAddr.String(),
Sender: mintAddr,
Target: l1WithdrawalAddr,
Value: math.NewIntFromBigInt(transferAmount).Add(math.OneInt()),
GasLimit: new(big.Int).SetUint64(100_000_000).Bytes(),
Expand All @@ -96,7 +101,7 @@ func TestRollup(t *testing.T) {

// send a successful MsgInitiateWithdrawal
_, err = integrationApp.RunMsg(&rolluptypes.MsgInitiateWithdrawal{
Sender: recipientAddr.String(),
Sender: recipientAddr,
Target: l1WithdrawalAddr,
Value: math.NewIntFromBigInt(transferAmount),
GasLimit: new(big.Int).SetUint64(100_000_000).Bytes(),
Expand Down Expand Up @@ -161,19 +166,19 @@ func setupIntegrationApp(t *testing.T) *integration.App {
return integrationApp
}

func queryUserBalance(t *testing.T, queryClient banktypes.QueryClient, userAddr sdk.AccAddress, denom string, app *integration.App) math.Int {
func queryUserBalance(t *testing.T, queryClient banktypes.QueryClient, userAddr, denom string, app *integration.App) math.Int {
resp, err := queryClient.Balance(app.Context(), &banktypes.QueryBalanceRequest{
Address: userAddr.String(),
Address: userAddr,
Denom: denom,
})
require.NoError(t, err)
return resp.Balance.Amount
}

func queryUserETHBalance(t *testing.T, queryClient banktypes.QueryClient, userAddr sdk.AccAddress, app *integration.App) math.Int {
func queryUserETHBalance(t *testing.T, queryClient banktypes.QueryClient, userAddr string, app *integration.App) math.Int {
return queryUserBalance(t, queryClient, userAddr, rolluptypes.WEI, app)
}

func queryUserERC20Balance(t *testing.T, queryClient banktypes.QueryClient, userAddr sdk.AccAddress, erc20addr common.Address, app *integration.App) math.Int {
func queryUserERC20Balance(t *testing.T, queryClient banktypes.QueryClient, userAddr string, erc20addr common.Address, app *integration.App) math.Int {
return queryUserBalance(t, queryClient, userAddr, "erc20/"+erc20addr.String()[2:], app)
}

0 comments on commit 5c5511b

Please sign in to comment.