Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace ECMH with Muhash #1624

Merged
merged 3 commits into from
Mar 22, 2021
Merged

Replace ECMH with Muhash #1624

merged 3 commits into from
Mar 22, 2021

Conversation

elichai
Copy link
Member

@elichai elichai commented Mar 22, 2021

On my machine(without turbo 2.4Ghz), ECMH:

goos: linux
goarch: amd64
pkg: github.com/kaspanet/go-secp256k1
cpu: Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
BenchmarkMultiSet_Add-16                  120726             10044 ns/op               0 B/op          0 allocs/op
BenchmarkMultiSet_Remove-16               119720              9966 ns/op               0 B/op          0 allocs/op
BenchmarkMultiSet_CombineRand-16         1549694             785.7 ns/op               0 B/op          0 allocs/op
BenchmarkMultiSet_Finalize-16             124837              9371 ns/op             128 B/op          2 allocs/op
PASS
ok      github.com/kaspanet/go-secp256k1        5.896s

MuHash:

goos: linux                                                                                              
goarch: amd64                                                                                            
pkg: github.com/kaspanet/go-muhash                                                                                                                                                                                 cpu: Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz                                                                                                                                                                     
BenchmarkMuHash_Add-16                    199528              5717 ns/op             768 B/op          2 allocs/op                                                                                                 
BenchmarkMuHash_Remove-16                 210734              5640 ns/op             768 B/op          2 allocs/op                                                                                                 
BenchmarkMuHash_CombineWorst-16           155590              7809 ns/op               0 B/op          0 allocs/op                                                                                                 
BenchmarkMuHash_CombineBest-16            157988              7815 ns/op               0 B/op          0 allocs/op                                                                                                 
BenchmarkMuHash_CombineRand-16            149949              7820 ns/op               0 B/op          0 allocs/op                                                                                                 
BenchmarkMuHash_Clone-16                1000000000          0.4664 ns/op               0 B/op          0 allocs/op                                                                                                 
BenchmarkMuHash_normalizeWorst-16         175725              6946 ns/op            3384 B/op         13 allocs/op                                                                                                 
BenchmarkMuHash_normalizeBest-16          236372              5293 ns/op            2880 B/op         10 allocs/op                                                                                                 
BenchmarkMuHash_normalizeRand-16           10000            115390 ns/op            8528 B/op         34 allocs/op                                                                                                 
BenchmarkMuHash_Finalize-16                10000            117512 ns/op            8912 B/op         35 allocs/op                                                                                                         
PASS                                                                                                     
ok      github.com/kaspanet/go-muhash   11.783s                                                          

And potentially if we add assembly implementations using SSE2/SSE4.1/AVX this can get even faster.

Note that the MuHash code isn't audited or reviewed and need to be audited before we go to mainnet.

@elichai elichai added this to the 0.10.0 milestone Mar 22, 2021
@elichai elichai added this to In progress in Mainnet via automation Mar 22, 2021
@codecov
Copy link

codecov bot commented Mar 22, 2021

Codecov Report

Merging #1624 (96794dd) into v0.10.0-dev (6824be9) will decrease coverage by 0.05%.
The diff coverage is 100.00%.

❗ Current head 96794dd differs from pull request most recent head 5cfd067. Consider uploading reports for the commit 5cfd067 to get more accurate results
Impacted file tree graph

@@               Coverage Diff               @@
##           v0.10.0-dev    #1624      +/-   ##
===============================================
- Coverage        59.97%   59.92%   -0.06%     
===============================================
  Files              516      516              
  Lines            20471    20469       -2     
===============================================
- Hits             12278    12266      -12     
- Misses            6238     6246       +8     
- Partials          1955     1957       +2     
Impacted Files Coverage Δ
domain/consensus/utils/multiset/multiset.go 68.75% <100.00%> (-3.48%) ⬇️
infrastructure/network/netadapter/router/route.go 78.57% <0.00%> (-7.15%) ⬇️
...k/netadapter/server/grpcserver/connection_loops.go 53.84% <0.00%> (-5.77%) ⬇️
...ersselectedchainstore/headersselectedchainstore.go 68.22% <0.00%> (-2.81%) ⬇️
.../consensus/processes/blockbuilder/block_builder.go 76.27% <0.00%> (-1.70%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6824be9...5cfd067. Read the comment docs.

@@ -101,10 +102,10 @@ var devnetGenesisBlock = externalapi.DomainBlock{
[]*externalapi.DomainHash{},
devnetGenesisMerkleRoot,
&externalapi.DomainHash{},
&externalapi.DomainHash{},
externalapi.NewDomainHashFromByteArray((*[32]byte)(&muhash.EmptyMuHashHash)),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be nice if Hash would have .ToArray() or something to save the (*[32]byte) ugliness

@@ -21,28 +22,27 @@ func (m multiset) Remove(data []byte) {

func (m multiset) Hash() *externalapi.DomainHash {
finalizedHash := m.ms.Finalize()
finalizedHashAsByteArray := (*[secp256k1.HashSize]byte)(finalizedHash)
return externalapi.NewDomainHashFromByteArray(finalizedHashAsByteArray)
finalizedHashAsByteArray := ([secp256k1.HashSize]byte)(finalizedHash)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use .AsArray here I think

svarogg
svarogg previously approved these changes Mar 22, 2021
@svarogg svarogg merged commit 6ec0a8a into v0.10.0-dev Mar 22, 2021
Mainnet automation moved this from In progress to Done Mar 22, 2021
@svarogg svarogg deleted the muhash branch March 22, 2021 16:15
@elichai elichai added this to Done in Mainnet May 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants