From e1dfbab33d3802be14795bb19a48bfa684674785 Mon Sep 17 00:00:00 2001 From: Nazarii-4chain Date: Thu, 25 Jan 2024 11:34:39 +0200 Subject: [PATCH 1/9] Added error handler --- record_tx_strategy_outgoing_tx.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/record_tx_strategy_outgoing_tx.go b/record_tx_strategy_outgoing_tx.go index edc7465a..ab2d20b0 100644 --- a/record_tx_strategy_outgoing_tx.go +++ b/record_tx_strategy_outgoing_tx.go @@ -82,7 +82,12 @@ func (strategy *outgoingTx) Validate() error { } func (strategy *outgoingTx) TxID() string { - btTx, _ := bt.NewTxFromString(strategy.Hex) + btTx, err := bt.NewTxFromString(strategy.Hex) + + // Return hex if hex is invalid. Handle error in error handlers + if err != nil { + return strategy.Hex + } return btTx.TxID() } From 7ca3886f775dc0cea14871dd1d23f4959646dd83 Mon Sep 17 00:00:00 2001 From: Nazarii-4chain Date: Fri, 26 Jan 2024 09:43:13 +0200 Subject: [PATCH 2/9] Improved tests --- action_transaction_test.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/action_transaction_test.go b/action_transaction_test.go index e4b05114..301b126e 100644 --- a/action_transaction_test.go +++ b/action_transaction_test.go @@ -187,6 +187,30 @@ func Test_RevertTransaction(t *testing.T) { }) } +func Test_RecordTransaction(t *testing.T) { + //var secondTransaction *Transaction + ctx, client, _ := initSimpleTestCase(t) + draftTransaction := newDraftTransaction( + testXPub, &TransactionConfig{ + Outputs: []*TransactionOutput{{ + To: "1A1PjKqjWMNBzTVdcBru27EV1PHcXWc63W", + Satoshis: 1000, + }}, + ChangeNumberOfDestinations: 1, + Sync: &SyncConfig{ + Broadcast: true, + BroadcastInstant: false, + PaymailP2P: false, + SyncOnChain: false, + }, + }, + append(client.DefaultModelOptions(), New())..., + ) + + _, err := client.RecordTransaction(ctx, testXPub, "test", draftTransaction.ID, client.DefaultModelOptions()...) + require.Error(t, err) +} + func initRevertTransactionData(t *testing.T) (context.Context, ClientInterface, *Transaction, *bip32.ExtendedKey, func()) { // this creates an xpub, destination and utxo ctx, client, deferMe := initSimpleTestCase(t) From b24452579daa8e8583affa1fb5f2968cc5dd45e5 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 30 Jan 2024 09:32:03 +0000 Subject: [PATCH 3/9] build(deps): bump github.com/99designs/gqlgen from 0.17.42 to 0.17.43 Bumps [github.com/99designs/gqlgen](https://github.com/99designs/gqlgen) from 0.17.42 to 0.17.43. - [Release notes](https://github.com/99designs/gqlgen/releases) - [Changelog](https://github.com/99designs/gqlgen/blob/master/CHANGELOG.md) - [Commits](https://github.com/99designs/gqlgen/compare/v0.17.42...v0.17.43) --- updated-dependencies: - dependency-name: github.com/99designs/gqlgen dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- go.mod | 4 ++-- go.sum | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index 1a91ebd1..5179430a 100644 --- a/go.mod +++ b/go.mod @@ -41,7 +41,7 @@ require ( ) require ( - github.com/99designs/gqlgen v0.17.42 // indirect + github.com/99designs/gqlgen v0.17.43 // indirect github.com/acobaugh/osrelease v0.1.0 // indirect github.com/bitcoinschema/go-bpu v0.1.3 // indirect github.com/bitcoinsv/bsvd v0.0.0-20190609155523-4c29707f7173 // indirect @@ -110,7 +110,7 @@ require ( github.com/tidwall/pretty v1.2.1 // indirect github.com/tidwall/sjson v1.2.5 // indirect github.com/valyala/bytebufferpool v1.0.0 // indirect - github.com/vektah/gqlparser/v2 v2.5.10 // indirect + github.com/vektah/gqlparser/v2 v2.5.11 // indirect github.com/vmihailenco/msgpack/v5 v5.4.1 // indirect github.com/vmihailenco/tagparser/v2 v2.0.0 // indirect github.com/xdg-go/pbkdf2 v1.0.0 // indirect diff --git a/go.sum b/go.sum index 184266a8..6410b289 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,5 @@ -github.com/99designs/gqlgen v0.17.42 h1:BVWDOb2VVHQC5k3m6oa0XhDnxltLLrU4so7x/u39Zu4= -github.com/99designs/gqlgen v0.17.42/go.mod h1:GQ6SyMhwFbgHR0a8r2Wn8fYgEwPxxmndLFPhU63+cJE= +github.com/99designs/gqlgen v0.17.43 h1:I4SYg6ahjowErAQcHFVKy5EcWuwJ3+Xw9z2fLpuFCPo= +github.com/99designs/gqlgen v0.17.43/go.mod h1:lO0Zjy8MkZgBdv4T1U91x09r0e0WFOdhVUutlQs1Rsc= github.com/DATA-DOG/go-sqlmock v1.5.2 h1:OcvFkGmslmlZibjAjaHm3L//6LiuBgolP7OputlJIzU= github.com/DATA-DOG/go-sqlmock v1.5.2/go.mod h1:88MAG/4G7SMwSE3CeA0ZKzrT5CiOU3OJ+JlNzwDqpNU= github.com/DataDog/datadog-go v3.7.1+incompatible/go.mod h1:LButxg5PwREeZtORoXG3tL4fMGNddJ+vMq1mwgfaqoQ= @@ -320,8 +320,8 @@ github.com/tylertreat/BoomFilters v0.0.0-20210315201527-1a82519a3e43 h1:QEePdg0t github.com/tylertreat/BoomFilters v0.0.0-20210315201527-1a82519a3e43/go.mod h1:OYRfF6eb5wY9VRFkXJH8FFBi3plw2v+giaIu7P054pM= github.com/valyala/bytebufferpool v1.0.0 h1:GqA5TC/0021Y/b9FG4Oi9Mr3q7XYx6KllzawFIhcdPw= github.com/valyala/bytebufferpool v1.0.0/go.mod h1:6bBcMArwyJ5K/AmCkWv1jt77kVWyCJ6HpOuEn7z0Csc= -github.com/vektah/gqlparser/v2 v2.5.10 h1:6zSM4azXC9u4Nxy5YmdmGu4uKamfwsdKTwp5zsEealU= -github.com/vektah/gqlparser/v2 v2.5.10/go.mod h1:1rCcfwB2ekJofmluGWXMSEnPMZgbxzwj6FaZ/4OT8Cc= +github.com/vektah/gqlparser/v2 v2.5.11 h1:JJxLtXIoN7+3x6MBdtIP59TP1RANnY7pXOaDnADQSf8= +github.com/vektah/gqlparser/v2 v2.5.11/go.mod h1:1rCcfwB2ekJofmluGWXMSEnPMZgbxzwj6FaZ/4OT8Cc= github.com/vmihailenco/msgpack/v5 v5.4.1 h1:cQriyiUvjTwOHg8QZaPihLWeRAAVoCpE00IUPn0Bjt8= github.com/vmihailenco/msgpack/v5 v5.4.1/go.mod h1:GaZTsDaehaPpQVyxrf5mtQlH+pc21PIudVV/E3rRQok= github.com/vmihailenco/tagparser/v2 v2.0.0 h1:y09buUbR+b5aycVFQs/g70pqKVZNBmxwAhO7/IwNM9g= From 058ff37ca84f568a52e6c7bb5dd9c51c994b83ea Mon Sep 17 00:00:00 2001 From: Nazarii-4chain Date: Thu, 1 Feb 2024 16:15:05 +0200 Subject: [PATCH 4/9] Added validation --- paymail_service_provider.go | 4 ++++ record_tx_strategy_external_incoming_tx.go | 4 ++++ record_tx_strategy_internal_incoming_tx.go | 5 +++++ record_tx_strategy_outgoing_tx.go | 11 +++++------ 4 files changed, 18 insertions(+), 6 deletions(-) diff --git a/paymail_service_provider.go b/paymail_service_provider.go index 5c8bd1b7..54266c9f 100644 --- a/paymail_service_provider.go +++ b/paymail_service_provider.go @@ -157,6 +157,10 @@ func (p *PaymailDefaultServiceProvider) RecordTransaction(ctx context.Context, metadata[p2pMetadataField] = p2pTx.MetaData metadata[ReferenceIDField] = p2pTx.Reference + var rts recordIncomingTxStrategy + if err := rts.Validate(); err != nil { + return nil, err + } // Record the transaction rts, err := getIncomingTxRecordStrategy(ctx, p.client, p2pTx.Hex) if err != nil { diff --git a/record_tx_strategy_external_incoming_tx.go b/record_tx_strategy_external_incoming_tx.go index b4b68b65..5641df23 100644 --- a/record_tx_strategy_external_incoming_tx.go +++ b/record_tx_strategy_external_incoming_tx.go @@ -59,6 +59,10 @@ func (strategy *externalIncomingTx) Validate() error { return ErrMissingFieldHex } + if _, err := bt.NewTxFromString(strategy.Hex); err != nil { + return fmt.Errorf("invalid hex: %w", err) + } + return nil // is valid } diff --git a/record_tx_strategy_internal_incoming_tx.go b/record_tx_strategy_internal_incoming_tx.go index 20ecf12d..fa9b04e5 100644 --- a/record_tx_strategy_internal_incoming_tx.go +++ b/record_tx_strategy_internal_incoming_tx.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" + "github.com/libsv/go-bt/v2" "github.com/rs/zerolog" ) @@ -52,6 +53,10 @@ func (strategy *internalIncomingTx) Validate() error { return errors.New("tx cannot be nil") } + if _, err := bt.NewTxFromString(strategy.Tx.Hex); err != nil { + return fmt.Errorf("invalid hex: %w", err) + } + return nil // is valid } diff --git a/record_tx_strategy_outgoing_tx.go b/record_tx_strategy_outgoing_tx.go index ab2d20b0..84a237d6 100644 --- a/record_tx_strategy_outgoing_tx.go +++ b/record_tx_strategy_outgoing_tx.go @@ -70,6 +70,10 @@ func (strategy *outgoingTx) Validate() error { return ErrMissingFieldHex } + if _, err := bt.NewTxFromString(strategy.Hex); err != nil { + return fmt.Errorf("invalid hex: %w", err) + } + if strategy.RelatedDraftID == "" { return errors.New("empty RelatedDraftID") } @@ -82,12 +86,7 @@ func (strategy *outgoingTx) Validate() error { } func (strategy *outgoingTx) TxID() string { - btTx, err := bt.NewTxFromString(strategy.Hex) - - // Return hex if hex is invalid. Handle error in error handlers - if err != nil { - return strategy.Hex - } + btTx, _ := bt.NewTxFromString(strategy.Hex) return btTx.TxID() } From d296d0b3b1197f7b9d6ade17e599ba6deb1e6987 Mon Sep 17 00:00:00 2001 From: Nazarii-4chain Date: Tue, 6 Feb 2024 10:18:11 +0200 Subject: [PATCH 5/9] Changed tests and validation order --- action_transaction_test.go | 43 +++++++++++++++++++++---------------- paymail_service_provider.go | 11 ++++++---- 2 files changed, 32 insertions(+), 22 deletions(-) diff --git a/action_transaction_test.go b/action_transaction_test.go index 301b126e..408e3ec1 100644 --- a/action_transaction_test.go +++ b/action_transaction_test.go @@ -189,26 +189,33 @@ func Test_RevertTransaction(t *testing.T) { func Test_RecordTransaction(t *testing.T) { //var secondTransaction *Transaction - ctx, client, _ := initSimpleTestCase(t) - draftTransaction := newDraftTransaction( - testXPub, &TransactionConfig{ - Outputs: []*TransactionOutput{{ - To: "1A1PjKqjWMNBzTVdcBru27EV1PHcXWc63W", - Satoshis: 1000, - }}, - ChangeNumberOfDestinations: 1, - Sync: &SyncConfig{ - Broadcast: true, - BroadcastInstant: false, - PaymailP2P: false, - SyncOnChain: false, + t.Run("record transaction", func(t *testing.T) { + ctx, client, _ := initSimpleTestCase(t) + // given + draftTransaction := newDraftTransaction( + testXPub, &TransactionConfig{ + Outputs: []*TransactionOutput{{ + To: "1A1PjKqjWMNBzTVdcBru27EV1PHcXWc63W", + Satoshis: 1000, + }}, + ChangeNumberOfDestinations: 1, + Sync: &SyncConfig{ + Broadcast: true, + BroadcastInstant: false, + PaymailP2P: false, + SyncOnChain: false, + }, }, - }, - append(client.DefaultModelOptions(), New())..., - ) + append(client.DefaultModelOptions(), New())..., + ) + + // when + _, err := client.RecordTransaction(ctx, testXPub, "test", draftTransaction.ID, client.DefaultModelOptions()...) + + //then + require.Error(t, err) + }) - _, err := client.RecordTransaction(ctx, testXPub, "test", draftTransaction.ID, client.DefaultModelOptions()...) - require.Error(t, err) } func initRevertTransactionData(t *testing.T) (context.Context, ClientInterface, *Transaction, *bip32.ExtendedKey, func()) { diff --git a/paymail_service_provider.go b/paymail_service_provider.go index 651fe063..18ec3928 100644 --- a/paymail_service_provider.go +++ b/paymail_service_provider.go @@ -157,15 +157,18 @@ func (p *PaymailDefaultServiceProvider) RecordTransaction(ctx context.Context, metadata[p2pMetadataField] = p2pTx.MetaData metadata[ReferenceIDField] = p2pTx.Reference - var rts recordIncomingTxStrategy - if err := rts.Validate(); err != nil { - return nil, err - } + //var rts recordIncomingTxStrategy + //if err := rts.Validate(); err != nil { + // return nil, err + //} // Record the transaction rts, err := getIncomingTxRecordStrategy(ctx, p.client, p2pTx.Hex) if err != nil { return nil, err } + if err := rts.Validate(); err != nil { + return nil, err + } rts.ForceBroadcast(true) From db9f986b169940a596e873cde3c6b3692d410509 Mon Sep 17 00:00:00 2001 From: Nazarii-4chain Date: Wed, 7 Feb 2024 09:26:36 +0200 Subject: [PATCH 6/9] Removed comments Added tests --- action_transaction_test.go | 59 ++++++++++++++++++++++++------------- paymail_service_provider.go | 4 --- 2 files changed, 39 insertions(+), 24 deletions(-) diff --git a/action_transaction_test.go b/action_transaction_test.go index 408e3ec1..bf9bb1b2 100644 --- a/action_transaction_test.go +++ b/action_transaction_test.go @@ -188,34 +188,53 @@ func Test_RevertTransaction(t *testing.T) { } func Test_RecordTransaction(t *testing.T) { - //var secondTransaction *Transaction - t.Run("record transaction", func(t *testing.T) { - ctx, client, _ := initSimpleTestCase(t) - // given - draftTransaction := newDraftTransaction( - testXPub, &TransactionConfig{ - Outputs: []*TransactionOutput{{ - To: "1A1PjKqjWMNBzTVdcBru27EV1PHcXWc63W", - Satoshis: 1000, - }}, - ChangeNumberOfDestinations: 1, - Sync: &SyncConfig{ - Broadcast: true, - BroadcastInstant: false, - PaymailP2P: false, - SyncOnChain: false, - }, + ctx, client, _ := initSimpleTestCase(t) + // given + draftTransaction := newDraftTransaction( + testXPub, &TransactionConfig{ + Outputs: []*TransactionOutput{{ + To: "1A1PjKqjWMNBzTVdcBru27EV1PHcXWc63W", + Satoshis: 1000, + }}, + ChangeNumberOfDestinations: 1, + Sync: &SyncConfig{ + Broadcast: true, + BroadcastInstant: false, + PaymailP2P: false, + SyncOnChain: false, }, - append(client.DefaultModelOptions(), New())..., - ) + }, + append(client.DefaultModelOptions(), New())..., + ) + draftTransactionID := draftTransaction.ID + t.Run("hex validation -> invalid hex", func(t *testing.T) { + invalidHex := "test" // when - _, err := client.RecordTransaction(ctx, testXPub, "test", draftTransaction.ID, client.DefaultModelOptions()...) + _, err := client.RecordTransaction(ctx, testXPub, invalidHex, draftTransactionID, client.DefaultModelOptions()...) //then require.Error(t, err) }) + t.Run("hex validation -> empty hex", func(t *testing.T) { + emptyHex := "" + // when + _, err := client.RecordTransaction(ctx, testXPub, emptyHex, draftTransactionID, client.DefaultModelOptions()...) + + //then + require.Error(t, err) + }) + + t.Run("hex validation -> valid hex", func(t *testing.T) { + validHex := "020000000165bb8d2733298b2d3b441a871868d6323c5392facf0d3eced3a6c6a17dc84c10000000006a473044022057b101e9a017cdcc333ef66a4a1e78720ae15adf7d1be9c33abec0fe56bc849d022013daa203095522039fadaba99e567ec3cf8615861d3b7258d5399c9f1f4ace8f412103b9c72aebee5636664b519e5f7264c78614f1e57fa4097ae83a3012a967b1c4b9ffffffff03e0930400000000001976a91413473d21dc9e1fb392f05a028b447b165a052d4d88acf9020000000000001976a91455decebedd9a6c2c2d32cf0ee77e2640c3955d3488ac00000000000000000c006a09446f7457616c6c657400000000" + // when + _, err := client.RecordTransaction(ctx, testXPub, validHex, "", client.DefaultModelOptions()...) + + //then + require.NotContains(t, err.Error(), "invalid hex") + }) + } func initRevertTransactionData(t *testing.T) (context.Context, ClientInterface, *Transaction, *bip32.ExtendedKey, func()) { diff --git a/paymail_service_provider.go b/paymail_service_provider.go index 18ec3928..ba562676 100644 --- a/paymail_service_provider.go +++ b/paymail_service_provider.go @@ -157,10 +157,6 @@ func (p *PaymailDefaultServiceProvider) RecordTransaction(ctx context.Context, metadata[p2pMetadataField] = p2pTx.MetaData metadata[ReferenceIDField] = p2pTx.Reference - //var rts recordIncomingTxStrategy - //if err := rts.Validate(); err != nil { - // return nil, err - //} // Record the transaction rts, err := getIncomingTxRecordStrategy(ctx, p.client, p2pTx.Hex) if err != nil { From 1270bc851ef4eae456e25cbd5fe8db6452d2320e Mon Sep 17 00:00:00 2001 From: Damian Orzepowski Date: Wed, 7 Feb 2024 10:10:14 +0100 Subject: [PATCH 7/9] ci(BUX-000): add github label "tested" --- .github/labels.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/labels.yml b/.github/labels.yml index 3c0d062e..01acb0c4 100644 --- a/.github/labels.yml +++ b/.github/labels.yml @@ -52,3 +52,6 @@ - color: c2e0c6 description: "Old, unused, stale" name: "stale" +- color: 50e061 + description: "PR was tested by a team member" + name: "tested" From e530b2c6d538dba4d90e0a684f97a94bfeffe9c2 Mon Sep 17 00:00:00 2001 From: Damian Orzepowski Date: Thu, 8 Feb 2024 13:55:39 +0100 Subject: [PATCH 8/9] ci(BUX-582): add check for manual tests --- .github/workflows/on-pull-request.yml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 .github/workflows/on-pull-request.yml diff --git a/.github/workflows/on-pull-request.yml b/.github/workflows/on-pull-request.yml new file mode 100644 index 00000000..94324b20 --- /dev/null +++ b/.github/workflows/on-pull-request.yml @@ -0,0 +1,15 @@ +name: "On pull request" + +on: + pull_request_target: + types: + - labeled + - unlabeled + - opened + +permissions: + pull-requests: write + +jobs: + on-pr: + uses: bactions/workflows/.github/workflows/on-pull-request.yml@main From 88df6cff2680d222cedf118eed32503e1f49468e Mon Sep 17 00:00:00 2001 From: Nazarii-4chain Date: Fri, 9 Feb 2024 11:41:06 +0200 Subject: [PATCH 9/9] Rerun tests --- record_tx.go | 1 + 1 file changed, 1 insertion(+) diff --git a/record_tx.go b/record_tx.go index a338f9b5..4e53df12 100644 --- a/record_tx.go +++ b/record_tx.go @@ -76,6 +76,7 @@ func getIncomingTxRecordStrategy(ctx context.Context, c ClientInterface, txHex s } return rts, nil + } func waitForRecordTxWriteLock(ctx context.Context, c ClientInterface, key string) func() {