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

minor channel fixes #8665

Merged
merged 10 commits into from
Mar 1, 2021
3 changes: 1 addition & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,10 @@ Ref: https://keepachangelog.com/en/1.0.0/

* (x/bank) [\#8614](https://github.com/cosmos/cosmos-sdk/issues/8614) Add `Name` and `Symbol` fields to denom metadata
* (x/auth) [\#8522](https://github.com/cosmos/cosmos-sdk/pull/8522) Allow to query all stored accounts
* (x/ibc) [\#7949](https://github.com/cosmos/cosmos-sdk/issues/7949) Standardized channel `Acknowledgement` moved to its own file. Codec registration redundancy removed.
* (crypto/types) [\#8600](https://github.com/cosmos/cosmos-sdk/pull/8600) `CompactBitArray`: optimize the `NumTrueBitsBefore` method and add an `Equal` method.
* (store/cachekv), (x/bank/types) [\#8719](https://github.com/cosmos/cosmos-sdk/pull/8719) algorithmically fix pathologically slow code



### Bug Fixes

* (keyring) [#\8635](https://github.com/cosmos/cosmos-sdk/issues/8635) Remove hardcoded default passphrase value on `NewMnemonic`
Expand Down
50 changes: 50 additions & 0 deletions x/ibc/core/04-channel/types/acknowledgement.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package types

import (
"strings"

sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
)

// NewResultAcknowledgement returns a new instance of Acknowledgement using an Acknowledgement_Result
// type in the Response field.
func NewResultAcknowledgement(result []byte) Acknowledgement {
return Acknowledgement{
Response: &Acknowledgement_Result{
Result: result,
},
}
}

// NewErrorAcknowledgement returns a new instance of Acknowledgement using an Acknowledgement_Error
// type in the Response field.
func NewErrorAcknowledgement(err string) Acknowledgement {
return Acknowledgement{
Response: &Acknowledgement_Error{
Error: err,
},
}
}

// GetBytes is a helper for serialising acknowledgements
func (ack Acknowledgement) GetBytes() []byte {
return sdk.MustSortJSON(SubModuleCdc.MustMarshalJSON(&ack))
}

// ValidateBasic performs a basic validation of the acknowledgement
func (ack Acknowledgement) ValidateBasic() error {
switch resp := ack.Response.(type) {
case *Acknowledgement_Result:
if len(resp.Result) == 0 {
return sdkerrors.Wrap(ErrInvalidAcknowledgement, "acknowledgement result cannot be empty")
}
case *Acknowledgement_Error:
if strings.TrimSpace(resp.Error) == "" {
return sdkerrors.Wrap(ErrInvalidAcknowledgement, "acknowledgement error cannot be empty")
}
default:
return sdkerrors.Wrapf(ErrInvalidAcknowledgement, "unsupported acknowledgement response field type %T", resp)
}
return nil
}
63 changes: 63 additions & 0 deletions x/ibc/core/04-channel/types/acknowledgement_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
package types_test

import "github.com/cosmos/cosmos-sdk/x/ibc/core/04-channel/types"

// tests acknowledgement.ValidateBasic and acknowledgement.GetBytes
func (suite TypesTestSuite) TestAcknowledgement() {
testCases := []struct {
name string
ack types.Acknowledgement
expPass bool
}{
{
"valid successful ack",
types.NewResultAcknowledgement([]byte("success")),
true,
},
{
"valid failed ack",
types.NewErrorAcknowledgement("error"),
true,
},
{
"empty successful ack",
types.NewResultAcknowledgement([]byte{}),
false,
},
{
"empty faied ack",
types.NewErrorAcknowledgement(" "),
false,
},
{
"nil response",
types.Acknowledgement{
Response: nil,
},
false,
},
}

for _, tc := range testCases {
tc := tc

suite.Run(tc.name, func() {
suite.SetupTest()

err := tc.ack.ValidateBasic()

if tc.expPass {
suite.Require().NoError(err)
} else {
suite.Require().Error(err)
}

// expect all acks to be able to be marshaled
suite.NotPanics(func() {
bz := tc.ack.GetBytes()
suite.Require().NotNil(bz)
})
})
}

}
45 changes: 0 additions & 45 deletions x/ibc/core/04-channel/types/channel.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
package types

import (
"strings"

sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
host "github.com/cosmos/cosmos-sdk/x/ibc/core/24-host"
"github.com/cosmos/cosmos-sdk/x/ibc/core/exported"
Expand Down Expand Up @@ -128,45 +125,3 @@ func (ic IdentifiedChannel) ValidateBasic() error {
channel := NewChannel(ic.State, ic.Ordering, ic.Counterparty, ic.ConnectionHops, ic.Version)
return channel.ValidateBasic()
}

// NewResultAcknowledgement returns a new instance of Acknowledgement using an Acknowledgement_Result
// type in the Response field.
func NewResultAcknowledgement(result []byte) Acknowledgement {
return Acknowledgement{
Response: &Acknowledgement_Result{
Result: result,
},
}
}

// NewErrorAcknowledgement returns a new instance of Acknowledgement using an Acknowledgement_Error
// type in the Response field.
func NewErrorAcknowledgement(err string) Acknowledgement {
return Acknowledgement{
Response: &Acknowledgement_Error{
Error: err,
},
}
}

// GetBytes is a helper for serialising acknowledgements
func (ack Acknowledgement) GetBytes() []byte {
return sdk.MustSortJSON(SubModuleCdc.MustMarshalJSON(&ack))
}

// ValidateBasic performs a basic validation of the acknowledgement
func (ack Acknowledgement) ValidateBasic() error {
switch resp := ack.Response.(type) {
case *Acknowledgement_Result:
if len(resp.Result) == 0 {
return sdkerrors.Wrap(ErrInvalidAcknowledgement, "acknowledgement result cannot be empty")
}
case *Acknowledgement_Error:
if strings.TrimSpace(resp.Error) == "" {
return sdkerrors.Wrap(ErrInvalidAcknowledgement, "acknowledgement error cannot be empty")
}
default:
return sdkerrors.Wrapf(ErrInvalidAcknowledgement, "unsupported acknowledgement response field type %T", resp)
}
return nil
}
60 changes: 0 additions & 60 deletions x/ibc/core/04-channel/types/channel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,63 +57,3 @@ func TestCounterpartyValidateBasic(t *testing.T) {
}
}
}

// tests acknowledgement.ValidateBasic and acknowledgement.GetBytes
func (suite TypesTestSuite) TestAcknowledgement() {
testCases := []struct {
name string
ack types.Acknowledgement
expPass bool
}{
{
"valid successful ack",
types.NewResultAcknowledgement([]byte("success")),
true,
},
{
"valid failed ack",
types.NewErrorAcknowledgement("error"),
true,
},
{
"empty successful ack",
types.NewResultAcknowledgement([]byte{}),
false,
},
{
"empty faied ack",
types.NewErrorAcknowledgement(" "),
false,
},
{
"nil response",
types.Acknowledgement{
Response: nil,
},
false,
},
}

for _, tc := range testCases {
tc := tc

suite.Run(tc.name, func() {
suite.SetupTest()

err := tc.ack.ValidateBasic()

if tc.expPass {
suite.Require().NoError(err)
} else {
suite.Require().Error(err)
}

// expect all acks to be able to be marshaled
suite.NotPanics(func() {
bz := tc.ack.GetBytes()
suite.Require().NotNil(bz)
})
})
}

}
13 changes: 2 additions & 11 deletions x/ibc/core/04-channel/types/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,25 +14,16 @@ func RegisterInterfaces(registry codectypes.InterfaceRegistry) {
registry.RegisterInterface(
"ibc.core.channel.v1.ChannelI",
(*exported.ChannelI)(nil),
&Channel{},
)
registry.RegisterInterface(
"ibc.core.channel.v1.CounterpartyChannelI",
(*exported.CounterpartyChannelI)(nil),
&Counterparty{},
)
registry.RegisterInterface(
"ibc.core.channel.v1.PacketI",
(*exported.PacketI)(nil),
)
registry.RegisterImplementations(
Copy link
Member

Choose a reason for hiding this comment

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

Is this no longer needed @colin-axner ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Duplicate registration. RegisterInterface will register implementations as well

(*exported.ChannelI)(nil),
&Channel{},
)
registry.RegisterImplementations(
(*exported.CounterpartyChannelI)(nil),
&Counterparty{},
)
registry.RegisterImplementations(
(*exported.PacketI)(nil),
&Packet{},
)
registry.RegisterImplementations(
Expand Down