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

Bind Transfer Port on InitChain #5954

Merged
merged 9 commits into from
Apr 8, 2020
28 changes: 15 additions & 13 deletions simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ type SimApp struct {
CrisisKeeper crisis.Keeper
UpgradeKeeper upgrade.Keeper
ParamsKeeper params.Keeper
IBCKeeper ibc.Keeper
IBCKeeper *ibc.Keeper // IBC Keeper must be a pointer in the app, so we can SetRouter on it correctly
EvidenceKeeper evidence.Keeper
TransferKeeper transfer.Keeper

Expand Down Expand Up @@ -212,16 +212,6 @@ func NewSimApp(
)
app.UpgradeKeeper = upgrade.NewKeeper(skipUpgradeHeights, keys[upgrade.StoreKey], appCodec, homePath)

// create evidence keeper with router
evidenceKeeper := evidence.NewKeeper(
appCodec, keys[evidence.StoreKey], app.subspaces[evidence.ModuleName], &app.StakingKeeper, app.SlashingKeeper,
)
evidenceRouter := evidence.NewRouter().
AddRoute(ibcclient.RouterKey, ibcclient.HandlerClientMisbehaviour(app.IBCKeeper.ClientKeeper))

evidenceKeeper.SetRouter(evidenceRouter)
app.EvidenceKeeper = *evidenceKeeper

// register the proposal types
govRouter := gov.NewRouter()
govRouter.AddRoute(gov.RouterKey, gov.ProposalHandler).
Expand All @@ -247,7 +237,8 @@ func NewSimApp(
// Create Transfer Keepers
app.TransferKeeper = transfer.NewKeeper(
app.cdc, keys[transfer.StoreKey],
app.IBCKeeper.ChannelKeeper, app.BankKeeper, app.SupplyKeeper,
app.IBCKeeper.ChannelKeeper, &app.IBCKeeper.PortKeeper,
app.BankKeeper, app.SupplyKeeper,
scopedTransferKeeper,
)
transferModule := transfer.NewAppModule(app.TransferKeeper)
Expand All @@ -257,6 +248,16 @@ func NewSimApp(
ibcRouter.AddRoute(transfer.ModuleName, transferModule)
app.IBCKeeper.SetRouter(ibcRouter)

// create evidence keeper with router
evidenceKeeper := evidence.NewKeeper(
appCodec, keys[evidence.StoreKey], app.subspaces[evidence.ModuleName], &app.StakingKeeper, app.SlashingKeeper,
)
evidenceRouter := evidence.NewRouter().
AddRoute(ibcclient.RouterKey, ibcclient.HandlerClientMisbehaviour(app.IBCKeeper.ClientKeeper))

evidenceKeeper.SetRouter(evidenceRouter)
app.EvidenceKeeper = *evidenceKeeper

// NOTE: Any module instantiated in the module manager that is later modified
// must be passed by reference here.
app.mm = module.NewManager(
Expand Down Expand Up @@ -289,6 +290,7 @@ func NewSimApp(
auth.ModuleName, distr.ModuleName, staking.ModuleName, bank.ModuleName,
slashing.ModuleName, gov.ModuleName, mint.ModuleName, supply.ModuleName,
crisis.ModuleName, ibc.ModuleName, genutil.ModuleName, evidence.ModuleName,
transfer.ModuleName,
)

app.mm.RegisterInvariants(&app.CrisisKeeper)
Expand Down Expand Up @@ -321,7 +323,7 @@ func NewSimApp(
app.SetBeginBlocker(app.BeginBlocker)
app.SetAnteHandler(
ante.NewAnteHandler(
app.AccountKeeper, app.SupplyKeeper, app.IBCKeeper,
app.AccountKeeper, app.SupplyKeeper, *app.IBCKeeper,
ante.DefaultSigVerificationGasConsumer,
),
)
Expand Down
26 changes: 13 additions & 13 deletions x/auth/ante/ante_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func TestSimulateGasCost(t *testing.T) {
// setup
app, ctx := createTestApp(true)
ctx = ctx.WithBlockHeight(1)
anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.SupplyKeeper, app.IBCKeeper, ante.DefaultSigVerificationGasConsumer)
anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.SupplyKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer)

// keys and addresses
priv1, _, addr1 := types.KeyTestPubAddr()
Expand Down Expand Up @@ -92,7 +92,7 @@ func TestSimulateGasCost(t *testing.T) {
func TestAnteHandlerSigErrors(t *testing.T) {
// setup
app, ctx := createTestApp(true)
anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.SupplyKeeper, app.IBCKeeper, ante.DefaultSigVerificationGasConsumer)
anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.SupplyKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer)

// keys and addresses
priv1, _, addr1 := types.KeyTestPubAddr()
Expand Down Expand Up @@ -142,7 +142,7 @@ func TestAnteHandlerAccountNumbers(t *testing.T) {
// setup
app, ctx := createTestApp(false)
ctx = ctx.WithBlockHeight(1)
anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.SupplyKeeper, app.IBCKeeper, ante.DefaultSigVerificationGasConsumer)
anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.SupplyKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer)

// keys and addresses
priv1, _, addr1 := types.KeyTestPubAddr()
Expand Down Expand Up @@ -201,7 +201,7 @@ func TestAnteHandlerAccountNumbersAtBlockHeightZero(t *testing.T) {
// setup
app, ctx := createTestApp(false)
ctx = ctx.WithBlockHeight(0)
anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.SupplyKeeper, app.IBCKeeper, ante.DefaultSigVerificationGasConsumer)
anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.SupplyKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer)

// keys and addresses
priv1, _, addr1 := types.KeyTestPubAddr()
Expand Down Expand Up @@ -259,7 +259,7 @@ func TestAnteHandlerSequences(t *testing.T) {
// setup
app, ctx := createTestApp(false)
ctx = ctx.WithBlockHeight(1)
anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.SupplyKeeper, app.IBCKeeper, ante.DefaultSigVerificationGasConsumer)
anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.SupplyKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer)

// keys and addresses
priv1, _, addr1 := types.KeyTestPubAddr()
Expand Down Expand Up @@ -335,7 +335,7 @@ func TestAnteHandlerSequences(t *testing.T) {
func TestAnteHandlerFees(t *testing.T) {
// setup
app, ctx := createTestApp(true)
anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.SupplyKeeper, app.IBCKeeper, ante.DefaultSigVerificationGasConsumer)
anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.SupplyKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer)

// keys and addresses
priv1, _, addr1 := types.KeyTestPubAddr()
Expand Down Expand Up @@ -377,7 +377,7 @@ func TestAnteHandlerMemoGas(t *testing.T) {
// setup
app, ctx := createTestApp(true)
ctx = ctx.WithBlockHeight(1)
anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.SupplyKeeper, app.IBCKeeper, ante.DefaultSigVerificationGasConsumer)
anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.SupplyKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer)

// keys and addresses
priv1, _, addr1 := types.KeyTestPubAddr()
Expand Down Expand Up @@ -417,7 +417,7 @@ func TestAnteHandlerMultiSigner(t *testing.T) {
// setup
app, ctx := createTestApp(false)
ctx = ctx.WithBlockHeight(1)
anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.SupplyKeeper, app.IBCKeeper, ante.DefaultSigVerificationGasConsumer)
anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.SupplyKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer)

// keys and addresses
priv1, _, addr1 := types.KeyTestPubAddr()
Expand Down Expand Up @@ -467,7 +467,7 @@ func TestAnteHandlerBadSignBytes(t *testing.T) {
// setup
app, ctx := createTestApp(true)
ctx = ctx.WithBlockHeight(1)
anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.SupplyKeeper, app.IBCKeeper, ante.DefaultSigVerificationGasConsumer)
anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.SupplyKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer)

// keys and addresses
priv1, _, addr1 := types.KeyTestPubAddr()
Expand Down Expand Up @@ -544,7 +544,7 @@ func TestAnteHandlerSetPubKey(t *testing.T) {
// setup
app, ctx := createTestApp(true)
ctx = ctx.WithBlockHeight(1)
anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.SupplyKeeper, app.IBCKeeper, ante.DefaultSigVerificationGasConsumer)
anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.SupplyKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer)

// keys and addresses
priv1, _, addr1 := types.KeyTestPubAddr()
Expand Down Expand Up @@ -662,7 +662,7 @@ func TestAnteHandlerSigLimitExceeded(t *testing.T) {
// setup
app, ctx := createTestApp(true)
ctx = ctx.WithBlockHeight(1)
anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.SupplyKeeper, app.IBCKeeper, ante.DefaultSigVerificationGasConsumer)
anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.SupplyKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer)

// keys and addresses
priv1, _, addr1 := types.KeyTestPubAddr()
Expand Down Expand Up @@ -703,7 +703,7 @@ func TestCustomSignatureVerificationGasConsumer(t *testing.T) {
app, ctx := createTestApp(true)
ctx = ctx.WithBlockHeight(1)
// setup an ante handler that only accepts PubKeyEd25519
anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.SupplyKeeper, app.IBCKeeper, func(meter sdk.GasMeter, sig []byte, pubkey crypto.PubKey, params types.Params) error {
anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.SupplyKeeper, *app.IBCKeeper, func(meter sdk.GasMeter, sig []byte, pubkey crypto.PubKey, params types.Params) error {
switch pubkey := pubkey.(type) {
case ed25519.PubKeyEd25519:
meter.ConsumeGas(params.SigVerifyCostED25519, "ante verify: ed25519")
Expand Down Expand Up @@ -761,7 +761,7 @@ func TestAnteHandlerReCheck(t *testing.T) {
app.AccountKeeper.SetAccount(ctx, acc1)
app.BankKeeper.SetBalances(ctx, addr1, types.NewTestCoins())

antehandler := ante.NewAnteHandler(app.AccountKeeper, app.SupplyKeeper, app.IBCKeeper, ante.DefaultSigVerificationGasConsumer)
antehandler := ante.NewAnteHandler(app.AccountKeeper, app.SupplyKeeper, *app.IBCKeeper, ante.DefaultSigVerificationGasConsumer)

// test that operations skipped on recheck do not run

Expand Down
6 changes: 6 additions & 0 deletions x/capability/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,12 @@ func (k *Keeper) InitializeAndSeal(ctx sdk.Context) {
k.sealed = true
}

// GetLatestIndex returns the latest index of the CapabilityKeeper
func (k Keeper) GetLatestIndex(ctx sdk.Context) uint64 {
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
store := ctx.KVStore(k.storeKey)
return types.IndexFromKey(store.Get(types.KeyIndex))
}

// NewCapability attempts to create a new capability with a given name. If the
// capability already exists in the in-memory store, an error will be returned.
// Otherwise, a new capability is created with the current global unique index.
Expand Down
8 changes: 4 additions & 4 deletions x/capability/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ type KeeperTestSuite struct {

ctx sdk.Context
keeper *keeper.Keeper
app *simapp.SimApp
}

func (suite *KeeperTestSuite) SetupTest() {
Expand All @@ -35,19 +34,20 @@ func (suite *KeeperTestSuite) SetupTest() {

suite.ctx = app.BaseApp.NewContext(checkTx, abci.Header{Height: 1})
suite.keeper = keeper
suite.app = app
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
}

func (suite *KeeperTestSuite) TestInitializeAndSeal() {
sk := suite.keeper.ScopeToModule(bank.ModuleName)

caps := make([]*types.Capability, 5)
// Get Latest Index before creating new ones to sychronize indices correctly
prevIndex := suite.keeper.GetLatestIndex(suite.ctx)
fedekunze marked this conversation as resolved.
Show resolved Hide resolved

for i := range caps {
cap, err := sk.NewCapability(suite.ctx, fmt.Sprintf("transfer-%d", i))
suite.Require().NoError(err)
suite.Require().NotNil(cap)
suite.Require().Equal(uint64(i), cap.GetIndex())
suite.Require().Equal(uint64(i)+prevIndex, cap.GetIndex())

caps[i] = cap
}
Expand All @@ -60,7 +60,7 @@ func (suite *KeeperTestSuite) TestInitializeAndSeal() {
got, ok := sk.GetCapability(suite.ctx, fmt.Sprintf("transfer-%d", i))
suite.Require().True(ok)
suite.Require().Equal(cap, got)
suite.Require().Equal(uint64(i), got.GetIndex())
suite.Require().Equal(uint64(i)+prevIndex, got.GetIndex())
}

suite.Require().Panics(func() {
Expand Down
3 changes: 2 additions & 1 deletion x/ibc/05-port/types/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ func (rtr *Router) AddRoute(module string, cbs IBCModule) *Router {

// HasRoute returns true if the Router has a module registered or false otherwise.
func (rtr *Router) HasRoute(module string) bool {
return rtr.routes[module] != nil
_, ok := rtr.routes[module]
return ok
}

// GetRoute returns a IBCModule for a given module.
Expand Down
18 changes: 18 additions & 0 deletions x/ibc/20-transfer/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,26 @@ import (
"github.com/cosmos/cosmos-sdk/x/ibc/20-transfer/types"
)

// GenesisState is currently only used to ensure that the InitGenesis gets run
// by the module manager
type GenesisState struct {
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
Version string `json:"version,omitempty" yaml:"version,omitempty"`
}

func DefaultGenesis() GenesisState {
return GenesisState{
Version: types.Version,
}
}

// InitGenesis sets distribution information for genesis
func InitGenesis(ctx sdk.Context, keeper Keeper) {
// transfer module binds to the transfer port on InitChain
// and claims the returned capability
err := keeper.BindPort(ctx, types.PortID)
if err != nil {
panic(fmt.Sprintf("could not claim port capability: %v", err))
}
// check if the module account exists
moduleAcc := keeper.GetTransferAccount(ctx)
if moduleAcc == nil {
Expand Down
20 changes: 15 additions & 5 deletions x/ibc/20-transfer/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/cosmos/cosmos-sdk/x/capability"
channel "github.com/cosmos/cosmos-sdk/x/ibc/04-channel"
channelexported "github.com/cosmos/cosmos-sdk/x/ibc/04-channel/exported"
porttypes "github.com/cosmos/cosmos-sdk/x/ibc/05-port/types"
"github.com/cosmos/cosmos-sdk/x/ibc/20-transfer/types"
ibctypes "github.com/cosmos/cosmos-sdk/x/ibc/types"
supplyexported "github.com/cosmos/cosmos-sdk/x/supply/exported"
Expand All @@ -26,16 +27,17 @@ type Keeper struct {
storeKey sdk.StoreKey
cdc *codec.Codec

channelKeeper types.ChannelKeeper
bankKeeper types.BankKeeper
supplyKeeper types.SupplyKeeper
scopedKeeper capability.ScopedKeeper
channelKeeper types.ChannelKeeper
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

File is not gofmt-ed with -s (from gofmt)

portKeeper types.PortKeeper
bankKeeper types.BankKeeper
supplyKeeper types.SupplyKeeper
scopedKeeper capability.ScopedKeeper
}

// NewKeeper creates a new IBC transfer Keeper instance
func NewKeeper(
cdc *codec.Codec, key sdk.StoreKey,
channelKeeper types.ChannelKeeper,
channelKeeper types.ChannelKeeper, portKeeper types.PortKeeper,
bankKeeper types.BankKeeper, supplyKeeper types.SupplyKeeper,
scopedKeeper capability.ScopedKeeper,
) Keeper {
Expand All @@ -49,6 +51,7 @@ func NewKeeper(
storeKey: key,
cdc: cdc,
channelKeeper: channelKeeper,
portKeeper: portKeeper,
bankKeeper: bankKeeper,
supplyKeeper: supplyKeeper,
scopedKeeper: scopedKeeper,
Expand Down Expand Up @@ -82,6 +85,13 @@ func (k Keeper) ChanCloseInit(ctx sdk.Context, portID, channelID string) error {
return k.channelKeeper.ChanCloseInit(ctx, portID, channelID, chanCap)
}

// BindPort defines a wrapper function for the ort Keeper's function in
// order to expose it to module's InitGenesis function
func (k Keeper) BindPort(ctx sdk.Context, portID string) error {
cap := k.portKeeper.BindPort(ctx, portID)
return k.ClaimCapability(ctx, cap, porttypes.PortPath(portID))
}

// TimeoutExecuted defines a wrapper function for the channel Keeper's function
// in order to expose it to the ICS20 transfer handler.
func (k Keeper) TimeoutExecuted(ctx sdk.Context, packet channelexported.PacketI) error {
Expand Down
6 changes: 3 additions & 3 deletions x/ibc/20-transfer/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ func (AppModuleBasic) RegisterCodec(cdc *codec.Codec) {

// DefaultGenesis returns default genesis state as raw bytes for the ibc
// transfer module.
func (AppModuleBasic) DefaultGenesis(_ codec.JSONMarshaler) json.RawMessage {
return nil
func (AppModuleBasic) DefaultGenesis(cdc codec.JSONMarshaler) json.RawMessage {
return cdc.MustMarshalJSON(DefaultGenesis())
}

// ValidateGenesis performs genesis state validation for the ibc transfer module.
Expand Down Expand Up @@ -235,7 +235,7 @@ func (am AppModule) OnRecvPacket(
packet channeltypes.Packet,
) (*sdk.Result, error) {
var data FungibleTokenPacketData
if err := types.ModuleCdc.UnmarshalBinaryBare(packet.GetData(), &data); err != nil {
if err := types.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil {
return nil, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "cannot unmarshal ICS-20 transfer packet data: %s", err.Error())
}
return handlePacketDataTransfer(ctx, am.keeper, packet, data)
Expand Down
5 changes: 5 additions & 0 deletions x/ibc/20-transfer/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ type ConnectionKeeper interface {
GetConnection(ctx sdk.Context, connectionID string) (connection connection.ConnectionEnd, found bool)
}

// PortKeeper defines the expected IBC port keeper
type PortKeeper interface {
BindPort(ctx sdk.Context, portID string) *capability.Capability
}

// SupplyKeeper expected supply keeper
type SupplyKeeper interface {
GetModuleAddress(name string) sdk.AccAddress
Expand Down
2 changes: 1 addition & 1 deletion x/ibc/20-transfer/types/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const (
Version = "ics20-1"

// PortID that transfer module binds to
PortID = "bank"
PortID = "transfer"
Copy link
Collaborator

Choose a reason for hiding this comment

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

double-checking if is this correct?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Yes this is good. I prefer transfer to bank tbh.


// StoreKey is the store key string for IBC transfer
StoreKey = ModuleName
Expand Down
9 changes: 6 additions & 3 deletions x/ibc/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,13 @@ type Keeper struct {
// NewKeeper creates a new ibc Keeper
func NewKeeper(
cdc *codec.Codec, key sdk.StoreKey, stakingKeeper client.StakingKeeper, scopedKeeper capability.ScopedKeeper,
) Keeper {
) *Keeper {
clientKeeper := client.NewKeeper(cdc, key, stakingKeeper)
connectionKeeper := connection.NewKeeper(cdc, key, clientKeeper)
portKeeper := port.NewKeeper(scopedKeeper)
channelKeeper := channel.NewKeeper(cdc, key, clientKeeper, connectionKeeper, portKeeper, scopedKeeper)

return Keeper{
return &Keeper{
ClientKeeper: clientKeeper,
ConnectionKeeper: connectionKeeper,
ChannelKeeper: channelKeeper,
Expand All @@ -37,7 +37,10 @@ func NewKeeper(
}

// Set the Router in IBC Keeper and seal it
func (k Keeper) SetRouter(rtr *port.Router) {
func (k *Keeper) SetRouter(rtr *port.Router) {
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
if k.Router != nil && k.Router.Sealed() {
panic("cannot reset a sealed router")
}
k.Router = rtr
k.Router.Seal()
}
Loading