Skip to content

Commit

Permalink
minor channel fixes (#8665)
Browse files Browse the repository at this point in the history
* Consolidating codec.go registrations. Moving Acknowledgement result/error to its own file

* Updating CHANGELOG.md with #7949 improvement as requested

* revert removing acknowledgement proto to its own file

* update changelog

* remove unnecessary pb.go file

Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>
  • Loading branch information
damiannolan and colin-axner authored Mar 1, 2021
1 parent 3832860 commit 0792db7
Show file tree
Hide file tree
Showing 6 changed files with 116 additions and 118 deletions.
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(
(*exported.ChannelI)(nil),
&Channel{},
)
registry.RegisterImplementations(
(*exported.CounterpartyChannelI)(nil),
&Counterparty{},
)
registry.RegisterImplementations(
(*exported.PacketI)(nil),
&Packet{},
)
registry.RegisterImplementations(
Expand Down

0 comments on commit 0792db7

Please sign in to comment.