From 8780b022e434a474c7393926dda6f295edea5a4d Mon Sep 17 00:00:00 2001 From: Arkadiusz Osowski Date: Mon, 8 Jul 2024 23:12:33 +0200 Subject: [PATCH 1/4] ARCO-155: unify transaction processing endpoints logic in default api handler --- pkg/api/handler/default.go | 226 ++++++++++++++----------------------- pkg/api/handler/parsers.go | 4 + 2 files changed, 91 insertions(+), 139 deletions(-) diff --git a/pkg/api/handler/default.go b/pkg/api/handler/default.go index 4612827ca..afb2d9869 100644 --- a/pkg/api/handler/default.go +++ b/pkg/api/handler/default.go @@ -146,22 +146,22 @@ func (m ArcDefaultHandler) POSTTransaction(ctx echo.Context, params api.POSTTran return ctx.JSON(e.Status, e) } - var transaction *bt.Tx - var response *api.TransactionResponse - var responseErr *api.ErrorFields + txs, outputs, failOutputs, e := m.processTransactions(ctx.Request().Context(), transactionHex, transactionOptions) - hexFormat := validator.GetHexFormat(transactionHex) - if hexFormat == validator.BeefHex { - transaction, response, responseErr = m.processBEEFTransaction(ctx.Request().Context(), transactionHex, transactionOptions) - } else { - transaction, response, responseErr = m.processEFTransaction(ctx.Request().Context(), transactionHex, transactionOptions) + if e != nil { + // if an error is returned, the processing failed, and we should return a 500 error + return ctx.JSON(e.Status, e) } - if responseErr != nil { + if len(failOutputs) > 0 { + e = failOutputs[0] // if an error is returned, the processing failed, and we should return a 500 error - return ctx.JSON(responseErr.Status, responseErr) + return ctx.JSON(e.Status, e) } + transaction := txs[0] + response := outputs[0] + sizingInfo := make([][]uint64, 1) normalBytes, dataBytes, feeAmount := getSizings(transaction) sizingInfo[0] = []uint64{normalBytes, dataBytes, feeAmount} @@ -215,14 +215,14 @@ func (m ArcDefaultHandler) POSTTransactions(ctx echo.Context, params api.POSTTra return ctx.JSON(e.Status, e) } - // process all transactions - transactions, responses, e := m.processTransactions(ctx.Request().Context(), txsHexes, transactionOptions) + // process all txs + txs, outputs, failOutputs, e := m.processTransactions(ctx.Request().Context(), txsHexes, transactionOptions) if e != nil { return ctx.JSON(e.Status, e) } sizingInfo := make([][]uint64, 0) - for _, btTx := range transactions { + for _, btTx := range txs { normalBytes, dataBytes, feeAmount := getSizings(btTx) sizingInfo = append(sizingInfo, []uint64{normalBytes, dataBytes, feeAmount}) } @@ -230,6 +230,16 @@ func (m ArcDefaultHandler) POSTTransactions(ctx echo.Context, params api.POSTTra ctx.SetRequest(ctx.Request().WithContext(sizingCtx)) // we cannot really return any other status here // each transaction in the slice will have the result of the transaction submission + + // merge success and fail outputs + responses := make([]any, 0, len(outputs)+len(failOutputs)) + for _, o := range outputs { + responses = append(responses, o) + } + for _, fo := range failOutputs { + responses = append(responses, fo) + } + return ctx.JSON(int(api.StatusOK), responses) } @@ -295,143 +305,52 @@ func getTransactionsOptions(params api.POSTTransactionsParams, rejectedCallbackU return transactionOptions, nil } -func (m ArcDefaultHandler) processEFTransaction(ctx context.Context, transactionHex []byte, transactionOptions *metamorph.TransactionOptions) (*bt.Tx, *api.TransactionResponse, *api.ErrorFields) { - - transaction, err := bt.NewTxFromBytes(transactionHex) - if err != nil { - return nil, nil, api.NewErrorFields(api.ErrStatusBadRequest, err.Error()) - } - - v := defaultValidator.New(m.NodePolicy) - if arcError := m.validateEFTransaction(ctx, v, transaction, transactionOptions); arcError != nil { - return nil, nil, arcError - } - - timeoutCtx, cancel := context.WithTimeout(ctx, time.Duration(transactionOptions.MaxTimeout+2)*time.Second) - defer cancel() - - tx, err := m.TransactionHandler.SubmitTransaction(timeoutCtx, transaction, transactionOptions) - if err != nil { - statusCode, arcError := m.handleError(ctx, transaction, err) - m.logger.Error("failed to submit transaction", slog.String("id", transaction.TxID()), slog.Int("status", int(statusCode)), slog.String("err", err.Error())) - - return nil, nil, arcError - } - - return transaction, &api.TransactionResponse{ - Status: int(api.StatusOK), - Title: "OK", - BlockHash: &tx.BlockHash, - BlockHeight: &tx.BlockHeight, - TxStatus: tx.Status, - ExtraInfo: &tx.ExtraInfo, - Timestamp: m.now(), - Txid: transaction.TxID(), - MerklePath: &tx.MerklePath, - }, nil -} - -func (m ArcDefaultHandler) processBEEFTransaction(ctx context.Context, transactionHex []byte, transactionOptions *metamorph.TransactionOptions) (*bt.Tx, *api.TransactionResponse, *api.ErrorFields) { - - beefTx, _, err := beef.DecodeBEEF(transactionHex) - if err != nil { - errStr := fmt.Sprintf("error decoding BEEF: %s", err.Error()) - return nil, nil, api.NewErrorFields(api.ErrStatusMalformed, errStr) - } - - v := beefValidator.New(m.NodePolicy) - if err := m.validateBEEFTransaction(ctx, v, beefTx, transactionOptions); err != nil { - return nil, nil, err - } - - transactions := make([]*bt.Tx, 0) - - for _, tx := range beefTx.Transactions { - if !tx.IsMined() { - transactions = append(transactions, tx.Transaction) - } - } - - if len(transactions) == 0 { - return nil, nil, api.NewErrorFields(api.ErrStatusBadRequest, "all transactions in BEEF are mined") - } - - txStatuses, err := m.TransactionHandler.SubmitTransactions(ctx, transactions, transactionOptions) - if err != nil { - statusCode, arcError := m.handleError(ctx, nil, err) - m.logger.Error("failed to submit transactions", slog.Int("txs", len(transactions)), slog.Int("id", int(statusCode)), slog.String("err", err.Error())) - return nil, nil, arcError - } - - lastStatus := findStatusByTxID(beefTx.GetLatestTx().TxID(), txStatuses) - if lastStatus == nil { - return nil, nil, api.NewErrorFields(api.ErrStatusNotFound, "last tx of BEEF not found in metamorph submit response") - } - - return beefTx.GetLatestTx(), &api.TransactionResponse{ - Status: int(api.StatusOK), - Title: "OK", - BlockHash: &lastStatus.BlockHash, - BlockHeight: &lastStatus.BlockHeight, - TxStatus: lastStatus.Status, - ExtraInfo: &lastStatus.ExtraInfo, - Timestamp: m.now(), - Txid: lastStatus.TxID, - MerklePath: &lastStatus.MerklePath, - }, nil -} - // processTransactions validates all the transactions in the array and submits to metamorph for processing. -func (m ArcDefaultHandler) processTransactions(ctx context.Context, transactionsHexes []byte, transactionOptions *metamorph.TransactionOptions) ([]*bt.Tx, []interface{}, *api.ErrorFields) { +func (m ArcDefaultHandler) processTransactions(ctx context.Context, txsHex []byte, transactionOptions *metamorph.TransactionOptions) ( + submitedTxs []*bt.Tx, outputs []*api.TransactionResponse, failOutputs []*api.ErrorFields, pErr *api.ErrorFields) { m.logger.Info("Starting to process transactions") - // validate before submitting array of transactions to metamorph - transactions := make([]*bt.Tx, 0) + // decode and validate txs txIds := make([]string, 0) - txErrors := make([]interface{}, 0) - - for len(transactionsHexes) != 0 { - hexFormat := validator.GetHexFormat(transactionsHexes) + for len(txsHex) != 0 { + hexFormat := validator.GetHexFormat(txsHex) if hexFormat == validator.BeefHex { - beefTx, remainingBytes, err := beef.DecodeBEEF(transactionsHexes) + beefTx, remainingBytes, err := beef.DecodeBEEF(txsHex) + if err != nil { errStr := fmt.Sprintf("error decoding BEEF: %s", err.Error()) - return nil, nil, api.NewErrorFields(api.ErrStatusMalformed, errStr) + return nil, nil, nil, api.NewErrorFields(api.ErrStatusMalformed, errStr) } - transactionsHexes = remainingBytes + txsHex = remainingBytes - feeOpts, scriptOpts := toValidationOpts(transactionOptions) v := beefValidator.New(m.NodePolicy) - - if errTx, err := v.ValidateTransaction(ctx, beefTx, feeOpts, scriptOpts); err != nil { - _, arcError := m.handleError(ctx, errTx, err) - txErrors = append(txErrors, arcError) + if arcError := m.validateBEEFTransaction(ctx, v, beefTx, transactionOptions); arcError != nil { + failOutputs = append(failOutputs, arcError) continue } for _, tx := range beefTx.Transactions { if !tx.IsMined() { - transactions = append(transactions, tx.Transaction) + submitedTxs = append(submitedTxs, tx.Transaction) } } - transactions = append(transactions, beefTx.GetLatestTx()) txIds = append(txIds, beefTx.GetLatestTx().TxID()) } else { - transaction, bytesUsed, err := bt.NewTxFromStream(transactionsHexes) + transaction, bytesUsed, err := bt.NewTxFromStream(txsHex) if err != nil { - return nil, nil, api.NewErrorFields(api.ErrStatusBadRequest, err.Error()) + return nil, nil, nil, api.NewErrorFields(api.ErrStatusBadRequest, err.Error()) } - transactionsHexes = transactionsHexes[bytesUsed:] + txsHex = txsHex[bytesUsed:] v := defaultValidator.New(m.NodePolicy) if arcError := m.validateEFTransaction(ctx, v, transaction, transactionOptions); arcError != nil { - txErrors = append(txErrors, arcError) + failOutputs = append(failOutputs, arcError) continue } - transactions = append(transactions, transaction) + submitedTxs = append(submitedTxs, transaction) txIds = append(txIds, transaction.TxID()) } } @@ -439,40 +358,36 @@ func (m ArcDefaultHandler) processTransactions(ctx context.Context, transactions timeoutCtx, cancel := context.WithTimeout(ctx, time.Duration(transactionOptions.MaxTimeout+2)*time.Second) defer cancel() - // submit all the validated array of transactions to metamorph endpoint - txStatuses, err := m.TransactionHandler.SubmitTransactions(timeoutCtx, transactions, transactionOptions) - if err != nil { - statusCode, arcError := m.handleError(ctx, nil, err) - m.logger.Error("failed to submit transactions", slog.Int("txs", len(transactions)), slog.Int("id", int(statusCode)), slog.String("err", err.Error())) - return nil, nil, arcError + // submit validated transactions to metamorph + txStatuses, e := m.submitTransactions(timeoutCtx, submitedTxs, transactionOptions) + if e != nil { + return nil, nil, nil, e } + // process returned transaction statuses and return to user txStatuses = filterStatusesByTxIDs(txIds, txStatuses) - // process returned transaction statuses and return to user - transactionsOutputs := make([]interface{}, 0, len(transactions)) + outputs = make([]*api.TransactionResponse, 0, len(submitedTxs)) - for ind, tx := range txStatuses { + for idx, tx := range txStatuses { txID := tx.TxID if txID == "" { - txID = transactions[ind].TxID() + txID = submitedTxs[idx].TxID() } - transactionsOutputs = append(transactionsOutputs, api.TransactionResponse{ + outputs = append(outputs, &api.TransactionResponse{ Status: int(api.StatusOK), Title: "OK", - BlockHash: &txStatuses[ind].BlockHash, - BlockHeight: &txStatuses[ind].BlockHeight, + BlockHash: &txStatuses[idx].BlockHash, + BlockHeight: &txStatuses[idx].BlockHeight, TxStatus: tx.Status, - ExtraInfo: &txStatuses[ind].ExtraInfo, + ExtraInfo: &txStatuses[idx].ExtraInfo, Timestamp: m.now(), Txid: txID, - MerklePath: &txStatuses[ind].MerklePath, + MerklePath: &tx.MerklePath, }) } - transactionsOutputs = append(transactionsOutputs, txErrors...) - - return transactions, transactionsOutputs, nil + return submitedTxs, outputs, failOutputs, nil } func (m ArcDefaultHandler) validateEFTransaction(ctx context.Context, txValidator validator.DefaultValidator, transaction *bt.Tx, transactionOptions *metamorph.TransactionOptions) *api.ErrorFields { @@ -562,6 +477,39 @@ func (m ArcDefaultHandler) validateBEEFTransaction(ctx context.Context, txValida return nil } +func (m ArcDefaultHandler) submitTransactions(ctx context.Context, txs []*bt.Tx, options *metamorph.TransactionOptions) ([]*metamorph.TransactionStatus, *api.ErrorFields) { + var submitStatuses []*metamorph.TransactionStatus + + if len(txs) == 1 { + tx := txs[0] + + // probably we could use SubmitTransactions() as well, but right now I don't want to create potential performance issues + status, err := m.TransactionHandler.SubmitTransaction(ctx, tx, options) + + if err != nil { + statusCode, arcError := m.handleError(ctx, tx, err) + m.logger.Error("failed to submit transaction", slog.String("id", tx.TxID()), slog.Int("status", int(statusCode)), slog.String("err", err.Error())) + + return nil, arcError + } + + submitStatuses = append(submitStatuses, status) + + } else { + var err error + submitStatuses, err = m.TransactionHandler.SubmitTransactions(ctx, txs, options) + + if err != nil { + statusCode, arcError := m.handleError(ctx, nil, err) + m.logger.Error("failed to submit transactions", slog.Int("txs", len(txs)), slog.Int("status", int(statusCode)), slog.String("err", err.Error())) + + return nil, arcError + } + } + + return submitStatuses, nil +} + func (m ArcDefaultHandler) getTransactionStatus(ctx context.Context, id string) (*metamorph.TransactionStatus, error) { tx, err := m.TransactionHandler.GetTransactionStatus(ctx, id) if err != nil { diff --git a/pkg/api/handler/parsers.go b/pkg/api/handler/parsers.go index 6908119f5..3284aac12 100644 --- a/pkg/api/handler/parsers.go +++ b/pkg/api/handler/parsers.go @@ -51,6 +51,10 @@ func parseTransactionFromRequest(request *http.Request) ([]byte, error) { return nil, fmt.Errorf("given content-type %s does not match any of the allowed content-types", contentType) } + if len(txHex) == 0 { + return nil, errEmptyBody + } + return txHex, nil } From cd47dc5b9ccde48a50810b5dd862b60fc2e8145e Mon Sep 17 00:00:00 2001 From: Arkadiusz Osowski Date: Mon, 8 Jul 2024 23:13:02 +0200 Subject: [PATCH 2/4] ARCO-155: fix tests --- pkg/api/handler/default_test.go | 60 ++++++++++++++++++++++++++------- 1 file changed, 47 insertions(+), 13 deletions(-) diff --git a/pkg/api/handler/default_test.go b/pkg/api/handler/default_test.go index f975ead50..87f89b8b6 100644 --- a/pkg/api/handler/default_test.go +++ b/pkg/api/handler/default_test.go @@ -41,6 +41,8 @@ var ( validTxBytes, _ = hex.DecodeString(validTx) validExtendedTx = "010000000000000000ef01358eb38f1f910e76b33788ff9395a5d2af87721e950ebd3d60cf64bb43e77485010000006a47304402203be8a3ba74e7b770afa2addeff1bbc1eaeb0cedf6b4096c8eb7ec29f1278752602205dc1d1bedf2cab46096bb328463980679d4ce2126cdd6ed191d6224add9910884121021358f252895263cd7a85009fcc615b57393daf6f976662319f7d0c640e6189fcffffffffc70a0000000000001976a914f1e6837cf17b485a1dcea9e943948fafbe5e9f6888ac02bf010000000000001976a91449f066fccf8d392ff6a0a33bc766c9f3436c038a88acfc080000000000001976a914a7dcbd14f83c564e0025a57f79b0b8b591331ae288ac00000000" validTxID = "a147cc3c71cc13b29f18273cf50ffeb59fc9758152e2b33e21a8092f0b049118" + validTxParentHex = "0100000001fbbe01d83cb1f53a63ef91c0fce5750cbd8075efef5acd2ff229506a45ab832c010000006a473044022064be2f304950a87782b44e772390836aa613f40312a0df4993e9c5123d0c492d02202009b084b66a3da939fb7dc5d356043986539cac4071372d0a6481d5b5e418ca412103fc12a81e5213e30c7facc15581ac1acbf26a8612a3590ffb48045084b097d52cffffffff02bf010000000000001976a914c2ca67db517c0c972b9a6eb1181880ed3a528e3188acc70a0000000000001976a914f1e6837cf17b485a1dcea9e943948fafbe5e9f6888ac00000000" + validTxParentBytes, _ = hex.DecodeString(validTxParentHex) validBeef = "0100beef01fe636d0c0007021400fe507c0c7aa754cef1f7889d5fd395cf1f785dd7de98eed895dbedfe4e5bc70d1502ac4e164f5bc16746bb0868404292ac8318bbac3800e4aad13a014da427adce3e010b00bc4ff395efd11719b277694cface5aa50d085a0bb81f613f70313acd28cf4557010400574b2d9142b8d28b61d88e3b2c3f44d858411356b49a28a4643b6d1a6a092a5201030051a05fc84d531b5d250c23f4f886f6812f9fe3f402d61607f977b4ecd2701c19010000fd781529d58fc2523cf396a7f25440b409857e7e221766c57214b1d38c7b481f01010062f542f45ea3660f86c013ced80534cb5fd4c19d66c56e7e8c5d4bf2d40acc5e010100b121e91836fd7cd5102b654e9f72f3cf6fdbfd0b161c53a9c54b12c841126331020100000001cd4e4cac3c7b56920d1e7655e7e260d31f29d9a388d04910f1bbd72304a79029010000006b483045022100e75279a205a547c445719420aa3138bf14743e3f42618e5f86a19bde14bb95f7022064777d34776b05d816daf1699493fcdf2ef5a5ab1ad710d9c97bfb5b8f7cef3641210263e2dee22b1ddc5e11f6fab8bcd2378bdd19580d640501ea956ec0e786f93e76ffffffff013e660000000000001976a9146bfd5c7fbe21529d45803dbcf0c87dd3c71efbc288ac0000000001000100000001ac4e164f5bc16746bb0868404292ac8318bbac3800e4aad13a014da427adce3e000000006a47304402203a61a2e931612b4bda08d541cfb980885173b8dcf64a3471238ae7abcd368d6402204cbf24f04b9aa2256d8901f0ed97866603d2be8324c2bfb7a37bf8fc90edd5b441210263e2dee22b1ddc5e11f6fab8bcd2378bdd19580d640501ea956ec0e786f93e76ffffffff013c660000000000001976a9146bfd5c7fbe21529d45803dbcf0c87dd3c71efbc288ac0000000000" validBeefBytes, _ = hex.DecodeString(validBeef) validBeefTxID = "157428aee67d11123203735e4c540fa1bdab3b36d5882c6f8c5ff79f07d20d1c" @@ -311,7 +313,7 @@ func TestPOSTTransaction(t *testing.T) { //nolint:funlen expectedResponse: *api.NewErrorFields(api.ErrStatusBadRequest, "error parsing transaction from request: no transaction found - empty request body"), }, { - name: "invalid tx - application/json", + name: "invalid tx - empty payload, application/json", contentType: contentTypes[1], expectedStatus: 400, @@ -349,15 +351,15 @@ func TestPOSTTransaction(t *testing.T) { //nolint:funlen expectedResponse: *api.NewErrorFields(api.ErrStatusBadRequest, "error parsing transaction from request: invalid character 'e' in literal true (expecting 'r')"), }, { - name: "invalid tx - application/json", + name: "invalid tx - incorrect json field, application/json", contentType: contentTypes[1], txHexString: fmt.Sprintf("{\"txHex\": \"%s\"}", validTx), expectedStatus: 400, - expectedResponse: *api.NewErrorFields(api.ErrStatusBadRequest, "EOF"), + expectedResponse: *api.NewErrorFields(api.ErrStatusBadRequest, "error parsing transaction from request: no transaction found - empty request body"), }, { - name: "invalid tx - application/octet-stream", + name: "invalid tx - invalid hex, application/octet-stream", contentType: contentTypes[2], txHexString: "test", @@ -732,6 +734,9 @@ func TestPOSTTransactions(t *testing.T) { //nolint:funlen } // set the node/metamorph responses for the 3 test requests txHandler := &mtmMocks.TransactionHandlerMock{ + SubmitTransactionFunc: func(ctx context.Context, tx *bt.Tx, options *metamorph.TransactionOptions) (*metamorph.TransactionStatus, error) { + return txResult, nil + }, SubmitTransactionsFunc: func(ctx context.Context, tx []*bt.Tx, options *metamorph.TransactionOptions) ([]*metamorph.TransactionStatus, error) { txStatuses := []*metamorph.TransactionStatus{txResult} return txStatuses, nil @@ -770,10 +775,6 @@ func TestPOSTTransactions(t *testing.T) { //nolint:funlen errBEEFDecode := *api.NewErrorFields(api.ErrStatusMalformed, "error decoding BEEF: invalid BEEF - HasBUMP flag set, but no BUMP index") // set the node/metamorph responses for the 3 test requests txHandler := &mtmMocks.TransactionHandlerMock{ - SubmitTransactionsFunc: func(ctx context.Context, tx []*bt.Tx, options *metamorph.TransactionOptions) ([]*metamorph.TransactionStatus, error) { - return nil, nil - }, - HealthFunc: func(ctx context.Context) error { return nil }, @@ -813,6 +814,9 @@ func TestPOSTTransactions(t *testing.T) { //nolint:funlen } // set the node/metamorph responses for the 3 test requests txHandler := &mtmMocks.TransactionHandlerMock{ + SubmitTransactionFunc: func(ctx context.Context, tx *bt.Tx, options *metamorph.TransactionOptions) (*metamorph.TransactionStatus, error) { + return txResult, nil + }, SubmitTransactionsFunc: func(ctx context.Context, tx []*bt.Tx, options *metamorph.TransactionOptions) ([]*metamorph.TransactionStatus, error) { txStatuses := []*metamorph.TransactionStatus{txResult} return txStatuses, nil @@ -823,7 +827,13 @@ func TestPOSTTransactions(t *testing.T) { //nolint:funlen }, } - defaultHandler, err := NewDefault(testLogger, txHandler, nil, defaultPolicy, nil, nil) + merkleRootsVerifier := &btxMocks.MerkleRootsVerifierMock{ + VerifyMerkleRootsFunc: func(ctx context.Context, merkleRootVerificationRequest []blocktx.MerkleRootVerificationRequest) ([]uint64, error) { + return nil, nil + }, + } + + defaultHandler, err := NewDefault(testLogger, txHandler, merkleRootsVerifier, defaultPolicy, nil, nil) require.NoError(t, err) inputTxs := map[string]io.Reader{ @@ -866,11 +876,19 @@ func TestPOSTTransactions(t *testing.T) { //nolint:funlen // set the node/metamorph responses for the 3 test requests txHandler := &mtmMocks.TransactionHandlerMock{ GetTransactionFunc: func(ctx context.Context, txID string) ([]byte, error) { - return inputTxLowFeesBytes, nil + return validTxParentBytes, nil }, - SubmitTransactionsFunc: func(ctx context.Context, tx []*bt.Tx, options *metamorph.TransactionOptions) ([]*metamorph.TransactionStatus, error) { - return txResults, nil + SubmitTransactionsFunc: func(ctx context.Context, txs []*bt.Tx, options *metamorph.TransactionOptions) ([]*metamorph.TransactionStatus, error) { + var res []*metamorph.TransactionStatus + for _, t := range txs { + txID := t.TxID() + if status, found := find(txResults, func(e *metamorph.TransactionStatus) bool { return e.TxID == txID }); found { + res = append(res, status) + } + } + + return res, nil }, HealthFunc: func(ctx context.Context) error { @@ -878,7 +896,13 @@ func TestPOSTTransactions(t *testing.T) { //nolint:funlen }, } - defaultHandler, err := NewDefault(testLogger, txHandler, nil, defaultPolicy, nil, nil) + merkleRootsVerifier := &btxMocks.MerkleRootsVerifierMock{ + VerifyMerkleRootsFunc: func(ctx context.Context, merkleRootVerificationRequest []blocktx.MerkleRootVerificationRequest) ([]uint64, error) { + return nil, nil + }, + } + + defaultHandler, err := NewDefault(testLogger, txHandler, merkleRootsVerifier, defaultPolicy, nil, nil) require.NoError(t, err) inputTxs := map[string]io.Reader{ @@ -1207,3 +1231,13 @@ func Test_handleError(t *testing.T) { }) } } + +func find[T any](arr []T, predicate func(T) bool) (T, bool) { + for _, element := range arr { + if predicate(element) { + return element, true + } + } + var zero T + return zero, false +} From cc99c65de2d2e0914ff689d603ca8d999a571852 Mon Sep 17 00:00:00 2001 From: Arkadiusz Osowski Date: Mon, 8 Jul 2024 23:20:03 +0200 Subject: [PATCH 3/4] ARCO-155: improve code --- pkg/api/handler/default.go | 18 ++++++++++++------ pkg/api/handler/helpers.go | 11 ++++------- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/pkg/api/handler/default.go b/pkg/api/handler/default.go index afb2d9869..ddc8dc65f 100644 --- a/pkg/api/handler/default.go +++ b/pkg/api/handler/default.go @@ -154,8 +154,8 @@ func (m ArcDefaultHandler) POSTTransaction(ctx echo.Context, params api.POSTTran } if len(failOutputs) > 0 { + // if an error is returned, the processing failed e = failOutputs[0] - // if an error is returned, the processing failed, and we should return a 500 error return ctx.JSON(e.Status, e) } @@ -221,7 +221,7 @@ func (m ArcDefaultHandler) POSTTransactions(ctx echo.Context, params api.POSTTra return ctx.JSON(e.Status, e) } - sizingInfo := make([][]uint64, 0) + sizingInfo := make([][]uint64, 0, len(txs)) for _, btTx := range txs { normalBytes, dataBytes, feeAmount := getSizings(btTx) sizingInfo = append(sizingInfo, []uint64{normalBytes, dataBytes, feeAmount}) @@ -336,6 +336,7 @@ func (m ArcDefaultHandler) processTransactions(ctx context.Context, txsHex []byt submitedTxs = append(submitedTxs, tx.Transaction) } } + txIds = append(txIds, beefTx.GetLatestTx().TxID()) } else { transaction, bytesUsed, err := bt.NewTxFromStream(txsHex) @@ -355,6 +356,10 @@ func (m ArcDefaultHandler) processTransactions(ctx context.Context, txsHex []byt } } + if len(submitedTxs) == 0 { + return nil, nil, failOutputs, nil + } + timeoutCtx, cancel := context.WithTimeout(ctx, time.Duration(transactionOptions.MaxTimeout+2)*time.Second) defer cancel() @@ -367,6 +372,7 @@ func (m ArcDefaultHandler) processTransactions(ctx context.Context, txsHex []byt // process returned transaction statuses and return to user txStatuses = filterStatusesByTxIDs(txIds, txStatuses) + now := m.now() outputs = make([]*api.TransactionResponse, 0, len(submitedTxs)) for idx, tx := range txStatuses { @@ -377,11 +383,11 @@ func (m ArcDefaultHandler) processTransactions(ctx context.Context, txsHex []byt outputs = append(outputs, &api.TransactionResponse{ Status: int(api.StatusOK), Title: "OK", - BlockHash: &txStatuses[idx].BlockHash, - BlockHeight: &txStatuses[idx].BlockHeight, + BlockHash: &tx.BlockHash, + BlockHeight: &tx.BlockHeight, TxStatus: tx.Status, - ExtraInfo: &txStatuses[idx].ExtraInfo, - Timestamp: m.now(), + ExtraInfo: &tx.ExtraInfo, + Timestamp: now, Txid: txID, MerklePath: &tx.MerklePath, }) diff --git a/pkg/api/handler/helpers.go b/pkg/api/handler/helpers.go index 741db06b1..43942e540 100644 --- a/pkg/api/handler/helpers.go +++ b/pkg/api/handler/helpers.go @@ -124,16 +124,13 @@ func convertMerkleRootsRequest(beefMerkleRoots []beef.MerkleRootVerificationRequ return merkleRoots } -func findStatusByTxID(txID string, statuses []*metamorph.TransactionStatus) *metamorph.TransactionStatus { - for _, status := range statuses { - if status.TxID == txID { - return status +func filterStatusesByTxIDs(txIDs []string, allStatuses []*metamorph.TransactionStatus) []*metamorph.TransactionStatus { + if len(txIDs) == 1 && len(allStatuses) == 1 { + if allStatuses[0].TxID == txIDs[0] { + return allStatuses } } - return nil -} -func filterStatusesByTxIDs(txIDs []string, allStatuses []*metamorph.TransactionStatus) []*metamorph.TransactionStatus { idsMap := make(map[string]struct{}) for _, id := range txIDs { idsMap[id] = struct{}{} From ab78a7c28b481c0d783577aced9aaab7d0835ee5 Mon Sep 17 00:00:00 2001 From: Arkadiusz Osowski Date: Thu, 11 Jul 2024 14:35:40 +0200 Subject: [PATCH 4/4] ARCO-155: change args/var names; add missing tests and benchmark --- pkg/api/handler/default.go | 105 ++++++++++++++++---------------- pkg/api/handler/helpers.go | 12 ++-- pkg/api/handler/helpers_test.go | 91 +++++++++++++++++++++++++++ 3 files changed, 151 insertions(+), 57 deletions(-) diff --git a/pkg/api/handler/default.go b/pkg/api/handler/default.go index ddc8dc65f..3a1ab6a32 100644 --- a/pkg/api/handler/default.go +++ b/pkg/api/handler/default.go @@ -140,34 +140,30 @@ func (m ArcDefaultHandler) POSTTransaction(ctx echo.Context, params api.POSTTran return ctx.JSON(e.Status, e) } - transactionHex, err := parseTransactionFromRequest(ctx.Request()) + txHex, err := parseTransactionFromRequest(ctx.Request()) if err != nil { e := api.NewErrorFields(api.ErrStatusBadRequest, fmt.Sprintf("error parsing transaction from request: %s", err.Error())) return ctx.JSON(e.Status, e) } - txs, outputs, failOutputs, e := m.processTransactions(ctx.Request().Context(), transactionHex, transactionOptions) + reqCtx := ctx.Request().Context() + txs, successes, fails, e := m.processTransactions(reqCtx, txHex, transactionOptions) if e != nil { - // if an error is returned, the processing failed, and we should return a 500 error + // if an error is returned, the processing failed return ctx.JSON(e.Status, e) } - if len(failOutputs) > 0 { - // if an error is returned, the processing failed - e = failOutputs[0] + if len(fails) > 0 { + // if an fail result is returned, the processing/validation failed + e = fails[0] return ctx.JSON(e.Status, e) } - transaction := txs[0] - response := outputs[0] - - sizingInfo := make([][]uint64, 1) - normalBytes, dataBytes, feeAmount := getSizings(transaction) - sizingInfo[0] = []uint64{normalBytes, dataBytes, feeAmount} - sizingCtx := context.WithValue(ctx.Request().Context(), ContextSizings, sizingInfo) + sizingCtx := context.WithValue(reqCtx, ContextSizings, prepareSizingInfo(txs)) ctx.SetRequest(ctx.Request().WithContext(sizingCtx)) + response := successes[0] return ctx.JSON(response.Status, response) } @@ -209,34 +205,29 @@ func (m ArcDefaultHandler) POSTTransactions(ctx echo.Context, params api.POSTTra return ctx.JSON(e.Status, e) } - txsHexes, err := parseTransactionsFromRequest(ctx.Request()) + txsHex, err := parseTransactionsFromRequest(ctx.Request()) if err != nil { e := api.NewErrorFields(api.ErrStatusBadRequest, fmt.Sprintf("error parsing transaction from request: %s", err.Error())) return ctx.JSON(e.Status, e) } - // process all txs - txs, outputs, failOutputs, e := m.processTransactions(ctx.Request().Context(), txsHexes, transactionOptions) + reqCtx := ctx.Request().Context() + txs, successes, fails, e := m.processTransactions(reqCtx, txsHex, transactionOptions) if e != nil { return ctx.JSON(e.Status, e) } - sizingInfo := make([][]uint64, 0, len(txs)) - for _, btTx := range txs { - normalBytes, dataBytes, feeAmount := getSizings(btTx) - sizingInfo = append(sizingInfo, []uint64{normalBytes, dataBytes, feeAmount}) - } - sizingCtx := context.WithValue(ctx.Request().Context(), ContextSizings, sizingInfo) + sizingCtx := context.WithValue(reqCtx, ContextSizings, prepareSizingInfo(txs)) ctx.SetRequest(ctx.Request().WithContext(sizingCtx)) // we cannot really return any other status here // each transaction in the slice will have the result of the transaction submission - // merge success and fail outputs - responses := make([]any, 0, len(outputs)+len(failOutputs)) - for _, o := range outputs { + // merge success and fail results + responses := make([]any, 0, len(successes)+len(fails)) + for _, o := range successes { responses = append(responses, o) } - for _, fo := range failOutputs { + for _, fo := range fails { responses = append(responses, fo) } @@ -306,12 +297,13 @@ func getTransactionsOptions(params api.POSTTransactionsParams, rejectedCallbackU } // processTransactions validates all the transactions in the array and submits to metamorph for processing. -func (m ArcDefaultHandler) processTransactions(ctx context.Context, txsHex []byte, transactionOptions *metamorph.TransactionOptions) ( - submitedTxs []*bt.Tx, outputs []*api.TransactionResponse, failOutputs []*api.ErrorFields, pErr *api.ErrorFields) { +func (m ArcDefaultHandler) processTransactions(ctx context.Context, txsHex []byte, options *metamorph.TransactionOptions) ( + submittedTxs []*bt.Tx, successes []*api.TransactionResponse, fails []*api.ErrorFields, processingErr *api.ErrorFields) { m.logger.Info("Starting to process transactions") // decode and validate txs - txIds := make([]string, 0) + var txIds []string + for len(txsHex) != 0 { hexFormat := validator.GetHexFormat(txsHex) @@ -326,14 +318,14 @@ func (m ArcDefaultHandler) processTransactions(ctx context.Context, txsHex []byt txsHex = remainingBytes v := beefValidator.New(m.NodePolicy) - if arcError := m.validateBEEFTransaction(ctx, v, beefTx, transactionOptions); arcError != nil { - failOutputs = append(failOutputs, arcError) + if arcError := m.validateBEEFTransaction(ctx, v, beefTx, options); arcError != nil { + fails = append(fails, arcError) continue } for _, tx := range beefTx.Transactions { if !tx.IsMined() { - submitedTxs = append(submitedTxs, tx.Transaction) + submittedTxs = append(submittedTxs, tx.Transaction) } } @@ -346,41 +338,41 @@ func (m ArcDefaultHandler) processTransactions(ctx context.Context, txsHex []byt txsHex = txsHex[bytesUsed:] v := defaultValidator.New(m.NodePolicy) - if arcError := m.validateEFTransaction(ctx, v, transaction, transactionOptions); arcError != nil { - failOutputs = append(failOutputs, arcError) + if arcError := m.validateEFTransaction(ctx, v, transaction, options); arcError != nil { + fails = append(fails, arcError) continue } - submitedTxs = append(submitedTxs, transaction) + submittedTxs = append(submittedTxs, transaction) txIds = append(txIds, transaction.TxID()) } } - if len(submitedTxs) == 0 { - return nil, nil, failOutputs, nil + if len(submittedTxs) == 0 { + return nil, nil, fails, nil } - timeoutCtx, cancel := context.WithTimeout(ctx, time.Duration(transactionOptions.MaxTimeout+2)*time.Second) + timeoutCtx, cancel := context.WithTimeout(ctx, time.Duration(options.MaxTimeout+2)*time.Second) defer cancel() - // submit validated transactions to metamorph - txStatuses, e := m.submitTransactions(timeoutCtx, submitedTxs, transactionOptions) + // submit valid transactions to metamorph + txStatuses, e := m.submitTransactions(timeoutCtx, submittedTxs, options) if e != nil { return nil, nil, nil, e } - // process returned transaction statuses and return to user + // prepare success results txStatuses = filterStatusesByTxIDs(txIds, txStatuses) now := m.now() - outputs = make([]*api.TransactionResponse, 0, len(submitedTxs)) + successes = make([]*api.TransactionResponse, 0, len(submittedTxs)) for idx, tx := range txStatuses { txID := tx.TxID if txID == "" { - txID = submitedTxs[idx].TxID() + txID = submittedTxs[idx].TxID() } - outputs = append(outputs, &api.TransactionResponse{ + successes = append(successes, &api.TransactionResponse{ Status: int(api.StatusOK), Title: "OK", BlockHash: &tx.BlockHash, @@ -393,13 +385,12 @@ func (m ArcDefaultHandler) processTransactions(ctx context.Context, txsHex []byt }) } - return submitedTxs, outputs, failOutputs, nil + return submittedTxs, successes, fails, nil } -func (m ArcDefaultHandler) validateEFTransaction(ctx context.Context, txValidator validator.DefaultValidator, transaction *bt.Tx, transactionOptions *metamorph.TransactionOptions) *api.ErrorFields { +func (m ArcDefaultHandler) validateEFTransaction(ctx context.Context, txValidator validator.DefaultValidator, transaction *bt.Tx, options *metamorph.TransactionOptions) *api.ErrorFields { // the validator expects an extended transaction // we must enrich the transaction with the missing data - // TODO: move morphing to validator if !txValidator.IsExtended(transaction) { err := m.extendTransaction(ctx, transaction) if err != nil { @@ -409,8 +400,8 @@ func (m ArcDefaultHandler) validateEFTransaction(ctx context.Context, txValidato } } - if !transactionOptions.SkipTxValidation { - feeOpts, scriptOpts := toValidationOpts(transactionOptions) + if !options.SkipTxValidation { + feeOpts, scriptOpts := toValidationOpts(options) if err := txValidator.ValidateTransaction(ctx, transaction, feeOpts, scriptOpts); err != nil { statusCode, arcError := m.handleError(ctx, transaction, err) @@ -455,8 +446,8 @@ func (m ArcDefaultHandler) extendTransaction(ctx context.Context, transaction *b return nil } -func (m ArcDefaultHandler) validateBEEFTransaction(ctx context.Context, txValidator validator.BeefValidator, beefTx *beef.BEEF, transactionOptions *metamorph.TransactionOptions) *api.ErrorFields { - feeOpts, scriptOpts := toValidationOpts(transactionOptions) +func (m ArcDefaultHandler) validateBEEFTransaction(ctx context.Context, txValidator validator.BeefValidator, beefTx *beef.BEEF, options *metamorph.TransactionOptions) *api.ErrorFields { + feeOpts, scriptOpts := toValidationOpts(options) if errTx, err := txValidator.ValidateTransaction(ctx, beefTx, feeOpts, scriptOpts); err != nil { _, arcError := m.handleError(ctx, errTx, err) @@ -489,7 +480,7 @@ func (m ArcDefaultHandler) submitTransactions(ctx context.Context, txs []*bt.Tx, if len(txs) == 1 { tx := txs[0] - // probably we could use SubmitTransactions() as well, but right now I don't want to create potential performance issues + // SubmitTransaction() used to avoid performance issue status, err := m.TransactionHandler.SubmitTransaction(ctx, tx, options) if err != nil { @@ -582,6 +573,16 @@ func (m ArcDefaultHandler) getTransaction(ctx context.Context, inputTxID string) return nil, metamorph.ErrParentTransactionNotFound } +func prepareSizingInfo(txs []*bt.Tx) [][]uint64 { + sizingInfo := make([][]uint64, 0, len(txs)) + for _, btTx := range txs { + normalBytes, dataBytes, feeAmount := getSizings(btTx) + sizingInfo = append(sizingInfo, []uint64{normalBytes, dataBytes, feeAmount}) + } + + return sizingInfo +} + func getSizings(tx *bt.Tx) (uint64, uint64, uint64) { var feeAmount uint64 diff --git a/pkg/api/handler/helpers.go b/pkg/api/handler/helpers.go index 43942e540..9f60ed641 100644 --- a/pkg/api/handler/helpers.go +++ b/pkg/api/handler/helpers.go @@ -124,11 +124,13 @@ func convertMerkleRootsRequest(beefMerkleRoots []beef.MerkleRootVerificationRequ return merkleRoots } -func filterStatusesByTxIDs(txIDs []string, allStatuses []*metamorph.TransactionStatus) []*metamorph.TransactionStatus { - if len(txIDs) == 1 && len(allStatuses) == 1 { - if allStatuses[0].TxID == txIDs[0] { - return allStatuses +func filterStatusesByTxIDs(txIDs []string, statuses []*metamorph.TransactionStatus) []*metamorph.TransactionStatus { + if len(txIDs) == 1 && len(statuses) == 1 { // optimization for a common scenario + if statuses[0].TxID == txIDs[0] { + return statuses } + + return make([]*metamorph.TransactionStatus, 0) } idsMap := make(map[string]struct{}) @@ -137,7 +139,7 @@ func filterStatusesByTxIDs(txIDs []string, allStatuses []*metamorph.TransactionS } filteredStatuses := make([]*metamorph.TransactionStatus, 0) - for _, txStatus := range allStatuses { + for _, txStatus := range statuses { if _, ok := idsMap[txStatus.TxID]; ok { filteredStatuses = append(filteredStatuses, txStatus) } diff --git a/pkg/api/handler/helpers_test.go b/pkg/api/handler/helpers_test.go index 20fe606f8..bfd7a9ca7 100644 --- a/pkg/api/handler/helpers_test.go +++ b/pkg/api/handler/helpers_test.go @@ -1,13 +1,16 @@ package handler import ( + "fmt" "net/http" "net/http/httptest" + "reflect" "strings" "testing" "github.com/bitcoin-sv/arc/internal/beef" "github.com/bitcoin-sv/arc/pkg/blocktx" + "github.com/bitcoin-sv/arc/pkg/metamorph" "github.com/labstack/echo/v4" "github.com/stretchr/testify/assert" ) @@ -73,3 +76,91 @@ func TestConvertMerkleRootsRequest(t *testing.T) { }) } } + +func TestFilterStatusesByTxIDs(t *testing.T) { + tcs := []struct { + name string + txIDs []string + statuses []*metamorph.TransactionStatus + expected []*metamorph.TransactionStatus + }{ + { + name: "Single txID with matching status", + txIDs: []string{"tx1"}, + statuses: []*metamorph.TransactionStatus{ + {TxID: "tx1"}, + }, + expected: []*metamorph.TransactionStatus{ + {TxID: "tx1"}, + }, + }, + { + name: "Single txID with non-matching status", + txIDs: []string{"tx1"}, + statuses: []*metamorph.TransactionStatus{ + {TxID: "tx2"}, + }, + expected: []*metamorph.TransactionStatus{}, + }, + { + name: "Multiple txIDs with some matching statuses", + txIDs: []string{"tx1", "tx3"}, + statuses: []*metamorph.TransactionStatus{ + {TxID: "tx1"}, + {TxID: "tx2"}, + {TxID: "tx3"}, + }, + expected: []*metamorph.TransactionStatus{ + {TxID: "tx1"}, + {TxID: "tx3"}, + }, + }, + { + name: "No txIDs", + txIDs: []string{}, + statuses: []*metamorph.TransactionStatus{ + {TxID: "tx1"}, + {TxID: "tx2"}, + }, + expected: []*metamorph.TransactionStatus{}, + }, + { + name: "No statuses", + txIDs: []string{"tx1", "tx2"}, + statuses: []*metamorph.TransactionStatus{}, + expected: []*metamorph.TransactionStatus{}, + }, + } + + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + res := filterStatusesByTxIDs(tc.txIDs, tc.statuses) + if !reflect.DeepEqual(res, tc.expected) { + t.Errorf("expected %v, got %v", tc.expected, res) + } + }) + } +} + +func BenchmarkFilterStatusesByTxIDs(b *testing.B) { + bcs := []int{1, 10, 100, 1000, 10000} + + for _, n := range bcs { + b.Run(fmt.Sprintf("txIDs-%d", n), func(b *testing.B) { + txIDs := make([]string, n) + for i := 0; i < n; i++ { + txIDs[i] = fmt.Sprintf("tx-%d", i) + } + + statuses := make([]*metamorph.TransactionStatus, n) + for i := 0; i < n; i++ { + statuses[i] = &metamorph.TransactionStatus{TxID: fmt.Sprintf("tx-%d", i)} + } + + b.ResetTimer() + for i := 0; i < b.N; i++ { + _ = filterStatusesByTxIDs(txIDs, statuses) + } + }) + } +}