hedera-services icon indicating copy to clipboard operation
hedera-services copied to clipboard

chore: move towards making `Hash` immutable

Open lpetrovic05 opened this issue 9 months ago • 8 comments

part of #12456

lpetrovic05 avatar May 07 '24 13:05 lpetrovic05

Node: HAPI Test (Restart) Results

2 tests   2 :white_check_mark:  8m 6s :stopwatch: 2 suites  0 :zzz: 2 files    0 :x:

Results for commit e4acee4e.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar May 08 '24 09:05 github-actions[bot]

Node: HAPI Test (Node Death Reconnect) Results

2 tests   2 :white_check_mark:  8m 52s :stopwatch: 2 suites  0 :zzz: 2 files    0 :x:

Results for commit e4acee4e.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar May 08 '24 09:05 github-actions[bot]

Node: HAPI Test (Token) Results

235 tests   233 :white_check_mark:  22m 2s :stopwatch:  17 suites    2 :zzz:  17 files      0 :x:

Results for commit e4acee4e.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar May 08 '24 10:05 github-actions[bot]

Node: HAPI Test (Crypto) Results

335 tests   335 :white_check_mark:  45m 12s :stopwatch:  25 suites    0 :zzz:  25 files      0 :x:

Results for commit e4acee4e.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar May 08 '24 10:05 github-actions[bot]

Node: HAPI Test (Misc) Results

459 tests   449 :white_check_mark:  45m 2s :stopwatch:  77 suites   10 :zzz:  77 files      0 :x:

Results for commit e4acee4e.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar May 08 '24 10:05 github-actions[bot]

Node: HAPI Test (Time Consuming) Results

21 tests   21 :white_check_mark:  53m 48s :stopwatch:  3 suites   0 :zzz:  3 files     0 :x:

Results for commit e4acee4e.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar May 08 '24 10:05 github-actions[bot]

Node: Unit Test Results

  2 282 files  ±0    2 282 suites  ±0   2h 49m 19s :stopwatch: - 53m 2s 118 828 tests +1  118 761 :white_check_mark: +1  67 :zzz: ±0  0 :x: ±0  127 316 runs  +1  127 249 :white_check_mark: +1  67 :zzz: ±0  0 :x: ±0 

Results for commit e4acee4e. ± Comparison against base commit 9f5c1683.

This pull request removes 4018 and adds 3782 tests. Note that renamed tests count towards both.

  
             IssuerDN: CN=s-aaaa
            SubjectDN: CN=s-aaaa
           Final Date: Fri Jan 01 00:00:00 UTC 2100
           Public Key: RSA Public Key [2e:28:bc:1e:d3:83:25:92:8e:cb:98:b1:b6:84:06:9c:d5:d8:14:d5],[56:66:d1:a4]
           Start Date: Sat Jan 01 00:00:00 UTC 2000
         SerialNumber: 12482092706667292405
        modulus: c1a0ff5d2372b53d12d12bb87dd03f5e…
        modulus: c1a0ff5d2372b53d12d12bb87dd03f5…
…
com.hedera.node.app.grpc.impl.netty.GrpcServiceBuilderTest ‑ [4] 

com.hedera.node.app.grpc.impl.netty.GrpcServiceBuilderTest ‑ [6] 

com.hedera.node.app.grpc.impl.netty.GrpcServiceBuilderTest ‑ [7]   
  
com.hedera.node.app.service.contract.impl.test.exec.systemcontracts.ExchangeRateSystemContractTest ‑ valueShouldNotBeSentToThePrecompile()
com.hedera.node.app.service.mono.state.codec.VirtualKeySerdesAdapterTest ‑ [10] com.hedera.node.app.service.mono.state.codec.VirtualBlobKey@d537009f
com.hedera.node.app.service.mono.state.codec.VirtualKeySerdesAdapterTest ‑ [11] com.hedera.node.app.service.mono.state.codec.VirtualBlobKey@b8746ac0
com.hedera.node.app.service.mono.state.codec.VirtualKeySerdesAdapterTest ‑ [12] com.hedera.node.app.service.mono.state.codec.VirtualBlobKey@b2882a92
com.hedera.node.app.service.mono.state.codec.VirtualKeySerdesAdapterTest ‑ [13] com.hedera.node.app.service.mono.state.codec.VirtualBlobKey@9b33bc9f
com.hedera.node.app.service.mono.state.codec.VirtualKeySerdesAdapterTest ‑ [14] com.hedera.node.app.service.mono.state.codec.VirtualBlobKey@cfb0351a
com.hedera.node.app.service.mono.state.codec.VirtualKeySerdesAdapterTest ‑ [15] com.hedera.node.app.service.mono.state.codec.VirtualBlobKey@f9b5ec5
…

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar May 08 '24 10:05 github-actions[bot]

Node: HAPI Test (Smart Contract) Results

592 tests   591 :white_check_mark:  1h 11m 52s :stopwatch:  63 suites    0 :zzz:  63 files      1 :x:

For more details on these failures, see this check.

Results for commit e4acee4e.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar May 09 '24 12:05 github-actions[bot]

My general thoughts on Hash: If we refactor Hash to an interface ImmutableHash could be a record. That would make it way easier to define it as a public api in future.

hendrikebbers avatar May 13 '24 09:05 hendrikebbers

My general thoughts on Hash: If we refactor Hash to an interface ImmutableHash could be a record. That would make it way easier to define it as a public api in future.

We can make it a record in the future. Right now, my goal is to remove the access to the underlying byte[]. We also need to stop making it self serializable as well.

lpetrovic05 avatar May 13 '24 10:05 lpetrovic05

Lots of comments for your consideration.

I don't think I've done so many code reviews for you. Perhaps not any! Anyway, so you know: My style is to comment on anything I find interesting, and to let you use your own judgment on what you think of them. If I find a bug then I "request changes", otherwise I "approve" and your actions are up to you. I'm doing this in the hopes that when you do code reviews for me you'll let me know what you think and that way I'll learn some good stuff.

P.S. please fix the PR description by copying relevant bits from the issue's description. This will help you create a proper commit message for the merge, later. Also, this is a "feat" not a "chore" IMO.

Feat as opposed to chore makes sense. As for copying the ticket into the description, I don't see the point in that, I don't like duplicating data/code/text 😄

lpetrovic05 avatar May 15 '24 09:05 lpetrovic05

Feat as opposed to chore makes sense. As for copying the ticket into the description, I don't see the point in that, I don't like duplicating data/code/text 😄

Up to you of course, but 1) I hope the final merge commit message makes sense for a person just looking at a cloned repo and 2) the advantage would be for the reader of the PR to have it right there.

david-bakin-sl avatar May 15 '24 10:05 david-bakin-sl