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

Receive and send valset reward on cosmos side instead of ethereum. #91

Merged
merged 1 commit into from
Jul 5, 2022
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: 5 additions & 0 deletions .github/workflows/go.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@ on:
push:
branches:
- main
- native-reward # temp
pull_request:
branches:
- main
- native-reward # temp


jobs:
build:
Expand Down
8 changes: 6 additions & 2 deletions .github/workflows/integration-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,13 @@ name: Integration tests

on:
push:
branches: [master, main]
branches:
- main
- native-reward # temp
pull_request:
branches: [master, main]
branches:
- main
- native-reward # temp

jobs:
happy-path-hardhat:
Expand Down
8 changes: 6 additions & 2 deletions .github/workflows/node.js.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,13 @@ name: Solidity contract build and test

on:
push:
branches: [master, main]
branches:
- main
- native-reward # temp
pull_request:
branches: [master, main]
branches:
- main
- native-reward # temp

jobs:
core-tests:
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@ on:
push:
branches:
- main
- native-reward # temp
pull_request:
branches:
- main
- native-reward # temp

env:
CARGO_TERM_COLOR: always
Expand Down
5 changes: 3 additions & 2 deletions module/proto/gravity/v1/msgs.proto
Original file line number Diff line number Diff line change
Expand Up @@ -246,8 +246,9 @@ message MsgValsetUpdatedClaim {
(gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Int",
(gogoproto.nullable) = false
];
string reward_token = 6;
string orchestrator = 7;
string reward_denom = 6;
string reward_recipient = 7;
string orchestrator = 8;
}

message MsgValsetUpdatedClaimResponse {}
Expand Down
4 changes: 2 additions & 2 deletions module/proto/gravity/v1/types.proto
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ message Valset {
(gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Int",
(gogoproto.nullable) = false
];
// the reward token in it's Ethereum hex address representation
string reward_token = 5;
// the cosmos native reward token/denom
string reward_denom = 5;

}

Expand Down
7 changes: 4 additions & 3 deletions module/x/gravity/abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/onomyprotocol/cosmos-gravity-bridge/module/x/gravity/keeper"
"github.com/onomyprotocol/cosmos-gravity-bridge/module/x/gravity/types"
sdk "github.com/cosmos/cosmos-sdk/types"
slashingtypes "github.com/cosmos/cosmos-sdk/x/slashing/types"
"github.com/cosmos/cosmos-sdk/x/staking"

"github.com/onomyprotocol/cosmos-gravity-bridge/module/x/gravity/keeper"
"github.com/onomyprotocol/cosmos-gravity-bridge/module/x/gravity/types"
)

func TestValsetCreationIfNotAvailable(t *testing.T) {
Expand All @@ -21,7 +22,7 @@ func TestValsetCreationIfNotAvailable(t *testing.T) {

// EndBlocker should set a new validator set if not available
EndBlocker(ctx, pk)
require.NotNil(t, pk.GetValset(ctx, uint64(pk.GetLatestValsetNonce(ctx))))
require.NotNil(t, pk.GetValset(ctx, pk.GetLatestValsetNonce(ctx)))
valsets := pk.GetValsets(ctx)
require.True(t, len(valsets) == 1)
}
Expand Down
2 changes: 1 addition & 1 deletion module/x/gravity/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -656,7 +656,7 @@ func TestMsgValsetConfirm(t *testing.T) {
blockHeight int64 = 200
signature = "7c331bd8f2f586b04a2e2cafc6542442ef52e8b8be49533fa6b8962e822bc01e295a62733abfd65a412a8de8286f2794134c160c27a2827bdb71044b94b003cc1c"
badSignature = "6c331bd8f2f586b04a2e2cafc6542442ef52e8b8be49533fa6b8962e822bc01e295a62733abfd65a412a8de8286f2794134c160c27a2827bdb71044b94b003cc1c"
ethAddress = "0xd62FF457C6165FF214C1658c993A8a203E601B03"
ethAddress = "0x6DBd7922e7f9502191ECe180635942f2DcEa73BC"
wrongAddress = "0xb9a2c7853F181C3dd4a0517FCb9470C0f709C08C"
)
ethAddressParsed, err := types.NewEthAddress(ethAddress)
Expand Down
2 changes: 1 addition & 1 deletion module/x/gravity/keeper/attestation.go
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ func (k Keeper) GetLastObservedValset(ctx sdk.Context) *types.Valset {
Members: []types.BridgeValidator{},
Height: 0,
RewardAmount: sdk.Int{},
RewardToken: "",
RewardDenom: "",
}
k.cdc.MustUnmarshal(bytes, &valset)
return &valset
Expand Down
92 changes: 38 additions & 54 deletions module/x/gravity/keeper/attestation_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@ package keeper

import (
"fmt"
distrkeeper "github.com/cosmos/cosmos-sdk/x/distribution/keeper"
"math/big"
"strconv"

sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
bankkeeper "github.com/cosmos/cosmos-sdk/x/bank/keeper"

distrkeeper "github.com/cosmos/cosmos-sdk/x/distribution/keeper"
distypes "github.com/cosmos/cosmos-sdk/x/distribution/types"

"github.com/onomyprotocol/cosmos-gravity-bridge/module/x/gravity/types"
)

Expand Down Expand Up @@ -271,71 +271,55 @@ func (a AttestationHandler) Handle(ctx sdk.Context, att types.Attestation, claim
),
)
case *types.MsgValsetUpdatedClaim:
rewardAddress, err := types.NewEthAddress(claim.RewardToken)
if err != nil {
return sdkerrors.Wrap(err, "invalid reward token on claim")
}
// TODO here we should check the contents of the validator set against
dzmitryhil marked this conversation as resolved.
Show resolved Hide resolved
// the store, if they differ we should take some action to indicate to the
// user that bridge highjacking has occurred
a.keeper.SetLastObservedValset(ctx, types.Valset{
Nonce: claim.ValsetNonce,
Members: claim.Members,
Height: 0,
RewardAmount: claim.RewardAmount,
RewardToken: claim.RewardToken,
RewardDenom: claim.RewardDenom,
})
// if the reward is greater than zero and the reward token
// is valid then some reward was issued by this validator set
// and we need to either add to the total tokens for a Cosmos native
// token, or burn non cosmos native tokens
if claim.RewardAmount.GT(sdk.ZeroInt()) && claim.RewardToken != types.ZeroAddressString {
// Check if coin is Cosmos-originated asset and get denom
isCosmosOriginated, denom := a.keeper.ERC20ToDenomLookup(ctx, *rewardAddress)
if isCosmosOriginated {
// If it is cosmos originated, mint some coins to account
// for coins that now exist on Ethereum and may eventually come
// back to Cosmos.
//
// Note the flow is
// user relays valset and gets reward -> event relayed to cosmos mints tokens to module
// -> user sends tokens to cosmos and gets the minted tokens from the module
//
// it is not possible for this to be a race condition thanks to the event nonces
// no matter how long it takes to relay the valset updated event the deposit event
// for the user will always come after.
//
// Note we are minting based on the claim! This is important as the reward value
// could change between when this event occurred and the present
coins := sdk.Coins{sdk.NewCoin(denom, claim.RewardAmount)}
if err := a.bankKeeper.MintCoins(ctx, types.ModuleName, coins); err != nil {
ctx.EventManager().EmitEvent(
sdk.NewEvent(
sdk.EventTypeMessage,
sdk.NewAttribute("MsgValsetUpdatedClaim", strconv.Itoa(int(claim.GetEventNonce()))),
),
)
return sdkerrors.Wrapf(err, "unable to mint cosmos originated coins %v", coins)
}
} else {
// // If it is not cosmos originated, burn the coins (aka Vouchers)
// // so that we don't think we have more in the bridge than we actually do
// coins := sdk.Coins{sdk.NewCoin(denom, claim.RewardAmount)}
// a.bankKeeper.BurnCoins(ctx, types.ModuleName, coins)

// if you want to issue Ethereum originated tokens remove this panic and uncomment
// the above code but note that you will have to constantly replenish the tokens in the
// module or your chain will eventually halt.
panic("Can not use Ethereum originated token as reward!")
}
}
ctx.EventManager().EmitEvent(
sdk.NewEvent(
sdk.EventTypeMessage,
sdk.NewAttribute("MsgValsetUpdatedClaim", strconv.Itoa(int(claim.GetEventNonce()))),
),
)

// if the reward is greater than zero and the reward token isn't nil we process the reward
if !claim.RewardAmount.IsNil() && claim.RewardAmount.GT(sdk.ZeroInt()) && claim.RewardDenom != "" {
// Mint some coins to account and send to the recipient.
//
// Note the flow is
// user relays valset -> event relayed to cosmos mints tokens to module and sends to recipient
//
// it is not possible for this to be a race condition thanks to the event nonces
// no matter how long it takes to relay the valset updated event the deposit event
// for the user will always come after.
//
// Note we are minting based on the claim! This is important as the reward value
// could change between when this event occurred and the present
coins := sdk.Coins{sdk.NewCoin(claim.RewardDenom, claim.RewardAmount)}
if err := a.bankKeeper.MintCoins(ctx, types.ModuleName, coins); err != nil {
return sdkerrors.Wrapf(err, "unable to mint cosmos originated coins for valset reward, coins: %v",
coins)
}

// If the recipient address is valid we send the minted coins to the provided address.
// If the address is wrong, the minted coins will be sent to the community pool.
recipient, err := sdk.AccAddressFromBech32(claim.RewardRecipient)
if err != nil {
if err := a.SendToCommunityPool(ctx, coins); err != nil {
return sdkerrors.Wrapf(err, "unable to send coins to community pool for valset reward, coins: %v",
coins)
}
return nil
}
if err := a.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, recipient, coins); err != nil {
return sdkerrors.Wrapf(err, "unable to send coins to recipient for valset reward, coins: %v, %s",
coins, recipient.String())
}
}

default:
panic(fmt.Sprintf("Invalid event type for attestations %s", claim.GetType()))
}
Expand Down
56 changes: 53 additions & 3 deletions module/x/gravity/keeper/attestation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,13 @@ package keeper
import (
"testing"

"github.com/onomyprotocol/cosmos-gravity-bridge/module/x/gravity/types"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
sdktypes "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/crypto/keys/ed25519"
sdk "github.com/cosmos/cosmos-sdk/types"
distypes "github.com/cosmos/cosmos-sdk/x/distribution/types"
"github.com/stretchr/testify/require"

"github.com/onomyprotocol/cosmos-gravity-bridge/module/x/gravity/types"
)

// Sets up 10 attestations and checks that they are returned in the correct order
Expand All @@ -24,7 +27,7 @@ func TestGetMostRecentAttestations(t *testing.T) {
EventNonce: nonce,
BlockHeight: 1,
TokenContract: "0x00000000000000000001",
Amount: sdktypes.NewInt(10000000000 + int64(i)),
Amount: sdk.NewInt(10000000000 + int64(i)),
EthereumSender: "0x00000000000000000002",
CosmosReceiver: "0x00000000000000000003",
Orchestrator: "0x00000000000000000004",
Expand All @@ -51,3 +54,50 @@ func TestGetMostRecentAttestations(t *testing.T) {
"The %vth claim does not match our message: claim %v\n message %v", n, attest.Claim, msgs[n])
}
}

func TestHandleMsgValsetUpdatedClaim(t *testing.T) {
rewardAmount := sdk.NewInt(100)

rewardRecipient := sdk.AccAddress(ed25519.GenPrivKey().PubKey().Address())

testEnv := CreateTestEnv(t)

accountKeeper := testEnv.AccountKeeper
bankKeeper := testEnv.BankKeeper
gravityKeeper := testEnv.GravityKeeper
stakingKeeper := testEnv.StakingKeeper

ctx := testEnv.Context

rewardDenom := stakingKeeper.BondDenom(ctx)
distAccount := accountKeeper.GetModuleAddress(distypes.ModuleName)
initialDistBalanceAmount := bankKeeper.GetBalance(ctx, distAccount, rewardDenom).Amount

// empty message
msg := &types.MsgValsetUpdatedClaim{}
err := gravityKeeper.AttestationHandler.Handle(ctx, types.Attestation{}, msg)
require.NoError(t, err)

// with valid reward and recipient
msg = &types.MsgValsetUpdatedClaim{
RewardAmount: rewardAmount,
RewardDenom: rewardDenom,
RewardRecipient: rewardRecipient.String(),
}
err = gravityKeeper.AttestationHandler.Handle(ctx, types.Attestation{}, msg)
require.NoError(t, err)
recipientBalanceAmount := bankKeeper.GetBalance(ctx, rewardRecipient, rewardDenom).Amount
require.Equal(t, rewardAmount, recipientBalanceAmount)

// with valid reward and invalid recipient (goes to community pool)
msg = &types.MsgValsetUpdatedClaim{
RewardAmount: rewardAmount,
RewardDenom: rewardDenom,
RewardRecipient: "invalid-recipient-address",
}
err = gravityKeeper.AttestationHandler.Handle(ctx, types.Attestation{}, msg)
require.NoError(t, err)

distBalanceAmount := bankKeeper.GetBalance(ctx, distAccount, rewardDenom).Amount
require.Equal(t, rewardAmount, distBalanceAmount.Sub(initialDistBalanceAmount))
}
21 changes: 0 additions & 21 deletions module/x/gravity/keeper/cosmos-originated.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,27 +64,6 @@ func (k Keeper) DenomToERC20Lookup(ctx sdk.Context, denom string) (bool, *types.
return false, tc1, nil
}

// RewardToERC20Lookup is a specialized function wrapping DenomToERC20Lookup designed to validate
// the validator set reward any time we generate a validator set
func (k Keeper) RewardToERC20Lookup(ctx sdk.Context, coin sdk.Coin) (*types.EthAddress, sdk.Int) {
if !coin.IsValid() || coin.IsZero() {
panic("Bad validator set relaying reward!")
} else {
// reward case, pass to DenomToERC20Lookup
_, address, err := k.DenomToERC20Lookup(ctx, coin.Denom)
if err != nil {
// This can only ever happen if governance sets a value for the reward
// which is not a valid ERC20 that as been bridged before (either from or to Cosmos)
// We'll classify that as operator error and just panic
panic("Invalid Valset reward! Correct or remove the paramater value")
}
if err != nil {
panic("Invalid Valset reward! Correct or remove the paramater value")
}
return address, coin.Amount
}
}

// ERC20ToDenom returns (bool isCosmosOriginated, string denom, err)
// Using this information, you can see if an ERC20 address representing an asset is native to Cosmos or Ethereum,
// and get its corresponding denom
Expand Down
9 changes: 4 additions & 5 deletions module/x/gravity/keeper/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,14 +158,13 @@ func InitGenesis(ctx sdk.Context, k Keeper, data types.GenesisState) {
k.setCosmosOriginatedDenomToERC20(ctx, item.Denom, *ethAddr)
}

// now that we have the denom-erc20 mapping we need to validate
// that the valset reward is possible and cosmos originated remove
// this if you want a non-cosmos originated reward
dzmitryhil marked this conversation as resolved.
Show resolved Hide resolved
// the valset reward is possible in cosmos native tokens only
valsetReward := k.GetParams(ctx).ValsetReward
if valsetReward.IsValid() && !valsetReward.IsZero() {
_, exists := k.GetCosmosOriginatedERC20(ctx, valsetReward.Denom)
_, exists := k.bankKeeper.GetDenomMetaData(ctx, valsetReward.Denom)
if !exists {
panic("Invalid Cosmos originated denom for valset reward")
panic(fmt.Sprintf("Invalid Cosmos originated denom for valset reward, denom %s "+
"not found in the bank keeper metadata", valsetReward.Denom))
}
}

Expand Down
Loading