From a3f31254024d0913ed8c648e30daaa566c601b29 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 28 Jul 2021 11:08:12 -0400 Subject: [PATCH] x/gravity: support tokens without metadata (#104) --- .../gravity/keeper/ethereum_event_handler.go | 166 ++++++++++++------ module/x/gravity/types/errors.go | 9 +- 2 files changed, 116 insertions(+), 59 deletions(-) diff --git a/module/x/gravity/keeper/ethereum_event_handler.go b/module/x/gravity/keeper/ethereum_event_handler.go index f80313b1d..5dd351af4 100644 --- a/module/x/gravity/keeper/ethereum_event_handler.go +++ b/module/x/gravity/keeper/ethereum_event_handler.go @@ -1,11 +1,11 @@ package keeper import ( - "fmt" "math/big" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" "github.com/ethereum/go-ethereum/common" "github.com/peggyjv/gravity-bridge/module/x/gravity/types" @@ -34,79 +34,36 @@ func (a EthereumEventProcessor) Handle(ctx sdk.Context, eve types.EthereumEvent) isCosmosOriginated, denom := a.keeper.ERC20ToDenomLookup(ctx, event.TokenContract) addr, _ := sdk.AccAddressFromBech32(event.CosmosReceiver) coins := sdk.Coins{sdk.NewCoin(denom, event.Amount)} + if !isCosmosOriginated { if err := a.DetectMaliciousSupply(ctx, denom, event.Amount); err != nil { return err } - // If it is not cosmos originated, mint the coins (aka vouchers) + + // if it is not cosmos originated, mint the coins (aka vouchers) if err := a.bankKeeper.MintCoins(ctx, types.ModuleName, coins); err != nil { return sdkerrors.Wrapf(err, "mint vouchers coins: %s", coins) } } + return a.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, addr, coins) + case *types.BatchExecutedEvent: a.keeper.batchTxExecuted(ctx, common.HexToAddress(event.TokenContract), event.BatchNonce) return - case *types.ERC20DeployedEvent: - // Check if it already exists - if existingERC20, exists := a.keeper.getCosmosOriginatedERC20(ctx, event.CosmosDenom); exists { - return sdkerrors.Wrap( - types.ErrInvalid, - fmt.Sprintf("ERC20 %s already exists for denom %s", existingERC20.Hex(), event.CosmosDenom)) - } - // Check if denom exists - // TODO: document that peggy chains require denom metadata set - metadata := a.keeper.bankKeeper.GetDenomMetaData(ctx, event.CosmosDenom) - if metadata.Base == "" { - return sdkerrors.Wrap(types.ErrInvalid, fmt.Sprintf("denom not found %s", event.CosmosDenom)) - } - - // Check if attributes of ERC20 match Cosmos denom - if event.Erc20Name != metadata.Display { - return sdkerrors.Wrap( - types.ErrInvalid, - fmt.Sprintf("ERC20 name %s does not match denom display %s", event.Erc20Name, metadata.Description)) - } - - if event.Erc20Symbol != metadata.Display { - return sdkerrors.Wrap( - types.ErrInvalid, - fmt.Sprintf("ERC20 symbol %s does not match denom display %s", event.Erc20Symbol, metadata.Display)) - } - - // ERC20 tokens use a very simple mechanism to tell you where to display the decimal point. - // The "decimals" field simply tells you how many decimal places there will be. - // Cosmos denoms have a system that is much more full featured, with enterprise-ready token denominations. - // There is a DenomUnits array that tells you what the name of each denomination of the - // token is. - // To correlate this with an ERC20 "decimals" field, we have to search through the DenomUnits array - // to find the DenomUnit which matches up to the main token "display" value. Then we take the - // "exponent" from this DenomUnit. - // If the correct DenomUnit is not found, it will default to 0. This will result in there being no decimal places - // in the token's ERC20 on Ethereum. So, for example, if this happened with Atom, 1 Atom would appear on Ethereum - // as 1 million Atoms, having 6 extra places before the decimal point. - // This will only happen with a Denom Metadata which is for all intents and purposes invalid, but I am not sure - // this is checked for at any other point. - decimals := uint32(0) - for _, denomUnit := range metadata.DenomUnits { - if denomUnit.Denom == metadata.Display { - decimals = denomUnit.Exponent - break - } - } - - if decimals != uint32(event.Erc20Decimals) { - return sdkerrors.Wrap( - types.ErrInvalid, - fmt.Sprintf("ERC20 decimals %d does not match denom decimals %d", event.Erc20Decimals, decimals)) + case *types.ERC20DeployedEvent: + if err := a.verifyERC20DeployedEvent(ctx, event); err != nil { + return err } - // Add to denom-erc20 mapping + // add to denom-erc20 mapping a.keeper.setCosmosOriginatedDenomToERC20(ctx, event.CosmosDenom, event.TokenContract) + return nil case *types.ContractCallExecutedEvent: // TODO: issue event hook for consumer modules + case *types.SignerSetTxExecutedEvent: // TODO here we should check the contents of the validator set against // the store, if they differ we should take some action to indicate to the @@ -121,3 +78,102 @@ func (a EthereumEventProcessor) Handle(ctx sdk.Context, eve types.EthereumEvent) } return nil } + +func (a EthereumEventProcessor) verifyERC20DeployedEvent(ctx sdk.Context, event *types.ERC20DeployedEvent) error { + if existingERC20, exists := a.keeper.getCosmosOriginatedERC20(ctx, event.CosmosDenom); exists { + return sdkerrors.Wrapf( + types.ErrInvalidERC20Event, + "ERC20 token %s already exists for denom %s", existingERC20.Hex(), event.CosmosDenom, + ) + } + + // We expect that all Cosmos-based tokens have metadata defined. In the case + // a token does not have metadata defined, e.g. an IBC token, we successfully + // handle the token under the following conditions: + // + // 1. The ERC20 name is equal to the token's denomination. Otherwise, this + // means that ERC20 tokens would have an untenable UX. + // 2. The ERC20 token has zero decimals as this is what we default to since + // we cannot know or infer the real decimal value for the Cosmos token. + // 3. The ERC20 symbol is empty. + // + // NOTE: This path is not encouraged and all supported assets should have + // metadata defined. If metadata cannot be defined, consider adding the token's + // metadata on the fly. + metadata := a.keeper.bankKeeper.GetDenomMetaData(ctx, event.CosmosDenom) + if metadata.Base == "" { + if event.Erc20Name != event.CosmosDenom { + return sdkerrors.Wrapf( + types.ErrInvalidERC20Event, + "invalid ERC20 name for token without metadata; got: %s, expected: %s", event.Erc20Name, event.CosmosDenom, + ) + } + + if event.Erc20Symbol != "" { + return sdkerrors.Wrapf( + types.ErrInvalidERC20Event, + "expected empty ERC20 symbol for token without metadata; got: %s", event.Erc20Symbol, + ) + } + + if event.Erc20Decimals != 0 { + return sdkerrors.Wrapf( + types.ErrInvalidERC20Event, + "expected zero ERC20 decimals for token without metadata; got: %d", event.Erc20Decimals, + ) + } + + return nil + } + + return verifyERC20Token(metadata, event) +} + +func verifyERC20Token(metadata banktypes.Metadata, event *types.ERC20DeployedEvent) error { + if event.Erc20Name != metadata.Display { + return sdkerrors.Wrapf( + types.ErrInvalidERC20Event, + "ERC20 name %s does not match the denom display %s", event.Erc20Name, metadata.Description, + ) + } + + if event.Erc20Symbol != metadata.Display { + return sdkerrors.Wrapf( + types.ErrInvalidERC20Event, + "ERC20 symbol %s does not match denom display %s", event.Erc20Symbol, metadata.Display, + ) + } + + // ERC20 tokens use a very simple mechanism to tell you where to display the + // decimal point. The "decimals" field simply tells you how many decimal places + // there will be. + // + // Cosmos denoms have a system that is much more full featured, with + // enterprise-ready token denominations. There is a DenomUnits array that + // tells you what the name of each denomination of the token is. + // + // To correlate this with an ERC20 "decimals" field, we have to search through + // the DenomUnits array to find the DenomUnit which matches up to the main + // token "display" value. Then we take the "exponent" from this DenomUnit. + // + // If the correct DenomUnit is not found, it will default to 0. This will + // result in there being no decimal places in the token's ERC20 on Ethereum. + // For example, if this happened with ATOM, 1 ATOM would appear on Ethereum + // as 1 million ATOM, having 6 extra places before the decimal point. + var decimals uint32 + for _, denomUnit := range metadata.DenomUnits { + if denomUnit.Denom == metadata.Display { + decimals = denomUnit.Exponent + break + } + } + + if uint64(decimals) != event.Erc20Decimals { + return sdkerrors.Wrapf( + types.ErrInvalidERC20Event, + "ERC20 decimals %d does not match denom decimals %d", event.Erc20Decimals, decimals, + ) + } + + return nil +} diff --git a/module/x/gravity/types/errors.go b/module/x/gravity/types/errors.go index bb3574aeb..c04f8ff15 100644 --- a/module/x/gravity/types/errors.go +++ b/module/x/gravity/types/errors.go @@ -5,8 +5,9 @@ import ( ) var ( - ErrInvalid = sdkerrors.Register(ModuleName, 3, "invalid") - ErrSupplyOverflow = sdkerrors.Register(ModuleName, 4, "malicious ERC20 with invalid supply sent over bridge") - ErrDelegateKeys = sdkerrors.Register(ModuleName, 5, "failed to delegate keys") - ErrEmptyEthSig = sdkerrors.Register(ModuleName, 6, "empty Ethereum signature") + ErrInvalid = sdkerrors.Register(ModuleName, 3, "invalid") + ErrSupplyOverflow = sdkerrors.Register(ModuleName, 4, "malicious ERC20 with invalid supply sent over bridge") + ErrDelegateKeys = sdkerrors.Register(ModuleName, 5, "failed to delegate keys") + ErrEmptyEthSig = sdkerrors.Register(ModuleName, 6, "empty Ethereum signature") + ErrInvalidERC20Event = sdkerrors.Register(ModuleName, 7, "invalid ERC20 deployed event") )