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.

Sign up for GitHub

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

Remove duplication of ConvertToErrorEvents #7467

Merged
merged 2 commits into from
Oct 15, 2024
Merged

Remove duplication of ConvertToErrorEvents #7467

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
35eb834
chore: move convert to errors to internal directory
chatton Oct 15, 2024
16197dd
chore: removed empty file
chatton Oct 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 2 additions & 23 deletions modules/core/04-channel/v2/keeper/msg_server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import (
channeltypesv1 "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types"
channeltypesv2 "github.com/cosmos/ibc-go/v9/modules/core/04-channel/v2/types"
ibcerrors "github.com/cosmos/ibc-go/v9/modules/core/errors"
internalerrors "github.com/cosmos/ibc-go/v9/modules/core/internal/errors"
telemetryv2 "github.com/cosmos/ibc-go/v9/modules/core/internal/v2/telemetry"
coretypes "github.com/cosmos/ibc-go/v9/modules/core/types"
)

var _ channeltypesv2.MsgServer = &Keeper{}
Expand Down Expand Up @@ -127,7 +127,7 @@ func (k *Keeper) RecvPacket(ctx context.Context, msg *channeltypesv2.MsgRecvPack
writeFn()
} else {
// Modify events in cached context to reflect unsuccessful acknowledgement
sdkCtx.EventManager().EmitEvents(convertToErrorEvents(cacheCtx.EventManager().Events()))
sdkCtx.EventManager().EmitEvents(internalerrors.ConvertToErrorEvents(cacheCtx.EventManager().Events()))
Copy link
Contributor

Choose a reason for hiding this comment

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

think this is duplicate import, see ibcerrors

Copy link
Contributor

Choose a reason for hiding this comment

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

oh wait, its not duplicate... should we just put it in ibcerrors none the less?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess we can do that yeah, no harm

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, ibcerrors is exported, it's not under internal.

}

ack.AcknowledgementResults = append(ack.AcknowledgementResults, channeltypesv2.AcknowledgementResult{
Expand Down Expand Up @@ -233,24 +233,3 @@ func (k *Keeper) ProvideCounterparty(goCtx context.Context, msg *channeltypesv2.

return &channeltypesv2.MsgProvideCounterpartyResponse{}, nil
}

// convertToErrorEvents converts all events to error events by appending the
// error attribute prefix to each event's attribute key.
// TODO: https://github.com/cosmos/ibc-go/issues/7436
func convertToErrorEvents(events sdk.Events) sdk.Events {
if events == nil {
return nil
}

newEvents := make(sdk.Events, len(events))
for i, event := range events {
newAttributes := make([]sdk.Attribute, len(event.Attributes))
for j, attribute := range event.Attributes {
newAttributes[j] = sdk.NewAttribute(coretypes.ErrorAttributeKeyPrefix+attribute.Key, attribute.Value)
}

newEvents[i] = sdk.NewEvent(coretypes.ErrorAttributeKeyPrefix+event.Type, newAttributes...)
}

return newEvents
}
25 changes: 25 additions & 0 deletions modules/core/internal/errors/errors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package errors

import (
sdk "github.com/cosmos/cosmos-sdk/types"

coretypes "github.com/cosmos/ibc-go/v9/modules/core/types"
)

// ConvertToErrorEvents converts all events to error events by appending the
// error attribute prefix to each event's attribute key.
func ConvertToErrorEvents(events sdk.Events) sdk.Events {
if events == nil {
return nil
}

newEvents := make(sdk.Events, len(events))
for i, event := range events {
newEvents[i] = sdk.NewEvent(coretypes.ErrorAttributeKeyPrefix + event.Type)
for _, attribute := range event.Attributes {
newEvents[i] = newEvents[i].AppendAttributes(sdk.NewAttribute(coretypes.ErrorAttributeKeyPrefix+attribute.Key, attribute.Value))
}
}

return newEvents
}
5 changes: 2 additions & 3 deletions modules/core/keeper/events_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,11 @@ package keeper_test

import (
"testing"

"github.com/stretchr/testify/require"

sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/cosmos/ibc-go/v9/modules/core/keeper"
internalerrors "github.com/cosmos/ibc-go/v9/modules/core/internal/errors"
"github.com/cosmos/ibc-go/v9/modules/core/types"
)

Expand Down Expand Up @@ -99,7 +98,7 @@ func TestConvertToErrorEvents(t *testing.T) {

tc.malleate()

newEvents := keeper.ConvertToErrorEvents(events)
newEvents := internalerrors.ConvertToErrorEvents(events)
require.Equal(t, expEvents, newEvents)
})
}
Expand Down
9 changes: 0 additions & 9 deletions modules/core/keeper/export_test.go
View file
Open in desktop

This file was deleted.

25 changes: 2 additions & 23 deletions modules/core/keeper/msg_server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"errors"
"fmt"

errorsmod "cosmossdk.io/errors"

sdk "github.com/cosmos/cosmos-sdk/types"
Expand All @@ -15,8 +14,8 @@ import (
channeltypes "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types"
porttypes "github.com/cosmos/ibc-go/v9/modules/core/05-port/types"
ibcerrors "github.com/cosmos/ibc-go/v9/modules/core/errors"
internalerrors "github.com/cosmos/ibc-go/v9/modules/core/internal/errors"
"github.com/cosmos/ibc-go/v9/modules/core/internal/telemetry"
coretypes "github.com/cosmos/ibc-go/v9/modules/core/types"
)

var (
Expand Down Expand Up @@ -442,7 +441,7 @@ func (k *Keeper) RecvPacket(goCtx context.Context, msg *channeltypes.MsgRecvPack
writeFn()
} else {
// Modify events in cached context to reflect unsuccessful acknowledgement
ctx.EventManager().EmitEvents(convertToErrorEvents(cacheCtx.EventManager().Events()))
ctx.EventManager().EmitEvents(internalerrors.ConvertToErrorEvents(cacheCtx.EventManager().Events()))
}

// Set packet acknowledgement only if the acknowledgement is not nil.
Expand Down Expand Up @@ -999,26 +998,6 @@ func (k *Keeper) UpdateChannelParams(goCtx context.Context, msg *channeltypes.Ms
return &channeltypes.MsgUpdateParamsResponse{}, nil
}

// convertToErrorEvents converts all events to error events by appending the
// error attribute prefix to each event's attribute key.
func convertToErrorEvents(events sdk.Events) sdk.Events {
if events == nil {
return nil
}

newEvents := make(sdk.Events, len(events))
for i, event := range events {
newAttributes := make([]sdk.Attribute, len(event.Attributes))
for j, attribute := range event.Attributes {
newAttributes[j] = sdk.NewAttribute(coretypes.ErrorAttributeKeyPrefix+attribute.Key, attribute.Value)
}

newEvents[i] = sdk.NewEvent(coretypes.ErrorAttributeKeyPrefix+event.Type, newAttributes...)
}

return newEvents
}

// getPacketHandlerAndModule returns the appropriate packet handler
// given the protocol version. An error is returned if the protocol
// version is not supported.
Expand Down
11 changes: 5 additions & 6 deletions modules/core/keeper/msg_server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"errors"
"fmt"

upgradetypes "cosmossdk.io/x/upgrade/types"

sdk "github.com/cosmos/cosmos-sdk/types"
Expand All @@ -20,7 +19,7 @@ import (
host "github.com/cosmos/ibc-go/v9/modules/core/24-host"
ibcerrors "github.com/cosmos/ibc-go/v9/modules/core/errors"
"github.com/cosmos/ibc-go/v9/modules/core/exported"
"github.com/cosmos/ibc-go/v9/modules/core/keeper"
internalerrors "github.com/cosmos/ibc-go/v9/modules/core/internal/errors"
packetservertypes "github.com/cosmos/ibc-go/v9/modules/core/packet-server/types"
ibctm "github.com/cosmos/ibc-go/v9/modules/light-clients/07-tendermint"
ibctesting "github.com/cosmos/ibc-go/v9/testing"
Expand Down Expand Up @@ -186,13 +185,13 @@ func (suite *KeeperTestSuite) TestHandleRecvPacket() {

if tc.expRevert {
// context events should contain error events
suite.Require().Contains(events, keeper.ConvertToErrorEvents(sdk.Events{ibcmock.NewMockRecvPacketEvent()})[0])
suite.Require().Contains(events, internalerrors.ConvertToErrorEvents(sdk.Events{ibcmock.NewMockRecvPacketEvent()})[0])
suite.Require().NotContains(events, ibcmock.NewMockRecvPacketEvent())
} else {
if tc.replay {
// context should not contain application events
suite.Require().NotContains(events, ibcmock.NewMockRecvPacketEvent())
suite.Require().NotContains(events, keeper.ConvertToErrorEvents(sdk.Events{ibcmock.NewMockRecvPacketEvent()})[0])
suite.Require().NotContains(events, internalerrors.ConvertToErrorEvents(sdk.Events{ibcmock.NewMockRecvPacketEvent()})[0])
} else {
// context events should contain application events
suite.Require().Contains(events, ibcmock.NewMockRecvPacketEvent())
Expand Down Expand Up @@ -353,13 +352,13 @@ func (suite *KeeperTestSuite) TestRecvPacketV2() {

if tc.expRevert {
// context events should contain error events
suite.Require().Contains(events, keeper.ConvertToErrorEvents(sdk.Events{ibcmock.NewMockRecvPacketEvent()})[0])
suite.Require().Contains(events, internalerrors.ConvertToErrorEvents(sdk.Events{ibcmock.NewMockRecvPacketEvent()})[0])
suite.Require().NotContains(events, ibcmock.NewMockRecvPacketEvent())
} else {
if tc.replay {
// context should not contain application events
suite.Require().NotContains(events, ibcmock.NewMockRecvPacketEvent())
suite.Require().NotContains(events, keeper.ConvertToErrorEvents(sdk.Events{ibcmock.NewMockRecvPacketEvent()})[0])
suite.Require().NotContains(events, internalerrors.ConvertToErrorEvents(sdk.Events{ibcmock.NewMockRecvPacketEvent()})[0])
} else {
// context events should contain application events
suite.Require().Contains(events, ibcmock.NewMockRecvPacketEvent())
Expand Down
Loading