From c3bdb0d4c760be6b690c84234d6afea104ac4fb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20B=C3=B6ckli?= Date: Thu, 14 Nov 2024 13:45:38 +0100 Subject: [PATCH 1/3] feat(ARCO-212): Move tx finder out of internal package --- cmd/arc/services/api.go | 31 +++++- examples/custom/main.go | 2 +- examples/simple/main.go | 5 +- pkg/api/handler/default.go | 20 +--- pkg/api/handler/default_test.go | 94 +++++++++++++------ .../tx_finder/cached_tx_finder.go | 27 ++++-- .../tx_finder/cached_tx_finder_test.go | 39 ++++---- .../internal => }/tx_finder/tx_finder.go | 0 .../internal => }/tx_finder/tx_finder_test.go | 21 +---- 9 files changed, 142 insertions(+), 97 deletions(-) rename pkg/api/{handler/internal => }/tx_finder/cached_tx_finder.go (69%) rename pkg/api/{handler/internal => }/tx_finder/cached_tx_finder_test.go (58%) rename pkg/api/{handler/internal => }/tx_finder/tx_finder.go (100%) rename pkg/api/{handler/internal => }/tx_finder/tx_finder_test.go (90%) diff --git a/cmd/arc/services/api.go b/cmd/arc/services/api.go index 49770e9b7..4712a9601 100644 --- a/cmd/arc/services/api.go +++ b/cmd/arc/services/api.go @@ -24,9 +24,12 @@ import ( "github.com/bitcoin-sv/arc/internal/message_queue/nats/client/nats_jetstream" "github.com/bitcoin-sv/arc/internal/message_queue/nats/nats_connection" "github.com/bitcoin-sv/arc/internal/metamorph/metamorph_api" + "github.com/bitcoin-sv/arc/internal/node_client" "github.com/bitcoin-sv/arc/internal/tracing" + "github.com/bitcoin-sv/arc/internal/woc_client" "github.com/bitcoin-sv/arc/pkg/api" "github.com/bitcoin-sv/arc/pkg/api/handler" + txfinder "github.com/bitcoin-sv/arc/pkg/api/tx_finder" "github.com/bitcoin-sv/arc/pkg/blocktx" "github.com/bitcoin-sv/arc/pkg/metamorph" ) @@ -50,11 +53,12 @@ func StartAPIServer(logger *slog.Logger, arcConfig *config.ArcConfig) (func(), e metamorph.WithMqClient(mqClient), metamorph.WithLogger(logger), } - // TODO: WithSecurityConfig(appConfig.Security) apiOpts := []handler.Option{ handler.WithCallbackURLRestrictions(arcConfig.Metamorph.RejectCallbackContaining), } + var finderOpts []func(f *txfinder.CachedFinder) + var nodeClientOpts []func(client *node_client.NodeClient) shutdownFns := make([]func(), 0) @@ -68,6 +72,8 @@ func StartAPIServer(logger *slog.Logger, arcConfig *config.ArcConfig) (func(), e mtmOpts = append(mtmOpts, metamorph.WithTracer(arcConfig.Tracing.KeyValueAttributes...)) apiOpts = append(apiOpts, handler.WithTracer(arcConfig.Tracing.KeyValueAttributes...)) + finderOpts = append(finderOpts, txfinder.WithTracerCachedFinder(arcConfig.Tracing.KeyValueAttributes...)) + nodeClientOpts = append(nodeClientOpts, node_client.WithTracer(arcConfig.Tracing.KeyValueAttributes...)) } conn, err := metamorph.DialGRPC(arcConfig.Metamorph.DialAddr, arcConfig.PrometheusEndpoint, arcConfig.GrpcMessageSize, arcConfig.Tracing) @@ -92,7 +98,28 @@ func StartAPIServer(logger *slog.Logger, arcConfig *config.ArcConfig) (func(), e policy = arcConfig.API.DefaultPolicy } - apiHandler, err := handler.NewDefault(logger, metamorphClient, blockTxClient, policy, arcConfig.PeerRPC, arcConfig.API, apiOpts...) + wocClient := woc_client.New(arcConfig.API.WocMainnet, woc_client.WithAuth(arcConfig.API.WocAPIKey)) + + pc := arcConfig.PeerRPC + rpcURL, err := url.Parse(fmt.Sprintf("rpc://%s:%s@%s:%d", pc.User, pc.Password, pc.Host, pc.Port)) + if err != nil { + return nil, fmt.Errorf("failed to parse node rpc url: %w", err) + } + + // get the transaction from the bitcoin node rpc + bitcoinClient, err := bitcoin.NewFromURL(rpcURL, false) + if err != nil { + return nil, fmt.Errorf("failed to create node client: %w", err) + } + + nodeClient, err := node_client.New(bitcoinClient, nodeClientOpts...) + if err != nil { + return nil, fmt.Errorf("failed to create node client: %v", err) + } + + finder := txfinder.NewCached(metamorphClient, nodeClient, wocClient, logger, finderOpts...) + + apiHandler, err := handler.NewDefault(logger, metamorphClient, blockTxClient, policy, finder, apiOpts...) if err != nil { return nil, err } diff --git a/examples/custom/main.go b/examples/custom/main.go index 675ead390..cb239cb0d 100644 --- a/examples/custom/main.go +++ b/examples/custom/main.go @@ -95,7 +95,7 @@ func main() { // initialise the arc default api handler, with our txHandler and any handler options var handler api.ServerInterface - if handler, err = apiHandler.NewDefault(logger, metamorphClient, blockTxClient, arcConfig.API.DefaultPolicy, arcConfig.PeerRPC, arcConfig.API); err != nil { + if handler, err = apiHandler.NewDefault(logger, metamorphClient, blockTxClient, arcConfig.API.DefaultPolicy, nil); err != nil { panic(err) } diff --git a/examples/simple/main.go b/examples/simple/main.go index 483bc77ab..8adcf3571 100644 --- a/examples/simple/main.go +++ b/examples/simple/main.go @@ -5,12 +5,13 @@ import ( "log/slog" "os" + "github.com/labstack/echo/v4" + "github.com/bitcoin-sv/arc/config" "github.com/bitcoin-sv/arc/pkg/api" apiHandler "github.com/bitcoin-sv/arc/pkg/api/handler" merklerootsverifier "github.com/bitcoin-sv/arc/pkg/api/merkle_roots_verifier" "github.com/bitcoin-sv/arc/pkg/api/transaction_handler" - "github.com/labstack/echo/v4" ) func main() { @@ -36,7 +37,7 @@ func main() { // initialise the arc default api handler, with our txHandler and any handler options var handler api.ServerInterface - if handler, err = apiHandler.NewDefault(logger, txHandler, merkleRootsVerifier, arcConfig.API.DefaultPolicy, arcConfig.PeerRPC, arcConfig.API); err != nil { + if handler, err = apiHandler.NewDefault(logger, txHandler, merkleRootsVerifier, arcConfig.API.DefaultPolicy, nil); err != nil { panic(err) } diff --git a/pkg/api/handler/default.go b/pkg/api/handler/default.go index 8115c8220..f58b9dd1d 100644 --- a/pkg/api/handler/default.go +++ b/pkg/api/handler/default.go @@ -16,7 +16,6 @@ import ( "github.com/ordishs/go-bitcoin" "go.opentelemetry.io/otel/attribute" - "github.com/bitcoin-sv/arc/config" "github.com/bitcoin-sv/arc/internal/beef" "github.com/bitcoin-sv/arc/internal/metamorph/metamorph_api" "github.com/bitcoin-sv/arc/internal/tracing" @@ -24,10 +23,8 @@ import ( beefValidator "github.com/bitcoin-sv/arc/internal/validator/beef" defaultValidator "github.com/bitcoin-sv/arc/internal/validator/default" "github.com/bitcoin-sv/arc/internal/version" - "github.com/bitcoin-sv/arc/internal/woc_client" "github.com/bitcoin-sv/arc/pkg/api" "github.com/bitcoin-sv/arc/pkg/api/handler/internal/merkle_verifier" - txfinder "github.com/bitcoin-sv/arc/pkg/api/handler/internal/tx_finder" "github.com/bitcoin-sv/arc/pkg/blocktx" "github.com/bitcoin-sv/arc/pkg/metamorph" ) @@ -90,17 +87,9 @@ func NewDefault( transactionHandler metamorph.TransactionHandler, merkleRootsVerifier blocktx.MerkleRootsVerifier, policy *bitcoin.Settings, - peerRPCConfig *config.PeerRPCConfig, - apiConfig *config.APIConfig, + cachedFinder validator.TxFinderI, opts ...Option, ) (*ArcDefaultHandler, error) { - var wocClient *woc_client.WocClient - if apiConfig != nil { - wocClient = woc_client.New(apiConfig.WocMainnet, woc_client.WithAuth(apiConfig.WocAPIKey)) - } else { - wocClient = woc_client.New(false) - } - mr := merkle_verifier.New(merkleRootsVerifier) handler := &ArcDefaultHandler{ @@ -109,18 +98,13 @@ func NewDefault( logger: logger, now: time.Now, mrVerifier: mr, + txFinder: cachedFinder, } // apply options for _, opt := range opts { opt(handler) } - var finderOpts []func(f *txfinder.CachedFinder) - if handler.tracingEnabled { - finderOpts = append(finderOpts, txfinder.WithTracerCachedFinder(handler.tracingAttributes...)) - } - - handler.txFinder = txfinder.NewCached(transactionHandler, peerRPCConfig, wocClient, logger, finderOpts...) return handler, nil } diff --git a/pkg/api/handler/default_test.go b/pkg/api/handler/default_test.go index 3eb0630e4..0026a99ea 100644 --- a/pkg/api/handler/default_test.go +++ b/pkg/api/handler/default_test.go @@ -17,6 +17,7 @@ import ( "time" defaultvalidator "github.com/bitcoin-sv/arc/internal/validator/default" + "github.com/bitcoin-sv/arc/internal/validator/mocks" sdkTx "github.com/bitcoin-sv/go-sdk/transaction" "github.com/labstack/echo/v4" @@ -24,7 +25,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/bitcoin-sv/arc/config" "github.com/bitcoin-sv/arc/internal/metamorph/metamorph_api" "github.com/bitcoin-sv/arc/internal/validator" "github.com/bitcoin-sv/arc/pkg/api" @@ -92,7 +92,7 @@ var ( func TestNewDefault(t *testing.T) { t.Run("simple init", func(t *testing.T) { - defaultHandler, err := NewDefault(testLogger, nil, nil, nil, nil, nil) + defaultHandler, err := NewDefault(testLogger, nil, nil, nil, nil) require.NoError(t, err) assert.NotNil(t, defaultHandler) }) @@ -101,7 +101,7 @@ func TestNewDefault(t *testing.T) { func TestGETPolicy(t *testing.T) { t.Run("default policy", func(t *testing.T) { // given - sut, err := NewDefault(testLogger, nil, nil, defaultPolicy, nil, nil) + sut, err := NewDefault(testLogger, nil, nil, defaultPolicy, nil) require.NoError(t, err) e := echo.New() req := httptest.NewRequest(http.MethodPost, "/v1/policy", strings.NewReader("")) @@ -138,7 +138,7 @@ func TestGETHealth(t *testing.T) { }, } - sut, err := NewDefault(testLogger, txHandler, nil, defaultPolicy, nil, nil) + sut, err := NewDefault(testLogger, txHandler, nil, defaultPolicy, nil) require.NoError(t, err) e := echo.New() req := httptest.NewRequest(http.MethodPost, "/v1/health", strings.NewReader("")) @@ -276,7 +276,7 @@ func TestGETTransactionStatus(t *testing.T) { }, } - defaultHandler, err := NewDefault(testLogger, txHandler, nil, nil, nil, nil, WithNow(func() time.Time { return time.Date(2023, 5, 3, 10, 0, 0, 0, time.UTC) })) + defaultHandler, err := NewDefault(testLogger, txHandler, nil, nil, nil, WithNow(func() time.Time { return time.Date(2023, 5, 3, 10, 0, 0, 0, time.UTC) })) require.NoError(t, err) err = defaultHandler.GETTransactionStatus(ctx, "c9648bf65a734ce64614dc92877012ba7269f6ea1f55be9ab5a342a2f768cf46") @@ -307,7 +307,7 @@ func TestGETTransactionStatus(t *testing.T) { } func TestPOSTTransaction(t *testing.T) { //nolint:funlen - errFieldMissingInputs := *api.NewErrorFields(api.ErrStatusTxFormat, "arc error 460: parent transaction not found") + errFieldMissingInputs := *api.NewErrorFields(api.ErrStatusTxFormat, "arc error 460: failed to get raw transactions for parent") errFieldMissingInputs.Txid = PtrTo("a147cc3c71cc13b29f18273cf50ffeb59fc9758152e2b33e21a8092f0b049118") errFieldSubmitTx := *api.NewErrorFields(api.ErrStatusGeneric, "failed to submit tx") @@ -331,6 +331,7 @@ func TestPOSTTransaction(t *testing.T) { //nolint:funlen getTx []byte submitTxResponse *metamorph.TransactionStatus submitTxErr error + getRawTxsErr error expectedStatus api.StatusCode expectedResponse any @@ -404,13 +405,14 @@ func TestPOSTTransaction(t *testing.T) { //nolint:funlen expectedResponse: *api.NewErrorFields(api.ErrStatusBadRequest, "could not read varint type: EOF"), }, { - name: "valid tx - missing inputs, text/plain", - contentType: contentTypes[0], - txHexString: validTx, + name: "valid tx - missing inputs, text/plain", + contentType: contentTypes[0], + txHexString: validTx, + getRawTxsErr: errors.New("failed to get raw txs"), expectedStatus: 460, expectedResponse: errFieldMissingInputs, - expectedError: defaultvalidator.ErrParentNotFound, + expectedError: defaultvalidator.ErrFailedToGetRawTxs, }, { name: "valid tx - fees too low", @@ -638,11 +640,27 @@ func TestPOSTTransaction(t *testing.T) { //nolint:funlen }, } - // load default config - arcConfig, err := config.Load() - require.NoError(t, err, "error loading config") + tx, err := sdkTx.NewTransactionFromHex("0100000001fbbe01d83cb1f53a63ef91c0fce5750cbd8075efef5acd2ff229506a45ab832c010000006a473044022064be2f304950a87782b44e772390836aa613f40312a0df4993e9c5123d0c492d02202009b084b66a3da939fb7dc5d356043986539cac4071372d0a6481d5b5e418ca412103fc12a81e5213e30c7facc15581ac1acbf26a8612a3590ffb48045084b097d52cffffffff02bf010000000000001976a914c2ca67db517c0c972b9a6eb1181880ed3a528e3188acc70a0000000000001976a914f1e6837cf17b485a1dcea9e943948fafbe5e9f6888ac00000000") + require.NoError(t, err) + txMap := map[string]*sdkTx.Transaction{ + "8574e743bb64cf603dbd0e951e7287afd2a59593ff8837b3760e911f8fb38e35": tx, + } + finder := &mocks.TxFinderIMock{ + GetRawTxsFunc: func(_ context.Context, _ validator.FindSourceFlag, ids []string) ([]*sdkTx.Transaction, error) { + var rawTxs []*sdkTx.Transaction + for _, id := range ids { + rawTx, ok := txMap[id] + if !ok { + continue + } + rawTxs = append(rawTxs, rawTx) + } + + return rawTxs, tc.getRawTxsErr + }, + } - sut, err := NewDefault(testLogger, txHandler, merkleRootsVerifier, &policy, arcConfig.PeerRPC, arcConfig.API, WithNow(func() time.Time { return now })) + sut, err := NewDefault(testLogger, txHandler, merkleRootsVerifier, &policy, finder, WithNow(func() time.Time { return now })) require.NoError(t, err) inputTx := strings.NewReader(tc.txHexString) @@ -670,7 +688,15 @@ func TestPOSTTransaction(t *testing.T) { //nolint:funlen err = json.Unmarshal(b, &actualError) require.NoError(t, err) - assert.Equal(t, tc.expectedResponse, actualError) + expectedResp, ok := tc.expectedResponse.(api.ErrorFields) + require.True(t, ok) + + assert.Equal(t, expectedResp.Txid, actualError.Txid) + assert.Equal(t, expectedResp.Status, actualError.Status) + assert.Equal(t, expectedResp.Detail, actualError.Detail) + assert.Equal(t, expectedResp.Title, actualError.Title) + assert.Equal(t, expectedResp.Instance, actualError.Instance) + assert.Equal(t, expectedResp.Type, actualError.Type) if tc.expectedError != nil { assert.ErrorContains(t, errors.New(*actualError.ExtraInfo), tc.expectedError.Error()) } @@ -684,7 +710,7 @@ func TestPOSTTransaction(t *testing.T) { //nolint:funlen func TestPOSTTransactions(t *testing.T) { //nolint:funlen t.Run("empty tx", func(t *testing.T) { // when - sut, err := NewDefault(testLogger, nil, nil, defaultPolicy, nil, nil) + sut, err := NewDefault(testLogger, nil, nil, defaultPolicy, nil) require.NoError(t, err) for _, contentType := range contentTypes { @@ -708,7 +734,7 @@ func TestPOSTTransactions(t *testing.T) { //nolint:funlen // given inputTx := strings.NewReader(validExtendedTx) rec, ctx := createEchoPostRequest(inputTx, echo.MIMETextPlain, "/v1/tx") - sut, err := NewDefault(testLogger, nil, nil, defaultPolicy, nil, nil) + sut, err := NewDefault(testLogger, nil, nil, defaultPolicy, nil) require.NoError(t, err) req := httptest.NewRequest(http.MethodPost, "/v1/tx", strings.NewReader("")) @@ -737,7 +763,7 @@ func TestPOSTTransactions(t *testing.T) { //nolint:funlen t.Run("invalid mime type", func(t *testing.T) { // given - sut, err := NewDefault(testLogger, nil, nil, defaultPolicy, nil, nil) + sut, err := NewDefault(testLogger, nil, nil, defaultPolicy, nil) require.NoError(t, err) e := echo.New() @@ -756,7 +782,7 @@ func TestPOSTTransactions(t *testing.T) { //nolint:funlen t.Run("invalid txs", func(t *testing.T) { // given - sut, err := NewDefault(testLogger, nil, nil, defaultPolicy, nil, nil) + sut, err := NewDefault(testLogger, nil, nil, defaultPolicy, nil) require.NoError(t, err) expectedErrors := map[string]string{ @@ -807,10 +833,12 @@ func TestPOSTTransactions(t *testing.T) { //nolint:funlen }, } - arcConfig, err := config.Load() - require.NoError(t, err, "could not load default config") - - sut, err := NewDefault(testLogger, txHandler, nil, defaultPolicy, arcConfig.PeerRPC, arcConfig.API) + //arcConfig, err := config.Load() + //require.NoError(t, err, "could not load default config") + finder := &mocks.TxFinderIMock{GetRawTxsFunc: func(_ context.Context, _ validator.FindSourceFlag, _ []string) ([]*sdkTx.Transaction, error) { + return nil, errors.New("error getting raw transactions") + }} + sut, err := NewDefault(testLogger, txHandler, nil, defaultPolicy, finder) require.NoError(t, err) validTxBytes, _ := hex.DecodeString(validTx) @@ -834,8 +862,8 @@ func TestPOSTTransactions(t *testing.T) { //nolint:funlen _ = json.Unmarshal(b, &bErr) assert.Equal(t, int(api.ErrStatusTxFormat), bErr[0].Status) - assert.Equal(t, "arc error 460: parent transaction not found", *bErr[0].ExtraInfo) - assert.ErrorContains(t, errors.New(*bErr[0].ExtraInfo), defaultvalidator.ErrParentNotFound.Error()) + + assert.ErrorContains(t, errors.New(*bErr[0].ExtraInfo), defaultvalidator.ErrFailedToGetRawTxs.Error()) } }) @@ -863,7 +891,7 @@ func TestPOSTTransactions(t *testing.T) { //nolint:funlen }, } - sut, err := NewDefault(testLogger, txHandler, nil, defaultPolicy, nil, nil) + sut, err := NewDefault(testLogger, txHandler, nil, defaultPolicy, nil) require.NoError(t, err) validExtendedTxBytes, _ := hex.DecodeString(validExtendedTx) @@ -900,7 +928,7 @@ func TestPOSTTransactions(t *testing.T) { //nolint:funlen }, } - sut, err := NewDefault(testLogger, txHandler, nil, defaultPolicy, nil, nil) + sut, err := NewDefault(testLogger, txHandler, nil, defaultPolicy, nil) require.NoError(t, err) invalidBeefNoBUMPIndexBytes, _ := hex.DecodeString(invalidBeefNoBUMPIndex) @@ -958,7 +986,7 @@ func TestPOSTTransactions(t *testing.T) { //nolint:funlen }, } - sut, err := NewDefault(testLogger, txHandler, merkleRootsVerifier, defaultPolicy, nil, nil) + sut, err := NewDefault(testLogger, txHandler, merkleRootsVerifier, defaultPolicy, nil) require.NoError(t, err) inputTxs := map[string]io.Reader{ @@ -1013,7 +1041,9 @@ func TestPOSTTransactions(t *testing.T) { //nolint:funlen }, }, nil }, - + SubmitTransactionFunc: func(_ context.Context, _ *sdkTx.Transaction, _ *metamorph.TransactionOptions) (*metamorph.TransactionStatus, error) { + return txResults[0], nil + }, SubmitTransactionsFunc: func(_ context.Context, txs sdkTx.Transactions, _ *metamorph.TransactionOptions) ([]*metamorph.TransactionStatus, error) { var res []*metamorph.TransactionStatus for _, t := range txs { @@ -1036,8 +1066,10 @@ func TestPOSTTransactions(t *testing.T) { //nolint:funlen return nil, nil }, } - - sut, err := NewDefault(testLogger, txHandler, merkleRootsVerifier, defaultPolicy, nil, nil) + finder := &mocks.TxFinderIMock{GetRawTxsFunc: func(_ context.Context, _ validator.FindSourceFlag, _ []string) ([]*sdkTx.Transaction, error) { + return nil, errors.New("error getting raw transactions") + }} + sut, err := NewDefault(testLogger, txHandler, merkleRootsVerifier, defaultPolicy, finder) require.NoError(t, err) inputTxs := map[string]io.Reader{ diff --git a/pkg/api/handler/internal/tx_finder/cached_tx_finder.go b/pkg/api/tx_finder/cached_tx_finder.go similarity index 69% rename from pkg/api/handler/internal/tx_finder/cached_tx_finder.go rename to pkg/api/tx_finder/cached_tx_finder.go index 86eba3719..d7bfcdfba 100644 --- a/pkg/api/handler/internal/tx_finder/cached_tx_finder.go +++ b/pkg/api/tx_finder/cached_tx_finder.go @@ -6,10 +6,10 @@ import ( "runtime" "time" + sdkTx "github.com/bitcoin-sv/go-sdk/transaction" "github.com/patrickmn/go-cache" "go.opentelemetry.io/otel/attribute" - "github.com/bitcoin-sv/arc/config" "github.com/bitcoin-sv/arc/internal/tracing" "github.com/bitcoin-sv/arc/internal/validator" "github.com/bitcoin-sv/arc/internal/woc_client" @@ -42,7 +42,13 @@ func WithTracerCachedFinder(attr ...attribute.KeyValue) func(s *CachedFinder) { } } -func NewCached(th metamorph.TransactionHandler, pc *config.PeerRPCConfig, w *woc_client.WocClient, l *slog.Logger, opts ...func(f *CachedFinder)) CachedFinder { +func WithCacheStore(store *cache.Cache) func(s *CachedFinder) { + return func(p *CachedFinder) { + p.cacheStore = store + } +} + +func NewCached(th metamorph.TransactionHandler, n NodeClient, w *woc_client.WocClient, l *slog.Logger, opts ...func(f *CachedFinder)) CachedFinder { c := CachedFinder{ cacheStore: cache.New(cacheExpiration, cacheCleanup), } @@ -55,23 +61,30 @@ func NewCached(th metamorph.TransactionHandler, pc *config.PeerRPCConfig, w *woc finderOpts = append(finderOpts, WithTracerFinder(c.tracingAttributes...)) } - c.finder = New(th, pc, w, l, finderOpts...) + c.finder = New(th, n, w, l, finderOpts...) return c } -func (f CachedFinder) GetRawTxs(ctx context.Context, source validator.FindSourceFlag, ids []string) ([]validator.RawTx, error) { +func (f CachedFinder) GetMempoolAncestors(ctx context.Context, ids []string) ([]string, error) { + return f.finder.GetMempoolAncestors(ctx, ids) +} + +func (f CachedFinder) GetRawTxs(ctx context.Context, source validator.FindSourceFlag, ids []string) ([]*sdkTx.Transaction, error) { ctx, span := tracing.StartTracing(ctx, "CachedFinder_GetRawTxs", f.tracingEnabled, f.tracingAttributes...) defer tracing.EndTracing(span) - cachedTxs := make([]validator.RawTx, 0, len(ids)) + cachedTxs := make([]*sdkTx.Transaction, 0, len(ids)) var toFindIDs []string // check cache for _, id := range ids { value, found := f.cacheStore.Get(id) if found { - cachedTxs = append(cachedTxs, value.(validator.RawTx)) + cahchedTx, ok := value.(sdkTx.Transaction) + if ok { + cachedTxs = append(cachedTxs, &cahchedTx) + } } else { toFindIDs = append(toFindIDs, id) } @@ -89,7 +102,7 @@ func (f CachedFinder) GetRawTxs(ctx context.Context, source validator.FindSource // update cache for _, tx := range foundTxs { - f.cacheStore.Set(tx.TxID, tx, cacheExpiration) + f.cacheStore.Set(tx.TxID(), *tx, cacheExpiration) } return append(cachedTxs, foundTxs...), nil diff --git a/pkg/api/handler/internal/tx_finder/cached_tx_finder_test.go b/pkg/api/tx_finder/cached_tx_finder_test.go similarity index 58% rename from pkg/api/handler/internal/tx_finder/cached_tx_finder_test.go rename to pkg/api/tx_finder/cached_tx_finder_test.go index 1d54f3fab..cc48e1d8a 100644 --- a/pkg/api/handler/internal/tx_finder/cached_tx_finder_test.go +++ b/pkg/api/tx_finder/cached_tx_finder_test.go @@ -2,39 +2,44 @@ package txfinder import ( "context" + "log/slog" + "os" "testing" "time" + sdkTx "github.com/bitcoin-sv/go-sdk/transaction" "github.com/patrickmn/go-cache" "github.com/stretchr/testify/require" + "github.com/bitcoin-sv/arc/internal/testdata" "github.com/bitcoin-sv/arc/internal/validator" "github.com/bitcoin-sv/arc/pkg/metamorph" "github.com/bitcoin-sv/arc/pkg/metamorph/mocks" ) -// Mocked data for RawTx -var rawTx1 = validator.RawTx{TxID: "tx1"} -var rawTx2 = validator.RawTx{TxID: "tx2"} - func TestCachedFinder_GetRawTxs_AllFromCache(t *testing.T) { tt := []struct { name string - cachedTx []validator.RawTx + cachedTx []sdkTx.Transaction fetchedTx []*metamorph.Transaction }{ { name: "all from cache", - cachedTx: []validator.RawTx{rawTx1, rawTx2}, + cachedTx: []sdkTx.Transaction{*testdata.TX1Raw, *testdata.TX6Raw}, }, { - name: "all from finder", - fetchedTx: []*metamorph.Transaction{{TxID: rawTx1.TxID}, {TxID: rawTx2.TxID}}, + name: "all from finder", + fetchedTx: []*metamorph.Transaction{ + {TxID: testdata.TX1Raw.TxID(), Bytes: testdata.TX1Raw.Bytes()}, + {TxID: testdata.TX6Raw.TxID(), Bytes: testdata.TX6Raw.Bytes()}, + }, }, { - name: "cached and fetched mixed", - cachedTx: []validator.RawTx{rawTx1}, - fetchedTx: []*metamorph.Transaction{{TxID: rawTx2.TxID}}, + name: "cached and fetched mixed", + cachedTx: []sdkTx.Transaction{*testdata.TX1Raw}, + fetchedTx: []*metamorph.Transaction{ + {TxID: testdata.TX6Raw.TxID(), Bytes: testdata.TX6Raw.Bytes()}, + }, }, } @@ -49,19 +54,15 @@ func TestCachedFinder_GetRawTxs_AllFromCache(t *testing.T) { c := cache.New(10*time.Second, 10*time.Second) for _, r := range tc.cachedTx { - c.Set(r.TxID, r, cache.DefaultExpiration) + c.Set(r.TxID(), r, cache.DefaultExpiration) } - sut := CachedFinder{ - finder: &Finder{ - transactionHandler: thMq, - }, - cacheStore: c, - } + logger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelDebug})) + sut := NewCached(thMq, nil, nil, logger, WithCacheStore(c)) // when // try to find in cache or with TransactionHandler only - res, err := sut.GetRawTxs(context.Background(), validator.SourceTransactionHandler, []string{rawTx1.TxID, rawTx2.TxID}) + res, err := sut.GetRawTxs(context.Background(), validator.SourceTransactionHandler, []string{testdata.TX1Raw.TxID(), testdata.TX6Raw.TxID()}) // then require.NoError(t, err) diff --git a/pkg/api/handler/internal/tx_finder/tx_finder.go b/pkg/api/tx_finder/tx_finder.go similarity index 100% rename from pkg/api/handler/internal/tx_finder/tx_finder.go rename to pkg/api/tx_finder/tx_finder.go diff --git a/pkg/api/handler/internal/tx_finder/tx_finder_test.go b/pkg/api/tx_finder/tx_finder_test.go similarity index 90% rename from pkg/api/handler/internal/tx_finder/tx_finder_test.go rename to pkg/api/tx_finder/tx_finder_test.go index 2fd2ae7e5..53f1f1580 100644 --- a/pkg/api/handler/internal/tx_finder/tx_finder_test.go +++ b/pkg/api/tx_finder/tx_finder_test.go @@ -6,11 +6,11 @@ import ( "os" "testing" - "github.com/bitcoin-sv/arc/config" + "github.com/stretchr/testify/require" + "github.com/bitcoin-sv/arc/internal/validator" "github.com/bitcoin-sv/arc/internal/woc_client" "github.com/bitcoin-sv/arc/pkg/metamorph" - "github.com/stretchr/testify/require" transactionHandlerMock "github.com/bitcoin-sv/arc/pkg/metamorph/mocks" ) @@ -106,19 +106,14 @@ func Test_GetRawTxs(t *testing.T) { th = transactionHandler(t, tc.thResponse) } - var pc *config.PeerRPCConfig - if tc.source.Has(validator.SourceNodes) { - pc = peerRPCConfig(t) - } - var w *woc_client.WocClient if tc.source.Has(validator.SourceWoC) { - w = wocClient() + w = woc_client.New(true) } l := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelDebug})) - sut := New(th, pc, w, l) + sut := New(th, nil, w, l) // then @@ -138,11 +133,3 @@ func transactionHandler(_ *testing.T, thResponse func() ([]*metamorph.Transactio return &mq } - -func peerRPCConfig(_ *testing.T) *config.PeerRPCConfig { - return &config.PeerRPCConfig{} -} - -func wocClient() *woc_client.WocClient { - return woc_client.New(true) -} From cd5ee3eb43e530905b1f06620617cf8ad86cbb3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20B=C3=B6ckli?= Date: Thu, 14 Nov 2024 13:47:25 +0100 Subject: [PATCH 2/3] fix(ARCO-212): Cumulative fee validation uses GetMempoolAncestors function --- cmd/arc/services/api.go | 2 +- .../validator/default/default_validator.go | 13 +- .../default/default_validator_test.go | 138 +++++-------- internal/validator/default/helpers.go | 106 ++++------ internal/validator/default/helpers_test.go | 138 ++++++------- .../validator/default/testdata/fixture.go | 84 ++------ internal/validator/helpers.go | 11 +- .../mocks/tx_finder_interface_mock.go | 59 +++++- pkg/api/handler/default_test.go | 2 - pkg/api/tx_finder/tx_finder.go | 183 +++++++----------- 10 files changed, 292 insertions(+), 444 deletions(-) diff --git a/cmd/arc/services/api.go b/cmd/arc/services/api.go index 4712a9601..21037be9c 100644 --- a/cmd/arc/services/api.go +++ b/cmd/arc/services/api.go @@ -109,7 +109,7 @@ func StartAPIServer(logger *slog.Logger, arcConfig *config.ArcConfig) (func(), e // get the transaction from the bitcoin node rpc bitcoinClient, err := bitcoin.NewFromURL(rpcURL, false) if err != nil { - return nil, fmt.Errorf("failed to create node client: %w", err) + return nil, fmt.Errorf("failed to create bitcoin client: %w", err) } nodeClient, err := node_client.New(bitcoinClient, nodeClientOpts...) diff --git a/internal/validator/default/default_validator.go b/internal/validator/default/default_validator.go index fabd75fff..13f98f695 100644 --- a/internal/validator/default/default_validator.go +++ b/internal/validator/default/default_validator.go @@ -127,9 +127,16 @@ func cumulativeCheckFees(ctx context.Context, txFinder validator.TxFinderI, tx * cumulativeSize := 0 cumulativePaidFee := uint64(0) - for _, tx := range txSet { - cumulativeSize += tx.Size() - cumulativePaidFee += tx.TotalInputSatoshis() - tx.TotalOutputSatoshis() + for _, txFromSet := range txSet { + cumulativeSize += txFromSet.Size() + totalInput := txFromSet.TotalInputSatoshis() + totalOutput := txFromSet.TotalOutputSatoshis() + + if totalOutput > totalInput { + return validator.NewError(fmt.Errorf("total outputs %d is larger than total inputs %d for tx %s", totalOutput, totalInput, tx.TxID()), api.ErrStatusCumulativeFees) + } + + cumulativePaidFee += totalInput - totalOutput } expectedFee, err := feeModel.ComputeFeeBasedOnSize(uint64(cumulativeSize)) diff --git a/internal/validator/default/default_validator_test.go b/internal/validator/default/default_validator_test.go index 62354e895..89422795b 100644 --- a/internal/validator/default/default_validator_test.go +++ b/internal/validator/default/default_validator_test.go @@ -151,13 +151,13 @@ func TestValidator(t *testing.T) { t.Run("valid Raw Format tx - expect success", func(t *testing.T) { // given txFinder := mocks.TxFinderIMock{ - GetRawTxsFunc: func(_ context.Context, _ validation.FindSourceFlag, _ []string) ([]validation.RawTx, error) { - res := []validation.RawTx{fixture.ParentTx1, fixture.ParentTx2} + GetRawTxsFunc: func(_ context.Context, _ validation.FindSourceFlag, _ []string) ([]*sdkTx.Transaction, error) { + res := []*sdkTx.Transaction{fixture.ParentTx1} return res, nil }, } - rawTx, _ := sdkTx.NewTransactionFromHex(fixture.ValidTxRawHex) + rawTx := fixture.ValidTx sut := New(getPolicy(5), &txFinder) @@ -352,127 +352,62 @@ func TestNeedExtension(t *testing.T) { } func TestCumulativeCheckFees(t *testing.T) { + txMap := map[string]*sdkTx.Transaction{ + fixture.ParentTxID1: fixture.ParentTx1, + fixture.AncestorTxID1: fixture.AncestorTx1, + fixture.AncestorOfAncestorTx1ID1: fixture.AncestorOfAncestor1Tx1, + } + tcs := []struct { - name string - hex string - feeModel *fees.SatoshisPerKilobyte - getTxFinderFn func(t *testing.T) mocks.TxFinderIMock + name string + feeModel *fees.SatoshisPerKilobyte + mempoolAncestors []string + getMempoolAncestorsErr error + getRawTxsErr error expectedErr *validation.Error }{ { name: "no unmined ancestors - valid fee", - hex: fixture.ValidTxRawHex, feeModel: func() *fees.SatoshisPerKilobyte { return &fees.SatoshisPerKilobyte{Satoshis: 1} }(), - getTxFinderFn: func(_ *testing.T) mocks.TxFinderIMock { - return mocks.TxFinderIMock{ - GetRawTxsFunc: func(_ context.Context, _ validation.FindSourceFlag, _ []string) ([]validation.RawTx, error) { - return []validation.RawTx{fixture.ParentTx1, fixture.ParentTx2}, nil - }, - } - }, + mempoolAncestors: []string{}, }, { - name: "no unmined ancestors - to low fee", - hex: fixture.ValidTxRawHex, + name: "no unmined ancestors - too low fee", feeModel: func() *fees.SatoshisPerKilobyte { return &fees.SatoshisPerKilobyte{Satoshis: 50} }(), - getTxFinderFn: func(_ *testing.T) mocks.TxFinderIMock { - return mocks.TxFinderIMock{ - GetRawTxsFunc: func(_ context.Context, _ validation.FindSourceFlag, _ []string) ([]validation.RawTx, error) { - return []validation.RawTx{fixture.ParentTx1, fixture.ParentTx2}, nil - }, - } - }, + mempoolAncestors: []string{}, + expectedErr: validation.NewError(ErrTxFeeTooLow, api.ErrStatusCumulativeFees), }, { name: "cumulative fees too low", - hex: fixture.ValidTxRawHex, feeModel: func() *fees.SatoshisPerKilobyte { return &fees.SatoshisPerKilobyte{Satoshis: 50} }(), - getTxFinderFn: func(t *testing.T) mocks.TxFinderIMock { - var getRawTxCount = 0 - var counterPtr = &getRawTxCount - - return mocks.TxFinderIMock{ - GetRawTxsFunc: func(_ context.Context, _ validation.FindSourceFlag, _ []string) ([]validation.RawTx, error) { - i := *counterPtr - *counterPtr = i + 1 - - if i == 0 { - p1 := validation.RawTx{ - TxID: fixture.ParentTx1.TxID, - Bytes: fixture.ParentTx1.Bytes, - IsMined: false, - } - return []validation.RawTx{p1, fixture.ParentTx2}, nil - } + mempoolAncestors: []string{fixture.AncestorTxID1}, - if i == 1 { - return []validation.RawTx{fixture.AncestorTx1, fixture.AncestorTx2}, nil - } - - t.Fatal("to many calls") - return nil, nil - }, - } - }, expectedErr: validation.NewError(ErrTxFeeTooLow, api.ErrStatusCumulativeFees), }, { name: "cumulative fees sufficient", - hex: fixture.ValidTxRawHex, feeModel: func() *fees.SatoshisPerKilobyte { return &fees.SatoshisPerKilobyte{Satoshis: 1} }(), - getTxFinderFn: func(t *testing.T) mocks.TxFinderIMock { - var getRawTxCount = 0 - var counterPtr = &getRawTxCount - - return mocks.TxFinderIMock{ - GetRawTxsFunc: func(_ context.Context, _ validation.FindSourceFlag, _ []string) ([]validation.RawTx, error) { - i := *counterPtr - *counterPtr = i + 1 - - if i == 0 { - p1 := validation.RawTx{ - TxID: fixture.ParentTx1.TxID, - Bytes: fixture.ParentTx1.Bytes, - IsMined: false, - } - return []validation.RawTx{p1, fixture.ParentTx2}, nil - } - - if i == 1 { - return []validation.RawTx{fixture.AncestorTx1, fixture.AncestorTx2}, nil - } - - t.Fatal("to many calls") - return nil, nil - }, - } - }, + mempoolAncestors: []string{fixture.AncestorTxID1}, }, { - name: "issue with getUnminedAncestors", - hex: fixture.ValidTxRawHex, + name: "failed to get mempool ancestors", feeModel: func() *fees.SatoshisPerKilobyte { return &fees.SatoshisPerKilobyte{Satoshis: 5} }(), - getTxFinderFn: func(_ *testing.T) mocks.TxFinderIMock { - return mocks.TxFinderIMock{ - GetRawTxsFunc: func(_ context.Context, _ validation.FindSourceFlag, _ []string) ([]validation.RawTx, error) { - return nil, errors.New("test error") - }, - } - }, + getMempoolAncestorsErr: errors.New("some error"), + expectedErr: validation.NewError( - ErrFailedToGetRawTxs, + ErrFailedToGetMempoolAncestors, api.ErrStatusCumulativeFees), }, } @@ -480,11 +415,30 @@ func TestCumulativeCheckFees(t *testing.T) { for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { // given - txFinder := tc.getTxFinderFn(t) - tx, _ := sdkTx.NewTransactionFromHex(tc.hex) + txFinder := &mocks.TxFinderIMock{ + GetMempoolAncestorsFunc: func(_ context.Context, _ []string) ([]string, error) { + return tc.mempoolAncestors, tc.getMempoolAncestorsErr + }, + GetRawTxsFunc: func(_ context.Context, _ validation.FindSourceFlag, ids []string) ([]*sdkTx.Transaction, error) { + rawTxs := make([]*sdkTx.Transaction, len(ids)) + for i, id := range ids { + rawTx, ok := txMap[id] + if !ok { + t.Fatalf("tx id %s not found", id) + } + rawTxs[i] = rawTx + } + + return rawTxs, tc.getRawTxsErr + }, + } + tx := fixture.ValidTx + + err := extendTx(context.TODO(), txFinder, tx, false) + require.NoError(t, err) // when - actualError := cumulativeCheckFees(context.TODO(), &txFinder, tx, tc.feeModel, false) + actualError := cumulativeCheckFees(context.TODO(), txFinder, tx, tc.feeModel, false) // then if tc.expectedErr == nil { diff --git a/internal/validator/default/helpers.go b/internal/validator/default/helpers.go index b4fa54350..6cefc1b67 100644 --- a/internal/validator/default/helpers.go +++ b/internal/validator/default/helpers.go @@ -13,11 +13,12 @@ import ( ) var ( - ErrParentNotFound = errors.New("parent transaction not found") - ErrFailedToGetRawTxs = errors.New("failed to get raw transactions for parent") + ErrParentNotFound = errors.New("parent transaction not found") + ErrFailedToGetRawTxs = errors.New("failed to get raw transactions for parent") + ErrFailedToGetMempoolAncestors = errors.New("failed to get mempool ancestors") ) -func extendTx(ctx context.Context, f validator.TxFinderI, rawTx *sdkTx.Transaction, tracingEnabled bool, tracingAttributes ...attribute.KeyValue) error { +func extendTx(ctx context.Context, txFinder validator.TxFinderI, rawTx *sdkTx.Transaction, tracingEnabled bool, tracingAttributes ...attribute.KeyValue) error { ctx, span := tracing.StartTracing(ctx, "extendTx", tracingEnabled, tracingAttributes...) defer tracing.EndTracing(span) @@ -45,9 +46,9 @@ func extendTx(ctx context.Context, f validator.TxFinderI, rawTx *sdkTx.Transacti // get parents const finderSource = validator.SourceTransactionHandler | validator.SourceNodes | validator.SourceWoC - parentsTxs, err := f.GetRawTxs(ctx, finderSource, parentsIDs) + parentsTxs, err := txFinder.GetRawTxs(ctx, finderSource, parentsIDs) if err != nil { - return fmt.Errorf("failed to get raw transactions for parent: %v. Reason: %w", parentsIDs, err) + return errors.Join(ErrFailedToGetRawTxs, fmt.Errorf("failed to get raw transactions for parents %v: %w", parentsIDs, err)) } if len(parentsTxs) != len(parentsIDs) { @@ -56,110 +57,71 @@ func extendTx(ctx context.Context, f validator.TxFinderI, rawTx *sdkTx.Transacti // extend inputs with parents data for _, p := range parentsTxs { - childInputs, found := parentInputMap[p.TxID] + childInputs, found := parentInputMap[p.TxID()] if !found { return ErrParentNotFound } - bTx, err := sdkTx.NewTransactionFromBytes(p.Bytes) - if err != nil { - return fmt.Errorf("cannot parse parent tx: %w", err) + if err = extendInputs(p, childInputs); err != nil { + return err } + } - if err = extendInputs(bTx, childInputs); err != nil { - return err + return nil +} + +func extendInputs(tx *sdkTx.Transaction, childInputs []*sdkTx.TransactionInput) error { + for _, input := range childInputs { + if len(tx.Outputs) < int(input.SourceTxOutIndex) { + return fmt.Errorf("output %d not found in transaction %s", input.SourceTxOutIndex, input.PreviousTxIDStr()) } + output := tx.Outputs[input.SourceTxOutIndex] + + input.SetPrevTxFromOutput(output) } return nil } -// getUnminedAncestors returns unmined ancestors with data necessary to perform Deep Fee validation -func getUnminedAncestors(ctx context.Context, w validator.TxFinderI, tx *sdkTx.Transaction, tracingEnabled bool, tracingAttributes ...attribute.KeyValue) (map[string]*sdkTx.Transaction, error) { +// getUnminedAncestors returns unmined ancestors with data necessary to perform cumulative fee validation +func getUnminedAncestors(ctx context.Context, txFinder validator.TxFinderI, tx *sdkTx.Transaction, tracingEnabled bool, tracingAttributes ...attribute.KeyValue) (map[string]*sdkTx.Transaction, error) { ctx, span := tracing.StartTracing(ctx, "getUnminedAncestors", tracingEnabled, tracingAttributes...) defer tracing.EndTracing(span) unmindedAncestorsSet := make(map[string]*sdkTx.Transaction) // get distinct parents // map parentID with inputs collection to avoid duplication and simplify later processing - parentInputMap := make(map[string][]*sdkTx.TransactionInput) + parentInputMap := make(map[string]struct{}) parentsIDs := make([]string, 0, len(tx.Inputs)) for _, in := range tx.Inputs { prevTxID := in.PreviousTxIDStr() - - inputs, found := parentInputMap[prevTxID] + _, found := parentInputMap[prevTxID] if !found { // first occurrence of the parent - inputs = make([]*sdkTx.TransactionInput, 0) parentsIDs = append(parentsIDs, prevTxID) } - - inputs = append(inputs, in) - parentInputMap[prevTxID] = inputs } - - // get parents - const finderSource = validator.SourceTransactionHandler | validator.SourceWoC - parentsTxs, err := w.GetRawTxs(ctx, finderSource, parentsIDs) + mempoolAncestorTxIDs, err := txFinder.GetMempoolAncestors(ctx, parentsIDs) if err != nil { - return nil, errors.Join(ErrFailedToGetRawTxs, fmt.Errorf("parent: %v", parentsIDs), err) + return nil, errors.Join(ErrFailedToGetMempoolAncestors, err) } - if len(parentsTxs) != len(parentsIDs) { - return nil, ErrParentNotFound - } - - for _, p := range parentsTxs { - if _, found := unmindedAncestorsSet[p.TxID]; found { - continue // parent was proccesed already - } + allTxIDs := append(parentsIDs, mempoolAncestorTxIDs...) - childInputs, found := parentInputMap[p.TxID] - if !found { - return nil, ErrParentNotFound - } - - // fulfill data about the parent for further validation - bTx, err := sdkTx.NewTransactionFromBytes(p.Bytes) - if err != nil { - return nil, fmt.Errorf("cannot parse parent tx: %w", err) - } + ancestorTxs, err := txFinder.GetRawTxs(ctx, validator.SourceNodes, allTxIDs) + if err != nil { + return nil, errors.Join(ErrFailedToGetRawTxs, err) + } - err = extendInputs(bTx, childInputs) + for _, ancestorTx := range ancestorTxs { + err = extendTx(ctx, txFinder, ancestorTx, tracingEnabled, tracingAttributes...) if err != nil { return nil, err } - if p.IsMined { - continue // we don't need its ancestors - } - - unmindedAncestorsSet[p.TxID] = bTx - - // get parent ancestors - parentAncestorsSet, err := getUnminedAncestors(ctx, w, bTx, tracingEnabled, tracingAttributes...) - for aID, aTx := range parentAncestorsSet { - unmindedAncestorsSet[aID] = aTx - } - - if err != nil { - return unmindedAncestorsSet, err - } + unmindedAncestorsSet[ancestorTx.TxID()] = ancestorTx } return unmindedAncestorsSet, nil } - -func extendInputs(tx *sdkTx.Transaction, childInputs []*sdkTx.TransactionInput) error { - for _, input := range childInputs { - if len(tx.Outputs) < int(input.SourceTxOutIndex) { - return fmt.Errorf("output %d not found in transaction %s", input.SourceTxOutIndex, input.PreviousTxIDStr()) - } - output := tx.Outputs[input.SourceTxOutIndex] - - input.SetPrevTxFromOutput(output) - } - - return nil -} diff --git a/internal/validator/default/helpers_test.go b/internal/validator/default/helpers_test.go index ba10dd6e4..a95576dba 100644 --- a/internal/validator/default/helpers_test.go +++ b/internal/validator/default/helpers_test.go @@ -2,13 +2,14 @@ package defaultvalidator import ( "context" + "errors" "testing" sdkTx "github.com/bitcoin-sv/go-sdk/transaction" "github.com/stretchr/testify/require" "github.com/bitcoin-sv/arc/internal/validator" - "github.com/bitcoin-sv/arc/internal/validator/default/testdata" + fixture "github.com/bitcoin-sv/arc/internal/validator/default/testdata" "github.com/bitcoin-sv/arc/internal/validator/mocks" ) @@ -16,31 +17,25 @@ func TestDefaultValidator_helpers_extendTx(t *testing.T) { tcs := []struct { name string txHex string - foundTransactions []validator.RawTx + foundTransactions []*sdkTx.Transaction expectedErr error }{ { name: "cannot find parents", - txHex: testdata.ValidTxRawHex, + txHex: fixture.ValidTxRawHex, foundTransactions: nil, expectedErr: ErrParentNotFound, }, - { - name: "cannot find all parents", - txHex: testdata.ValidTxRawHex, - foundTransactions: []validator.RawTx{testdata.ParentTx1}, - expectedErr: ErrParentNotFound, - }, { name: "tx finder returns rubbish", - txHex: testdata.ValidTxRawHex, - foundTransactions: []validator.RawTx{testdata.ParentTx1, testdata.RandomTx1}, + txHex: fixture.ValidTxRawHex, + foundTransactions: []*sdkTx.Transaction{fixture.ParentTx1, fixture.RandomTx1}, expectedErr: ErrParentNotFound, }, { name: "success", - txHex: testdata.ValidTxRawHex, - foundTransactions: []validator.RawTx{testdata.ParentTx1, testdata.ParentTx2}, + txHex: fixture.ValidTxRawHex, + foundTransactions: []*sdkTx.Transaction{fixture.ParentTx1}, expectedErr: nil, }, } @@ -49,7 +44,7 @@ func TestDefaultValidator_helpers_extendTx(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // given txFinder := mocks.TxFinderIMock{ - GetRawTxsFunc: func(_ context.Context, _ validator.FindSourceFlag, _ []string) ([]validator.RawTx, error) { + GetRawTxsFunc: func(_ context.Context, _ validator.FindSourceFlag, _ []string) ([]*sdkTx.Transaction, error) { return tc.foundTransactions, nil }, } @@ -77,99 +72,78 @@ func TestDefaultValidator_helpers_extendTx(t *testing.T) { } func TestDefaultValidator_helpers_getUnminedAncestors(t *testing.T) { + txMap := map[string]*sdkTx.Transaction{ + fixture.ParentTxID1: fixture.ParentTx1, + fixture.AncestorTxID1: fixture.AncestorTx1, + fixture.AncestorOfAncestorTx1ID1: fixture.AncestorOfAncestor1Tx1, + fixture.RandomTxID1: fixture.RandomTx1, + } + tcs := []struct { - name string - txHex string - foundTransactionsFn func(iteration int) []validator.RawTx - expectedErr error + name string + txHex string + mempoolAncestors []string + getMempoolAncestorsErr error + + expectedError error }{ { - name: "cannot find all parents", - txHex: testdata.ValidTxRawHex, - foundTransactionsFn: func(_ int) []validator.RawTx { - return []validator.RawTx{testdata.ParentTx1} - }, - expectedErr: ErrParentNotFound, + name: "tx finder returns rubbish", + txHex: fixture.ValidTxRawHex, + mempoolAncestors: []string{fixture.ParentTx1.TxID(), fixture.RandomTx1.TxID()}, + + expectedError: ErrParentNotFound, }, { - name: "tx finder returns rubbish", - txHex: testdata.ValidTxRawHex, - foundTransactionsFn: func(_ int) []validator.RawTx { - return []validator.RawTx{testdata.ParentTx1, testdata.RandomTx1} - }, - expectedErr: ErrParentNotFound, + name: "with mined parents only", + txHex: fixture.ValidTxRawHex, + mempoolAncestors: []string{fixture.ParentTx1.TxID()}, }, { - name: "with unmined parents", - txHex: testdata.ValidTxRawHex, - foundTransactionsFn: func(i int) []validator.RawTx { - if i == 0 { - p1 := testdata.ParentTx1 - p2 := testdata.ParentTx2 - - return []validator.RawTx{ - { - TxID: p1.TxID, - Bytes: p1.Bytes, - IsMined: false, - }, - { - TxID: p2.TxID, - Bytes: p2.Bytes, - IsMined: true, - }, - } - } + name: "with mined parents only", + txHex: fixture.ValidTxRawHex, + mempoolAncestors: nil, + getMempoolAncestorsErr: errors.New("some error"), - if i == 1 { - return []validator.RawTx{testdata.AncestorTx1, testdata.AncestorTx2} - } - - panic("too many calls") - }, - }, - { - name: "with mined parents only", - txHex: testdata.ValidTxRawHex, - foundTransactionsFn: func(_ int) []validator.RawTx { - return []validator.RawTx{testdata.ParentTx1, testdata.ParentTx2} - }, + expectedError: ErrFailedToGetMempoolAncestors, }, } for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { // given - var getTxsCounter int - var counterPtr = &getTxsCounter txFinder := mocks.TxFinderIMock{ - GetRawTxsFunc: func(_ context.Context, _ validator.FindSourceFlag, _ []string) ([]validator.RawTx, error) { - iteration := *counterPtr - *counterPtr = iteration + 1 - return tc.foundTransactionsFn(iteration), nil + GetMempoolAncestorsFunc: func(_ context.Context, _ []string) ([]string, error) { + return tc.mempoolAncestors, tc.getMempoolAncestorsErr + }, + GetRawTxsFunc: func(_ context.Context, _ validator.FindSourceFlag, ids []string) ([]*sdkTx.Transaction, error) { + var rawTxs []*sdkTx.Transaction + for _, id := range ids { + rawTx, ok := txMap[id] + if !ok { + continue + } + rawTxs = append(rawTxs, rawTx) + } + + return rawTxs, nil }, } tx, _ := sdkTx.NewTransactionFromHex(tc.txHex) // when - res, err := getUnminedAncestors(context.TODO(), &txFinder, tx, false) + actual, actualError := getUnminedAncestors(context.TODO(), &txFinder, tx, false) // then - require.Equal(t, tc.expectedErr, err) - if tc.expectedErr == nil { - expectedUnminedAncestors := make([]validator.RawTx, 0) - - for i := 0; i < getTxsCounter; i++ { - for _, t := range tc.foundTransactionsFn(i) { - if !t.IsMined { - expectedUnminedAncestors = append(expectedUnminedAncestors, t) - } - } - } - require.Len(t, res, len(expectedUnminedAncestors)) + if tc.expectedError != nil { + require.ErrorIs(t, actualError, tc.expectedError) + return } + + require.NoError(t, actualError) + require.Len(t, actual, 1) }) } } diff --git a/internal/validator/default/testdata/fixture.go b/internal/validator/default/testdata/fixture.go index 0d30f331f..7ad6ce1ef 100644 --- a/internal/validator/default/testdata/fixture.go +++ b/internal/validator/default/testdata/fixture.go @@ -1,84 +1,28 @@ package testdata -import ( - "encoding/hex" - - "github.com/bitcoin-sv/arc/internal/validator" -) +import sdkTx "github.com/bitcoin-sv/go-sdk/transaction" const ( - ValidTxID = "900d18f42034586544fb6876e712b2621f14ca3f7e639018ef2e4ed6e22caf25" - ValidTxRawHex = "01000000023df7585811e737e1607b95f32f01574c5a9ce49bb872528a88cb2c7a372b6dec000000006b483045022100809f8409850ed8e1a4ad0abee60fe6e60707eff40af08f55cbb5af80ec363b8e02203204193b7a563463f8844be198228d1356d1ee0a25a73662c7488f45f046936dc321033df74ab4534eef6cb47badf56c115f627e87fd6be47f426ee63a3e94920d38ccffffffffeb48bd7fb162a94b41728b3d29d5aec5f2bc56eac46c026ee053642fd6ff1acc010000006a4730440220154a0a8b4096b7137dcfde373d262f007fd43bc6a127ffc82c1b851020cdde7b02202dbefd508892de0b82a967b0e95c415981b661dc07aab86929cba07405d5d5adc32103128c33a42ed19cf84fb7342d86f9344ab966e4d182bd678ecf9adbe63b162591ffffffff0201000000000000001976a914b44bcdac5ad9657b4558da182bf0ec2ba761291588ac803f0f00000000001976a914747a796c43d2953f9fde6ccbc302f8e8a5e1417088ac00000000" - - ParentTxID1 = "cc1affd62f6453e06e026cc4ea56bcf2c5aed5293d8b72414ba962b17fbd48eb" - ParentTxRawHex1 = "0100000002e5237325a9b80a4411f72ac4605d53c723a08763853db6c37fcc990cef2e80bd000000006a47304402204c140bff029f8ce91840a56a86c135d3fee314bb684c517428176cc2c18c17780220680593f9a3710d470b688bfbd7aee128a118832dc179370049aa1411a22510ffc321033df74ab4534eef6cb47badf56c115f627e87fd6be47f426ee63a3e94920d38ccffffffff44f668c0f15f918f32b2b342602e9623a87ae8240f5560f1a680e3d018edae88010000006b483045022100eb738eed8b213a036262b050d113c801d8a91cdf23daaa127d6a933fbf2d727d022073ad1c36a1b7669fb07ad0d22719a45a9068c23a76eae1f7ef7d9417fe4caad0c32103128c33a42ed19cf84fb7342d86f9344ab966e4d182bd678ecf9adbe63b162591ffffffff0201000000000000001976a914b44bcdac5ad9657b4558da182bf0ec2ba761291588ac903f0f00000000001976a914747a796c43d2953f9fde6ccbc302f8e8a5e1417088ac00000000" + ValidTxID = "516016c90be02b335abe8fc421a4cd9f6ae24a0c865f9b9bd36b9e23c953e40d" + ValidTxRawHex = "0100000001600fa3a28128783c7d862998325e49b541b18214bf211291a8bce0ecf5b06725000000006a47304402200bf8c4755ad4108910ab1f097f1ecf161b55432236c836b5dfa8246c619b73d4022036c1e4e5d0f042da059196fe27e69f07712d0b8a68ddd20a392877c4bf01655f412102f87ce69f6ba5444aed49c34470041189c1e1060acd99341959c0594002c61bf0ffffffff01db030000000000001976a914c2b6fd4319122b9b5156a2a0060d19864c24f49a88ac00000000" - ParentTxID2 = "ec6d2b377a2ccb888a5272b89be49c5a4c57012ff3957b60e137e7115858f73d" - ParentTxRawHex2 = "010000000262a6ce5a8c0f166d78301884e2a081ff8050fa22a823401b13272e458cca3c88490000006b483045022100c99f5e0cdbe4ca86d5bc449a30b58d4ae4b991965217588c5043e5b483efc46d022004e0ea9161253338a759b2851a0af97f52e72ce562debd86e1cdecd6865a3ec5c32102cff8e38e0e1924f5ac8621eb68f64e7b2094e05f9a7a4de833d56ceab00ff5eeffffffff97f04aef1b7f0eaad59f4e185eda32c5e555a685888eb82bed8b9f055300d00c1f0000006b483045022100d598aa21fc2184fda7e688327d535998ac59b8d58a9e95edf5cca05bfe608d120220328688bcb2617f01148ad3f6ab3ea469bc531c61646297abde9dcc27fb4657b7c321035d16a59a5366917098ddb79172b60a81b1ac7a85324621339c3b6c04cb8df10cffffffff0201000000000000001976a914aceaa49221d8199c8cf969caf955d1d977b4373a88acc1030000000000001976a9145795b5515cb3a30c5fcb22bb8c44e7681158310888ac00000000" + ParentTxID1 = "2567b0f5ece0bca8911221bf1482b141b5495e329829867d3c782881a2a30f60" // parent of ValidTx + ParentTxRawHex1 = "010000000103f03f1e80e818f863240e94564ed3a82cddfb7d79d4d831997b649a65557a14000000006b483045022100f025e05513d953e0dc0ee3a9d0da8d07b8653df8ff71cec61aeb86f00607a6b5022066b12631c98f2b9479daf501296d8c384c76058e8f041f7ade678106bfdf45fb412102f87ce69f6ba5444aed49c34470041189c1e1060acd99341959c0594002c61bf0ffffffff01dc030000000000001976a914c2b6fd4319122b9b5156a2a0060d19864c24f49a88ac00000000" - AncestorTxID1 = "88aeed18d0e380a6f160550f24e87aa823962e6042b3b2328f915ff1c068f644" - AncestorTxRawHex1 = "0100000002c764855acb56af959e4352123d55c961c57f6d093042153242ab3e05ebbdf7b7000000006a47304402202840b62d9ed60551dbe037cfa78ebccc1e8a03096ec472ba930dc3cd21eaad9d02204503b4a7deed4ffe4805f5e0608a0b421401955db6ed7c711cba4bd7bcc52835c321033df74ab4534eef6cb47badf56c115f627e87fd6be47f426ee63a3e94920d38ccffffffff1ba7c60827c39ea968571788a62cb6c447535b72b37a691bc0b95e8bc3d3bd6c010000006b483045022100c57b5f37f4a39718f0c0ceeebc7e09bdb0f7176b31491bcda1e89fba678d4f140220571ef8965e5385f451c04685a6f41c84cd425e6af9960c56d2f409d2e9884e58c32103128c33a42ed19cf84fb7342d86f9344ab966e4d182bd678ecf9adbe63b162591ffffffff0201000000000000001976a914b44bcdac5ad9657b4558da182bf0ec2ba761291588aca03f0f00000000001976a914747a796c43d2953f9fde6ccbc302f8e8a5e1417088ac00000000" + AncestorTxID1 = "147a55659a647b9931d8d4797dfbdd2ca8d34e56940e2463f818e8801e3ff003" // parent of ParentTx + AncestorTxRawHex1 = "0100000001ab2e0b0d77126781e3f20f9577b711529229315b5066185382ee74eb6a3057a8000000006a4730440220132be7364b62755fcbafed814492bb12ccb17a11a3df6e76589453a22e35c4ba02206bd0dd40b68cad7bb611e84934b94d13f0db84042038c734431126b040991cae412102f87ce69f6ba5444aed49c34470041189c1e1060acd99341959c0594002c61bf0ffffffff01dd030000000000001976a914c2b6fd4319122b9b5156a2a0060d19864c24f49a88ac00000000" - AncestorTxID2 = "bd802eef0c99cc7fc3b63d856387a023c7535d60c42af711440ab8a9257323e5" - AncestorTxRawHex2 = "010000000262a6ce5a8c0f166d78301884e2a081ff8050fa22a823401b13272e458cca3c88040000006b483045022100bfd7cd3b163e9189f651dbeacbb1a22795487a10b2c2b584bc972cf691012a9102201805a9be04a5b5b960b9f5d65d267c4e375abca7ae648d732e787f4a7ff77e3cc32102cff8e38e0e1924f5ac8621eb68f64e7b2094e05f9a7a4de833d56ceab00ff5eeffffffff97f04aef1b7f0eaad59f4e185eda32c5e555a685888eb82bed8b9f055300d00c380000006a47304402204535ba866eab61e0869b66a963413af2bb228bc069158c58eff94ccb39a5775602202da81ab79688e5049093541a4b4998aecae1ab1facdf258a932949671d3e7b8ec321035d16a59a5366917098ddb79172b60a81b1ac7a85324621339c3b6c04cb8df10cffffffff0201000000000000001976a914aceaa49221d8199c8cf969caf955d1d977b4373a88acc1030000000000001976a9145795b5515cb3a30c5fcb22bb8c44e7681158310888ac00000000" - - AncestorOfAncestorTx1ID1 = "b7f7bdeb053eab4232154230096d7fc561c9553d1252439e95af56cb5a8564c7" - AncestorOfAncestorTx1RawHex1 = "01000000028e72e02a087d0fcd0b41d26bddc631253893e92820210087481a1926e380331f5d0000006a473044022077fd2c5f31fd4dfa443f984d633c9d30197cfbee76e65b953c9bccc2c857945e0220470f66132be8e8641b88c346aaff5b93e6f03a2f5b726693a83b3f819e332706c32102cff8e38e0e1924f5ac8621eb68f64e7b2094e05f9a7a4de833d56ceab00ff5eeffffffffbba4d0c692678475dfb4fc5bf269b697bf1276258fa3081885805840c773be10a00000006a4730440220543453d1473ec81e0f6b97c04113d688cee0cc263e3086b46bf287f4b817d8400220131803900b89198de553ec95496ab5e4d30ace1fe6e5a7a5895a9f4595867247c321035d16a59a5366917098ddb79172b60a81b1ac7a85324621339c3b6c04cb8df10cffffffff0201000000000000001976a914aceaa49221d8199c8cf969caf955d1d977b4373a88acc1030000000000001976a9145795b5515cb3a30c5fcb22bb8c44e7681158310888ac00000000" - - AncestorOfAncestorTx1ID2 = "6cbdd3c38b5eb9c01b697ab3725b5347c4b62ca688175768a99ec32708c6a71b" - AncestorOfAncestorTx1RawHex2 = "0100000002412b690ccfa8bb33174e048bf31bb2d0aa15183610387d26c2294ecfe9470c04000000006a4730440220705b098b6e9c011f91b4f0508fc27bfdeea24ab96950f8ef5033d55e8f04457f02203f716c8f2511789c89a05f911184c32eddf07b2f914ec91959e6f80301947202c321033df74ab4534eef6cb47badf56c115f627e87fd6be47f426ee63a3e94920d38ccffffffff1c3447803b8d55ef31a401f64ae0a90e47c927f556c6a60ce9de55c30ef77580010000006b483045022100856e2a5d3d9cf92ba69310b8931d6665a488cfa36c819159e17282dcd9ad4f5b02203485d73daf57cb16ff96398d0c4430c26ee4c78b5828a7cbc86b4c79dd3fb1ebc32103128c33a42ed19cf84fb7342d86f9344ab966e4d182bd678ecf9adbe63b162591ffffffff0201000000000000001976a914b44bcdac5ad9657b4558da182bf0ec2ba761291588acb03f0f00000000001976a914747a796c43d2953f9fde6ccbc302f8e8a5e1417088ac00000000" + AncestorOfAncestorTx1ID1 = "a857306aeb74ee82531866505b3129925211b777950ff2e3816712770d0b2eab" // parent of AncestorTx1 + AncestorOfAncestorTx1RawHex1 = "0100000001c756be83382ebf65aeb3de9a8a40f6054572580d50b834d760b7f04df58d807f000000006a473044022056c1412bc6dd1e590c1949c79b79b13363744443f3c932f0dff5adf0c39902ce022039d7548625f5c97e3a3c31a994f9024a455d2183a081ab9acb1ca11380a37427412102f87ce69f6ba5444aed49c34470041189c1e1060acd99341959c0594002c61bf0ffffffff01de030000000000001976a914c2b6fd4319122b9b5156a2a0060d19864c24f49a88ac00000000" RandomTxID1 = "519e7f337268d682eb1bf512e4f57b3380fc9adc18d19e2fc9abdb42872cc1d6" RandomTxRawHex1 = "010000000199f9598199fcdad12290d2c3edb695e1887d1db7f8029af6bf18644079b051c9000000006b483045022100a0281159947cd9fbe2bdc1f485b573d7802de632b57556346789ad32c533c96002201d091dd3430cbb7e49520f894c05353ae07d8c315aa59000d125099a410431054121030d4fc707a6d8a7dfecaa68e03e60a6f450ac1f7d7b73277a5e215effa6c91982ffffffff07404b4c00000000001976a9145eff88f5b70b7cd70c1fd77c6710da349cf8ca1b88ac40420f00000000001976a9141dc792e37b026c2adc2cd3b861965c4c6a8f6dde88ac400d0300000000001976a9144667e5157c8e3fe1ae97b724a2e2098fcdc6f74c88ac50c30000000000001976a914c88a4c29090691126b210efc0de5e409282b581a88ac204e0000000000001976a914bc2b8015f25c31de0e04f0c1c092a914f96794c288acd0070000000000001976a914878e7d66085e49dee31765ea0abb7e6d1d91223e88aceef20100000000001976a914853b8b2182c24d810e82f5c88f2064ebe01e8c7f88ac00000000" ) var ( - parentTxBytes1, _ = hex.DecodeString(ParentTxRawHex1) - ParentTx1 = validator.RawTx{ - TxID: ParentTxID1, - Bytes: parentTxBytes1, - IsMined: true, - } - - parentTxBytes2, _ = hex.DecodeString(ParentTxRawHex2) - ParentTx2 = validator.RawTx{ - TxID: ParentTxID2, - Bytes: parentTxBytes2, - IsMined: true, - } - - randomTxBytes1, _ = hex.DecodeString(RandomTxRawHex1) - RandomTx1 = validator.RawTx{ - TxID: RandomTxID1, - Bytes: randomTxBytes1, - IsMined: true, - } - - ancestorTxBytes1, _ = hex.DecodeString(AncestorTxRawHex1) - AncestorTx1 = validator.RawTx{ - TxID: AncestorTxID1, - Bytes: ancestorTxBytes1, - IsMined: true, - } - - ancestorTxBytes2, _ = hex.DecodeString(AncestorTxRawHex2) - AncestorTx2 = validator.RawTx{ - TxID: AncestorTxID2, - Bytes: ancestorTxBytes2, - IsMined: true, - } - - ancestorOfAncestorTxBytes1, _ = hex.DecodeString(AncestorOfAncestorTx1RawHex1) - AncestorOfAncestorTx1 = validator.RawTx{ - TxID: AncestorOfAncestorTx1ID1, - Bytes: ancestorOfAncestorTxBytes1, - IsMined: true, - } - - ancestorOfAncestorTxBytes2, _ = hex.DecodeString(AncestorOfAncestorTx1RawHex2) - AncestorOfAncestorTx2 = validator.RawTx{ - TxID: AncestorOfAncestorTx1ID2, - Bytes: ancestorOfAncestorTxBytes2, - IsMined: true, - } + RandomTx1, _ = sdkTx.NewTransactionFromHex(RandomTxRawHex1) + ValidTx, _ = sdkTx.NewTransactionFromHex(ValidTxRawHex) + ParentTx1, _ = sdkTx.NewTransactionFromHex(ParentTxRawHex1) + AncestorTx1, _ = sdkTx.NewTransactionFromHex(AncestorTxRawHex1) + AncestorOfAncestor1Tx1, _ = sdkTx.NewTransactionFromHex(AncestorOfAncestorTx1RawHex1) ) diff --git a/internal/validator/helpers.go b/internal/validator/helpers.go index e98106416..ef663b24e 100644 --- a/internal/validator/helpers.go +++ b/internal/validator/helpers.go @@ -3,6 +3,8 @@ package validator import ( "context" + sdkTx "github.com/bitcoin-sv/go-sdk/transaction" + "github.com/bitcoin-sv/arc/internal/beef" ) @@ -27,13 +29,8 @@ func (flag FindSourceFlag) Has(v FindSourceFlag) bool { } type TxFinderI interface { - GetRawTxs(ctx context.Context, source FindSourceFlag, ids []string) ([]RawTx, error) -} - -type RawTx struct { - TxID string - Bytes []byte - IsMined bool + GetRawTxs(ctx context.Context, source FindSourceFlag, ids []string) ([]*sdkTx.Transaction, error) + GetMempoolAncestors(ctx context.Context, ids []string) ([]string, error) } type MerkleVerifierI interface { diff --git a/internal/validator/mocks/tx_finder_interface_mock.go b/internal/validator/mocks/tx_finder_interface_mock.go index 8f12075a0..fdc13fbe4 100644 --- a/internal/validator/mocks/tx_finder_interface_mock.go +++ b/internal/validator/mocks/tx_finder_interface_mock.go @@ -6,6 +6,7 @@ package mocks import ( "context" "github.com/bitcoin-sv/arc/internal/validator" + sdkTx "github.com/bitcoin-sv/go-sdk/transaction" "sync" ) @@ -19,7 +20,10 @@ var _ validator.TxFinderI = &TxFinderIMock{} // // // make and configure a mocked validator.TxFinderI // mockedTxFinderI := &TxFinderIMock{ -// GetRawTxsFunc: func(ctx context.Context, source validator.FindSourceFlag, ids []string) ([]validator.RawTx, error) { +// GetMempoolAncestorsFunc: func(ctx context.Context, ids []string) ([]string, error) { +// panic("mock out the GetMempoolAncestors method") +// }, +// GetRawTxsFunc: func(ctx context.Context, source validator.FindSourceFlag, ids []string) ([]*sdkTx.Transaction, error) { // panic("mock out the GetRawTxs method") // }, // } @@ -29,11 +33,21 @@ var _ validator.TxFinderI = &TxFinderIMock{} // // } type TxFinderIMock struct { + // GetMempoolAncestorsFunc mocks the GetMempoolAncestors method. + GetMempoolAncestorsFunc func(ctx context.Context, ids []string) ([]string, error) + // GetRawTxsFunc mocks the GetRawTxs method. - GetRawTxsFunc func(ctx context.Context, source validator.FindSourceFlag, ids []string) ([]validator.RawTx, error) + GetRawTxsFunc func(ctx context.Context, source validator.FindSourceFlag, ids []string) ([]*sdkTx.Transaction, error) // calls tracks calls to the methods. calls struct { + // GetMempoolAncestors holds details about calls to the GetMempoolAncestors method. + GetMempoolAncestors []struct { + // Ctx is the ctx argument value. + Ctx context.Context + // Ids is the ids argument value. + Ids []string + } // GetRawTxs holds details about calls to the GetRawTxs method. GetRawTxs []struct { // Ctx is the ctx argument value. @@ -44,11 +58,48 @@ type TxFinderIMock struct { Ids []string } } - lockGetRawTxs sync.RWMutex + lockGetMempoolAncestors sync.RWMutex + lockGetRawTxs sync.RWMutex +} + +// GetMempoolAncestors calls GetMempoolAncestorsFunc. +func (mock *TxFinderIMock) GetMempoolAncestors(ctx context.Context, ids []string) ([]string, error) { + if mock.GetMempoolAncestorsFunc == nil { + panic("TxFinderIMock.GetMempoolAncestorsFunc: method is nil but TxFinderI.GetMempoolAncestors was just called") + } + callInfo := struct { + Ctx context.Context + Ids []string + }{ + Ctx: ctx, + Ids: ids, + } + mock.lockGetMempoolAncestors.Lock() + mock.calls.GetMempoolAncestors = append(mock.calls.GetMempoolAncestors, callInfo) + mock.lockGetMempoolAncestors.Unlock() + return mock.GetMempoolAncestorsFunc(ctx, ids) +} + +// GetMempoolAncestorsCalls gets all the calls that were made to GetMempoolAncestors. +// Check the length with: +// +// len(mockedTxFinderI.GetMempoolAncestorsCalls()) +func (mock *TxFinderIMock) GetMempoolAncestorsCalls() []struct { + Ctx context.Context + Ids []string +} { + var calls []struct { + Ctx context.Context + Ids []string + } + mock.lockGetMempoolAncestors.RLock() + calls = mock.calls.GetMempoolAncestors + mock.lockGetMempoolAncestors.RUnlock() + return calls } // GetRawTxs calls GetRawTxsFunc. -func (mock *TxFinderIMock) GetRawTxs(ctx context.Context, source validator.FindSourceFlag, ids []string) ([]validator.RawTx, error) { +func (mock *TxFinderIMock) GetRawTxs(ctx context.Context, source validator.FindSourceFlag, ids []string) ([]*sdkTx.Transaction, error) { if mock.GetRawTxsFunc == nil { panic("TxFinderIMock.GetRawTxsFunc: method is nil but TxFinderI.GetRawTxs was just called") } diff --git a/pkg/api/handler/default_test.go b/pkg/api/handler/default_test.go index 0026a99ea..679b6870e 100644 --- a/pkg/api/handler/default_test.go +++ b/pkg/api/handler/default_test.go @@ -833,8 +833,6 @@ func TestPOSTTransactions(t *testing.T) { //nolint:funlen }, } - //arcConfig, err := config.Load() - //require.NoError(t, err, "could not load default config") finder := &mocks.TxFinderIMock{GetRawTxsFunc: func(_ context.Context, _ validator.FindSourceFlag, _ []string) ([]*sdkTx.Transaction, error) { return nil, errors.New("error getting raw transactions") }} diff --git a/pkg/api/tx_finder/tx_finder.go b/pkg/api/tx_finder/tx_finder.go index c1ea47228..58bda94b2 100644 --- a/pkg/api/tx_finder/tx_finder.go +++ b/pkg/api/tx_finder/tx_finder.go @@ -2,33 +2,39 @@ package txfinder import ( "context" - "encoding/hex" "errors" "fmt" "log/slog" - "net/url" "runtime" - "github.com/ordishs/go-bitcoin" + sdkTx "github.com/bitcoin-sv/go-sdk/transaction" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" - "github.com/bitcoin-sv/arc/config" "github.com/bitcoin-sv/arc/internal/tracing" "github.com/bitcoin-sv/arc/internal/validator" "github.com/bitcoin-sv/arc/internal/woc_client" "github.com/bitcoin-sv/arc/pkg/metamorph" ) +var ( + ErrFailedToGetMempoolAncestors = errors.New("failed to get mempool ancestors") +) + type Finder struct { transactionHandler metamorph.TransactionHandler - bitcoinClient *bitcoin.Bitcoind + bitcoinClient NodeClient wocClient *woc_client.WocClient logger *slog.Logger tracingEnabled bool tracingAttributes []attribute.KeyValue } +type NodeClient interface { + GetMempoolAncestors(ctx context.Context, ids []string) ([]string, error) + GetRawTransaction(ctx context.Context, id string) (*sdkTx.Transaction, error) +} + func WithTracerFinder(attr ...attribute.KeyValue) func(s *Finder) { return func(p *Finder) { p.tracingEnabled = true @@ -42,22 +48,8 @@ func WithTracerFinder(attr ...attribute.KeyValue) func(s *Finder) { } } -func New(th metamorph.TransactionHandler, pc *config.PeerRPCConfig, w *woc_client.WocClient, l *slog.Logger, opts ...func(f *Finder)) *Finder { +func New(th metamorph.TransactionHandler, n NodeClient, w *woc_client.WocClient, l *slog.Logger, opts ...func(f *Finder)) *Finder { l = l.With(slog.String("module", "tx-finder")) - var n *bitcoin.Bitcoind - - if pc != nil { - rpcURL, err := url.Parse(fmt.Sprintf("rpc://%s:%s@%s:%d", pc.User, pc.Password, pc.Host, pc.Port)) - if err != nil { - l.Warn("cannot parse node rpc url. Finder will not use node as source") - } else { - // get the transaction from the bitcoin node rpc - n, err = bitcoin.NewFromURL(rpcURL, false) - if err != nil { - l.Warn("cannot create node client. Finder will not use node as source") - } - } - } f := &Finder{ transactionHandler: th, @@ -73,133 +65,102 @@ func New(th metamorph.TransactionHandler, pc *config.PeerRPCConfig, w *woc_clien return f } -func (f Finder) GetRawTxs(ctx context.Context, source validator.FindSourceFlag, ids []string) ([]validator.RawTx, error) { +func (f Finder) GetMempoolAncestors(ctx context.Context, ids []string) ([]string, error) { + txIDs, err := f.bitcoinClient.GetMempoolAncestors(ctx, ids) + if err != nil { + return nil, errors.Join(ErrFailedToGetMempoolAncestors, err) + } + + return txIDs, nil +} + +func (f Finder) GetRawTxs(ctx context.Context, source validator.FindSourceFlag, ids []string) ([]*sdkTx.Transaction, error) { ctx, span := tracing.StartTracing(ctx, "Finder_GetRawTxs", f.tracingEnabled, f.tracingAttributes...) defer tracing.EndTracing(span) // NOTE: we can ignore ALL errors from providers, if one returns err we go to another - foundTxs := make([]validator.RawTx, 0, len(ids)) - var remainingIDs []string + foundTxs := make([]*sdkTx.Transaction, 0, len(ids)) + remainingIDs := map[string]struct{}{} + + for _, id := range ids { + remainingIDs[id] = struct{}{} + } // first get transactions from the handler if source.Has(validator.SourceTransactionHandler) { var thGetRawTxSpan trace.Span ctx, thGetRawTxSpan = tracing.StartTracing(ctx, "TransactionHandler_GetRawTxs", f.tracingEnabled, f.tracingAttributes...) - txs, thErr := f.transactionHandler.GetTransactions(ctx, ids) + txs, err := f.transactionHandler.GetTransactions(ctx, ids) tracing.EndTracing(thGetRawTxSpan) - for _, tx := range txs { - rt := validator.RawTx{ - TxID: tx.TxID, - Bytes: tx.Bytes, - IsMined: tx.BlockHeight > 0, - } - - foundTxs = append(foundTxs, rt) - } + if err != nil { + f.logger.WarnContext(ctx, "failed to get transactions from TransactionHandler", slog.Any("ids", ids), slog.Any("err", err)) + } else { + for _, tx := range txs { + var rt *sdkTx.Transaction + rt, err = sdkTx.NewTransactionFromBytes(tx.Bytes) + if err != nil { + f.logger.Error("failed to parse TransactionHandler tx bytes to transaction", slog.Any("id", tx.TxID), slog.String("err", err.Error())) + continue + } - // add remaining ids - remainingIDs = outerRightJoin(foundTxs, ids) - if len(remainingIDs) > 0 || thErr != nil { - f.logger.WarnContext(ctx, "couldn't find transactions in TransactionHandler", slog.Any("ids", remainingIDs), slog.Any("source-err", thErr)) + delete(remainingIDs, tx.TxID) + foundTxs = append(foundTxs, rt) + } } - - ids = remainingIDs[:] - remainingIDs = nil } + ids = getKeys(remainingIDs) + // try to get remaining txs from the node if source.Has(validator.SourceNodes) && f.bitcoinClient != nil { - var nErr error for _, id := range ids { - var bitcoinGetRawTxSpan trace.Span - ctx, bitcoinGetRawTxSpan = tracing.StartTracing(ctx, "Bitcoind_GetRawTxs", f.tracingEnabled, f.tracingAttributes...) - nTx, err := f.bitcoinClient.GetRawTransaction(id) - tracing.EndTracing(bitcoinGetRawTxSpan) + rawTx, err := f.bitcoinClient.GetRawTransaction(ctx, id) if err != nil { - nErr = errors.Join(nErr, fmt.Errorf("%s: %w", id, err)) - } - if nTx != nil { - rt, e := newRawTx(nTx.TxID, nTx.Hex, nTx.BlockHeight) - if e != nil { - return nil, e - } - - foundTxs = append(foundTxs, rt) - } else { - remainingIDs = append(remainingIDs, id) + f.logger.WarnContext(ctx, "failed to get transactions from bitcoin client", slog.String("id", id), slog.Any("err", err)) + continue } - } - if len(remainingIDs) > 0 || nErr != nil { - f.logger.WarnContext(ctx, "couldn't find transactions in node", slog.Any("ids", remainingIDs), slog.Any("source-error", nErr)) + delete(remainingIDs, id) + foundTxs = append(foundTxs, rawTx) } - - ids = remainingIDs[:] - remainingIDs = nil } + ids = getKeys(remainingIDs) + // at last try the WoC if source.Has(validator.SourceWoC) && len(ids) > 0 { var wocSpan trace.Span ctx, wocSpan = tracing.StartTracing(ctx, "WocClient_GetRawTxs", f.tracingEnabled, f.tracingAttributes...) - wocTxs, wocErr := f.wocClient.GetRawTxs(ctx, ids) + wocTxs, err := f.wocClient.GetRawTxs(ctx, ids) defer tracing.EndTracing(wocSpan) + if err != nil { + f.logger.WarnContext(ctx, "failed to get transactions from WoC", slog.Any("ids", ids), slog.Any("err", err)) + } else { + for _, wTx := range wocTxs { + if wTx.Error != "" { + f.logger.WarnContext(ctx, "WoC tx reports error", slog.Any("id", ids), slog.Any("err", wTx.Error)) + continue + } + tx, err := sdkTx.NewTransactionFromHex(wTx.Hex) + if err != nil { + return nil, fmt.Errorf("failed to parse WoC hex string to transaction: %v", err) + } - for _, wTx := range wocTxs { - if wTx.Error != "" { - wocErr = errors.Join(wocErr, fmt.Errorf("returned error data tx %s: %s", wTx.TxID, wTx.Error)) - continue - } - - rt, e := newRawTx(wTx.TxID, wTx.Hex, wTx.BlockHeight) - if e != nil { - return nil, e + foundTxs = append(foundTxs, tx) } - - foundTxs = append(foundTxs, rt) - } - - // add remaining ids - remainingIDs = outerRightJoin(foundTxs, ids) - if len(remainingIDs) > 0 || wocErr != nil { - f.logger.WarnContext(ctx, "couldn't find transactions in WoC", slog.Any("ids", remainingIDs), slog.Any("source-error", wocErr)) } } return foundTxs, nil } -func newRawTx(id, hexTx string, blockH uint64) (validator.RawTx, error) { - b, e := hex.DecodeString(hexTx) - if e != nil { - return validator.RawTx{}, e - } - - rt := validator.RawTx{ - TxID: id, - Bytes: b, - IsMined: blockH > 0, - } - - return rt, nil -} - -func outerRightJoin(left []validator.RawTx, right []string) []string { - var outerRight []string - - for _, id := range right { - found := false - for _, tx := range left { - if tx.TxID == id { - found = true - break - } - } - - if !found { - outerRight = append(outerRight, id) - } +func getKeys(uniqueMap map[string]struct{}) []string { + uniqueKeys := make([]string, len(uniqueMap)) + counter := 0 + for id := range uniqueMap { + uniqueKeys[counter] = id + counter++ } - return outerRight + return uniqueKeys } From 8eed668b54205ec3c717dd322cb6e47f87cdb9ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20B=C3=B6ckli?= Date: Fri, 15 Nov 2024 14:24:12 +0100 Subject: [PATCH 3/3] fix(ARCO-212): Feedback --- cmd/arc/services/api.go | 14 ++++++++------ .../api => internal}/tx_finder/cached_tx_finder.go | 12 ++---------- .../tx_finder/cached_tx_finder_test.go | 4 ++-- {pkg/api => internal}/tx_finder/tx_finder.go | 0 {pkg/api => internal}/tx_finder/tx_finder_test.go | 0 internal/validator/default/helpers.go | 9 +++++---- 6 files changed, 17 insertions(+), 22 deletions(-) rename {pkg/api => internal}/tx_finder/cached_tx_finder.go (83%) rename {pkg/api => internal}/tx_finder/cached_tx_finder_test.go (96%) rename {pkg/api => internal}/tx_finder/tx_finder.go (100%) rename {pkg/api => internal}/tx_finder/tx_finder_test.go (100%) diff --git a/cmd/arc/services/api.go b/cmd/arc/services/api.go index 21037be9c..cef6a813f 100644 --- a/cmd/arc/services/api.go +++ b/cmd/arc/services/api.go @@ -26,10 +26,10 @@ import ( "github.com/bitcoin-sv/arc/internal/metamorph/metamorph_api" "github.com/bitcoin-sv/arc/internal/node_client" "github.com/bitcoin-sv/arc/internal/tracing" + tx_finder "github.com/bitcoin-sv/arc/internal/tx_finder" "github.com/bitcoin-sv/arc/internal/woc_client" "github.com/bitcoin-sv/arc/pkg/api" "github.com/bitcoin-sv/arc/pkg/api/handler" - txfinder "github.com/bitcoin-sv/arc/pkg/api/tx_finder" "github.com/bitcoin-sv/arc/pkg/blocktx" "github.com/bitcoin-sv/arc/pkg/metamorph" ) @@ -57,7 +57,8 @@ func StartAPIServer(logger *slog.Logger, arcConfig *config.ArcConfig) (func(), e apiOpts := []handler.Option{ handler.WithCallbackURLRestrictions(arcConfig.Metamorph.RejectCallbackContaining), } - var finderOpts []func(f *txfinder.CachedFinder) + var cachedFinderOpts []func(f *tx_finder.CachedFinder) + var finderOpts []func(f *tx_finder.Finder) var nodeClientOpts []func(client *node_client.NodeClient) shutdownFns := make([]func(), 0) @@ -72,7 +73,8 @@ func StartAPIServer(logger *slog.Logger, arcConfig *config.ArcConfig) (func(), e mtmOpts = append(mtmOpts, metamorph.WithTracer(arcConfig.Tracing.KeyValueAttributes...)) apiOpts = append(apiOpts, handler.WithTracer(arcConfig.Tracing.KeyValueAttributes...)) - finderOpts = append(finderOpts, txfinder.WithTracerCachedFinder(arcConfig.Tracing.KeyValueAttributes...)) + cachedFinderOpts = append(cachedFinderOpts, tx_finder.WithTracerCachedFinder(arcConfig.Tracing.KeyValueAttributes...)) + finderOpts = append(finderOpts, tx_finder.WithTracerFinder(arcConfig.Tracing.KeyValueAttributes...)) nodeClientOpts = append(nodeClientOpts, node_client.WithTracer(arcConfig.Tracing.KeyValueAttributes...)) } @@ -106,7 +108,6 @@ func StartAPIServer(logger *slog.Logger, arcConfig *config.ArcConfig) (func(), e return nil, fmt.Errorf("failed to parse node rpc url: %w", err) } - // get the transaction from the bitcoin node rpc bitcoinClient, err := bitcoin.NewFromURL(rpcURL, false) if err != nil { return nil, fmt.Errorf("failed to create bitcoin client: %w", err) @@ -117,9 +118,10 @@ func StartAPIServer(logger *slog.Logger, arcConfig *config.ArcConfig) (func(), e return nil, fmt.Errorf("failed to create node client: %v", err) } - finder := txfinder.NewCached(metamorphClient, nodeClient, wocClient, logger, finderOpts...) + finder := tx_finder.New(metamorphClient, nodeClient, wocClient, logger, finderOpts...) + cachedFinder := tx_finder.NewCached(finder, cachedFinderOpts...) - apiHandler, err := handler.NewDefault(logger, metamorphClient, blockTxClient, policy, finder, apiOpts...) + apiHandler, err := handler.NewDefault(logger, metamorphClient, blockTxClient, policy, cachedFinder, apiOpts...) if err != nil { return nil, err } diff --git a/pkg/api/tx_finder/cached_tx_finder.go b/internal/tx_finder/cached_tx_finder.go similarity index 83% rename from pkg/api/tx_finder/cached_tx_finder.go rename to internal/tx_finder/cached_tx_finder.go index d7bfcdfba..aaefca436 100644 --- a/pkg/api/tx_finder/cached_tx_finder.go +++ b/internal/tx_finder/cached_tx_finder.go @@ -2,7 +2,6 @@ package txfinder import ( "context" - "log/slog" "runtime" "time" @@ -12,8 +11,6 @@ import ( "github.com/bitcoin-sv/arc/internal/tracing" "github.com/bitcoin-sv/arc/internal/validator" - "github.com/bitcoin-sv/arc/internal/woc_client" - "github.com/bitcoin-sv/arc/pkg/metamorph" ) const ( @@ -48,20 +45,15 @@ func WithCacheStore(store *cache.Cache) func(s *CachedFinder) { } } -func NewCached(th metamorph.TransactionHandler, n NodeClient, w *woc_client.WocClient, l *slog.Logger, opts ...func(f *CachedFinder)) CachedFinder { +func NewCached(finder *Finder, opts ...func(f *CachedFinder)) CachedFinder { c := CachedFinder{ cacheStore: cache.New(cacheExpiration, cacheCleanup), + finder: finder, } for _, opt := range opts { opt(&c) } - var finderOpts []func(f *Finder) - if c.tracingEnabled { - finderOpts = append(finderOpts, WithTracerFinder(c.tracingAttributes...)) - } - - c.finder = New(th, n, w, l, finderOpts...) return c } diff --git a/pkg/api/tx_finder/cached_tx_finder_test.go b/internal/tx_finder/cached_tx_finder_test.go similarity index 96% rename from pkg/api/tx_finder/cached_tx_finder_test.go rename to internal/tx_finder/cached_tx_finder_test.go index cc48e1d8a..ed7170fa3 100644 --- a/pkg/api/tx_finder/cached_tx_finder_test.go +++ b/internal/tx_finder/cached_tx_finder_test.go @@ -56,9 +56,9 @@ func TestCachedFinder_GetRawTxs_AllFromCache(t *testing.T) { for _, r := range tc.cachedTx { c.Set(r.TxID(), r, cache.DefaultExpiration) } - logger := slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{Level: slog.LevelDebug})) - sut := NewCached(thMq, nil, nil, logger, WithCacheStore(c)) + finder := New(thMq, nil, nil, logger) + sut := NewCached(finder, WithCacheStore(c)) // when // try to find in cache or with TransactionHandler only diff --git a/pkg/api/tx_finder/tx_finder.go b/internal/tx_finder/tx_finder.go similarity index 100% rename from pkg/api/tx_finder/tx_finder.go rename to internal/tx_finder/tx_finder.go diff --git a/pkg/api/tx_finder/tx_finder_test.go b/internal/tx_finder/tx_finder_test.go similarity index 100% rename from pkg/api/tx_finder/tx_finder_test.go rename to internal/tx_finder/tx_finder_test.go diff --git a/internal/validator/default/helpers.go b/internal/validator/default/helpers.go index 6cefc1b67..07afc2660 100644 --- a/internal/validator/default/helpers.go +++ b/internal/validator/default/helpers.go @@ -25,9 +25,10 @@ func extendTx(ctx context.Context, txFinder validator.TxFinderI, rawTx *sdkTx.Tr // potential improvement: implement version for the rawTx with only one input // get distinct parents + parentsIDs := make([]string, 0, len(rawTx.Inputs)) + // map parentID with inputs collection to avoid duplication and simplify later processing parentInputMap := make(map[string][]*sdkTx.TransactionInput) - parentsIDs := make([]string, 0, len(rawTx.Inputs)) for _, in := range rawTx.Inputs { prevTxID := in.PreviousTxIDStr() @@ -90,9 +91,8 @@ func getUnminedAncestors(ctx context.Context, txFinder validator.TxFinderI, tx * unmindedAncestorsSet := make(map[string]*sdkTx.Transaction) // get distinct parents - // map parentID with inputs collection to avoid duplication and simplify later processing - parentInputMap := make(map[string]struct{}) parentsIDs := make([]string, 0, len(tx.Inputs)) + parentInputMap := make(map[string]struct{}) for _, in := range tx.Inputs { prevTxID := in.PreviousTxIDStr() @@ -109,7 +109,8 @@ func getUnminedAncestors(ctx context.Context, txFinder validator.TxFinderI, tx * allTxIDs := append(parentsIDs, mempoolAncestorTxIDs...) - ancestorTxs, err := txFinder.GetRawTxs(ctx, validator.SourceNodes, allTxIDs) + const finderSource = validator.SourceTransactionHandler | validator.SourceNodes | validator.SourceWoC + ancestorTxs, err := txFinder.GetRawTxs(ctx, finderSource, allTxIDs) if err != nil { return nil, errors.Join(ErrFailedToGetRawTxs, err) }