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

refactor: log when constructing IBC err ack #1090

Merged
merged 3 commits into from
Jun 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 3 additions & 4 deletions tests/integration/slashing.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ func (s *CCVTestSuite) TestSlashPacketAcknowledgement() {
err := consumerKeeper.OnAcknowledgementPacket(s.consumerCtx(), packet, channeltypes.NewResultAcknowledgement(ack.Acknowledgement()))
s.Require().NoError(err)

err = consumerKeeper.OnAcknowledgementPacket(s.consumerCtx(), packet, channeltypes.NewErrorAcknowledgement(fmt.Errorf("another error")))
err = consumerKeeper.OnAcknowledgementPacket(s.consumerCtx(), packet, ccv.NewErrorAcknowledgementWithLog(s.consumerCtx(), fmt.Errorf("another error")))
s.Require().Error(err)
}

Expand Down Expand Up @@ -330,7 +330,8 @@ func (suite *CCVTestSuite) TestOnRecvSlashPacketErrors() {
errAck := providerKeeper.OnRecvSlashPacket(ctx, packet, packetData)
suite.Require().False(errAck.Success())
errAckCast := errAck.(channeltypes.Acknowledgement)
// TODO: see if there's a way to get error reason like before
// Error strings in err acks are now thrown out by IBC core to prevent app hash.
// Hence a generic error string is expected.
suite.Require().Equal("ABCI code: 1: error handling packet: see events for details", errAckCast.GetError())

// Restore init chain height
Expand All @@ -341,7 +342,6 @@ func (suite *CCVTestSuite) TestOnRecvSlashPacketErrors() {
errAck = providerKeeper.OnRecvSlashPacket(ctx, packet, packetData)
suite.Require().False(errAck.Success())
errAckCast = errAck.(channeltypes.Acknowledgement)
// TODO: see if there's a way to get error reason like before
suite.Require().Equal("ABCI code: 1: error handling packet: see events for details", errAckCast.GetError())

// save current VSC ID
Expand All @@ -357,7 +357,6 @@ func (suite *CCVTestSuite) TestOnRecvSlashPacketErrors() {
errAck = providerKeeper.OnRecvSlashPacket(ctx, packet, packetData)
suite.Require().False(errAck.Success())
errAckCast = errAck.(channeltypes.Acknowledgement)
// TODO: see if there's a way to get error reason like before
suite.Require().Equal("ABCI code: 1: error handling packet: see events for details", errAckCast.GetError())

// construct slashing packet with non existing validator
Expand Down
2 changes: 1 addition & 1 deletion x/ccv/consumer/ibc_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ func (am AppModule) OnRecvPacket(
data types.ValidatorSetChangePacketData
)
if err := types.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil {
errAck := channeltypes.NewErrorAcknowledgement(fmt.Errorf("cannot unmarshal CCV packet data"))
errAck := types.NewErrorAcknowledgementWithLog(ctx, fmt.Errorf("cannot unmarshal CCV packet data"))
ack = &errAck
} else {
ack = am.keeper.OnRecvVSCPacket(ctx, packet, data)
Expand Down
2 changes: 1 addition & 1 deletion x/ccv/consumer/keeper/relay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ func TestOnAcknowledgementPacket(t *testing.T) {
).Return(nil).Times(1),
)

ack = channeltypes.NewErrorAcknowledgement(fmt.Errorf("error"))
ack = types.NewErrorAcknowledgementWithLog(ctx, fmt.Errorf("error"))
err = consumerKeeper.OnAcknowledgementPacket(ctx, packet, ack)
require.Nil(t, err)
}
Expand Down
4 changes: 2 additions & 2 deletions x/ccv/provider/ibc_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ func (am AppModule) OnRecvPacket(
)
// unmarshall consumer packet
if err := ccv.ModuleCdc.UnmarshalJSON(packet.GetData(), &consumerPacket); err != nil {
errAck := channeltypes.NewErrorAcknowledgement(fmt.Errorf("cannot unmarshal CCV packet data"))
errAck := ccv.NewErrorAcknowledgementWithLog(ctx, fmt.Errorf("cannot unmarshal CCV packet data"))
ack = &errAck
} else {
// TODO: call ValidateBasic method on consumer packet data
Expand All @@ -194,7 +194,7 @@ func (am AppModule) OnRecvPacket(
// handle SlashPacket
ack = am.keeper.OnRecvSlashPacket(ctx, packet, *consumerPacket.GetSlashPacketData())
default:
errAck := channeltypes.NewErrorAcknowledgement(fmt.Errorf("invalid consumer packet type: %q", consumerPacket.Type))
errAck := ccv.NewErrorAcknowledgementWithLog(ctx, fmt.Errorf("invalid consumer packet type: %q", consumerPacket.Type))
ack = &errAck
}
}
Expand Down
6 changes: 3 additions & 3 deletions x/ccv/provider/keeper/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func (k Keeper) OnRecvVSCMaturedPacket(
}

if err := k.QueueThrottledVSCMaturedPacketData(ctx, chainID, packet.Sequence, data); err != nil {
return channeltypes.NewErrorAcknowledgement(fmt.Errorf(
return ccv.NewErrorAcknowledgementWithLog(ctx, fmt.Errorf(
"failed to queue VSCMatured packet data: %s", err.Error()))
}

Expand Down Expand Up @@ -330,7 +330,7 @@ func (k Keeper) OnRecvSlashPacket(ctx sdk.Context, packet channeltypes.Packet, d
"vscID", data.ValsetUpdateId,
"infractionType", data.Infraction,
)
return channeltypes.NewErrorAcknowledgement(err)
return ccv.NewErrorAcknowledgementWithLog(ctx, err)
}

// The slash packet validator address may be known only on the consumer chain,
Expand Down Expand Up @@ -366,7 +366,7 @@ func (k Keeper) OnRecvSlashPacket(ctx sdk.Context, packet channeltypes.Packet, d
// Queue slash packet data in the same (consumer chain specific) queue as vsc matured packet data,
// to enforce order of handling between the two packet data types.
if err := k.QueueThrottledSlashPacketData(ctx, chainID, packet.Sequence, data); err != nil {
return channeltypes.NewErrorAcknowledgement(fmt.Errorf("failed to queue slash packet data: %s", err.Error()))
return ccv.NewErrorAcknowledgementWithLog(ctx, fmt.Errorf("failed to queue slash packet data: %s", err.Error()))
}

k.Logger(ctx).Info("slash packet received and enqueued",
Expand Down
5 changes: 5 additions & 0 deletions x/ccv/types/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,11 @@ func SendIBCPacket(
return err
}

func NewErrorAcknowledgementWithLog(ctx sdk.Context, err error) channeltypes.Acknowledgement {
ctx.Logger().Error("IBC ErrorAcknowledgement constructed", "error", err)
return channeltypes.NewErrorAcknowledgement(err)
}

// AppendMany appends a variable number of byte slices together
func AppendMany(byteses ...[]byte) (out []byte) {
for _, bytes := range byteses {
Expand Down
Loading