Skip to content

Commit

Permalink
refactor: log when constructing IBC err ack (#1090)
Browse files Browse the repository at this point in the history
* log with err ack

* linter
  • Loading branch information
shaspitz authored Jun 28, 2023
1 parent ce7bedb commit 07302ff
Show file tree
Hide file tree
Showing 6 changed files with 15 additions and 11 deletions.
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

0 comments on commit 07302ff

Please sign in to comment.