refactor(axelarnet): archive failed transfer (#2200)

axelarnetwork · Oct 30, 2024 · 31dd11d · 31dd11d
1 parent fe8587a
commit 31dd11d
Show file tree

Hide file tree

Showing 12 changed files with 191 additions and 106 deletions.
diff --git a/client/docs/static/openapi/index.html b/client/docs/static/openapi/index.html
diff --git a/client/docs/static/openapi/openapi.yaml b/client/docs/static/openapi/openapi.yaml
@@ -15268,6 +15268,7 @@ paths:
                             - TRANSFER_STATE_PENDING
                             - TRANSFER_STATE_ARCHIVED
                             - TRANSFER_STATE_INSUFFICIENT_AMOUNT
+                            - TRANSFER_STATE_FAILED
                           default: TRANSFER_STATE_UNSPECIFIED
                       title: >-
                         CrossChainTransfer represents a generalized transfer of
@@ -15487,6 +15488,7 @@ paths:
               - TRANSFER_STATE_PENDING
               - TRANSFER_STATE_ARCHIVED
               - TRANSFER_STATE_INSUFFICIENT_AMOUNT
+              - TRANSFER_STATE_FAILED
         - name: pagination.key
           description: |-
             key is a value returned in PageResponse.next_key to begin
@@ -51544,6 +51546,7 @@ components:
             - TRANSFER_STATE_PENDING
             - TRANSFER_STATE_ARCHIVED
             - TRANSFER_STATE_INSUFFICIENT_AMOUNT
+            - TRANSFER_STATE_FAILED
           default: TRANSFER_STATE_UNSPECIFIED
       title: |-
         CrossChainTransfer represents a generalized transfer of some asset to a
@@ -51667,6 +51670,7 @@ components:
         - TRANSFER_STATE_PENDING
         - TRANSFER_STATE_ARCHIVED
         - TRANSFER_STATE_INSUFFICIENT_AMOUNT
+        - TRANSFER_STATE_FAILED
       default: TRANSFER_STATE_UNSPECIFIED
     axelar.nexus.v1beta1.ActivateChainRequest:
       type: object
@@ -52373,6 +52377,7 @@ components:
                   - TRANSFER_STATE_PENDING
                   - TRANSFER_STATE_ARCHIVED
                   - TRANSFER_STATE_INSUFFICIENT_AMOUNT
+                  - TRANSFER_STATE_FAILED
                 default: TRANSFER_STATE_UNSPECIFIED
             title: >-
               CrossChainTransfer represents a generalized transfer of some asset

diff --git a/client/docs/static/swagger/swagger.yaml b/client/docs/static/swagger/swagger.yaml
@@ -16781,6 +16781,7 @@ paths:
                         - TRANSFER_STATE_PENDING
                         - TRANSFER_STATE_ARCHIVED
                         - TRANSFER_STATE_INSUFFICIENT_AMOUNT
+                        - TRANSFER_STATE_FAILED
                       default: TRANSFER_STATE_UNSPECIFIED
                   title: >-
                     CrossChainTransfer represents a generalized transfer of some
@@ -17020,6 +17021,7 @@ paths:
             - TRANSFER_STATE_PENDING
             - TRANSFER_STATE_ARCHIVED
             - TRANSFER_STATE_INSUFFICIENT_AMOUNT
+            - TRANSFER_STATE_FAILED
         - name: pagination.key
           description: |-
             key is a value returned in PageResponse.next_key to begin
@@ -55964,6 +55966,7 @@ definitions:
           - TRANSFER_STATE_PENDING
           - TRANSFER_STATE_ARCHIVED
           - TRANSFER_STATE_INSUFFICIENT_AMOUNT
+          - TRANSFER_STATE_FAILED
         default: TRANSFER_STATE_UNSPECIFIED
     title: |-
       CrossChainTransfer represents a generalized transfer of some asset to a
@@ -56089,6 +56092,7 @@ definitions:
       - TRANSFER_STATE_PENDING
       - TRANSFER_STATE_ARCHIVED
       - TRANSFER_STATE_INSUFFICIENT_AMOUNT
+      - TRANSFER_STATE_FAILED
     default: TRANSFER_STATE_UNSPECIFIED
   axelar.nexus.v1beta1.ActivateChainRequest:
     type: object
@@ -56798,6 +56802,7 @@ definitions:
                 - TRANSFER_STATE_PENDING
                 - TRANSFER_STATE_ARCHIVED
                 - TRANSFER_STATE_INSUFFICIENT_AMOUNT
+                - TRANSFER_STATE_FAILED
               default: TRANSFER_STATE_UNSPECIFIED
           title: >-
             CrossChainTransfer represents a generalized transfer of some asset

diff --git a/client/docs/statik/statik.go b/client/docs/statik/statik.go
diff --git a/docs/proto/proto-docs.md b/docs/proto/proto-docs.md
diff --git a/proto/axelar/nexus/exported/v1beta1/types.proto b/proto/axelar/nexus/exported/v1beta1/types.proto
@@ -44,6 +44,7 @@ enum TransferState {
   TRANSFER_STATE_ARCHIVED = 2 [ (gogoproto.enumvalue_customname) = "Archived" ];
   TRANSFER_STATE_INSUFFICIENT_AMOUNT = 3
       [ (gogoproto.enumvalue_customname) = "InsufficientAmount" ];
+  TRANSFER_STATE_FAILED = 4 [ (gogoproto.enumvalue_customname) = "TransferFailed" ];
 }
 
 // TransferFee represents accumulated fees generated by the network

diff --git a/x/axelarnet/keeper/msg_server.go b/x/axelarnet/keeper/msg_server.go
@@ -255,27 +255,34 @@ func (s msgServer) ExecutePendingTransfers(c context.Context, _ *types.ExecutePe
 	}
 
 	for _, pendingTransfer := range pendingTransfers {
-		recipient, err := sdk.AccAddressFromBech32(pendingTransfer.Recipient.Address)
-		if err != nil {
-			s.Logger(ctx).Error(fmt.Sprintf("discard invalid recipient %s and continue", pendingTransfer.Recipient.Address))
-			s.nexus.ArchivePendingTransfer(ctx, pendingTransfer)
-			continue
-		}
+		success := utils.RunCached(ctx, s, func(ctx sdk.Context) (bool, error) {
+			recipient, err := sdk.AccAddressFromBech32(pendingTransfer.Recipient.Address)
+			if err != nil {
+				// NOTICE: Addresses that previously failed validation were marked as Archived. Starting in v1.1, they are now marked as TransferFailed.
+				s.Logger(ctx).Error(fmt.Sprintf("transfer failed due to invalid recipient %s", pendingTransfer.Recipient.Address))
+				return false, err
+			}
 
-		if err = transfer(ctx, s.Keeper, s.nexus, s.bank, s.ibcK, recipient, pendingTransfer.Asset); err != nil {
-			s.Logger(ctx).Error("failed to transfer asset to axelarnet", "err", err)
-			continue
-		}
+			if err = transfer(ctx, s.Keeper, s.nexus, s.bank, s.ibcK, recipient, pendingTransfer.Asset); err != nil {
+				s.Logger(ctx).Error("failed to transfer asset to axelarnet", "err", err)
+				return false, err
+			}
 
-		events.Emit(ctx,
-			&types.AxelarTransferCompleted{
-				ID:         pendingTransfer.ID,
-				Receipient: pendingTransfer.Recipient.Address,
-				Asset:      pendingTransfer.Asset,
-				Recipient:  pendingTransfer.Recipient.Address,
-			})
+			s.nexus.ArchivePendingTransfer(ctx, pendingTransfer)
+			events.Emit(ctx,
+				&types.AxelarTransferCompleted{
+					ID:         pendingTransfer.ID,
+					Receipient: pendingTransfer.Recipient.Address,
+					Asset:      pendingTransfer.Asset,
+					Recipient:  pendingTransfer.Recipient.Address,
+				})
 
-		s.nexus.ArchivePendingTransfer(ctx, pendingTransfer)
+			return true, nil
+		})
+
+		if !success {
+			s.nexus.MarkTransferAsFailed(ctx, pendingTransfer)
+		}
 	}
 
 	// release transfer fees

diff --git a/x/axelarnet/keeper/msg_server_test.go b/x/axelarnet/keeper/msg_server_test.go
@@ -298,7 +298,8 @@ func TestHandleMsgExecutePendingTransfers(t *testing.T) {
 			GetTransferFeesFunc: func(sdk.Context) sdk.Coins {
 				return sdk.Coins{}
 			},
-			SubTransferFeeFunc: func(sdk.Context, sdk.Coin) {},
+			SubTransferFeeFunc:       func(sdk.Context, sdk.Coin) {},
+			MarkTransferAsFailedFunc: func(sdk.Context, nexus.CrossChainTransfer) {},
 		}
 		bankK = &mock.BankKeeperMock{}
 		transferK = &mock.IBCTransferKeeperMock{}
@@ -357,11 +358,13 @@ func TestHandleMsgExecutePendingTransfers(t *testing.T) {
 						}
 					}).
 					When2(requestIsMade).
-					Then("should not archive the transfer", func(t *testing.T) {
+					Then("should mark the transfer as failed", func(t *testing.T) {
 						_, err := server.ExecutePendingTransfers(sdk.WrapSDKContext(ctx), req)
 						assert.NoError(t, err)
 
 						assert.Len(t, nexusK.ArchivePendingTransferCalls(), 0)
+						assert.Len(t, nexusK.MarkTransferAsFailedCalls(), 1)
+						assert.Len(t, ctx.EventManager().Events(), 0)
 					}),
 
 				whenHasPendingTransfers.

diff --git a/x/axelarnet/types/expected_keepers.go b/x/axelarnet/types/expected_keepers.go
@@ -79,6 +79,7 @@ type Nexus interface {
 	EnqueueTransfer(ctx sdk.Context, senderChain nexus.Chain, recipient nexus.CrossChainAddress, asset sdk.Coin) (nexus.TransferID, error)
 	GetTransfersForChainPaginated(ctx sdk.Context, chain nexus.Chain, state nexus.TransferState, pageRequest *query.PageRequest) ([]nexus.CrossChainTransfer, *query.PageResponse, error)
 	ArchivePendingTransfer(ctx sdk.Context, transfer nexus.CrossChainTransfer)
+	MarkTransferAsFailed(ctx sdk.Context, transfer nexus.CrossChainTransfer)
 	RegisterAsset(ctx sdk.Context, chain nexus.Chain, asset nexus.Asset, limit sdk.Uint, window time.Duration) error
 	GetRecipient(ctx sdk.Context, sender nexus.CrossChainAddress) (nexus.CrossChainAddress, bool)
 	SetChain(ctx sdk.Context, chain nexus.Chain)

diff --git a/x/axelarnet/types/mock/expected_keepers.go b/x/axelarnet/types/mock/expected_keepers.go