feat(spv-642): return ArcError to prevent infinite broadcasting of invalid transactions

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.Nil(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

@@ -5,6 +5,8 @@ import (
 	"errors"
 	"fmt"
 	"strings"
+
+	"github.com/bitcoin-sv/go-broadcast-client/broadcast/internal/utils"
 )
 
 // ErrClientUndefined is returned when the client is undefined.
@@ -77,3 +79,31 @@ func (err ArcError) Error() string {
 	sb.WriteString("}")
 	return sb.String()
 }
+
+type FailureResponse struct {
+	Description      string
+	ArcErrorResponse *ArcError
+}
+
+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()
+}
+
+func Failure(description string, err error) *FailureResponse {
+	if arcErr, ok := err.(ArcError); ok {
+		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, *FailureResponse)
 }
 
 // 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, *FailureResponse)
 }
 
 // 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, *FailureResponse)
 }
 
 // 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, *FailureResponse)
 }
 
 // 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, *FailureResponse)
 }
 
 // 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)
 	})