Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Derive Cosmos addresses from public keys #273

Merged
merged 1 commit into from
Oct 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions builder/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
"github.com/polymerdao/monomer/monomerdb/localdb"
"github.com/polymerdao/monomer/testapp"
"github.com/polymerdao/monomer/testutils"
"github.com/polymerdao/monomer/utils"
"github.com/polymerdao/monomer/x/rollup/types"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -314,12 +313,12 @@ 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, err := utils.EvmToCosmosAddress("cosmos", *depositTxETH.To())
recipientAddr, err := monomer.CosmosETHAddress(*depositTxETH.To()).Encode("cosmos")
require.NoError(t, err)

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

withdrawalTx := testapp.ToTx(t, &types.MsgInitiateWithdrawal{
Expand Down
27 changes: 14 additions & 13 deletions e2e/stack_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ import (
"github.com/polymerdao/monomer/environment"
"github.com/polymerdao/monomer/node"
"github.com/polymerdao/monomer/testapp"
"github.com/polymerdao/monomer/utils"
rolluptypes "github.com/polymerdao/monomer/x/rollup/types"
"github.com/stretchr/testify/require"
"golang.org/x/exp/slog"
Expand Down Expand Up @@ -249,25 +248,27 @@ func ethRollupFlow(t *testing.T, stack *e2e.StackConfig) {

// instantiate L1 user, tx signer.
userPrivKey := stack.Users[0]
userAddress := crypto.PubkeyToAddress(userPrivKey.PublicKey)
userETHAddress := crypto.PubkeyToAddress(userPrivKey.PublicKey)
l1signer := types.NewEIP155Signer(l1ChainID)

l2GasLimit := l2blockGasLimit / 10
l1GasLimit := l2GasLimit * 2 // must be higher than l2Gaslimit, because of l1 gas burn (cross-chain gas accounting)

userCosmosETHAddress := monomer.CosmosETHAddress(userETHAddress)

//////////////////////////
////// ETH DEPOSITS //////
//////////////////////////

// get the user's balance before the deposit has been processed
balanceBeforeDeposit, err := l1Client.BalanceAt(stack.Ctx, userAddress, nil)
balanceBeforeDeposit, err := l1Client.BalanceAt(stack.Ctx, userETHAddress, nil)
require.NoError(t, err)

// send user Deposit Tx
depositAmount := big.NewInt(params.Ether)
depositTx, err := stack.OptimismPortal.DepositTransaction(
createL1TransactOpts(t, stack, userPrivKey, l1signer, l1GasLimit, depositAmount),
userAddress,
common.Address(userCosmosETHAddress),
depositAmount,
l2GasLimit,
false, // _isCreation
Expand All @@ -293,16 +294,16 @@ func ethRollupFlow(t *testing.T, stack *e2e.StackConfig) {
End: nil,
Context: stack.Ctx,
},
[]common.Address{userAddress},
[]common.Address{userAddress},
[]common.Address{userETHAddress},
[]common.Address{common.Address(userCosmosETHAddress)},
[]*big.Int{big.NewInt(0)},
)
require.NoError(t, err, "configuring 'TransactionDeposited' event listener")
require.True(t, depositLogs.Next(), "finding deposit event")
require.NoError(t, depositLogs.Close())

// get the user's balance after the deposit has been processed
balanceAfterDeposit, err := stack.L1Client.BalanceAt(stack.Ctx, userAddress, nil)
balanceAfterDeposit, err := stack.L1Client.BalanceAt(stack.Ctx, userETHAddress, nil)
require.NoError(t, err)

//nolint:gocritic
Expand All @@ -316,7 +317,7 @@ 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, err := utils.EvmToCosmosAddress("cosmos", userAddress)
userCosmosAddr, err := userCosmosETHAddress.Encode("cosmos")
require.NoError(t, err)
depositValueHex := hexutil.Encode(depositAmount.Bytes())
requireEthIsMinted(t, stack, userCosmosAddr, depositValueHex)
Expand All @@ -328,10 +329,10 @@ func ethRollupFlow(t *testing.T, stack *e2e.StackConfig) {
/////////////////////////////

// create a withdrawal tx to withdraw the deposited amount from L2 back to L1
withdrawalTx := e2e.NewWithdrawalTx(0, userAddress, userAddress, depositAmount, new(big.Int).SetUint64(params.TxGas))
withdrawalTx := e2e.NewWithdrawalTx(0, common.Address(userCosmosETHAddress), userETHAddress, depositAmount, new(big.Int).SetUint64(params.TxGas))

// initiate the withdrawal of the deposited amount on L2
senderAddr, err := utils.EvmToCosmosAddress("cosmos", *withdrawalTx.Sender)
senderAddr, err := monomer.CosmosETHAddress(*withdrawalTx.Sender).Encode("cosmos")
require.NoError(t, err)
withdrawalTxResult, err := stack.L2Client.BroadcastTxAsync(
stack.Ctx,
Expand Down Expand Up @@ -400,7 +401,7 @@ func ethRollupFlow(t *testing.T, stack *e2e.StackConfig) {
time.Sleep(time.Duration(finalizationPeriod.Uint64()) * time.Second)

// get the user's balance before the withdrawal has been finalized
balanceBeforeFinalization, err := stack.L1Client.BalanceAt(stack.Ctx, userAddress, nil)
balanceBeforeFinalization, err := stack.L1Client.BalanceAt(stack.Ctx, userETHAddress, nil)
require.NoError(t, err)

// send a withdrawal finalizing tx to finalize the withdrawal on L1
Expand Down Expand Up @@ -433,7 +434,7 @@ func ethRollupFlow(t *testing.T, stack *e2e.StackConfig) {
require.NoError(t, finalizeWithdrawalLogs.Close())

// get the user's balance after the withdrawal has been finalized
balanceAfterFinalization, err := stack.L1Client.BalanceAt(stack.Ctx, userAddress, nil)
balanceAfterFinalization, err := stack.L1Client.BalanceAt(stack.Ctx, userETHAddress, nil)
require.NoError(t, err)

//nolint:gocritic
Expand Down Expand Up @@ -533,7 +534,7 @@ func erc20RollupFlow(t *testing.T, stack *e2e.StackConfig) {
require.NoError(t, stack.WaitL2(1))

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

Expand Down
27 changes: 27 additions & 0 deletions monomer.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package monomer

import (
"context"
"crypto/ecdsa"
"crypto/sha256"
"encoding/binary"
"errors"
Expand All @@ -13,14 +14,17 @@ import (

abcitypes "github.com/cometbft/cometbft/abci/types"
bfttypes "github.com/cometbft/cometbft/types"
"github.com/cosmos/cosmos-sdk/types/bech32"
opeth "github.com/ethereum-optimism/optimism/op-service/eth"
"github.com/ethereum/go-ethereum/beacon/engine"
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/common/hexutil"
ethtypes "github.com/ethereum/go-ethereum/core/types"
"github.com/ethereum/go-ethereum/crypto/secp256k1"
"github.com/ethereum/go-ethereum/params"
"github.com/ethereum/go-ethereum/trie"
"github.com/polymerdao/monomer/utils"
"golang.org/x/crypto/ripemd160" //nolint:staticcheck
)

type Application interface {
Expand Down Expand Up @@ -237,3 +241,26 @@ func NewChainConfig(chainID *big.Int) *params.ChainConfig {
CanyonTime: utils.Ptr(uint64(0)),
}
}

// CosmosETHAddress is a Cosmos address packed into an Ethereum address.
// Only addresses derived from secp256k1 keys can be packed into an Ethereum address.
// See [ADR-28] for more details.
//
//nolint:lll // [ADR-28]: https://github.com/cosmos/cosmos-sdk/blob/8bfcf554275c1efbb42666cc8510d2da139b67fa/docs/architecture/adr-028-public-key-addresses.md?plain=1#L85
type CosmosETHAddress common.Address

// PubkeyToCosmosETHAddress converts a secp256k1 public key to a CosmosETHAddress.
// Passing in a non-secp256k1 key results in undefined behavior.
func PubkeyToCosmosETHAddress(pubKey *ecdsa.PublicKey) CosmosETHAddress {
sha := sha256.Sum256(secp256k1.CompressPubkey(pubKey.X, pubKey.Y))
hasherRIPEMD160 := ripemd160.New()
if _, err := hasherRIPEMD160.Write(sha[:]); err != nil {
// hash.Hash never returns an error on Write. This panic should never execute.
panic(fmt.Errorf("ripemd160: %v", err))
}
return CosmosETHAddress(hasherRIPEMD160.Sum(nil))
}

func (a CosmosETHAddress) Encode(hrp string) (string, error) {
return bech32.ConvertAndEncode(hrp, common.Address(a).Bytes())
}
18 changes: 18 additions & 0 deletions monomer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ import (
"time"

bfttypes "github.com/cometbft/cometbft/types"
cosmossecp256k1 "github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/decred/dcrd/dcrec/secp256k1/v4"
opeth "github.com/ethereum-optimism/optimism/op-service/eth"
"github.com/ethereum/go-ethereum/beacon/engine"
"github.com/ethereum/go-ethereum/common"
Expand Down Expand Up @@ -219,3 +222,18 @@ func TestPayloadAttributesValidForkchoiceUpdateResult(t *testing.T) {
PayloadID: payloadID,
}, result)
}

func TestCosmosETHAddress(t *testing.T) {
privKey, err := secp256k1.GeneratePrivateKey()
require.NoError(t, err)
pubKey := privKey.PubKey().ToECDSA()
got := monomer.PubkeyToCosmosETHAddress(pubKey)
wantBytes := (&cosmossecp256k1.PubKey{
Key: privKey.PubKey().SerializeCompressed(), // https://github.com/cosmos/cosmos-sdk/blob/346044afd0ecd4738c13993d2ac75da8e242266d/crypto/keys/secp256k1/secp256k1.go#L44-L45
}).Address().Bytes()
require.Equal(t, wantBytes, common.Address(got).Bytes())
// We have to use the `cosmos` hrp here because sdk.AccAddress.String() uses the global SDK config variable that uses the `cosmos` hrp.
gotEncoded, err := got.Encode("cosmos")
require.NoError(t, err)
require.Equal(t, sdk.AccAddress(wantBytes).String(), gotEncoded)
}
Comment on lines +226 to +239
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adding test cases for edge cases.

The current test only covers the happy path. Consider adding test cases for:

  1. Invalid public keys
  2. Different address prefixes
  3. Zero addresses
  4. Known test vectors from the Cosmos SDK

This would ensure robust handling of various scenarios.

Would you like me to provide an implementation for these additional test cases?

11 changes: 0 additions & 11 deletions utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import (
"fmt"
"io"

"github.com/cosmos/cosmos-sdk/types/bech32"
"github.com/ethereum/go-ethereum/common"
"github.com/hashicorp/go-multierror"
)

Expand All @@ -23,12 +21,3 @@ func WrapCloseErr(err error, closer io.Closer) error {
}
return nil
}

// 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
}
7 changes: 3 additions & 4 deletions x/rollup/keeper/deposits.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
ethtypes "github.com/ethereum/go-ethereum/core/types"
"github.com/polymerdao/monomer"
"github.com/polymerdao/monomer/bindings"
"github.com/polymerdao/monomer/utils"
"github.com/polymerdao/monomer/x/rollup/types"
"github.com/samber/lo"
)
Expand Down Expand Up @@ -122,13 +121,13 @@ func (k *Keeper) processL1UserDepositTxs(
return nil, types.WrapError(types.ErrInvalidL1Txs, "failed to get sender address: %v", err)
}
addrPrefix := sdk.GetConfig().GetBech32AccountAddrPrefix()
mintAddr, err := utils.EvmToCosmosAddress(addrPrefix, from)
mintAddr, err := monomer.CosmosETHAddress(from).Encode(addrPrefix)
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, err := utils.EvmToCosmosAddress(addrPrefix, *tx.To())
recipientAddr, err := monomer.CosmosETHAddress(*tx.To()).Encode(addrPrefix)
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)
Expand Down Expand Up @@ -192,7 +191,7 @@ 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)
toAddr, err := monomer.CosmosETHAddress(finalizeBridgeERC20.To).Encode(sdk.GetConfig().GetBech32AccountAddrPrefix())
if err != nil {
return nil, fmt.Errorf("evm to cosmos address: %v", err)
}
Expand Down
8 changes: 4 additions & 4 deletions x/rollup/tests/integration/rollup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ import (
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
"github.com/ethereum/go-ethereum/common"
gethtypes "github.com/ethereum/go-ethereum/core/types"
"github.com/polymerdao/monomer"
monomertestutils "github.com/polymerdao/monomer/testutils"
"github.com/polymerdao/monomer/utils"
"github.com/polymerdao/monomer/x/rollup"
rollupkeeper "github.com/polymerdao/monomer/x/rollup/keeper"
rolluptypes "github.com/polymerdao/monomer/x/rollup/types"
Expand Down Expand Up @@ -52,9 +52,9 @@ func TestRollup(t *testing.T) {
from, err := gethtypes.NewCancunSigner(depositTx.ChainId()).Sender(depositTx)
require.NoError(t, err)

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

// query the mint address ETH balance and assert it's zero
Expand All @@ -64,7 +64,7 @@ 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
erc20userCosmosAddr, err := utils.EvmToCosmosAddress(sdk.GetConfig().GetBech32AccountAddrPrefix(), erc20userAddr)
erc20userCosmosAddr, err := monomer.CosmosETHAddress(erc20userAddr).Encode(sdk.GetConfig().GetBech32AccountAddrPrefix())
require.NoError(t, err)
require.Equal(t, math.ZeroInt(), queryUserERC20Balance(t, queryClient, erc20userCosmosAddr, erc20tokenAddr, integrationApp))

Expand Down
Loading