feat(spv-642): return ArcError to prevent infinite broadcasting of in…

…valid transactions (#94)

Co-authored-by: wregulski <wojciechregulski@outlook.com>

broadcast/broadcast-client-mock/mock_client_builder.go

@@ -20,7 +20,7 @@ type builder struct {
 	factories []composite.BroadcastFactory
 }
 
-// Builder is used to prepare the mock broadcast client. It is recommended 
+// Builder is used to prepare the mock broadcast client. It is recommended
 // to use that builder for creating the mock broadcast client.
 func Builder() *builder {
 	return &builder{}

broadcast/broadcast-client-mock/mock_client_test.go

@@ -21,10 +21,10 @@ func TestMockClientSuccess(t *testing.T) {
 		expectedResult := []*broadcast.PolicyQuoteResponse{mocks.Policy1, mocks.Policy2}
 
 		// when
-		result, err := broadcaster.GetPolicyQuote(context.Background())
+		result, fail := broadcaster.GetPolicyQuote(context.Background())
 
 		// then
-		assert.NoError(t, err)
+		assert.NoError(t, fail)
 		assert.NotNil(t, result)
 		assert.Equal(t, result, expectedResult)
 	})
@@ -37,10 +37,10 @@ func TestMockClientSuccess(t *testing.T) {
 		expectedResult := []*broadcast.FeeQuote{mocks.Fee1, mocks.Fee2}
 
 		// when
-		result, err := broadcaster.GetFeeQuote(context.Background())
+		result, fail := broadcaster.GetFeeQuote(context.Background())
 
 		// then
-		assert.NoError(t, err)
+		assert.Nil(t, fail)
 		assert.NotNil(t, result)
 		assert.Equal(t, expectedResult, result)
 	})
@@ -56,7 +56,7 @@ func TestMockClientSuccess(t *testing.T) {
 		result, err := broadcaster.QueryTransaction(context.Background(), testTxId)
 
 		// then
-		assert.NoError(t, err)
+		assert.Nil(t, err)
 		assert.NotNil(t, result)
 		assert.Equal(t, result.Miner, fixtures.ProviderMain)
 		assert.Equal(t, result.TxID, testTxId)
@@ -71,10 +71,10 @@ func TestMockClientSuccess(t *testing.T) {
 			Build()
 
 		// when
-		result, err := broadcaster.SubmitTransaction(context.Background(), &broadcast.Transaction{Hex: "test-rawtx"})
+		result, fail := broadcaster.SubmitTransaction(context.Background(), &broadcast.Transaction{Hex: "test-rawtx"})
 
 		// then
-		assert.NoError(t, err)
+		assert.Nil(t, fail)
 		assert.NotNil(t, result)
 		assert.Equal(t, result.Miner, fixtures.ProviderMain)
 		assert.Equal(t, result.BlockHash, fixtures.TxBlockHash)
@@ -98,10 +98,10 @@ func TestMockClientSuccess(t *testing.T) {
 		}
 
 		// when
-		result, err := broadcaster.SubmitBatchTransactions(context.Background(), []*broadcast.Transaction{{Hex: "test-rawtx"}, {Hex: "test2-rawtx"}})
+		result, fail := broadcaster.SubmitBatchTransactions(context.Background(), []*broadcast.Transaction{{Hex: "test-rawtx"}, {Hex: "test2-rawtx"}})
 
 		// then
-		assert.NoError(t, err)
+		assert.Nil(t, fail)
 		assert.NotNil(t, result)
 		assert.Equal(t, expectedResult, result)
 	})
@@ -115,12 +115,12 @@ func TestMockClientFailure(t *testing.T) {
 			Build()
 
 		// when
-		result, err := broadcaster.GetPolicyQuote(context.Background())
+		result, fail := broadcaster.GetPolicyQuote(context.Background())
 
 		// then
-		assert.Error(t, err)
+		assert.Error(t, fail)
 		assert.Nil(t, result)
-		assert.EqualError(t, err, broadcast.ErrNoMinerResponse.Error())
+		assert.ErrorContains(t, fail, broadcast.ErrNoMinerResponse.Error())
 	})
 
 	t.Run("Should return error from GetFeeQuote method of mock Arc Client with Failure Mock Type", func(t *testing.T) {
@@ -135,7 +135,7 @@ func TestMockClientFailure(t *testing.T) {
 		// then
 		assert.Error(t, err)
 		assert.Nil(t, result)
-		assert.EqualError(t, err, broadcast.ErrNoMinerResponse.Error())
+		assert.ErrorContains(t, err, broadcast.ErrNoMinerResponse.Error())
 	})
 
 	t.Run("Should return error from QueryTransaction method of mock Arc Client with Failure Mock Type", func(t *testing.T) {
@@ -150,7 +150,7 @@ func TestMockClientFailure(t *testing.T) {
 		// then
 		assert.Error(t, err)
 		assert.Nil(t, result)
-		assert.EqualError(t, err, broadcast.ErrAllBroadcastersFailed.Error())
+		assert.ErrorContains(t, err, broadcast.ErrNoMinerResponse.Error())
 	})
 
 	t.Run("Should return error from SubmitTransaction method of mock Arc Client with Failure Mock Type", func(t *testing.T) {
@@ -165,7 +165,7 @@ func TestMockClientFailure(t *testing.T) {
 		// then
 		assert.Error(t, err)
 		assert.Nil(t, result)
-		assert.EqualError(t, err, broadcast.ErrAllBroadcastersFailed.Error())
+		assert.ErrorContains(t, err, broadcast.ErrAllBroadcastersFailed.Error())
 	})
 
 	t.Run("Should return error from SubmitBatchTransaction method of mock Arc Client with Failure Mock Type", func(t *testing.T) {
@@ -180,7 +180,7 @@ func TestMockClientFailure(t *testing.T) {
 		// then
 		assert.Error(t, err)
 		assert.Nil(t, result)
-		assert.EqualError(t, err, broadcast.ErrAllBroadcastersFailed.Error())
+		assert.ErrorContains(t, err, broadcast.ErrAllBroadcastersFailed.Error())
 	})
 }
 
@@ -201,7 +201,7 @@ func TestMockClientTimeout(t *testing.T) {
 		result, err := broadcaster.GetPolicyQuote(ctx)
 
 		// then
-		assert.NoError(t, err)
+		assert.Nil(t, err)
 		assert.NotNil(t, result)
 		assert.Greater(t, time.Since(startTime), defaultTestTime)
 		assert.Equal(t, expectedResult, result)
@@ -221,7 +221,7 @@ func TestMockClientTimeout(t *testing.T) {
 		result, err := broadcaster.GetFeeQuote(ctx)
 
 		// then
-		assert.NoError(t, err)
+		assert.Nil(t, err)
 		assert.NotNil(t, result)
 		assert.Greater(t, time.Since(startTime), defaultTestTime)
 		assert.Equal(t, expectedResult, result)
@@ -241,7 +241,7 @@ func TestMockClientTimeout(t *testing.T) {
 		result, err := broadcaster.QueryTransaction(ctx, testTxId)
 
 		// then
-		assert.NoError(t, err)
+		assert.Nil(t, err)
 		assert.NotNil(t, result)
 		assert.Greater(t, time.Since(startTime), defaultTestTime)
 		assert.Equal(t, result.Miner, fixtures.ProviderMain)
@@ -263,7 +263,7 @@ func TestMockClientTimeout(t *testing.T) {
 		result, err := broadcaster.SubmitTransaction(ctx, &broadcast.Transaction{Hex: "test-rawtx"})
 
 		// then
-		assert.NoError(t, err)
+		assert.Nil(t, err)
 		assert.NotNil(t, result)
 		assert.Greater(t, time.Since(startTime), defaultTestTime)
 		assert.Equal(t, result.Miner, fixtures.ProviderMain)
@@ -294,7 +294,7 @@ func TestMockClientTimeout(t *testing.T) {
 		result, err := broadcaster.SubmitBatchTransactions(ctx, []*broadcast.Transaction{{Hex: "test-rawtx"}, {Hex: "test2-rawtx"}})
 
 		// then
-		assert.NoError(t, err)
+		assert.Nil(t, err)
 		assert.NotNil(t, result)
 		assert.Greater(t, time.Since(startTime), defaultTestTime)
 		assert.Equal(t, expectedResult, result)

broadcast/errors.go

@@ -4,6 +4,7 @@ package broadcast
 import (
 	"errors"
 	"fmt"
+	"github.com/bitcoin-sv/go-broadcast-client/broadcast/internal/utils"
 	"strings"
 )
 
@@ -21,15 +22,9 @@ var ErrClientUndefined = errors.New("client is undefined")
 // ErrAllBroadcastersFailed is returned when all configured broadcasters failed to query or broadcast the transaction.
 var ErrAllBroadcastersFailed = errors.New("all broadcasters failed")
 
-// ErrBroadcastFailed is returned when the broadcast failed.
-var ErrBroadcasterFailed = errors.New("broadcaster failed")
-
 // ErrUnableToDecodeResponse is returned when the http response cannot be decoded.
 var ErrUnableToDecodeResponse = errors.New("unable to decode response")
 
-// ErrUnableToDecodeMerklePath is returned when merkle path from transaction response cannot be decoded.
-var ErrUnableToDecodeMerklePath = errors.New("unable to decode merkle path from response")
-
 // ErrMissingStatus is returned when the tx status is missing.
 var ErrMissingStatus = errors.New("missing tx status")
 
@@ -43,6 +38,12 @@ var ErrStrategyUnkown = errors.New("unknown strategy")
 // ErrNoMinerResponse is returned when no response is received from any miner.
 var ErrNoMinerResponse = errors.New("failed to get reponse from any miner")
 
+// ArcFailure is the interface for the error returned by the ArcClient.
+type ArcFailure interface {
+	error
+	Details() *FailureResponse
+}
+
 // ArcError is general type for the error returned by the ArcClient.
 type ArcError struct {
 	Type      string `json:"type"`
@@ -54,6 +55,11 @@ type ArcError struct {
 	ExtraInfo string `json:"extraInfo,omitempty"`
 }
 
+// Details returns the details of the error it's the implementation of the ArcFailure interface.
+func (failure *FailureResponse) Details() *FailureResponse {
+	return failure
+}
+
 // Error returns the error string it's the implementation of the error interface.
 func (err ArcError) Error() string {
 	sb := strings.Builder{}
@@ -77,3 +83,35 @@ func (err ArcError) Error() string {
 	sb.WriteString("}")
 	return sb.String()
 }
+
+// FailureResponse is the response returned by the ArcClient when the request fails.
+type FailureResponse struct {
+	Description      string
+	ArcErrorResponse *ArcError
+}
+
+// Error returns the error string it's the implementation of the error interface.
+func (failure *FailureResponse) Error() string {
+	sb := strings.Builder{}
+	sb.WriteString(failure.Description)
+
+	if failure.ArcErrorResponse != nil {
+		sb.WriteString(", ")
+		sb.WriteString(failure.ArcErrorResponse.Error())
+	}
+
+	return sb.String()
+}
+
+// Failure returns a new FailureResponse with the description and the error.
+func Failure(description string, err error) *FailureResponse {
+	var arcErr ArcError
+	if errors.As(err, &arcErr) {
+		return &FailureResponse{
+			Description:      description,
+			ArcErrorResponse: &arcErr,
+		}
+	}
+
+	return &FailureResponse{Description: utils.WithCause(errors.New(description), err).Error()}
+}

broadcast/interface.go

@@ -7,35 +7,35 @@ import (
 // FeeQuoter it the interface that wraps GetFeeQuote method.
 // It retrieves the Fee Quote from the configured miners.
 type FeeQuoter interface {
-	GetFeeQuote(ctx context.Context) ([]*FeeQuote, error)
+	GetFeeQuote(ctx context.Context) ([]*FeeQuote, ArcFailure)
 }
 
 // PolicyQuoter it the interface that wraps GetPolicyQuote method.
 // It retrieves the Policy Quote from the configured miners.
 type PolicyQuoter interface {
-	GetPolicyQuote(ctx context.Context) ([]*PolicyQuoteResponse, error)
+	GetPolicyQuote(ctx context.Context) ([]*PolicyQuoteResponse, ArcFailure)
 }
 
 // TransactionQuerier is the interface that wraps the QueryTransaction method.
 // It takes a transaction ID and returns the transaction details, like it's status, hash, height etc.
 // Everything is wrapped in the QueryTxResponse struct.
 type TransactionQuerier interface {
-	QueryTransaction(ctx context.Context, txID string) (*QueryTxResponse, error)
+	QueryTransaction(ctx context.Context, txID string) (*QueryTxResponse, ArcFailure)
 }
 
 // TransactionSubmitter is the interface that wraps the SubmitTransaction method.
 // It takes a transaction and tries to broadcast it to the P2P network.
 // Transaction object needs RawTx to be set. All other fields are optional and used to append headers related to status callbacks.
 // As a result it returns a SubmitTxResponse object.
 type TransactionSubmitter interface {
-	SubmitTransaction(ctx context.Context, tx *Transaction, opts ...TransactionOptFunc) (*SubmitTxResponse, error)
+	SubmitTransaction(ctx context.Context, tx *Transaction, opts ...TransactionOptFunc) (*SubmitTxResponse, ArcFailure)
 }
 
 // TransactionsSubmitter is the interface that wraps the SubmitBatchTransactions method.
 // It is the same as TransactionSubmitter but it takes a slice of transactions and tries to broadcast them to the P2P network.
 // As a result it returns a SubmitBatchTxResponse, which includes a slice of SubmitTxResponse objects.
 type TransactionsSubmitter interface {
-	SubmitBatchTransactions(ctx context.Context, tx []*Transaction, opts ...TransactionOptFunc) (*SubmitBatchTxResponse, error)
+	SubmitBatchTransactions(ctx context.Context, tx []*Transaction, opts ...TransactionOptFunc) (*SubmitBatchTxResponse, ArcFailure)
 }
 
 // Client is a grouping interface that represents the entire exposed functionality of the broadcast client.

broadcast/internal/acceptance_tests/arc_fee_quote_test.go

@@ -37,7 +37,7 @@ func TestFeeQuote(t *testing.T) {
 		result, err := broadcaster.GetFeeQuote(context.Background())
 
 		// then
-		assert.NoError(t, err)
+		assert.Nil(t, err)
 		assert.NotNil(t, result)
 		assert.Equal(t, 2, httpmock.GetTotalCallCount())
 		assert.Equal(t, int64(1), result[0].MiningFee.Satoshis)
@@ -66,7 +66,7 @@ func TestFeeQuote(t *testing.T) {
 		assert.Error(t, err)
 		assert.Nil(t, result)
 		assert.Equal(t, 2, httpmock.GetTotalCallCount())
-		assert.EqualError(t, err, broadcast.ErrNoMinerResponse.Error())
+		assert.Contains(t, err.Error(), broadcast.ErrNoMinerResponse.Error())
 	})
 
 	t.Run("Should successfully query from single ArcClient", func(t *testing.T) {
@@ -82,7 +82,7 @@ func TestFeeQuote(t *testing.T) {
 		result, err := broadcaster.GetFeeQuote(context.Background())
 
 		// then
-		assert.NoError(t, err)
+		assert.Nil(t, err)
 		assert.NotNil(t, result)
 		assert.Equal(t, 1, httpmock.GetTotalCallCount())
 		assert.Equal(t, int64(1), result[0].MiningFee.Satoshis)

broadcast/internal/acceptance_tests/arc_policy_quote_test.go

@@ -78,7 +78,7 @@ func TestPolicyQuote(t *testing.T) {
 		result, err := broadcaster.GetPolicyQuote(context.Background())
 
 		// then
-		assert.NoError(t, err)
+		assert.Nil(t, err)
 		assert.NotNil(t, result)
 	})
 
@@ -102,7 +102,7 @@ func TestPolicyQuote(t *testing.T) {
 		// then
 		assert.Error(t, err)
 		assert.Nil(t, result)
-		assert.EqualError(t, err, broadcast.ErrNoMinerResponse.Error())
+		assert.Contains(t, err.Error(), broadcast.ErrNoMinerResponse.Error())
 	})
 
 	t.Run("Should successfully query from single ArcClient", func(t *testing.T) {
@@ -125,7 +125,7 @@ func TestPolicyQuote(t *testing.T) {
 		result, err := broadcaster.GetPolicyQuote(context.Background())
 
 		// then
-		assert.NoError(t, err)
+		assert.Nil(t, err)
 		assert.NotNil(t, result)
 	})
 

broadcast/internal/acceptance_tests/arc_query_tx_test.go

@@ -54,7 +54,7 @@ func TestQueryTransaction(t *testing.T) {
 		result, err := broadcaster.QueryTransaction(context.Background(), mockTxID)
 
 		// then
-		assert.NoError(t, err)
+		assert.Nil(t, err)
 		assert.NotNil(t, result)
 		assert.Equal(t, 1, httpmock.GetTotalCallCount())
 		assert.Equal(t, "MINED", string(result.TxStatus))
@@ -77,7 +77,7 @@ func TestQueryTransaction(t *testing.T) {
 		result, err := broadcaster.QueryTransaction(context.Background(), mockTxID)
 
 		// then
-		assert.NoError(t, err)
+		assert.Nil(t, err)
 		assert.NotNil(t, result)
 		assert.Equal(t, 1, httpmock.GetTotalCallCount())
 		assert.Equal(t, "MINED", string(result.TxStatus))
@@ -100,7 +100,7 @@ func TestQueryTransaction(t *testing.T) {
 		result, err := broadcaster.QueryTransaction(context.Background(), mockTxID)
 
 		// then
-		assert.NoError(t, err)
+		assert.Nil(t, err)
 		assert.NotNil(t, result)
 		assert.Equal(t, 2, httpmock.GetTotalCallCount())
 		assert.Equal(t, "CONFIRMED", string(result.TxStatus))
@@ -125,7 +125,7 @@ func TestQueryTransaction(t *testing.T) {
 		// then
 		assert.Error(t, err)
 		assert.Nil(t, result)
-		assert.EqualError(t, err, broadcast.ErrAllBroadcastersFailed.Error())
+		assert.Contains(t, err.Error(), broadcast.ErrAllBroadcastersFailed.Error())
 		assert.Equal(t, 2, httpmock.GetTotalCallCount())
 	})
 
@@ -142,7 +142,7 @@ func TestQueryTransaction(t *testing.T) {
 		result, err := broadcaster.QueryTransaction(context.Background(), mockTxID)
 
 		// then
-		assert.NoError(t, err)
+		assert.Nil(t, err)
 		assert.NotNil(t, result)
 	})