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

Use nurkel for the tree instead of urkel. #804

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nodech
Copy link
Contributor

@nodech nodech commented Feb 24, 2023

nurkel is a node bindings for the liburkel.

It has not been tested in action so far, but I have run some tests with it, so if you are willing to experiment/test/run you are welcome.

There's also liburkel-urkel-nurkel integration suite here: https://github.com/nodech/liburkel-test, unfortunately not well documented.

Todo

  • Add more options for tree:
    • liburkel tree (disk)
    • urkel (memory)
    • urkel (disk)
  • Either expose blake2b from liburkel via patch or vendor blake2b from libtorsion/bcrypto with javascript fallback. This can enable performant in memory tree and proper urkel on disk variation.
  • Instead of current Proof, use UrkelProof wrapper w/o hash, bits parameters for each method. No reimplementation of the same logic and already in use serializers (encode/decode, toJSON/fromJSON)
  • Implement synchronous version of nurkel_finalize queue (linked list) for napi env_cleanup_hook. Currently, clean up hooks will crash, as async operations are no longer permitted. So unclosed tree will crash on node.js exit.

Benchmarks

The benchmarks are pretty promising for the actual sync.

First the most important - How much time difference does it make while syncing the node. These are the sync times until the block height (NOTE: Data is a single run):
total

Graph below shows PER BLOCK TIME differences between the two, both are recorded using single run so they should be less "bumpy" on multiple runs:

perblock

This will be DRAFT until it receives some feedback and has been tested out in the wild. I will try to rebase it on master as often as possible.

@nodech nodech added enhancement general - improving existing feature performance improvement classification blockchain part of the codebase labels Feb 24, 2023
@nodech nodech force-pushed the nurkel branch 3 times, most recently from 9f44f2c to 6dc5054 Compare February 24, 2023 19:26
@handshake-enthusiast
Copy link
Contributor

Nice! Does https://github.com/nodech/liburkel-test generates the images above?

I could at least test on Mac with SPV.

@nodech
Copy link
Contributor Author

nodech commented Feb 25, 2023

Nice! Does https://github.com/nodech/liburkel-test generates the images above?

I could at least test on Mac with SPV.

No, liburkel-test generates random tree (and some specific edge cases) and compares that tree files match byte by byte (urkel.js, liburkel, nurkel).

Unfortunately, SPV does not have a tree so it wont test the tree.

As for the graphs, they were generated from the log files of the hsd. Specifically [I:2023-02-23T15:33:07Z] (chain) Block 0000000000000004226d2a1178bd745c43dc7e4ff286e45e6a21194db04e4050 (144964) added to chain (size=286599 txs=133 time=307.788625).

These were then parsed accumulated in separate file for gnuplot: https://gist.github.com/nodech/da78e501b29cd05afc620648a721d97c

@coveralls
Copy link

coveralls commented Feb 28, 2023

Coverage Status

Coverage: 68.44% (+0.006%) from 68.434% when pulling 8c7dd0d on nodech:nurkel into 6b47c3a on handshake-org:master.

@rithvikvibhu
Copy link
Member

This is very promising and I had no trouble getting Bob to sync with nurkel, awesome work @nodech :D Even switching between urkel and nurkel with the same tree data worked fine.

Some stats from my machine up to block 150k (with checkpoint at 100k):
image

Bob saw a 2x speed sync that reduced time from 20 hours to 10 hours 🤯

Some observations

  • The orange line is probably an outlier / due to network effects
  • We're not sure what that small steep slope in the middle is
  • Indexers being enabled or not didn't make any noticeable difference

As for compacting 161k blocks (approx with ± 200 blocks diff):

time
urkel (Bob) 10m 34s
nurkel (Bob) 6m 29s

I'd like to run these tests again in a proper environment and syncing from a local node to rule out network weirdness and on a dedicated machine. But ^ gives a rough idea on what we can expect.

@handshake-enthusiast
Copy link
Contributor

Great results!

Unfortunately, SPV does not have a tree so it wont test the tree.

As far as I get https://github.com/nodech/liburkel-test wouldn't work in SPV mode. Can I still run hsd in SPV mode and compare sync times using https://gist.github.com/nodech/da78e501b29cd05afc620648a721d97c?

@nodech
Copy link
Contributor Author

nodech commented Mar 9, 2023

As far as I get https://github.com/nodech/liburkel-test wouldn't work in SPV mode.

https://github.com/nodech/liburkel-test - is not related to HSD at all. It is specific to the urkel/nurkel/liburkel integration test.

Can I still run hsd in SPV mode and compare sync times using https://gist.github.com/nodech/da78e501b29cd05afc620648a721d97c?

SPV mode in hsd does not have tree (as having 24 GB tree would not make sense with SPV), so whether SPV node has this update or not, there will be no difference.

@rithvikvibhu
Copy link
Member

Found a bug:

❯ hsd-cli rpc getnameproof theshake
proof.toJSON is not a function

Think nurkel's Proof doesn't have toJSON

@handshake-enthusiast
Copy link
Contributor

@nodech thanks, now it's explicit enough for me to understand what's going on. I hoped for an SPV speedup, but there won't be any.

Then I wanted to contribute and compare time required to sync a "real life" full node with https://github.com/nodech/nurkel in comparison with https://github.com/handshake-org/urkel. My results are incomplete, but anyway I'll better post them than nothing. Initially I hoped for a quick sync, like @rithvikvibhu showed 20 hours, but in my case I got not much in 20 hours and I couldn't keep my laptop always on connected to the same wi-fi network, so I stopped the experiment. I started two hsd full nodes simultaneously in order to make comparison better, but I'm not sure whether it was a good choice actually.

Environment

  • macOS 13.2.1
  • 2,6 GHz 6-Core Intel Core i7
  • 16 GB 2667 MHz DDR4
  • node v18.13.0
  • Wi-Fi should be stable and give up to 100 Mb/s
  • I used an external flash memory

Problems

  1. It took some time to figure out how to run two (actually three) hsd full nodes simultaneously. See the settings below in "Commands".
  2. hsd with urkel once got Abort trap: 6. I noticed this a few hours later:
[info] (chain) Block 0000000000000002078d3be05db816e9e4fca8a89dd7120740a7e8c3181a01e2 (72896) added to chain (size=84877 txs=12 time=1541.7390759999998).
[debug] (chain) Retargetting to: 00000000000000043c5b00000000000000000000000000000000000000000000 (0x19043c5b).
[debug] (chain) Memory: rss=257mb, js-heap=74/97mb native-heap=159mb
[info] (chain) Block 0000000000000003e74ac8254d41884dfe86cbc12961049377bb3198af801990 (72897) added to chain (size=95004 txs=81 time=1714.0104219999998).
[debug] (chain) Retargetting to: 000000000000000436ac00000000000000000000000000000000000000000000 (0x190436ac).
Abort trap: 6
  1. After I noticed the problem 2 I restarted another hsd node (with nurkel) and seemingly it started to syncronise faster. It happened after block height 107439. I noticed that in Terminal, but the chart shows this as well.

Commands

urkel (6b47c3a):
./bin/hsd --prefix .../nurkel-test/hsd-master/hsd_data/ --no-wallet --http-port=22037 --ns-port=2349 --rs-port=2350

nurkel (8c7dd0d):
./bin/hsd --prefix .../nurkel-test/hsd-nurkel/hsd_data/ --no-wallet --http-port=32037 --ns-port=3349 --rs-port=3350

Results

nurkel vs urkel

P.S.: I may retry or proceed with the experiment later. Let me know if you have any suggestions/recommendations, e.g. regarding running both hsd full nodes simultaneously.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blockchain part of the codebase enhancement general - improving existing feature performance improvement classification
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants