*: add Copy() to transaction.Transaction and payload.P2PNotaryRequest #3407

AliceInHunterland · 2024-04-09T08:14:03Z

Close #3288

pkg/core/transaction/transaction.go

pkg/core/transaction/witness.go

pkg/core/transaction/transaction_test.go

pkg/network/payload/notary_request.go

pkg/core/transaction/transaction.go

pkg/network/payload/notary_request.go

codecov · 2024-04-10T15:31:23Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.19%. Comparing base (198af7f) to head (8da7c0a).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3407      +/-   ##
==========================================
+ Coverage   86.13%   86.19%   +0.06%     
==========================================
  Files         331      331              
  Lines       38114    38210      +96     
==========================================
+ Hits        32828    32937     +109     
+ Misses       3773     3754      -19     
- Partials     1513     1519       +6

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

pkg/core/transaction/transaction.go

pkg/network/payload/notary_request.go

pkg/core/transaction/witness.go

pkg/services/notary/notary.go

pkg/core/transaction/transaction.go

pkg/core/transaction/transaction_test.go

pkg/core/transaction/witness.go

pkg/network/payload/notary_request_test.go

pkg/services/notary/notary.go

pkg/network/payload/notary_request.go

AliceInHunterland · 2024-04-11T17:21:31Z

@AnnaShaleva Let's decide if the current copy implementation is okay before writing tests for all new copy() methods?

pkg/core/transaction/attribute.go

pkg/core/transaction/signer.go

pkg/core/transaction/witness_condition.go

pkg/core/transaction/witness_condition_test.go

pkg/crypto/keys/publickey.go

pkg/core/transaction/signer.go

pkg/core/transaction/attribute.go

pkg/core/transaction/attribute_test.go

pkg/core/transaction/conflicts.go

pkg/core/transaction/attribute_test.go

pkg/crypto/keys/publickey.go

pkg/core/transaction/attribute_test.go

pkg/core/transaction/signer.go

pkg/core/transaction/attribute.go

pkg/core/transaction/witness.go

pkg/core/transaction/witness_condition.go

pkg/core/transaction/witness_condition_test.go

pkg/core/transaction/witness_rule.go

pkg/crypto/keys/publickey_test.go

Signed-off-by: Ekaterina Pavlova <[email protected]>

AnnaShaleva

Style comments, the logic LGTM.

pkg/core/transaction/witness_test.go

pkg/core/transaction/witness_condition.go

pkg/core/transaction/signer.go

pkg/core/transaction/attribute.go

pkg/services/notary/notary.go

pkg/core/transaction/transaction.go

pkg/crypto/keys/publickey.go

Signed-off-by: Ekaterina Pavlova <[email protected]>

AnnaShaleva

Nice!

AnnaShaleva · 2024-04-27T15:24:56Z

@roman-khimov, ready for review.

pkg/core/transaction/signer.go

roman-khimov · 2024-04-27T16:08:08Z

pkg/core/transaction/witness_test.go

-		InvocationScript:   make([]byte, MaxInvocationScript),
-		VerificationScript: make([]byte, MaxVerificationScript),
+		InvocationScript:   []byte{1, 2, 2},
+		VerificationScript: []byte{3, 2, 2},


pkg/core/transaction/signer.go

Since the AllowedGroups []*keys.PublicKey slice is used in the initialization, the test should use the same structures. Signed-off-by: Ekaterina Pavlova <[email protected]>

Add a method that makes a deep copy of all fields and resets size/hash caches. Close #3288 Signed-off-by: Ekaterina Pavlova <[email protected]>

Signed-off-by: Ekaterina Pavlova <[email protected]>

AnnaShaleva · 2024-05-01T07:45:06Z

Do you really need this change? It can break some initializations in existing code.

That was my concern as far, but I've checked the existing code and it seems that this type change doesn't break anything. Anyway, the current version looks better.

AliceInHunterland requested review from AnnaShaleva and roman-khimov as code owners April 9, 2024 08:14

roman-khimov reviewed Apr 9, 2024

View reviewed changes

AnnaShaleva reviewed Apr 9, 2024

View reviewed changes

pkg/core/transaction/transaction.go Outdated Show resolved Hide resolved

pkg/network/payload/notary_request.go Show resolved Hide resolved

AliceInHunterland force-pushed the copy-transaction branch from 112f9ad to 9cdcbc0 Compare April 10, 2024 15:21

AliceInHunterland requested review from AnnaShaleva and roman-khimov April 10, 2024 15:25

AnnaShaleva reviewed Apr 10, 2024

View reviewed changes

AliceInHunterland force-pushed the copy-transaction branch from 9cdcbc0 to b06d96b Compare April 10, 2024 21:17

AliceInHunterland requested a review from AnnaShaleva April 10, 2024 21:17

AliceInHunterland force-pushed the copy-transaction branch from b06d96b to 2743df3 Compare April 10, 2024 21:29

AnnaShaleva reviewed Apr 11, 2024

View reviewed changes

pkg/network/payload/notary_request.go Show resolved Hide resolved

AliceInHunterland force-pushed the copy-transaction branch from 2743df3 to 6cea4e8 Compare April 11, 2024 16:53

AliceInHunterland requested a review from AnnaShaleva April 11, 2024 17:02

AnnaShaleva reviewed Apr 15, 2024

View reviewed changes

pkg/core/transaction/signer.go Outdated Show resolved Hide resolved

AliceInHunterland force-pushed the copy-transaction branch from 6cea4e8 to acc3ed9 Compare April 18, 2024 14:18

AliceInHunterland requested a review from AnnaShaleva April 18, 2024 14:22

AliceInHunterland force-pushed the copy-transaction branch 3 times, most recently from 5098928 to 84b50f4 Compare April 22, 2024 08:51

AnnaShaleva reviewed Apr 22, 2024

View reviewed changes

AliceInHunterland force-pushed the copy-transaction branch 3 times, most recently from 1c84171 to 13ab7de Compare April 23, 2024 15:36

AliceInHunterland requested a review from AnnaShaleva April 23, 2024 16:08

AnnaShaleva reviewed Apr 25, 2024

View reviewed changes

core: move transaction Attribute value to a designated interface

df2a569

Signed-off-by: Ekaterina Pavlova <[email protected]>

AliceInHunterland force-pushed the copy-transaction branch from 13ab7de to 077f4f0 Compare April 26, 2024 16:12

AliceInHunterland requested a review from AnnaShaleva April 26, 2024 16:13

AnnaShaleva reviewed Apr 27, 2024

View reviewed changes

pkg/core/transaction/transaction.go Outdated Show resolved Hide resolved

pkg/crypto/keys/publickey.go Show resolved Hide resolved

AliceInHunterland force-pushed the copy-transaction branch from 077f4f0 to 9ac42e5 Compare April 27, 2024 09:25

crypto: add nil check for PublicKeys Copy()

7c8d2c3

Signed-off-by: Ekaterina Pavlova <[email protected]>

AliceInHunterland force-pushed the copy-transaction branch from 9ac42e5 to 682032f Compare April 27, 2024 09:28

AliceInHunterland requested a review from AnnaShaleva April 27, 2024 09:28

AliceInHunterland force-pushed the copy-transaction branch from 682032f to 11772fb Compare April 27, 2024 09:31

AnnaShaleva approved these changes Apr 27, 2024

View reviewed changes

roman-khimov reviewed Apr 27, 2024

View reviewed changes

AliceInHunterland force-pushed the copy-transaction branch from 11772fb to 3f5d84c Compare April 27, 2024 17:22

AliceInHunterland requested a review from roman-khimov April 27, 2024 17:25

roman-khimov reviewed Apr 27, 2024

View reviewed changes

pkg/core/transaction/signer.go Outdated Show resolved Hide resolved

AliceInHunterland force-pushed the copy-transaction branch from 3f5d84c to fcd5f24 Compare April 27, 2024 18:39

crypto: adjust TestPublicKeys_Copy

1292a00

Since the AllowedGroups []*keys.PublicKey slice is used in the initialization, the test should use the same structures. Signed-off-by: Ekaterina Pavlova <[email protected]>

AliceInHunterland force-pushed the copy-transaction branch from fcd5f24 to c7f60e6 Compare April 27, 2024 18:42

AliceInHunterland requested a review from roman-khimov April 27, 2024 18:44

AliceInHunterland added 2 commits April 28, 2024 00:59

*: add Copy() to transaction.Transaction and payload.P2PNotaryRequest

956fd08

Add a method that makes a deep copy of all fields and resets size/hash caches. Close #3288 Signed-off-by: Ekaterina Pavlova <[email protected]>

notary: reuse (*Transaction).Copy where possible

8da7c0a

Signed-off-by: Ekaterina Pavlova <[email protected]>

AliceInHunterland force-pushed the copy-transaction branch from c7f60e6 to 8da7c0a Compare April 27, 2024 19:29

AliceInHunterland requested a review from AnnaShaleva April 27, 2024 19:30

roman-khimov approved these changes Apr 27, 2024

View reviewed changes

AnnaShaleva approved these changes May 1, 2024

View reviewed changes

AnnaShaleva merged commit b54fcbc into master May 1, 2024
21 checks passed

AnnaShaleva deleted the copy-transaction branch May 1, 2024 07:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: add Copy() to transaction.Transaction and payload.P2PNotaryRequest #3407

*: add Copy() to transaction.Transaction and payload.P2PNotaryRequest #3407

AliceInHunterland commented Apr 9, 2024

codecov bot commented Apr 10, 2024 •

edited

Loading

AliceInHunterland commented Apr 11, 2024

AnnaShaleva left a comment

AnnaShaleva left a comment

AnnaShaleva commented Apr 27, 2024

roman-khimov Apr 27, 2024

AnnaShaleva commented May 1, 2024

*: add Copy() to transaction.Transaction and payload.P2PNotaryRequest #3407

*: add Copy() to transaction.Transaction and payload.P2PNotaryRequest #3407

Conversation

AliceInHunterland commented Apr 9, 2024

codecov bot commented Apr 10, 2024 • edited Loading

Codecov Report

AliceInHunterland commented Apr 11, 2024

AnnaShaleva left a comment

Choose a reason for hiding this comment

AnnaShaleva left a comment

Choose a reason for hiding this comment

AnnaShaleva commented Apr 27, 2024

roman-khimov Apr 27, 2024

Choose a reason for hiding this comment

AnnaShaleva commented May 1, 2024

codecov bot commented Apr 10, 2024 •

edited

Loading