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

feat: use native crypto functions #61

Merged
merged 10 commits into from
Jul 15, 2024
Merged

Conversation

wemeetagain
Copy link
Member

@wemeetagain wemeetagain commented Jul 12, 2024

@wemeetagain wemeetagain requested a review from a team as a code owner July 12, 2024 15:37
@nflaig
Copy link
Member

nflaig commented Jul 12, 2024

This does not change much for me when importing keys in Lodestar

Unstable branch

Jul-12 17:23:52.907[]                 info: Lodestar network=holesky, version=v1.20.0/6ab2697, commit=6ab26974616310ffa3786f9639913851f684c088
Jul-12 17:23:52.909[]                 info: Connecting to LevelDB database path=/home/devops/.local/share/lodestar/holesky/validator-db
Jul-12 17:24:02.933[]                 info: 2% of keystores imported. current=48 total=2000 rate=288.00keys/m
Jul-12 17:24:12.934[]                 info: 5% of keystores imported. current=96 total=2000 rate=287.97keys/m
Jul-12 17:24:22.934[]                 info: 7% of keystores imported. current=143 total=2000 rate=282.00keys/m
Jul-12 17:24:32.939[]                 info: 10% of keystores imported. current=193 total=2000 rate=299.88keys/m
Jul-12 17:24:42.939[]                 info: 13% of keystores imported. current=251 total=2000 rate=347.97keys/m
Jul-12 17:24:52.939[]                 info: 15% of keystores imported. current=300 total=2000 rate=294.00keys/m

Using this branch ChainSafe/lodestar@unstable...nflaig/web-crypto-pbkdf2

Jul-12 17:19:59.459[]                 info: Lodestar network=holesky, version=v1.20.0/nflaig/web-crypto-pbkdf2/5d78334, commit=5d783348f05d182b54383ddc0fdd50aaa510ec6c
Jul-12 17:19:59.461[]                 info: Connecting to LevelDB database path=/home/devops/.local/share/lodestar/holesky/validator-db
Jul-12 17:20:09.504[]                 info: 2% of keystores imported. current=36 total=2000 rate=215.98keys/m
Jul-12 17:20:19.506[]                 info: 4% of keystores imported. current=83 total=2000 rate=281.92keys/m
Jul-12 17:20:29.511[]                 info: 7% of keystores imported. current=132 total=2000 rate=293.85keys/m
Jul-12 17:20:39.513[]                 info: 9% of keystores imported. current=173 total=2000 rate=245.95keys/m
Jul-12 17:20:49.516[]                 info: 11% of keystores imported. current=226 total=2000 rate=317.90keys/m
Jul-12 17:20:59.517[]                 info: 14% of keystores imported. current=276 total=2000 rate=299.97keys/m

@nflaig
Copy link
Member

nflaig commented Jul 12, 2024

My guess is the decryption part takes most of the time, not key derivation

return await aesDecrypt(

We might be able to use https://developer.mozilla.org/en-US/docs/Web/API/SubtleCrypto/decrypt there

@nflaig nflaig changed the title chore: use webcrypto for pbkdf2 chore: use webcrypto Jul 13, 2024
@nflaig
Copy link
Member

nflaig commented Jul 13, 2024

I have updated the branch to use webcrypto everywhere but the results are still the same

Jul-13 12:38:30.349[]                 info: Lodestar network=holesky, version=v1.20.0/nflaig/web-crypto-pbkdf2/3eeaa40, commit=3eeaa40756b5bb77b3af529297a171d89b6ab392
Jul-13 12:38:30.353[]                 info: Connecting to LevelDB database path=/home/devops/.local/share/lodestar/holesky/validator-db
Jul-13 12:38:40.589[]                 info: 2% of keystores imported. current=35 total=2000 rate=209.96keys/m
Jul-13 12:38:50.591[]                 info: 4% of keystores imported. current=84 total=2000 rate=293.94keys/m
Jul-13 12:39:00.591[]                 info: 6% of keystores imported. current=127 total=2000 rate=258.00keys/m
Jul-13 12:39:10.591[]                 info: 9% of keystores imported. current=182 total=2000 rate=330.00keys/m
Jul-13 12:39:20.611[]                 info: 12% of keystores imported. current=231 total=2000 rate=293.41keys/m
Jul-13 12:39:30.612[]                 info: 14% of keystores imported. current=278 total=2000 rate=281.97keys/m
Jul-13 12:39:40.613[]                 info: 17% of keystores imported. current=332 total=2000 rate=324.00keys/m

Need to further investigate, might wanna benchmark each function individually and compare that way.

Also tried keystore decryption without thread pool, and with thread pool and increased concurrency, both made the results much worse.

Looking at standard thread pool configuration we use, CPU utilization is maxed out already, likely the only way to improve the situation is to use more efficient implementations which does not seem to be the case with webcrypto compared to previous performance.

@wemeetagain
Copy link
Member Author

strange, the unit tests run ~10x faster

@nflaig
Copy link
Member

nflaig commented Jul 13, 2024

strange, the unit tests run ~10x faster

I know why, it is using scrypt, not pbkdf2. I guess this depens on how the keystores are created?

@nflaig
Copy link
Member

nflaig commented Jul 13, 2024

Looking at decrypt timings

kdf: 2.057s
verify: 1.663ms
decrypt: 1.376ms
decrypt outer: 2.073s <-- in Lodestar

the majority of time is spend here

const decryptionKey = await kdf(keystore.crypto.kdf, normalizePassword(password));

@wemeetagain
Copy link
Member Author

I know why, it is using scrypt, not pbkdf2. I guess this depens on how the keystores are created?

Individual tests are running 10x faster. Specifically the round trip test (uses pbkdf2) and the pbkdf2 test vectors

@nflaig
Copy link
Member

nflaig commented Jul 13, 2024

I know why, it is using scrypt, not pbkdf2. I guess this depens on how the keystores are created?

Individual tests are running 10x faster. Specifically the round trip test (uses pbkdf2) and the pbkdf2 test vectors

Looks like all my keystores use scrypt

{"crypto": {"kdf": {"function": "scrypt", "params": {"dklen": 32, "n": 262144, "r": 8, "p": 1, "salt": "9b030cd77d229630253ffbb61708fb55e5206a3586de016b929f03315993d71b"}, "message": ""}, "checksum": {"function": "sha256", "params": {}, "message": "6455354b6666a28e4fa3b88e74d8b53e57d32a7dacbe0c299c8f7dfbb154c7c9"}, "cipher": {"function": "aes-128-ctr", "params": {"iv": "4c68e0d27df40979c5c466b801102ae5"}, "message": "fc6f971cf0d6863937e989c02be40b191b03b9c05dd36f35a9d85da1f73aedc5"}}, "description": "", "pubkey": "b8123417e2afcb2877fef62b74d8dc9b4405fa724953c9c61fd47b3efc346915902fbfd9f9c356328a1d37660c771e70", "path": "m/12381/3600/1047/0/0", "uuid": "0e120faa-39af-40a1-aad9-964bf45acacc", "version": 4}

Which is the default for staking deposit cli, see ethereum/staking-deposit-cli#140

@nflaig
Copy link
Member

nflaig commented Jul 13, 2024

Benchmarks for pbkdf2

This branch (before my changes)

pbkdf2
    ✔ create                      7.668867 ops/s    130.3974 ms/op
    ✔ verifyPassword              7.783849 ops/s    128.4712 ms/op
    ✔ decrypt                     7.748557 ops/s    129.0563 ms/op

This branch (after my changes)

pbkdf2
    ✔ create                      7.701212 ops/s    129.8497 ms/op 
    ✔ verifyPassword              7.749816 ops/s    129.0353 ms/op
    ✔ decrypt                     7.908960 ops/s    126.4389 ms/op 

Master branch (using ethereum-cryptography)

pbkdf2
    ✔ create                       1.009159 ops/s    990.9241 ms/op
    ✔ verifyPassword               1.123363 ops/s    890.1838 ms/op 
    ✔ decrypt                      1.034486 ops/s    966.6636 ms/op 

Looks like key derivation takes most of the time, using webcrypto for pbkdf2 might be sufficient.

PS: I was not able to get the benchmark tool to work without a few hacks, due to issues with loading files. Not sure if it's worth to get it to work with this repository.

@wemeetagain
Copy link
Member Author

wemeetagain commented Jul 14, 2024

@nflaig can you give another review/benchmark?
I added ability to use node crypto for both pbkdf2 and scrypt

@nflaig
Copy link
Member

nflaig commented Jul 14, 2024

Benchmarks before update to node crypto

Known Test Vectors
    ✔ pbkdf2 / create                                                     7.680681 ops/s    130.1968 ms/op        -         12 runs   2.09 s
    ✔ pbkdf2 / verifyPassword                                             7.719184 ops/s    129.5474 ms/op        -         12 runs   2.07 s
    ✔ pbkdf2 / decrypt                                                    7.769929 ops/s    128.7013 ms/op        -         12 runs   2.06 s
    ✔ scrypt / create                                                    0.5223365 ops/s    1.914475  s/op        -         24 runs   47.4 s
    ✔ scrypt / verifyPassword                                            0.5220932 ops/s    1.915367  s/op        -         28 runs   55.5 s
    ✔ scrypt / decrypt                                                   0.5045171 ops/s    1.982093  s/op        -         19 runs   39.0 s

Importing pbkdf2 keys in Lodestar (uses ethereum-cryptography)

Jul-14 10:55:37.191[]                 info: 5% of local keystores imported. current=93 total=2000 rate=557.78keys/m
Jul-14 10:55:47.192[]                 info: 10% of local keystores imported. current=204 total=2000 rate=666.00keys/m
Jul-14 10:55:57.199[]                 info: 16% of local keystores imported. current=311 total=2000 rate=641.49keys/m

Importing pbkdf2 keys in Lodestar (uses webcrypto)

Jul-14 10:22:22.148[]                 info: 18% of local keystores imported. current=358 total=2000 rate=2147.79keys/m
Jul-14 10:22:32.149[]                 info: 39% of local keystores imported. current=775 total=2000 rate=2501.75keys/m
Jul-14 10:22:42.149[]                 info: 59% of local keystores imported. current=1172 total=2000 rate=2382.00keys/m

Importing scrypt keys in Lodestar (uses ethereum-cryptogprahy)

Jul-14 09:27:09.784[]                 info: 2% of keystores imported. current=36 total=2000 rate=215.96keys/m
Jul-14 09:27:19.787[]                 info: 5% of keystores imported. current=94 total=2000 rate=347.90keys/m
Jul-14 09:27:29.788[]                 info: 7% of keystores imported. current=136 total=2000 rate=251.97keys/m

Benchmarks after update to node crypto

Known Test Vectors
    ✔ pbkdf2 / create                                                     7.923219 ops/s    126.2113 ms/op        -         12 runs   2.12 s
    ✔ pbkdf2 / verifyPassword                                             7.993852 ops/s    125.0961 ms/op        -         12 runs   2.01 s
    ✔ pbkdf2 / decrypt                                                    7.951492 ops/s    125.7626 ms/op        -         12 runs   2.02 s
    ✔ scrypt / create                                                    0.6795214 ops/s    1.471624  s/op        -         22 runs   34.0 s
    ✔ scrypt / verifyPassword                                            0.7001338 ops/s    1.428298  s/op        -         20 runs   30.2 s
    ✔ scrypt / decrypt                                                   0.6419487 ops/s    1.557757  s/op        -         18 runs   29.7 s

Importing pbkdf2 keys in Lodestar (uses node:crypto), it almost instantly imported the 2k keys. I am not quite sure why the benchmarks are the same but I observed some strange behavior when using webcrypto inside worker threads, it almost looks like it needs to warm up, it starts really slow, taking ~1-2 seconds for kdf but after few runs it goes down to a few ms.

Jul-14 10:42:59.120[]                 info: 50% of local keystores imported. current=1009 total=2000 rate=6054.00keys/m
Jul-14 10:43:07.833[]                 info: 100% of local keystores imported. current=2000 total=2000 rate=6824.29keys/m

Importing scrypt keys in Lodestar (uses node:crypto), it's a nice speed up here as well, I don't think we can speed up scrypt much more than that

Jul-14 09:50:48.114[]                 info: 4% of keystores imported. current=71 total=2000 rate=425.91keys/m
Jul-14 09:50:58.114[]                 info: 8% of keystores imported. current=154 total=2000 rate=498.00keys/m
Jul-14 09:51:08.115[]                 info: 12% of keystores imported. current=230 total=2000 rate=455.95keys/m

The speed up for pbkdf2 is ~10x based on data which is also the format used by Kurtosis which uses https://github.com/protolambda/eth2-val-tools to generate the keys. Although there is strange behavior if running Lodestar inside docker (see below)

Running the command

kurtosis run github.com/ethpandaops/ethereum-package --args-file https://raw.githubusercontent.com/ethpandaops/ethereum-package/main/.github/tests/geth-all.yaml

It seems to take a while to start decryption, might be related to workers inside docker? I don't see this when running locally (as shown in logs above)

Jul-14 10:03:14.591[]                 info: 0% of local keystores imported. current=0 total=64 rate=0.00keys/m
Jul-14 10:03:24.589[]                 info: 0% of local keystores imported. current=0 total=64 rate=0.00keys/m
Jul-14 10:03:28.690[]                 info: 100% of local keystores imported. current=64 total=64 rate=936.59keys/m

And looking at timings of each decrypt (inside worker), it looks pretty strange, some take a few ms, others more than second

decrypt: 798.274ms
decrypt: 1.398s
decrypt: 1.001s
decrypt: 198.203ms
decrypt: 1.300s
decrypt: 699.015ms
decrypt: 199.123ms
decrypt: 199.513ms
decrypt: 999.94ms
decrypt: 1.002s
decrypt: 1.879ms
decrypt: 99.586ms
.... towards the end it speeds up
decrypt: 1.368ms
decrypt: 196.697ms
decrypt: 196.034ms
decrypt: 2.626ms
decrypt: 96.339ms
decrypt: 96.326ms
decrypt: 1.777ms
decrypt: 197.557ms
decrypt: 98.913ms
decrypt: 1.089ms
decrypt: 1.156ms
decrypt: 97.333ms

Our current unstable branch image seems to be faster..

Jul-14 10:07:45.585[]                 info: 0% of local keystores imported. current=0 total=64 rate=0.00keys/m
Jul-14 10:07:55.583[]                 info: 28% of local keystores imported. current=18 total=64 rate=108.01keys/m
Jul-14 10:07:58.085[]                 info: 100% of local keystores imported. current=64 total=64 rate=1103.12keys/m

Could give this a try without using worker threads, maybe we check key count and only spawn threads at a certain amount, although for scrypt this is likely not a good idea.

So far the observation are

  • web crypto and node:crypto choke inside worker threads
  • great performance gains outside of docker

I have pushed code to take benchmarks here: master...nflaig/benchmark (wip)

@nflaig
Copy link
Member

nflaig commented Jul 15, 2024

Could give this a try without using worker threads, maybe we check key count and only spawn threads at a certain amount, although for scrypt this is likely not a good idea

Did more testing in Kurtosis, with and without workers threads, at least on my machine not using workers make this pretty much instant. The reason why this is so fast in Kurtosis is because they use pbkdf2 but in addition set c=2 in kdf params which is insecure but much faster and good for testing.

Disabling the worker pool, it takes ~1 sec to decrypt 64 keys

Jul-15 09:11:56.107[]                 info: Connecting to LevelDB database path=/root/.local/share/lodestar/testnet/validator-db
Jul-15 09:11:56.614[]                 info: 100% of local keystores imported. current=64 total=64 rate=7664.67keys/m
Jul-15 09:11:57.514[]                 info: 64 local keystores

It take sub ms time to decrypt those keystores, so spawning workers would anyhow be overkill in this environment.

My idea is to add a CLI flag which allows to disable worker thread for decryption.

@nflaig nflaig changed the title chore: use webcrypto chore: use native crypto functions Jul 15, 2024
@nflaig nflaig changed the title chore: use native crypto functions feat: use native crypto functions Jul 15, 2024
nflaig
nflaig previously approved these changes Jul 15, 2024
Copy link
Member

@nflaig nflaig left a comment

Choose a reason for hiding this comment

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

LGTM - might wanna get another review, also do we want to use webcrypto everywhere or just for key derivation?

nflaig
nflaig previously approved these changes Jul 15, 2024
@wemeetagain wemeetagain merged commit 750d2e8 into master Jul 15, 2024
7 checks passed
@wemeetagain wemeetagain deleted the cayman/web-crypto-pbkdf2 branch July 15, 2024 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants