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

feat(hard_fork): part2 (Delyayed ack callback) #1355

Merged
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
1 change: 1 addition & 0 deletions app/keepers/keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,7 @@ func (a *AppKeepers) InitKeepers(
a.DelayedAckKeeper = *delayedackkeeper.NewKeeper(
appCodec,
a.keys[delayedacktypes.StoreKey],
a.keys[ibcexported.StoreKey],
a.GetSubspace(delayedacktypes.ModuleName),
a.RollappKeeper,
a.IBCKeeper.ChannelKeeper,
Expand Down
1 change: 0 additions & 1 deletion proto/dymensionxyz/dymension/common/status.proto
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,4 @@ option go_package = "github.com/dymensionxyz/dymension/v3/x/common/types";
enum Status {
PENDING = 0;
FINALIZED = 1;
REVERTED = 3;
}
1 change: 1 addition & 0 deletions testutil/keeper/delayedack.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ func DelayedackKeeper(t testing.TB) (*keeper.Keeper, sdk.Context) {

k := keeper.NewKeeper(cdc,
storeKey,
nil,
paramsSubspace,
RollappKeeperStub{},
ICS4WrapperStub{},
Expand Down
4 changes: 0 additions & 4 deletions x/common/types/key_rollapp_packet.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ var (
PendingRollappPacketKeyPrefix = []byte{0x00, 0x01}
// FinalizedRollappPacketKeyPrefix is the prefix for finalized rollapp packets
FinalizedRollappPacketKeyPrefix = []byte{0x00, 0x02}
// RevertedRollappPacketKeyPrefix is the prefix for reverted rollapp packets
RevertedRollappPacketKeyPrefix = []byte{0x00, 0x03}
// keySeparatorBytes is used to separate the rollapp packet key parts
keySeparatorBytes = []byte("/")
)
Expand Down Expand Up @@ -100,8 +98,6 @@ func MustGetStatusBytes(status Status) []byte {
return PendingRollappPacketKeyPrefix
case Status_FINALIZED:
return FinalizedRollappPacketKeyPrefix
case Status_REVERTED:
return RevertedRollappPacketKeyPrefix
default:
panic(fmt.Sprintf("invalid packet status: %s", status))
}
Expand Down
18 changes: 7 additions & 11 deletions x/common/types/status.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions x/delayedack/client/cli/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func CmdGetPacketsByRollapp() *cobra.Command {
cmd := &cobra.Command{
Use: "packets-by-rollapp rollapp-id [status] [type]",
Short: "Get packets by rollapp-id",
Long: `Get packets by rollapp-id. Can filter by status (pending/finalized/reverted) and by type (recv/ack/timeout)
Long: `Get packets by rollapp-id. Can filter by status (pending/finalized) and by type (recv/ack/timeout)
Example:
packets rollapp1
packets rollapp1 PENDING
Expand Down Expand Up @@ -124,7 +124,7 @@ func CmdGetPacketsByStatus() *cobra.Command {
cmd := &cobra.Command{
Use: "packets-by-status status [type]",
Short: "Get packets by status",
Long: `Get packets by status (pending/finalized/reverted). Can filter by type (recv/ack/timeout)
Long: `Get packets by status (pending/finalized). Can filter by type (recv/ack/timeout)
Example:
packets-by-status pending
packets-by-status finalized recv`,
Expand Down
42 changes: 7 additions & 35 deletions x/delayedack/keeper/finalize.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"github.com/osmosis-labs/osmosis/v15/osmoutils"

commontypes "github.com/dymensionxyz/dymension/v3/x/common/types"
"github.com/dymensionxyz/dymension/v3/x/delayedack/types"
)

// FinalizeRollappPacket finalizes a singe packet by its rollapp packet key.
Expand All @@ -25,7 +24,7 @@ func (k Keeper) FinalizeRollappPacket(ctx sdk.Context, ibc porttypes.IBCModule,
// Verify the height is finalized
err = k.VerifyHeightFinalized(ctx, packet.RollappId, packet.ProofHeight)
if err != nil {
return packet, fmt.Errorf("verify height is finalized: rollapp '%s': %w", packet.RollappId, err)
return packet, fmt.Errorf("verify height: rollapp '%s': %w", packet.RollappId, err)
}

// Finalize the packet
Expand Down Expand Up @@ -137,41 +136,14 @@ func (k Keeper) onTimeoutPacket(rollappPacket commontypes.RollappPacket, ibc por
}

func (k Keeper) VerifyHeightFinalized(ctx sdk.Context, rollappID string, height uint64) error {
// Get the latest state info of the rollapp
latestIndex, found := k.rollappKeeper.GetLatestFinalizedStateIndex(ctx, rollappID)
if !found {
return gerrc.ErrNotFound.Wrapf("latest finalized state index is not found")
}
stateInfo, found := k.rollappKeeper.GetStateInfo(ctx, rollappID, latestIndex.Index)
if !found {
return gerrc.ErrNotFound.Wrapf("state info is not found")
latestFinalizedHeight, err := k.getRollappLatestFinalizedHeight(ctx, rollappID)
if err != nil {
return err
}

// Check the latest finalized height of the rollapp is higher than the height specified
if height > stateInfo.GetLatestHeight() {
return gerrc.ErrInvalidArgument.Wrapf("packet height is not finalized yet: height '%d', latest height '%d'", height, stateInfo.GetLatestHeight())
if height > latestFinalizedHeight {
return gerrc.ErrInvalidArgument.Wrapf("packet height is not finalized yet: height '%d', latest finalized height '%d'", height, latestFinalizedHeight)
}
return nil
}

func (k Keeper) GetRollappLatestFinalizedHeight(ctx sdk.Context, rollappID string) (uint64, error) {
latestIndex, found := k.rollappKeeper.GetLatestFinalizedStateIndex(ctx, rollappID)
if !found {
return 0, gerrc.ErrNotFound.Wrapf("latest finalized state index is not found")
}
stateInfo, found := k.rollappKeeper.GetStateInfo(ctx, rollappID, latestIndex.Index)
if !found {
return 0, gerrc.ErrNotFound.Wrapf("state info is not found")
}
return stateInfo.GetLatestHeight(), nil
}

func (k Keeper) GetPendingPacketsUntilLatestHeight(ctx sdk.Context, rollappID string) ([]commontypes.RollappPacket, uint64, error) {
// Get rollapp's latest finalized height
latestFinalizedHeight, err := k.GetRollappLatestFinalizedHeight(ctx, rollappID)
if err != nil {
return nil, 0, fmt.Errorf("get latest finalized height: rollapp '%s': %w", rollappID, err)
}

// Get all pending rollapp packets until the latest finalized height
return k.ListRollappPackets(ctx, types.PendingByRollappIDByMaxHeight(rollappID, latestFinalizedHeight)), latestFinalizedHeight, nil
}
2 changes: 1 addition & 1 deletion x/delayedack/keeper/finalize_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func (s *DelayedAckTestSuite) TestFinalizePacket() {
},
rollappHeight: 10,
expectErr: true,
errContains: "packet height is not finalized yet: height '15', latest height '10'",
errContains: "verify height",
},
}

Expand Down
35 changes: 21 additions & 14 deletions x/delayedack/keeper/fraud.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,16 @@

sdk "github.com/cosmos/cosmos-sdk/types"
porttypes "github.com/cosmos/ibc-go/v7/modules/core/05-port/types"
host "github.com/cosmos/ibc-go/v7/modules/core/24-host"

commontypes "github.com/dymensionxyz/dymension/v3/x/common/types"
)

func (k Keeper) HandleFraud(ctx sdk.Context, rollappID string, ibc porttypes.IBCModule) error {
// Get all the pending packets
rollappPendingPackets := k.ListRollappPackets(ctx, types.ByRollappIDByStatus(rollappID, commontypes.Status_PENDING))
if len(rollappPendingPackets) == 0 {
return nil
}
func (k Keeper) HandleHardFork(ctx sdk.Context, rollappID string, height uint64, ibc porttypes.IBCModule) error {
logger := ctx.Logger().With("module", "DelayedAckMiddleware")
logger.Info("reverting IBC rollapp packets", "rollappID", rollappID)

// Get all the pending packets from fork height inclusive
rollappPendingPackets := k.ListRollappPackets(ctx, types.PendingByRollappIDFromHeight(rollappID, height))
Copy link
Contributor

Choose a reason for hiding this comment

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

probably wont fix for now but I think possible DOS if a lot of packets and fraud is e.g few days ago.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

pls open research topic


// Iterate over all the pending packets and revert them
for _, rollappPacket := range rollappPendingPackets {
Expand All @@ -28,22 +26,31 @@
"sequence", rollappPacket.Packet.Sequence,
}

// refund all pending outgoing packets
if rollappPacket.Type == commontypes.RollappPacket_ON_ACK || rollappPacket.Type == commontypes.RollappPacket_ON_TIMEOUT {
// refund all pending outgoing packets
// we don't have access directly to `refundPacketToken` function, so we'll use the `OnTimeoutPacket` function
err := ibc.OnTimeoutPacket(ctx, *rollappPacket.Packet, rollappPacket.Relayer)
if err != nil {
logger.Error("failed to refund reverted packet", append(logContext, "error", err.Error())...)
}
Comment on lines +29 to 35
Copy link
Contributor

Choose a reason for hiding this comment

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

should be changed to follow the ADR:

  1. timeout - only deleting the rollapp packet
  2. on_ack - to delete packet and re-create the commitment

}
// Update status to reverted
_, err := k.UpdateRollappPacketWithStatus(ctx, rollappPacket, commontypes.Status_REVERTED)
if err != nil {
logger.Error("error reverting IBC rollapp packet", append(logContext, "error", err.Error())...)
return err
} else {
// for incoming packets, we need to reset the packet receipt
ibcPacket := rollappPacket.Packet
k.deletePacketReceipt(ctx, ibcPacket.GetDestPort(), ibcPacket.GetDestChannel(), ibcPacket.GetSequence())
}

// delete the packet
k.DeleteRollappPacket(ctx, &rollappPacket)

Check failure on line 43 in x/delayedack/keeper/fraud.go

View workflow job for this annotation

GitHub Actions / golangci-lint

Error return value of `k.DeleteRollappPacket` is not checked (errcheck)
logger.Debug("reverted IBC rollapp packet", logContext...)
}

logger.Info("reverting IBC rollapp packets", "rollappID", rollappID, "numPackets", len(rollappPendingPackets))

return nil
}

// DeleteRollappPacket deletes a packet receipt from the store
func (k Keeper) deletePacketReceipt(ctx sdk.Context, portID, channelID string, sequence uint64) {
store := ctx.KVStore(k.channelKeeperStoreKey)
store.Delete(host.PacketReceiptKey(portID, channelID, sequence))
}
63 changes: 21 additions & 42 deletions x/delayedack/keeper/fraud_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,66 +18,45 @@ func (suite *DelayedAckTestSuite) TestHandleFraud() {
)

rollappId := "testRollappId"
pkts := generatePackets(rollappId, 5)
pkts := generatePackets(rollappId, 10)
rollappId2 := "testRollappId2"
pkts2 := generatePackets(rollappId2, 5)
pkts2 := generatePackets(rollappId2, 10)
prefixPending1 := types.ByRollappIDByStatus(rollappId, commontypes.Status_PENDING)
prefixPending2 := types.ByRollappIDByStatus(rollappId2, commontypes.Status_PENDING)
prefixReverted := types.ByRollappIDByStatus(rollappId, commontypes.Status_REVERTED)
prefixFinalized := types.ByRollappIDByStatus(rollappId, commontypes.Status_FINALIZED)
prefixFinalized1 := types.ByRollappIDByStatus(rollappId, commontypes.Status_FINALIZED)
prefixFinalized2 := types.ByRollappIDByStatus(rollappId, commontypes.Status_FINALIZED)

for _, pkt := range append(pkts, pkts2...) {
keeper.SetRollappPacket(ctx, pkt)
}

suite.Require().Equal(5, len(keeper.ListRollappPackets(ctx, prefixPending1)))
suite.Require().Equal(5, len(keeper.ListRollappPackets(ctx, prefixPending2)))
suite.Require().Equal(10, len(keeper.ListRollappPackets(ctx, prefixPending1)))
suite.Require().Equal(10, len(keeper.ListRollappPackets(ctx, prefixPending2)))

// finalize some packets
// finalize one packet
_, err := keeper.UpdateRollappPacketWithStatus(ctx, pkts[0], commontypes.Status_FINALIZED)
suite.Require().Nil(err)
_, err = keeper.UpdateRollappPacketWithStatus(ctx, pkts2[0], commontypes.Status_FINALIZED)
suite.Require().Nil(err)

err = keeper.HandleFraud(ctx, rollappId, transferStack)
// call fraud on the 4 packet
err = keeper.HandleHardFork(ctx, rollappId, 4, transferStack)
suite.Require().Nil(err)

suite.Require().Equal(0, len(keeper.ListRollappPackets(ctx, prefixPending1)))
suite.Require().Equal(4, len(keeper.ListRollappPackets(ctx, prefixPending2)))
suite.Require().Equal(4, len(keeper.ListRollappPackets(ctx, prefixReverted)))
suite.Require().Equal(1, len(keeper.ListRollappPackets(ctx, prefixFinalized)))
suite.Require().Equal(1, len(keeper.ListRollappPackets(ctx, prefixFinalized2)))
}

func (suite *DelayedAckTestSuite) TestDeletionOfRevertedPackets() {
keeper, ctx := suite.App.DelayedAckKeeper, suite.Ctx
transferStack := damodule.NewIBCMiddleware(
damodule.WithIBCModule(ibctransfer.NewIBCModule(suite.App.TransferKeeper)),
damodule.WithKeeper(keeper),
damodule.WithRollappKeeper(suite.App.RollappKeeper),
)

rollappId := "testRollappId"
pkts := generatePackets(rollappId, 5)
rollappId2 := "testRollappId2"
pkts2 := generatePackets(rollappId2, 5)

for _, pkt := range append(pkts, pkts2...) {
keeper.SetRollappPacket(ctx, pkt)
}

err := keeper.HandleFraud(ctx, rollappId, transferStack)
suite.Require().Nil(err)
// expected result:
// rollappId:
// - packet 1 are finalized
// - packet 2-3 are still pending
// - packets 4-10 are deleted
// rollappId2:
// - packet 1 are finalized
// - packets 2-10 are still pending

suite.Require().Equal(10, len(keeper.GetAllRollappPackets(ctx)))
suite.Require().Equal(1, len(keeper.ListRollappPackets(ctx, prefixFinalized1)))
suite.Require().Equal(2, len(keeper.ListRollappPackets(ctx, prefixPending1)))

keeper.SetParams(ctx, types.Params{EpochIdentifier: "minute", BridgingFee: keeper.BridgingFee(ctx)})
epochHooks := keeper.GetEpochHooks()
err = epochHooks.AfterEpochEnd(ctx, "minute", 1)
suite.Require().NoError(err)

suite.Require().Equal(5, len(keeper.GetAllRollappPackets(ctx)))
suite.Require().Equal(1, len(keeper.ListRollappPackets(ctx, prefixFinalized2)))
suite.Require().Equal(9, len(keeper.ListRollappPackets(ctx, prefixPending2)))
}

// TODO: test refunds of pending packets
Expand All @@ -86,7 +65,7 @@ func (suite *DelayedAckTestSuite) TestDeletionOfRevertedPackets() {

func generatePackets(rollappId string, num uint64) []commontypes.RollappPacket {
var packets []commontypes.RollappPacket
for i := uint64(0); i < num; i++ {
for i := uint64(1); i <= num; i++ {
packets = append(packets, commontypes.RollappPacket{
RollappId: rollappId,
Packet: &channeltypes.Packet{
Expand Down
2 changes: 1 addition & 1 deletion x/delayedack/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func (q Querier) GetPendingPacketsByReceiver(goCtx context.Context, req *types.Q
ctx := sdk.UnwrapSDKContext(goCtx)

// Get all pending rollapp packets until the latest finalized height
rollappPendingPackets, _, err := q.GetPendingPacketsUntilLatestHeight(ctx, req.RollappId)
rollappPendingPackets, _, err := q.GetPendingPacketsUntilFinalizedHeight(ctx, req.RollappId)
if err != nil {
return nil, fmt.Errorf("get pending rollapp packets until the latest finalized height: rollapp '%s': %w", req.RollappId, err)
}
Expand Down
4 changes: 2 additions & 2 deletions x/delayedack/keeper/hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func (e epochHooks) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epoch
return nil
}

listFilter := types.ByStatus(commontypes.Status_FINALIZED, commontypes.Status_REVERTED).Take(int(deletePacketsBatchSize))
listFilter := types.ByStatus(commontypes.Status_FINALIZED).Take(int(deletePacketsBatchSize))
count := 0

// Get batch of rollapp packets with status != PENDING and delete them
Expand All @@ -83,7 +83,7 @@ func (e epochHooks) AfterEpochEnd(ctx sdk.Context, epochIdentifier string, epoch

for _, packet := range toDeletePackets {
err := osmoutils.ApplyFuncIfNoError(ctx, func(ctx sdk.Context) error {
return e.deleteRollappPacket(ctx, &packet)
return e.DeleteRollappPacket(ctx, &packet)
})
if err != nil {
e.Logger(ctx).Error("Failed to delete rollapp packet",
Expand Down
Loading
Loading