-
Notifications
You must be signed in to change notification settings - Fork 586
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
feat: #258 Register Counterparty Address #376
Changes from 5 commits
ef53ea1
1f6271b
325ff94
019baeb
b658d02
353fb97
9bfd299
89affa4
9e8a8d8
c1bf0a5
0b63aaf
acae9d6
7552b79
4e22511
0603eee
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,13 @@ | ||
package keeper | ||
|
||
/* | ||
import ( | ||
"github.com/tendermint/tendermint/libs/log" | ||
|
||
"github.com/cosmos/cosmos-sdk/codec" | ||
sdk "github.com/cosmos/cosmos-sdk/types" | ||
capabilitykeeper "github.com/cosmos/cosmos-sdk/x/capability/keeper" | ||
capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types" | ||
paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" | ||
"github.com/cosmos/ibc-go/modules/apps/transfer/types" | ||
"github.com/cosmos/ibc-go/modules/apps/29-fee/types" | ||
colin-axner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
host "github.com/cosmos/ibc-go/modules/core/24-host" | ||
) | ||
|
||
|
@@ -25,7 +23,6 @@ type Keeper struct { | |
scopedKeeper capabilitykeeper.ScopedKeeper | ||
} | ||
|
||
|
||
// NewKeeper creates a new 29-fee Keeper instance | ||
func NewKeeper( | ||
cdc codec.BinaryCodec, key sdk.StoreKey, paramSpace paramtypes.Subspace, | ||
|
@@ -49,6 +46,7 @@ func (k Keeper) Logger(ctx sdk.Context) log.Logger { | |
return ctx.Logger().With("module", "x/"+host.ModuleName+"-"+types.ModuleName) | ||
} | ||
|
||
/* | ||
// IsBound checks if the transfer module is already bound to the desired port | ||
func (k Keeper) IsBound(ctx sdk.Context, portID string) bool { | ||
_, ok := k.scopedKeeper.GetCapability(ctx, host.PortPath(portID)) | ||
|
@@ -62,12 +60,6 @@ func (k Keeper) BindPort(ctx sdk.Context, portID string) error { | |
return k.ClaimCapability(ctx, cap, host.PortPath(portID)) | ||
} | ||
|
||
// GetPort returns the portID for the transfer module. Used in ExportGenesis | ||
func (k Keeper) GetPort(ctx sdk.Context) string { | ||
store := ctx.KVStore(k.storeKey) | ||
return string(store.Get(types.PortKey)) | ||
} | ||
|
||
// SetPort sets the portID for the transfer module. Used in InitGenesis | ||
func (k Keeper) SetPort(ctx sdk.Context, portID string) { | ||
store := ctx.KVStore(k.storeKey) | ||
|
@@ -85,3 +77,22 @@ func (k Keeper) ClaimCapability(ctx sdk.Context, cap *capabilitytypes.Capability | |
return k.scopedKeeper.ClaimCapability(ctx, cap, name) | ||
} | ||
*/ | ||
|
||
// SetCounterPartyAddress maps the counterparty relayer address to the source relayer address | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
func (k Keeper) SetCounterpartyAddress(ctx sdk.Context, SourceAddress, CounterpartyAddress string) { | ||
seantking marked this conversation as resolved.
Show resolved
Hide resolved
|
||
store := ctx.KVStore(k.storeKey) | ||
store.Set(types.KeySourceAddress(SourceAddress), []byte(CounterpartyAddress)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This mapping is incorrect. We are mapping the address on the destination chain -> address on the source chain.
I'd use spec terminology of address -> counterpartyAddress, but the godoc should specify this is the address on the source chain of the forward relayer and that this is used in processing a packet recv on the destination chain There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated. wdyt? |
||
} | ||
|
||
// GetCounterPartyAddress gets the relayer counterparty address given a source address | ||
func (k Keeper) GetCounterPartyAddress(ctx sdk.Context, sourceAddress sdk.AccAddress) (sdk.AccAddress, bool) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
store := ctx.KVStore(k.storeKey) | ||
key := types.KeySourceAddress(sourceAddress.String()) | ||
|
||
if !store.Has(key) { | ||
// TODO: add logging here | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is logging useful here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, I was thinking of a Hadn't put much thought into it though tbh. Patterns for logging are always a funny topic of conversation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm thinking there might be times when the key not existing makes sense. Might make more sense to log where this function is called from (if we are certain the key should exist) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Happy to leave it out for now. 👍 |
||
return []byte{}, false | ||
} | ||
|
||
return store.Get(key), true | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,33 @@ | ||
package keeper | ||
|
||
// TODO | ||
//var _ types.MsgServer = Keeper{} | ||
import ( | ||
"context" | ||
|
||
sdk "github.com/cosmos/cosmos-sdk/types" | ||
|
||
"github.com/cosmos/ibc-go/modules/apps/29-fee/types" | ||
) | ||
|
||
var _ types.MsgServer = Keeper{} | ||
|
||
// RegisterCounterpartyAddress is called by the relayer on each channelEnd and allows them to specify their counterparty address before relaying | ||
// This ensures they will be properly compensated for forward relaying since destination chain must send back relayer's source address (counterparty address) in acknowledgement | ||
// This function may be called more than once by relayer, in which case, latest counterparty address is always used. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @AdityaSripal I took this to mean that the address can be overwritten by calling There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct |
||
func (k Keeper) RegisterCounterpartyAddress(goCtx context.Context, msg *types.MsgRegisterCounterpartyAddress) (*types.MsgRegisterCounterpartyAddressResponse, error) { | ||
ctx := sdk.UnwrapSDKContext(goCtx) | ||
|
||
k.SetCounterpartyAddress( | ||
ctx, msg.Address, msg.CounterpartyAddress, | ||
) | ||
|
||
k.Logger(ctx).Info("Registering counterparty address for relayer with", "Source address:", msg.Address, "With counterparty address:", msg.CounterpartyAddress) | ||
seantking marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
return &types.MsgRegisterCounterpartyAddressResponse{}, nil | ||
} | ||
|
||
// EscrowPacketFee defines a rpc handler method for MsgEscrowPacketFee | ||
// EscrowPacketFee is an open callback that may be called by any module/user that wishes to escrow funds in order to | ||
// incentivize the relaying of the given packet. | ||
func (k Keeper) EscrowPacketFee(goCtx context.Context, msg *types.MsgEscrowPacketFee) (*types.MsgEscrowPacketFeeResponse, error) { | ||
return &types.MsgEscrowPacketFeeResponse{}, nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,25 @@ | ||
package types | ||
|
||
import fmt "fmt" | ||
seantking marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
const ( | ||
// ModuleName defines the 29-fee name | ||
ModuleName = "ibcfee" | ||
|
||
// StoreKey is the store key string for IBC transfer | ||
StoreKey = ModuleName | ||
|
||
// PortKey is the port id that is wrapped by fee middleware | ||
PortKey = "feetransfer" | ||
|
||
// RouterKey is the message route for IBC transfer | ||
RouterKey = ModuleName | ||
|
||
// QuerierRoute is the querier route for IBC transfer | ||
QuerierRoute = ModuleName | ||
) | ||
|
||
// Key for relayer source address -> counteryparty address mapping | ||
func KeySourceAddress(sourceAddress string) []byte { | ||
return []byte(fmt.Sprintf("relayerSourceAddress/%s", sourceAddress)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It might be useful to make |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,32 +1,40 @@ | ||
package types | ||
|
||
/* | ||
import ( | ||
"strings" | ||
|
||
sdk "github.com/cosmos/cosmos-sdk/types" | ||
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" | ||
clienttypes "github.com/cosmos/ibc-go/modules/core/02-client/types" | ||
host "github.com/cosmos/ibc-go/modules/core/24-host" | ||
) | ||
|
||
// NewMsg | ||
func NewMsg() *Msg { | ||
return &Msg{ | ||
} | ||
// msg types | ||
const ( | ||
TypeMsgRegisterCounterPartyAddress = "registerCounterPartyAddress" | ||
) | ||
|
||
// NewMsgRegisterCounterPartyAddress | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. godoc |
||
func NewMsgRegisterCounterpartyAddress(sourceAddress, counterpartyAddress string) *MsgRegisterCounterpartyAddress { | ||
colin-axner marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return &MsgRegisterCounterpartyAddress{Address: sourceAddress, CounterpartyAddress: counterpartyAddress} | ||
seantking marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
// ValidateBasic performs a basic check of the Msg fields. | ||
func (msg Msg) ValidateBasic() error { | ||
// ValidateBasic performs a basic check of the Msg fields | ||
func (msg MsgRegisterCounterpartyAddress) ValidateBasic() error { | ||
_, err := sdk.AccAddressFromBech32(msg.Address) | ||
if err != nil { | ||
return sdkerrors.Wrap(err, "Incorrect source relayer address") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: golang error string best practice
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe the error message should indicate it failed to convert to SDK bech32 addr? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. updated. wdyt? |
||
} | ||
|
||
_, err = sdk.AccAddressFromBech32(msg.CounterpartyAddress) | ||
if err != nil { | ||
return sdkerrors.Wrap(err, "Incorrect counterparty relayer address") | ||
} | ||
|
||
return nil | ||
} | ||
|
||
// GetSigners implements sdk.Msg | ||
func (msg MsgTransfer) GetSigners() []sdk.AccAddress { | ||
signer, err := sdk.AccAddressFromBech32(msg.Sender) | ||
func (msg MsgRegisterCounterpartyAddress) GetSigners() []sdk.AccAddress { | ||
signer, err := sdk.AccAddressFromBech32(msg.Address) | ||
if err != nil { | ||
panic(err) | ||
} | ||
return []sdk.AccAddress{signer} | ||
} | ||
*/ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,36 +1,38 @@ | ||
package types | ||
|
||
/* | ||
import ( | ||
"fmt" | ||
"testing" | ||
|
||
sdk "github.com/cosmos/cosmos-sdk/types" | ||
"github.com/stretchr/testify/require" | ||
"github.com/tendermint/tendermint/crypto/secp256k1" | ||
) | ||
|
||
"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1" | ||
sdk "github.com/cosmos/cosmos-sdk/types" | ||
var ( | ||
validAadr1 = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I'm also fine with just a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. updated |
||
validAadr2 = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String() | ||
invalidAadr1 = "invalid_address" | ||
invalidAadr2 = "invalid_address" | ||
) | ||
|
||
func (suite *TypesTestSuite) TestMsgValidateBasic() { | ||
// TestMsgTransferValidation tests ValidateBasic for MsgTransfer | ||
func TestMsgRegisterCountepartyAddressValidation(t *testing.T) { | ||
testCases := []struct { | ||
name string | ||
msg *types.Msg | ||
msg *MsgRegisterCounterpartyAddress | ||
expPass bool | ||
}{ | ||
{"", types.NewMsg(), true}, | ||
{"validate with correct sdk.AccAddress", NewMsgRegisterCounterpartyAddress(validAadr1, validAadr2), true}, | ||
{"validate with incorrect source relayer address", NewMsgRegisterCounterpartyAddress(invalidAadr1, validAadr2), false}, | ||
{"validate with incorrect counterparty source relayer address", NewMsgRegisterCounterpartyAddress(validAadr1, invalidAadr2), false}, | ||
} | ||
|
||
for _, tc := range testCases { | ||
tc := tc | ||
|
||
suite.Run(tc.name, func() { | ||
err := tc.msg.ValidateBasic() | ||
if tc.expPass { | ||
suite.Require().NoError(err) | ||
} else { | ||
suite.Require().Error(err) | ||
} | ||
}) | ||
for i, tc := range testCases { | ||
err := tc.msg.ValidateBasic() | ||
if tc.expPass { | ||
require.NoError(t, err, "valid test case %d failed: %s", i, tc.name) | ||
} else { | ||
require.Error(t, err, "invalid test case %d passed: %s", i, tc.name) | ||
} | ||
} | ||
} | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for clarity?