Merge pull request #50 from babylonchain/hot-fix-proper-error-codes

Hot fix proper error codes - merge back to dev

itest/e2e_test.go

@@ -380,6 +380,47 @@ func TestSigningUnbondingTx(t *testing.T) {
 	require.NoError(t, err)
 }
 
+func TestProperResponseForInvalidRequest(t *testing.T) {
+	tm := StartManager(t, 100)
+
+	stakingData := defaultStakingData()
+
+	stakingTxInfo := tm.sendStakingTxToBtc(stakingData)
+
+	unb := tm.createUnbondingTx(stakingTxInfo, stakingData)
+
+	// staker signs unbonding tx
+	unbondingPathInfo, err := stakingTxInfo.stakingInfo.UnbondingPathSpendInfo()
+	require.NoError(t, err)
+
+	randomKey, err := btcec.NewPrivateKey()
+	require.NoError(t, err)
+
+	// We will send invalid signature in request, server should respond with
+	// bad request
+	badSig, err := btcstaking.SignTxWithOneScriptSpendInputFromTapLeaf(
+		unb.unbondingTx,
+		stakingTxInfo.stakingOutput,
+		randomKey,
+		unbondingPathInfo.RevealedLeaf,
+	)
+	require.NoError(t, err)
+
+	sig, err := signerservice.RequestCovenantSignaure(
+		context.Background(),
+		tm.SigningServerUrl(),
+		10*time.Second,
+		unb.unbondingTx,
+		badSig,
+		tm.localCovenantPubKey,
+		stakingTxInfo.stakingOutput.PkScript,
+	)
+
+	require.Error(t, err)
+	require.Nil(t, sig)
+	require.EqualError(t, err, "signing request failed. status code: 400, message: {\"errorCode\":\"BAD_REQUEST\",\"message\":\"staker unbonding signature verification failed: signature is not valid: invalid signing request\"}")
+}
+
 func TestRejectToLargeRequest(t *testing.T) {
 	tm := StartManager(t, 100)
 	r := rand.New(rand.NewSource(time.Now().UnixNano()))

signerapp/signer.go

@@ -15,6 +15,14 @@ import (
 	"github.com/btcsuite/btcd/wire"
 )
 
+var (
+	ErrInvalidSigningRequest = fmt.Errorf("invalid signing request")
+)
+
+func wrapInvalidSigningRequestError(err error) error {
+	return fmt.Errorf("%s: %w", err, ErrInvalidSigningRequest)
+}
+
 type SignerApp struct {
 	s   ExternalBtcSigner
 	r   BtcChainInfo
@@ -79,17 +87,17 @@ func (s *SignerApp) SignUnbondingTransaction(
 	covnentSignerPubKey *btcec.PublicKey,
 ) (*schnorr.Signature, error) {
 	if err := btcstaking.IsSimpleTransfer(unbondingTx); err != nil {
-		return nil, err
+		return nil, wrapInvalidSigningRequestError(err)
 	}
 
 	script, err := txscript.ParsePkScript(stakingOutputPkScript)
 
 	if err != nil {
-		return nil, err
+		return nil, wrapInvalidSigningRequestError(err)
 	}
 
 	if script.Class() != txscript.WitnessV1TaprootTy {
-		return nil, fmt.Errorf("invalid staking output pk script")
+		return nil, wrapInvalidSigningRequestError(fmt.Errorf("invalid staking output pk script"))
 	}
 
 	stakingTxHash := unbondingTx.TxIn[0].PreviousOutPoint.Hash
@@ -114,10 +122,10 @@ func (s *SignerApp) SignUnbondingTransaction(
 	}
 
 	if !isCovenantMember(covnentSignerPubKey, params.CovenantPublicKeys) {
-		return nil, fmt.Errorf("received covenant public key %s is not committee member at height %d",
+		return nil, wrapInvalidSigningRequestError(fmt.Errorf("received covenant public key %s is not committee member at height %d",
 			hex.EncodeToString(covnentSignerPubKey.SerializeCompressed()),
 			stakingTxInfo.TxInclusionHeight,
-		)
+		))
 	}
 
 	// We are using signed numbers here as calls to:
@@ -128,11 +136,11 @@ func (s *SignerApp) SignUnbondingTransaction(
 	numberOfStakingTxConfirmations := (int64(bestBlock) - int64(stakingTxInfo.TxInclusionHeight)) + 1
 
 	if numberOfStakingTxConfirmations < int64(params.ConfirmationDepth) {
-		return nil, fmt.Errorf(
+		return nil, wrapInvalidSigningRequestError(fmt.Errorf(
 			"staking tx does not have enough confirmations. Current confirmations: %d, required confirmations: %d",
 			numberOfStakingTxConfirmations,
 			params.ConfirmationDepth,
-		)
+		))
 	}
 
 	parsedStakingTransaction, err := btcstaking.ParseV0StakingTx(
@@ -143,31 +151,39 @@ func (s *SignerApp) SignUnbondingTransaction(
 		s.net)
 
 	if err != nil {
-		return nil, err
+		return nil, wrapInvalidSigningRequestError(err)
 	}
 
 	stakingOutputIndexFromUnbondingTx := unbondingTx.TxIn[0].PreviousOutPoint.Index
 
 	if stakingOutputIndexFromUnbondingTx != uint32(parsedStakingTransaction.StakingOutputIdx) {
-		return nil, fmt.Errorf("unbonding transaction has invalid input index")
-	}
-
-	expectedUnbondingOutputValue := parsedStakingTransaction.StakingOutput.Value - int64(params.UnbondingFee)
-
-	if expectedUnbondingOutputValue <= 0 {
-		// This is actually eror of our parameters configuaration and should not happen
-		// for honest requests.
-		return nil, fmt.Errorf("staking output value is too low")
+		return nil, wrapInvalidSigningRequestError(fmt.Errorf("unbonding transaction has invalid input index"))
 	}
 
 	if parsedStakingTransaction.OpReturnData.StakingTime < params.MinStakingTime ||
 		parsedStakingTransaction.OpReturnData.StakingTime > params.MaxStakingTime {
-		return nil, fmt.Errorf("staking time of staking tx with hash: %s is out of bounds", stakingTxHash.String())
+		return nil, wrapInvalidSigningRequestError(
+			fmt.Errorf(
+				"staking time of staking tx with hash: %s is out of bounds",
+				stakingTxHash.String(),
+			),
+		)
 	}
 
 	if parsedStakingTransaction.StakingOutput.Value < int64(params.MinStakingAmount) ||
 		parsedStakingTransaction.StakingOutput.Value > int64(params.MaxStakingAmount) {
-		return nil, fmt.Errorf("staking amount of staking tx with hash: %s is out of bounds", stakingTxHash.String())
+		return nil, wrapInvalidSigningRequestError(fmt.Errorf(
+			"staking amount of staking tx with hash: %s is out of bounds",
+			stakingTxHash.String(),
+		))
+	}
+
+	expectedUnbondingOutputValue := parsedStakingTransaction.StakingOutput.Value - int64(params.UnbondingFee)
+
+	if expectedUnbondingOutputValue <= 0 {
+		// This is actually eror of our parameters configuaration and should not happen
+		// for honest requests.
+		return nil, fmt.Errorf("staking output value is too low")
 	}
 
 	// build expected output in unbonding transaction
@@ -186,7 +202,9 @@ func (s *SignerApp) SignUnbondingTransaction(
 	}
 
 	if !outputsAreEqual(unbondingInfo.UnbondingOutput, unbondingTx.TxOut[0]) {
-		return nil, fmt.Errorf("unbonding output does not match expected output")
+		return nil, wrapInvalidSigningRequestError(
+			fmt.Errorf("unbonding output does not match expected output"),
+		)
 	}
 
 	// At this point we know that:
@@ -225,7 +243,12 @@ func (s *SignerApp) SignUnbondingTransaction(
 	)
 
 	if err != nil {
-		return nil, fmt.Errorf("staker unbonding signature verification failed: %w", err)
+		return nil, wrapInvalidSigningRequestError(
+			fmt.Errorf(
+				"staker unbonding signature verification failed: %w",
+				err,
+			),
+		)
 	}
 
 	covenantKeyAddress, err := s.pubKeyToAddress(covnentSignerPubKey)

signerapp/signer_test.go

@@ -2,8 +2,7 @@ package signerapp_test
 
 import (
 	"context"
-	"encoding/hex"
-	"fmt"
+	"errors"
 	"testing"
 
 	"github.com/babylonchain/babylon/btcstaking"
@@ -224,11 +223,5 @@ func TestErrRequestNotCovenantMember(t *testing.T) {
 
 	require.Error(t, err)
 	require.Nil(t, receivedSignature)
-
-	expectedError := fmt.Errorf("received covenant public key %s is not committee member at height %d",
-		hex.EncodeToString(unknownCovenantMember.PubKey().SerializeCompressed()),
-		200,
-	)
-
-	require.Equal(t, expectedError, err)
+	require.True(t, errors.Is(err, signerapp.ErrInvalidSigningRequest))
 }

signerservice/client.go

@@ -89,7 +89,7 @@ func RequestCovenantSignaure(
 	}
 
 	if res.StatusCode != http.StatusOK {
-		return nil, fmt.Errorf("signigning request failed. status code: %d, message: %s", res.StatusCode, string(resBody))
+		return nil, fmt.Errorf("signing request failed. status code: %d, message: %s", res.StatusCode, string(resBody))
 	}
 
 	var response handlers.PublicResponse[types.SignUnbondingTxResponse]

signerservice/handlers/sign_unbonding.go

@@ -3,8 +3,10 @@ package handlers
 import (
 	"encoding/hex"
 	"encoding/json"
+	"errors"
 	"net/http"
 
+	"github.com/babylonchain/covenant-signer/signerapp"
 	"github.com/babylonchain/covenant-signer/signerservice/types"
 	"github.com/babylonchain/covenant-signer/utils"
 	"github.com/btcsuite/btcd/btcec/v2"
@@ -70,7 +72,12 @@ func (h *Handler) SignUnbonding(request *http.Request) (*Result, *types.Error) {
 
 	if err != nil {
 		h.m.IncFailedSigningRequests()
-		// TODO Properly translate errors between layers
+
+		if errors.Is(err, signerapp.ErrInvalidSigningRequest) {
+			return nil, types.NewErrorWithMsg(http.StatusBadRequest, types.BadRequest, err.Error())
+		}
+
+		// if this is unknown error, return internal server error
 		return nil, types.NewErrorWithMsg(http.StatusInternalServerError, types.InternalServiceError, err.Error())
 	}