P2P AppError handling (#2248)

Signed-off-by: Joshua Kim <[email protected]>
Co-authored-by: Stephen Buttolph <[email protected]>

go.mod

@@ -11,7 +11,7 @@ require (
 	github.com/DataDog/zstd v1.5.2
 	github.com/Microsoft/go-winio v0.5.2
 	github.com/NYTimes/gziphandler v1.1.1
-	github.com/ava-labs/coreth v0.12.10-rc.0
+	github.com/ava-labs/coreth v0.12.9-rc.9.0.20231206202846-fc088a944e76
 	github.com/ava-labs/ledger-avalanche/go v0.0.0-20231102202641-ae2ebdaeac34
 	github.com/btcsuite/btcd/btcutil v1.1.3
 	github.com/cockroachdb/pebble v0.0.0-20230209160836-829675f94811

go.sum

@@ -66,8 +66,8 @@ github.com/allegro/bigcache v1.2.1-0.20190218064605-e24eb225f156 h1:eMwmnE/GDgah
 github.com/allegro/bigcache v1.2.1-0.20190218064605-e24eb225f156/go.mod h1:Cb/ax3seSYIx7SuZdm2G2xzfwmv3TPSk2ucNfQESPXM=
 github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY=
 github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8=
-github.com/ava-labs/coreth v0.12.10-rc.0 h1:qmuom7rtH5hc1E3lnqrMFNLFL1TMnEVa/2O8poB1YLU=
-github.com/ava-labs/coreth v0.12.10-rc.0/go.mod h1:plFm/xzvWmx1+qJ3JQSTzF8+FdaA2xu7GgY/AdaZDfk=
+github.com/ava-labs/coreth v0.12.9-rc.9.0.20231206202846-fc088a944e76 h1:0hYwcuOfndE1I61SvWv2dEa7N3QRJL3RpB9fmJy0sr0=
+github.com/ava-labs/coreth v0.12.9-rc.9.0.20231206202846-fc088a944e76/go.mod h1:dH0LOVIEm1uHKozwhDV223BOyP0ElVsYclizRJzEY4s=
 github.com/ava-labs/ledger-avalanche/go v0.0.0-20231102202641-ae2ebdaeac34 h1:mg9Uw6oZFJKytJxgxnl3uxZOs/SB8CVHg6Io4Tf99Zc=
 github.com/ava-labs/ledger-avalanche/go v0.0.0-20231102202641-ae2ebdaeac34/go.mod h1:pJxaT9bUgeRNVmNRgtCHb7sFDIRKy7CzTQVi8gGNT6g=
 github.com/aymerick/raymond v2.0.3-0.20180322193309-b565731e1464+incompatible/go.mod h1:osfaiScAUVup+UC9Nfq76eWqDhXlp+4UYaA8uhTBO6g=

message/inbound_msg_builder.go

@@ -284,6 +284,26 @@ func InboundAppRequest(
 	}
 }
 
+func InboundAppError(
+	nodeID ids.NodeID,
+	chainID ids.ID,
+	requestID uint32,
+	errorCode int32,
+	errorMessage string,
+) InboundMessage {
+	return &inboundMessage{
+		nodeID: nodeID,
+		op:     AppErrorOp,
+		message: &p2p.AppError{
+			ChainId:      chainID[:],
+			RequestId:    requestID,
+			ErrorCode:    errorCode,
+			ErrorMessage: errorMessage,
+		},
+		expiration: mockable.MaxTime,
+	}
+}
+
 func InboundAppResponse(
 	chainID ids.ID,
 	requestID uint32,

message/inbound_msg_builder_test.go

@@ -13,6 +13,7 @@ import (
 
 	"github.com/ava-labs/avalanchego/ids"
 	"github.com/ava-labs/avalanchego/proto/pb/p2p"
+	"github.com/ava-labs/avalanchego/utils/compression"
 	"github.com/ava-labs/avalanchego/utils/logging"
 	"github.com/ava-labs/avalanchego/utils/timer/mockable"
 )
@@ -396,3 +397,46 @@ func TestInboundMsgBuilder(t *testing.T) {
 		},
 	)
 }
+
+func TestAppError(t *testing.T) {
+	require := require.New(t)
+
+	mb, err := newMsgBuilder(
+		logging.NoLog{},
+		"",
+		prometheus.NewRegistry(),
+		time.Second,
+	)
+	require.NoError(err)
+
+	nodeID := ids.GenerateTestNodeID()
+	chainID := ids.GenerateTestID()
+	requestID := uint32(1)
+	errorCode := int32(2)
+	errorMessage := "hello world"
+
+	want := &p2p.Message{
+		Message: &p2p.Message_AppError{
+			AppError: &p2p.AppError{
+				ChainId:      chainID[:],
+				RequestId:    requestID,
+				ErrorCode:    errorCode,
+				ErrorMessage: errorMessage,
+			},
+		},
+	}
+
+	outMsg, err := mb.createOutbound(want, compression.TypeNone, false)
+	require.NoError(err)
+
+	got, err := mb.parseInbound(outMsg.Bytes(), nodeID, func() {})
+	require.NoError(err)
+
+	require.Equal(nodeID, got.NodeID())
+	require.Equal(AppErrorOp, got.Op())
+
+	msg, ok := got.Message().(*p2p.AppError)
+	require.True(ok)
+	require.Equal(errorCode, msg.ErrorCode)
+	require.Equal(errorMessage, msg.ErrorMessage)
+}

message/internal_msg_builder.go

@@ -52,10 +52,6 @@ var (
 	_ requestIDGetter  = (*QueryFailed)(nil)
 	_ engineTypeGetter = (*QueryFailed)(nil)
 
-	_ fmt.Stringer    = (*AppRequestFailed)(nil)
-	_ chainIDGetter   = (*AppRequestFailed)(nil)
-	_ requestIDGetter = (*AppRequestFailed)(nil)
-
 	_ fmt.Stringer        = (*CrossChainAppRequest)(nil)
 	_ sourceChainIDGetter = (*CrossChainAppRequest)(nil)
 	_ chainIDGetter       = (*CrossChainAppRequest)(nil)
@@ -365,42 +361,6 @@ func InternalQueryFailed(
 	}
 }
 
-type AppRequestFailed struct {
-	ChainID   ids.ID `json:"chain_id,omitempty"`
-	RequestID uint32 `json:"request_id,omitempty"`
-}
-
-func (m *AppRequestFailed) String() string {
-	return fmt.Sprintf(
-		"ChainID: %s RequestID: %d",
-		m.ChainID, m.RequestID,
-	)
-}
-
-func (m *AppRequestFailed) GetChainId() []byte {
-	return m.ChainID[:]
-}
-
-func (m *AppRequestFailed) GetRequestId() uint32 {
-	return m.RequestID
-}
-
-func InternalAppRequestFailed(
-	nodeID ids.NodeID,
-	chainID ids.ID,
-	requestID uint32,
-) InboundMessage {
-	return &inboundMessage{
-		nodeID: nodeID,
-		op:     AppRequestFailedOp,
-		message: &AppRequestFailed{
-			ChainID:   chainID,
-			RequestID: requestID,
-		},
-		expiration: mockable.MaxTime,
-	}
-}
-
 type CrossChainAppRequest struct {
 	SourceChainID      ids.ID `json:"source_chain_id,omitempty"`
 	DestinationChainID ids.ID `json:"destination_chain_id,omitempty"`
@@ -452,6 +412,8 @@ type CrossChainAppRequestFailed struct {
 	SourceChainID      ids.ID `json:"source_chain_id,omitempty"`
 	DestinationChainID ids.ID `json:"destination_chain_id,omitempty"`
 	RequestID          uint32 `json:"request_id,omitempty"`
+	ErrorCode          int32  `json:"error_code,omitempty"`
+	ErrorMessage       string `json:"error_message,omitempty"`
 }
 
 func (m *CrossChainAppRequestFailed) String() string {
@@ -473,19 +435,23 @@ func (m *CrossChainAppRequestFailed) GetRequestId() uint32 {
 	return m.RequestID
 }
 
-func InternalCrossChainAppRequestFailed(
+func InternalCrossChainAppError(
 	nodeID ids.NodeID,
 	sourceChainID ids.ID,
 	destinationChainID ids.ID,
 	requestID uint32,
+	errorCode int32,
+	errorMessage string,
 ) InboundMessage {
 	return &inboundMessage{
 		nodeID: nodeID,
-		op:     CrossChainAppRequestFailedOp,
+		op:     CrossChainAppErrorOp,
 		message: &CrossChainAppRequestFailed{
 			SourceChainID:      sourceChainID,
 			DestinationChainID: destinationChainID,
 			RequestID:          requestID,
+			ErrorCode:          errorCode,
+			ErrorMessage:       errorMessage,
 		},
 		expiration: mockable.MaxTime,
 	}

message/ops.go

@@ -51,12 +51,12 @@ const (
 	ChitsOp
 	// Application:
 	AppRequestOp
-	AppRequestFailedOp
+	AppErrorOp
 	AppResponseOp
 	AppGossipOp
 	// Cross chain:
 	CrossChainAppRequestOp
-	CrossChainAppRequestFailedOp
+	CrossChainAppErrorOp
 	CrossChainAppResponseOp
 	// Internal:
 	ConnectedOp
@@ -115,9 +115,9 @@ var (
 		GetAncestorsFailedOp,
 		GetFailedOp,
 		QueryFailedOp,
-		AppRequestFailedOp,
+		AppErrorOp,
 		CrossChainAppRequestOp,
-		CrossChainAppRequestFailedOp,
+		CrossChainAppErrorOp,
 		CrossChainAppResponseOp,
 		ConnectedOp,
 		ConnectedSubnetOp,
@@ -165,12 +165,12 @@ var (
 	AsynchronousOps = []Op{
 		// Application
 		AppRequestOp,
-		AppRequestFailedOp,
+		AppErrorOp,
 		AppGossipOp,
 		AppResponseOp,
 		// Cross chain
 		CrossChainAppRequestOp,
-		CrossChainAppRequestFailedOp,
+		CrossChainAppErrorOp,
 		CrossChainAppResponseOp,
 	}
 
@@ -182,8 +182,8 @@ var (
 		GetAncestorsFailedOp:            AncestorsOp,
 		GetFailedOp:                     PutOp,
 		QueryFailedOp:                   ChitsOp,
-		AppRequestFailedOp:              AppResponseOp,
-		CrossChainAppRequestFailedOp:    CrossChainAppResponseOp,
+		AppErrorOp:                      AppResponseOp,
+		CrossChainAppErrorOp:            CrossChainAppResponseOp,
 	}
 	UnrequestedOps = set.Of(
 		GetAcceptedFrontierOp,
@@ -265,17 +265,17 @@ func (op Op) String() string {
 	// Application
 	case AppRequestOp:
 		return "app_request"
-	case AppRequestFailedOp:
-		return "app_request_failed"
+	case AppErrorOp:
+		return "app_error"
 	case AppResponseOp:
 		return "app_response"
 	case AppGossipOp:
 		return "app_gossip"
 	// Cross chain
 	case CrossChainAppRequestOp:
 		return "cross_chain_app_request"
-	case CrossChainAppRequestFailedOp:
-		return "cross_chain_app_request_failed"
+	case CrossChainAppErrorOp:
+		return "cross_chain_app_error"
 	case CrossChainAppResponseOp:
 		return "cross_chain_app_response"
 		// Internal
@@ -347,6 +347,8 @@ func Unwrap(m *p2p.Message) (fmt.Stringer, error) {
 		return msg.AppRequest, nil
 	case *p2p.Message_AppResponse:
 		return msg.AppResponse, nil
+	case *p2p.Message_AppError:
+		return msg.AppError, nil
 	case *p2p.Message_AppGossip:
 		return msg.AppGossip, nil
 	default:
@@ -400,6 +402,8 @@ func ToOp(m *p2p.Message) (Op, error) {
 		return AppRequestOp, nil
 	case *p2p.Message_AppResponse:
 		return AppResponseOp, nil
+	case *p2p.Message_AppError:
+		return AppErrorOp, nil
 	case *p2p.Message_AppGossip:
 		return AppGossipOp, nil
 	default:

network/p2p/client.go

@@ -14,9 +14,8 @@ import (
 )
 
 var (
-	ErrAppRequestFailed = errors.New("app request failed")
-	ErrRequestPending   = errors.New("request pending")
-	ErrNoPeers          = errors.New("no peers")
+	ErrRequestPending = errors.New("request pending")
+	ErrNoPeers        = errors.New("no peers")
 )
 
 // AppResponseCallback is called upon receiving an AppResponse for an AppRequest

network/p2p/network.go

@@ -87,8 +87,8 @@ func (n *Network) AppResponse(ctx context.Context, nodeID ids.NodeID, requestID
 	return n.router.AppResponse(ctx, nodeID, requestID, response)
 }
 
-func (n *Network) AppRequestFailed(ctx context.Context, nodeID ids.NodeID, requestID uint32) error {
-	return n.router.AppRequestFailed(ctx, nodeID, requestID)
+func (n *Network) AppRequestFailed(ctx context.Context, nodeID ids.NodeID, requestID uint32, appErr *common.AppError) error {
+	return n.router.AppRequestFailed(ctx, nodeID, requestID, appErr)
 }
 
 func (n *Network) AppGossip(ctx context.Context, nodeID ids.NodeID, msg []byte) error {
@@ -103,8 +103,8 @@ func (n *Network) CrossChainAppResponse(ctx context.Context, chainID ids.ID, req
 	return n.router.CrossChainAppResponse(ctx, chainID, requestID, response)
 }
 
-func (n *Network) CrossChainAppRequestFailed(ctx context.Context, chainID ids.ID, requestID uint32) error {
-	return n.router.CrossChainAppRequestFailed(ctx, chainID, requestID)
+func (n *Network) CrossChainAppRequestFailed(ctx context.Context, chainID ids.ID, requestID uint32, appErr *common.AppError) error {
+	return n.router.CrossChainAppRequestFailed(ctx, chainID, requestID, appErr)
 }
 
 func (n *Network) Connected(_ context.Context, nodeID ids.NodeID, _ *version.Application) error {

network/p2p/network_test.go

@@ -25,6 +25,11 @@ import (
 	"github.com/ava-labs/avalanchego/version"
 )
 
+var errFoo = &common.AppError{
+	Code:    123,
+	Message: "foo",
+}
+
 func TestAppRequestResponse(t *testing.T) {
 	handlerID := uint64(0x0)
 	request := []byte("request")
@@ -85,7 +90,7 @@ func TestAppRequestResponse(t *testing.T) {
 				sender.SendAppRequestF = func(ctx context.Context, nodeIDs set.Set[ids.NodeID], requestID uint32, request []byte) error {
 					for range nodeIDs {
 						go func() {
-							require.NoError(t, network.AppRequestFailed(ctx, nodeID, requestID))
+							require.NoError(t, network.AppRequestFailed(ctx, nodeID, requestID, errFoo))
 						}()
 					}
 
@@ -95,7 +100,7 @@ func TestAppRequestResponse(t *testing.T) {
 				callback := func(_ context.Context, actualNodeID ids.NodeID, actualResponse []byte, err error) {
 					defer wg.Done()
 
-					require.ErrorIs(t, err, ErrAppRequestFailed)
+					require.ErrorIs(t, err, errFoo)
 					require.Equal(t, nodeID, actualNodeID)
 					require.Nil(t, actualResponse)
 				}
@@ -140,14 +145,14 @@ func TestAppRequestResponse(t *testing.T) {
 			requestFunc: func(t *testing.T, network *Network, client *Client, sender *common.SenderTest, handler *mocks.MockHandler, wg *sync.WaitGroup) {
 				sender.SendCrossChainAppRequestF = func(ctx context.Context, chainID ids.ID, requestID uint32, request []byte) {
 					go func() {
-						require.NoError(t, network.CrossChainAppRequestFailed(ctx, chainID, requestID))
+						require.NoError(t, network.CrossChainAppRequestFailed(ctx, chainID, requestID, errFoo))
 					}()
 				}
 
 				callback := func(_ context.Context, actualChainID ids.ID, actualResponse []byte, err error) {
 					defer wg.Done()
 
-					require.ErrorIs(t, err, ErrAppRequestFailed)
+					require.ErrorIs(t, err, errFoo)
 					require.Equal(t, chainID, actualChainID)
 					require.Nil(t, actualResponse)
 				}
@@ -273,7 +278,7 @@ func TestNetworkDropMessage(t *testing.T) {
 		{
 			name: "drop unrequested app request failed",
 			requestFunc: func(network *Network) error {
-				return network.AppRequestFailed(context.Background(), ids.GenerateTestNodeID(), 0)
+				return network.AppRequestFailed(context.Background(), ids.GenerateTestNodeID(), 0, errFoo)
 			},
 			err: ErrUnrequestedResponse,
 		},
@@ -287,7 +292,7 @@ func TestNetworkDropMessage(t *testing.T) {
 		{
 			name: "drop unrequested cross-chain request failed",
 			requestFunc: func(network *Network) error {
-				return network.CrossChainAppRequestFailed(context.Background(), ids.GenerateTestID(), 0)
+				return network.CrossChainAppRequestFailed(context.Background(), ids.GenerateTestID(), 0, errFoo)
 			},
 			err: ErrUnrequestedResponse,
 		},