From 9edbf2c1034064f11a0cd17f60da5412a9a7963c Mon Sep 17 00:00:00 2001 From: arkadiuszos4chain Date: Tue, 3 Oct 2023 18:26:53 +0200 Subject: [PATCH 1/5] fix(BUX-255): kahn sorting: ignore inputs we do not use for the transaction --- beef_tx.go | 4 ++-- beef_tx_sorting.go | 11 +++++++---- beef_tx_sorting_test.go | 4 ++-- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/beef_tx.go b/beef_tx.go index 34b4c93f..0d01670a 100644 --- a/beef_tx.go +++ b/beef_tx.go @@ -28,8 +28,8 @@ type beefTx struct { } func newBeefTx(version uint32, tx *Transaction) (*beefTx, error) { - if version > 65_535 { - return nil, errors.New("version above 65.535") + if version > 0xFFFF { + return nil, errors.New("version above 0xFFFF") } // get inputs parent transactions diff --git a/beef_tx_sorting.go b/beef_tx_sorting.go index de2dc836..129b063e 100644 --- a/beef_tx_sorting.go +++ b/beef_tx_sorting.go @@ -28,16 +28,19 @@ func prepareSortStructures(dag []*Transaction) (txByID map[string]*Transaction, incomingEdgesMap[tx.ID] = 0 } - calculateIncomingEdges(incomingEdgesMap, dag) + calculateIncomingEdges(incomingEdgesMap, txByID) zeroIncomingEdgeQueue = getTxWithZeroIncomingEdges(incomingEdgesMap) return } -func calculateIncomingEdges(inDegree map[string]int, transactions []*Transaction) { - for _, tx := range transactions { +func calculateIncomingEdges(inDegree map[string]int, txByID map[string]*Transaction) { + for _, tx := range txByID { for _, input := range tx.draftTransaction.Configuration.Inputs { - inDegree[input.UtxoPointer.TransactionID]++ + inputUtxoTxID := input.UtxoPointer.TransactionID + if _, ok := txByID[inputUtxoTxID]; ok { // transaction can contains inputs we are not interested in + inDegree[inputUtxoTxID]++ + } } } } diff --git a/beef_tx_sorting_test.go b/beef_tx_sorting_test.go index dec96e01..0fb5f1cd 100644 --- a/beef_tx_sorting_test.go +++ b/beef_tx_sorting_test.go @@ -12,10 +12,10 @@ func Test_kahnTopologicalSortTransaction(t *testing.T) { txsFromOldestToNewest := []*Transaction{ createTx("0"), createTx("1", "0"), - createTx("2", "1"), + createTx("2", "1", "101"), createTx("3", "2", "1"), createTx("4", "3", "1"), - createTx("5", "3", "2"), + createTx("5", "3", "2", "100"), createTx("6", "4", "2", "0"), createTx("7", "6", "5", "3", "1"), createTx("8", "7"), From 7250d53e41f463e62dd03bb870ccc7d1e035c85c Mon Sep 17 00:00:00 2001 From: arkadiuszos4chain Date: Wed, 4 Oct 2023 16:44:36 +0200 Subject: [PATCH 2/5] fix(BUX-252): hydrate the transaction model to encode it in BEEF --- beef_tx.go | 52 ++++++++++++++++++++++++++++---------- beef_tx_bytes.go | 3 ++- model_sync_transactions.go | 1 + paymail.go | 8 +++--- 4 files changed, 46 insertions(+), 18 deletions(-) diff --git a/beef_tx.go b/beef_tx.go index 0d01670a..396cd005 100644 --- a/beef_tx.go +++ b/beef_tx.go @@ -3,19 +3,21 @@ package bux import ( "context" "encoding/hex" - "errors" + "fmt" ) +const maxBeefVer = uint32(0xFFFF) // value from BRC-62 + // ToBeefHex generates BEEF Hex for transaction -func ToBeefHex(tx *Transaction) (string, error) { - beef, err := newBeefTx(1, tx) +func ToBeefHex(ctx context.Context, tx *Transaction) (string, error) { + beef, err := newBeefTx(ctx, 1, tx) if err != nil { - return "", err + return "", fmt.Errorf("ToBeefHex() error: %w", err) } beefBytes, err := beef.toBeefBytes() if err != nil { - return "", err + return "", fmt.Errorf("ToBeefHex() error: %w", err) } return hex.EncodeToString(beefBytes), nil @@ -27,9 +29,13 @@ type beefTx struct { transactions []*Transaction } -func newBeefTx(version uint32, tx *Transaction) (*beefTx, error) { - if version > 0xFFFF { - return nil, errors.New("version above 0xFFFF") +func newBeefTx(ctx context.Context, version uint32, tx *Transaction) (*beefTx, error) { + if version > maxBeefVer { + return nil, fmt.Errorf("version above 0x%X", maxBeefVer) + } + + if err := hydrateTransaction(ctx, tx); err != nil { + return nil, err } // get inputs parent transactions @@ -37,9 +43,9 @@ func newBeefTx(version uint32, tx *Transaction) (*beefTx, error) { transactions := make([]*Transaction, 0, len(inputs)+1) for _, input := range inputs { - prevTxs, err := getParentTransactionsForInput(tx.client, input) + prevTxs, err := getParentTransactionsForInput(ctx, tx.client, input) if err != nil { - return nil, err + return nil, fmt.Errorf("retrieve input parent transaction failed: %w", err) } transactions = append(transactions, prevTxs...) @@ -57,15 +63,35 @@ func newBeefTx(version uint32, tx *Transaction) (*beefTx, error) { return beef, nil } -func getParentTransactionsForInput(client ClientInterface, input *TransactionInput) ([]*Transaction, error) { - inputTx, err := client.GetTransactionByID(context.Background(), input.UtxoPointer.TransactionID) +func getParentTransactionsForInput(ctx context.Context, client ClientInterface, input *TransactionInput) ([]*Transaction, error) { + inputTx, err := client.GetTransactionByID(ctx, input.UtxoPointer.TransactionID) if err != nil { return nil, err } + if err = hydrateTransaction(ctx, inputTx); err != nil { + return nil, err + } + if inputTx.MerkleProof.TxOrID != "" { return []*Transaction{inputTx}, nil } - return nil, errors.New("transaction is not mined yet") // TODO: handle it in next iterration + return nil, fmt.Errorf("transaction is not mined yet (tx.ID: %s)", inputTx.ID) // TODO: handle it in next iterration +} + +func hydrateTransaction(ctx context.Context, tx *Transaction) error { + if tx.draftTransaction == nil { + dTx, err := getDraftTransactionID( + ctx, tx.XPubID, tx.DraftID, tx.GetOptions(false)..., + ) + + if err != nil { + return fmt.Errorf("retrieve DraftTransaction failed: %w", err) + } + + tx.draftTransaction = dTx + } + + return nil } diff --git a/beef_tx_bytes.go b/beef_tx_bytes.go index 0a7c8b73..a55d3ebf 100644 --- a/beef_tx_bytes.go +++ b/beef_tx_bytes.go @@ -3,6 +3,7 @@ package bux import ( "encoding/hex" "errors" + "fmt" "github.com/libsv/go-bt/v2" ) @@ -63,7 +64,7 @@ func (tx *Transaction) toBeefBytes(compountedPaths CMPSlice) ([]byte, error) { txBeefBytes, err := hex.DecodeString(tx.Hex) if err != nil { - return nil, err + return nil, fmt.Errorf("decoding tx (ID: %s) hex failed: %w", tx.ID, err) } cmpIdx := tx.getCompountedMarklePathIndex(compountedPaths) diff --git a/model_sync_transactions.go b/model_sync_transactions.go index 9dc2e0cc..68405eb3 100644 --- a/model_sync_transactions.go +++ b/model_sync_transactions.go @@ -855,6 +855,7 @@ func notifyPaymailProviders(ctx context.Context, transaction *Transaction) ([]*S // Notify each provider with the transaction if payload, err = finalizeP2PTransaction( + ctx, pm, out.PaymailP4, transaction, diff --git a/paymail.go b/paymail.go index a1800aeb..0d6f2643 100644 --- a/paymail.go +++ b/paymail.go @@ -155,7 +155,7 @@ func startP2PTransaction(client paymail.ClientInterface, } // finalizeP2PTransaction will notify the paymail provider about the transaction -func finalizeP2PTransaction(client paymail.ClientInterface, p4 *PaymailP4, transaction *Transaction) (*paymail.P2PTransactionPayload, error) { +func finalizeP2PTransaction(ctx context.Context, client paymail.ClientInterface, p4 *PaymailP4, transaction *Transaction) (*paymail.P2PTransactionPayload, error) { // Submit the P2P transaction /*logger.Data(2, logger.DEBUG, "sending p2p tx...", @@ -167,7 +167,7 @@ func finalizeP2PTransaction(client paymail.ClientInterface, p4 *PaymailP4, trans logger.MakeParameter("referenceID", referenceID), )*/ - p2pTransaction, err := buildP2pTx(p4, transaction) + p2pTransaction, err := buildP2pTx(ctx, p4, transaction) if err != nil { return nil, err } @@ -180,7 +180,7 @@ func finalizeP2PTransaction(client paymail.ClientInterface, p4 *PaymailP4, trans return &response.P2PTransactionPayload, nil } -func buildP2pTx(p4 *PaymailP4, transaction *Transaction) (*paymail.P2PTransaction, error) { +func buildP2pTx(ctx context.Context, p4 *PaymailP4, transaction *Transaction) (*paymail.P2PTransaction, error) { p2pTransaction := &paymail.P2PTransaction{ MetaData: &paymail.P2PMetaData{ Note: p4.Note, @@ -192,7 +192,7 @@ func buildP2pTx(p4 *PaymailP4, transaction *Transaction) (*paymail.P2PTransactio switch p4.Format { case BeefPaymailPayloadFormat: - beef, err := ToBeefHex(transaction) + beef, err := ToBeefHex(ctx, transaction) if err != nil { return nil, err From 183693deb35579d822d2964d0ca75f6212fd0162 Mon Sep 17 00:00:00 2001 From: arkadiuszos4chain Date: Thu, 5 Oct 2023 09:14:49 +0200 Subject: [PATCH 3/5] fix(BUX-252): validate CMPs before decoding --- beef_tx.go | 54 +++++++++++++++++++++++++++++++++--------------- beef_tx_bytes.go | 2 +- 2 files changed, 38 insertions(+), 18 deletions(-) diff --git a/beef_tx.go b/beef_tx.go index 396cd005..2675305a 100644 --- a/beef_tx.go +++ b/beef_tx.go @@ -3,6 +3,7 @@ package bux import ( "context" "encoding/hex" + "errors" "fmt" ) @@ -34,7 +35,12 @@ func newBeefTx(ctx context.Context, version uint32, tx *Transaction) (*beefTx, e return nil, fmt.Errorf("version above 0x%X", maxBeefVer) } - if err := hydrateTransaction(ctx, tx); err != nil { + var err error + if err = hydrateTransaction(ctx, tx); err != nil { + return nil, err + } + + if err = validateCompoundMerklePathes(tx.draftTransaction.CompoundMerklePathes); err != nil { return nil, err } @@ -63,6 +69,36 @@ func newBeefTx(ctx context.Context, version uint32, tx *Transaction) (*beefTx, e return beef, nil } +func hydrateTransaction(ctx context.Context, tx *Transaction) error { + if tx.draftTransaction == nil { + dTx, err := getDraftTransactionID( + ctx, tx.XPubID, tx.DraftID, tx.GetOptions(false)..., + ) + + if err != nil { + return fmt.Errorf("retrieve DraftTransaction failed: %w", err) + } + + tx.draftTransaction = dTx + } + + return nil +} + +func validateCompoundMerklePathes(compountedPaths CMPSlice) error { + if len(compountedPaths) == 0 { + return errors.New("empty compounted paths slice") + } + + for _, c := range compountedPaths { + if len(c) == 0 { + return errors.New("one of compounted merkle paths is empty") + } + } + + return nil +} + func getParentTransactionsForInput(ctx context.Context, client ClientInterface, input *TransactionInput) ([]*Transaction, error) { inputTx, err := client.GetTransactionByID(ctx, input.UtxoPointer.TransactionID) if err != nil { @@ -79,19 +115,3 @@ func getParentTransactionsForInput(ctx context.Context, client ClientInterface, return nil, fmt.Errorf("transaction is not mined yet (tx.ID: %s)", inputTx.ID) // TODO: handle it in next iterration } - -func hydrateTransaction(ctx context.Context, tx *Transaction) error { - if tx.draftTransaction == nil { - dTx, err := getDraftTransactionID( - ctx, tx.XPubID, tx.DraftID, tx.GetOptions(false)..., - ) - - if err != nil { - return fmt.Errorf("retrieve DraftTransaction failed: %w", err) - } - - tx.draftTransaction = dTx - } - - return nil -} diff --git a/beef_tx_bytes.go b/beef_tx_bytes.go index a55d3ebf..2e15497f 100644 --- a/beef_tx_bytes.go +++ b/beef_tx_bytes.go @@ -12,7 +12,7 @@ var hasCmp = byte(0x01) var hasNoCmp = byte(0x00) func (beefTx *beefTx) toBeefBytes() ([]byte, error) { - if beefTx.compoundMerklePaths == nil || beefTx.transactions == nil { + if len(beefTx.compoundMerklePaths) == 0 || len(beefTx.transactions) < 2 { // valid BEEF contains atleast two transactions (new transaction and one parent transaction) return nil, errors.New("beef tx is incomplete") } From 8f39b4b840f63df39c2a85b63f6ae08d291935a3 Mon Sep 17 00:00:00 2001 From: arkadiuszos4chain Date: Thu, 5 Oct 2023 10:41:06 +0200 Subject: [PATCH 4/5] fix(BUX-252): add tests --- beef_tx_sorting_test.go | 48 ++++++++++++++---- beef_tx_test.go | 107 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 146 insertions(+), 9 deletions(-) create mode 100644 beef_tx_test.go diff --git a/beef_tx_sorting_test.go b/beef_tx_sorting_test.go index 0fb5f1cd..6ac7e7dd 100644 --- a/beef_tx_sorting_test.go +++ b/beef_tx_sorting_test.go @@ -1,6 +1,7 @@ package bux import ( + "fmt" "math/rand" "testing" @@ -12,25 +13,54 @@ func Test_kahnTopologicalSortTransaction(t *testing.T) { txsFromOldestToNewest := []*Transaction{ createTx("0"), createTx("1", "0"), - createTx("2", "1", "101"), + createTx("2", "1"), createTx("3", "2", "1"), createTx("4", "3", "1"), - createTx("5", "3", "2", "100"), + createTx("5", "3", "2"), createTx("6", "4", "2", "0"), createTx("7", "6", "5", "3", "1"), createTx("8", "7"), } - unsortedTxs := shuffleTransactions(txsFromOldestToNewest) + txsFromOldestToNewestWithUnnecessaryInputs := []*Transaction{ + createTx("0"), + createTx("1", "0"), + createTx("2", "1", "101", "102"), + createTx("3", "2", "1"), + createTx("4", "3", "1"), + createTx("5", "3", "2", "100"), + createTx("6", "4", "2", "0"), + createTx("7", "6", "5", "3", "1", "103", "105", "106"), + createTx("8", "7"), + } + + tCases := []struct { + name string + expectedSortedTransactions []*Transaction + }{{ + name: "txs with neccessary data only", + expectedSortedTransactions: txsFromOldestToNewest, + }, + { + name: "txs with inputs from other txs", + expectedSortedTransactions: txsFromOldestToNewestWithUnnecessaryInputs, + }, + } - t.Run("kahnTopologicalSortTransaction sort from oldest to newest", func(t *testing.T) { - sortedGraph := kahnTopologicalSortTransactions(unsortedTxs) + for _, tc := range tCases { + t.Run(fmt.Sprint("sort from oldest to newest ", tc.name), func(t *testing.T) { + // given + unsortedTxs := shuffleTransactions(tc.expectedSortedTransactions) - for i, tx := range txsFromOldestToNewest { - assert.Equal(t, tx.ID, sortedGraph[i].ID) - } + // when + sortedGraph := kahnTopologicalSortTransactions(unsortedTxs) - }) + // then + for i, tx := range txsFromOldestToNewest { + assert.Equal(t, tx.ID, sortedGraph[i].ID) + } + }) + } } func createTx(txID string, inputsTxIDs ...string) *Transaction { diff --git a/beef_tx_test.go b/beef_tx_test.go new file mode 100644 index 00000000..d3d4f50a --- /dev/null +++ b/beef_tx_test.go @@ -0,0 +1,107 @@ +package bux + +import ( + "context" + "testing" + + "github.com/libsv/go-bc" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_ToBeefHex(t *testing.T) { + t.Run("all parents txs are already mined", func(t *testing.T) { + //given + ctx, client, deferMe := initSimpleTestCase(t) + defer deferMe() + + ancestorTx := addGrandPaTx(t, ctx, client) + minedParentTx := createTxWithDraft(t, ctx, client, ancestorTx, true) + + newTx := createTxWithDraft(t, ctx, client, minedParentTx, false) + + //when + hex, err := ToBeefHex(ctx, newTx) + + //then + assert.NoError(t, err) + assert.NotEmpty(t, hex) + }) + + t.Run("some parents txs are not mined yet", func(t *testing.T) { + // Error expeted! this should be changed in the future. right now the test case has been written to make sure the system doesn't panic in such a situation + + //given + ctx, client, deferMe := initSimpleTestCase(t) + defer deferMe() + + ancestorTx := addGrandPaTx(t, ctx, client) + notMinedParentTx := createTxWithDraft(t, ctx, client, ancestorTx, false) + + newTx := createTxWithDraft(t, ctx, client, notMinedParentTx, false) + + //when + hex, err := ToBeefHex(ctx, newTx) + + //then + assert.Error(t, err) + assert.Empty(t, hex) + }) +} + +func addGrandPaTx(t *testing.T, ctx context.Context, client ClientInterface) *Transaction { + // great ancestor + grandpaTx := newTransaction(testTx2Hex, append(client.DefaultModelOptions(), New())...) + grandpaTx.BlockHeight = 1 + // mark it as mined + grandpaTxMp := bc.MerkleProof{ + TxOrID: "111111111111111111111111111111111111111", + Nodes: []string{"n1", "n2"}, + } + grandpaTx.MerkleProof = MerkleProof(grandpaTxMp) + err := grandpaTx.Save(ctx) + require.NoError(t, err) + + return grandpaTx +} + +func createTxWithDraft(t *testing.T, ctx context.Context, client ClientInterface, parentTx *Transaction, mined bool) *Transaction { + draftTransaction := newDraftTransaction( + testXPub, &TransactionConfig{ + Inputs: []*TransactionInput{{Utxo: *newUtxoFromTxID(parentTx.GetID(), 0, append(client.DefaultModelOptions(), New())...)}}, + Outputs: []*TransactionOutput{{ + To: "1A1PjKqjWMNBzTVdcBru27EV1PHcXWc63W", + Satoshis: 1000, + }}, + ChangeNumberOfDestinations: 1, + Sync: &SyncConfig{ + Broadcast: true, + BroadcastInstant: false, + PaymailP2P: false, + SyncOnChain: false, + }, + }, + append(client.DefaultModelOptions(), New())..., + ) + + err := draftTransaction.Save(ctx) + require.NoError(t, err) + + var transaction *Transaction + transaction, err = client.RecordTransaction(ctx, testXPub, draftTransaction.Hex, draftTransaction.ID, client.DefaultModelOptions()...) + require.NoError(t, err) + assert.NotEmpty(t, transaction) + + if mined { + transaction.BlockHeight = 128 + mp := bc.MerkleProof{ + TxOrID: "423542156234627frafserg6gtrdsbd", Nodes: []string{"n1", "n2"}, + } + transaction.MerkleProof = MerkleProof(mp) + } + + err = transaction.Save(ctx) + require.NoError(t, err) + + return transaction +} From b1ff95443d83a4f7a5369fb30b3943fd3128ea64 Mon Sep 17 00:00:00 2001 From: arkadiuszos4chain Date: Thu, 5 Oct 2023 11:10:13 +0200 Subject: [PATCH 5/5] fxi(BUX-252): fix typo; remove unnecessary comment --- beef_tx_bytes.go | 2 +- beef_tx_test.go | 6 +++--- paymail.go | 11 ----------- 3 files changed, 4 insertions(+), 15 deletions(-) diff --git a/beef_tx_bytes.go b/beef_tx_bytes.go index 2e15497f..0bb92e14 100644 --- a/beef_tx_bytes.go +++ b/beef_tx_bytes.go @@ -12,7 +12,7 @@ var hasCmp = byte(0x01) var hasNoCmp = byte(0x00) func (beefTx *beefTx) toBeefBytes() ([]byte, error) { - if len(beefTx.compoundMerklePaths) == 0 || len(beefTx.transactions) < 2 { // valid BEEF contains atleast two transactions (new transaction and one parent transaction) + if len(beefTx.compoundMerklePaths) == 0 || len(beefTx.transactions) < 2 { // valid BEEF contains at least two transactions (new transaction and one parent transaction) return nil, errors.New("beef tx is incomplete") } diff --git a/beef_tx_test.go b/beef_tx_test.go index d3d4f50a..18d27211 100644 --- a/beef_tx_test.go +++ b/beef_tx_test.go @@ -15,7 +15,7 @@ func Test_ToBeefHex(t *testing.T) { ctx, client, deferMe := initSimpleTestCase(t) defer deferMe() - ancestorTx := addGrandPaTx(t, ctx, client) + ancestorTx := addGrandpaTx(t, ctx, client) minedParentTx := createTxWithDraft(t, ctx, client, ancestorTx, true) newTx := createTxWithDraft(t, ctx, client, minedParentTx, false) @@ -35,7 +35,7 @@ func Test_ToBeefHex(t *testing.T) { ctx, client, deferMe := initSimpleTestCase(t) defer deferMe() - ancestorTx := addGrandPaTx(t, ctx, client) + ancestorTx := addGrandpaTx(t, ctx, client) notMinedParentTx := createTxWithDraft(t, ctx, client, ancestorTx, false) newTx := createTxWithDraft(t, ctx, client, notMinedParentTx, false) @@ -49,7 +49,7 @@ func Test_ToBeefHex(t *testing.T) { }) } -func addGrandPaTx(t *testing.T, ctx context.Context, client ClientInterface) *Transaction { +func addGrandpaTx(t *testing.T, ctx context.Context, client ClientInterface) *Transaction { // great ancestor grandpaTx := newTransaction(testTx2Hex, append(client.DefaultModelOptions(), New())...) grandpaTx.BlockHeight = 1 diff --git a/paymail.go b/paymail.go index 0d6f2643..6f0ebda0 100644 --- a/paymail.go +++ b/paymail.go @@ -156,17 +156,6 @@ func startP2PTransaction(client paymail.ClientInterface, // finalizeP2PTransaction will notify the paymail provider about the transaction func finalizeP2PTransaction(ctx context.Context, client paymail.ClientInterface, p4 *PaymailP4, transaction *Transaction) (*paymail.P2PTransactionPayload, error) { - - // Submit the P2P transaction - /*logger.Data(2, logger.DEBUG, "sending p2p tx...", - logger.MakeParameter("alias", alias), - logger.MakeParameter("p2pSubmitURL", p2pSubmitURL), - logger.MakeParameter("domain", domain), - logger.MakeParameter("note", note), - logger.MakeParameter("senderPaymailAddress", senderPaymailAddress), - logger.MakeParameter("referenceID", referenceID), - )*/ - p2pTransaction, err := buildP2pTx(ctx, p4, transaction) if err != nil { return nil, err