Conversation
x/assetctl/README.md
Outdated
| @@ -0,0 +1,154 @@ | |||
| # `x/assetctl` - Asset Registry Module | |||
There was a problem hiding this comment.
Need to review this readme, as there were some stuff that didn't comply with this
| // GetSupportAssetsCmd returns the command to support assets | ||
| func GetSupportAssetsCmd() *cobra.Command { | ||
| cmd := &cobra.Command{ | ||
| Use: "manage-supported-assets [assets-to-add] [assets-to-remove]", |
There was a problem hiding this comment.
I think, it's a bit error-prone to keep both assets to add and assets to remove in the same command. It might be better to have one command to add assets and another one to remove (same as I suggested for proto rpcs).
| service Msg { | ||
| option (cosmos.msg.v1.service) = true; | ||
|
|
||
| rpc ManageRegisteredAssets(MsgManageRegisteredAssets) returns (MsgManageRegisteredAssetsResponse); |
There was a problem hiding this comment.
Please tell me if I am wrong: supported assets are assets available on the Hub and registered assets are assets available on some specific chainlet, am I correct?
| } | ||
|
|
||
|
|
||
| message MsgManageRegisteredAssets { |
There was a problem hiding this comment.
I would probably split both Manage... rpc calls into two, one to register/add and another to unregister/remove.
|
|
||
|
|
||
| // DenomUnit represents a unit of a asset | ||
| message DenomUnit { |
There was a problem hiding this comment.
Maybe just import it from https://raw.githubusercontent.com/cosmos/cosmos-sdk/refs/tags/v0.50.13/proto/cosmos/bank/v1beta1/bank.proto?
| } | ||
|
|
||
| message MsgManageSupportedAssetsResponse { | ||
| repeated cosmos.bank.v1beta1.Metadata added_assets = 1 [(gogoproto.nullable) = false]; |
There was a problem hiding this comment.
Why do we return added assets here but do not return anything in MsgManageRegisteredAssetsResponse?
|
|
||
| // checkMsgs is recursive in case there are nested authz messages, with a hard limit of 3 levels. | ||
| func (ah AssetControlAnteHandler) checkMsgs(ctx sdk.Context, msgs []sdk.Msg, level int) error { | ||
| if level >= 3 { |
There was a problem hiding this comment.
Is it something special in number 3 here?
| for _, msg := range msgs { | ||
| msgType := sdk.MsgTypeURL(msg) | ||
|
|
||
| if msgType == ibcTransferMsgType { |
There was a problem hiding this comment.
I guess, type switch is possible here.
| } | ||
|
|
||
| // OnRecvPacket implements types.IBCModule. | ||
| func (m *IBCMiddleware) OnRecvPacket(ctx types.Context, packet channeltypes.Packet, relayer types.AccAddress) exported.Acknowledgement { |
There was a problem hiding this comment.
I really suggest to add some unit tests for this function since it's basically the core logic of the whole assetctl module.
| } | ||
| } | ||
|
|
||
| return &types.MsgManageSupportedAssetsResponse{}, nil |
2 participants
No description provided.