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

BouncyCastle EC math is slow in Blazor WebAssembly #963

Open
JVanloofsvelt opened this issue Jan 26, 2021 · 24 comments
Open

BouncyCastle EC math is slow in Blazor WebAssembly #963

JVanloofsvelt opened this issue Jan 26, 2021 · 24 comments

Comments

@JVanloofsvelt
Copy link
Contributor

It takes about a second to derive the next key from an ExtendedKey. Perhaps this library can make use of JSInterop for the Webcrypto APIs when the platform is 'browser'.

@NicolasDorier
Copy link
Collaborator

I am surprised it even works. WASM does not support Span? Because if you use the netstandard2.1 version of NBitcoin it is not using Bouncycastle, but something which should be more efficient.

@NicolasDorier
Copy link
Collaborator

I can do a special build of NBitcoin for WASM environment, but you will need to guide me here about what should be done for browser environment.

@JVanloofsvelt
Copy link
Contributor Author

JVanloofsvelt commented Jan 27, 2021

I built it with following compilation symbols set:

  • NOCUSTOMSSLVALIDATION
  • NETSTANDARD
  • NO_ARRAY_FILL
  • NULLABLE_SHIMS
  • USEBC
  • NONATIVEHASH
  • NO_NATIVESHA1
  • NO_NATIVERIPEMD160
  • NO_NATIVE_RFC2898_HMACSHA512
  • NO_NATIVERIPEMD160
  • NO_NATIVE_HMACSHA512

This way, I managed to build it for target 'netstandard2.0'. Blazor WASM has no support for System.Security.Cryptography, that's why I toggled "USEBC" (and the other NO_NATIVE...'s). Or is there perhaps a more efficient implementation that is not using System.Security.Cryptography? (in that case I must have missed it)

I don't recall exactly if I was able to use Span, I can investigate this later.

@NicolasDorier
Copy link
Collaborator

NicolasDorier commented Jan 27, 2021

Can you try to toggle HAS_SPAN?

@NicolasDorier
Copy link
Collaborator

NicolasDorier commented Jan 27, 2021

Also check if you can use

using (var sha = new SHA256Managed())
			{
				return sha.ComputeHash(data, offset, count);
			}

So we can find what is supported or not. Same with SHA512Managed

@NicolasDorier
Copy link
Collaborator

I checked the code, toggling HAS_SPAN might already improve things if WASM support it.

@JVanloofsvelt
Copy link
Contributor Author

I will try. Should this speed up the bouncy castle EC math?

I can try the SHA256Managed, but this is probably not the slowest part.

@NicolasDorier
Copy link
Collaborator

If you set HAS_SPAN it will use BC only for hash things. Not EC.

@JVanloofsvelt
Copy link
Contributor Author

JVanloofsvelt commented Jan 27, 2021

I tried it again, now targetting 'netstandard2.1'.

I had to add these 2 compilation symbols, because System.Security.Cryptography is not supported in wasm:

  • NO_NATIVE_HMACSHA512
  • NO_NATIVE_RFC2898_HMACSHA512

However, the project won't compile like this because the BouncyCastle source files get removed from the project when targetting 'netstandard2.1', while BouncyCastle is the fallback for the native HMACSHA512 that is now missing. I can restore the files by editing the csproj file to switch RemoveBC off:
<RemoveBC>false</RemoveBC>

Furthermore, I also had to remove the compilation symbol NO_BC

There are a few more issues that I can fix, but the result is that key derivation still runs slow in the browser.

@JVanloofsvelt
Copy link
Contributor Author

JVanloofsvelt commented Jan 27, 2021

Also check if you can use

using (var sha = new SHA256Managed())
			{
				return sha.ComputeHash(data, offset, count);
			}

So we can find what is supported or not. Same with SHA512Managed

They are not supported.
EDIT: I was wrong, they are supported. HMACSHA is not however.

@JVanloofsvelt
Copy link
Contributor Author

JVanloofsvelt commented Jan 27, 2021

@NicolasDorier please review my pull request:
https://github.com/JVanloofsvelt/NBitcoin/pull/1

As I wrote earlier, managed SHA256 and SHA512 are not supported in Blazor wasm. After applying the changes from my patch/pull-request, you should be able to build the project for netstandard2.1 and get it to work in Blazor wasm. You'll need to set RemoveBC to false, and add the following compilation symbols:

NO_NATIVESHA1;NONATIVEHASH;NO_NATIVE_HMACSHA512;NO_NATIVE_RFC2898_HMACSHA512

Unlike my first attempt at compiling it for Blazor, I am not forcing the use of Bouncy Castle with the USEBC compilation symbol.

Now the sad news: in Blazor wasm it takes my system 300-500ms to derive an ExtPubKey from a parent key, which is slow.

@NicolasDorier
Copy link
Collaborator

NicolasDorier commented Jan 28, 2021

@JVanloofsvelt you did not open the PR on the right repo, you did on your fork. I will check what I can do about the RemoveBC. I should be smarter about it.

Your PR seems good.

@NicolasDorier
Copy link
Collaborator

NicolasDorier commented Jan 28, 2021

I think rather than <ItemGroup Condition=" '$(RemoveBC)' == 'true' "> in the csproj, I should just look for the NO_BC constant.

Now the sad news: in Blazor wasm it takes my system 300-500ms to derive an ExtPubKey from a parent key, which is slow.

Have you tried to set HAS_SPAN ?

@NicolasDorier
Copy link
Collaborator

NicolasDorier commented Jan 28, 2021

@JVanloofsvelt I am wondering if instead of relying on BC, we could not just import SHA256Managed.cs inside NBitcoin (keep it internal, change the namespace)

After all, this is open source.

@NicolasDorier
Copy link
Collaborator

NicolasDorier commented Jan 28, 2021

@JVanloofsvelt
Copy link
Contributor Author

@NicolasDorier I opened the pull request on your repo this time, sorry for that.

  1. I'll leave it up to you to use NO_BC or not when deciding whether or not to include those BC files.
  2. I believe HAS_SPAN was set to 'true' without any action on my part (perhaps because I was targetting netstandard2.1?).

@NicolasDorier
Copy link
Collaborator

Checking on here if we can't just copy/paste dotnet managed code rather than depending on BC

dotnet/runtime#40074

I don't know if hashing is the reason why it is slow though.
The EC operations if HAS_SPAN is set is using secp256k1 in C# which is normally very efficient.

@JVanloofsvelt
Copy link
Contributor Author

JVanloofsvelt commented Jan 28, 2021

@NicolasDorier I tried it once more, with the same configuration as I described here, but now using Microsoft Edge. The key derivation is taking only 10s of milliseconds (as opposed to 100s).

My dotnet version is 5.0.102 by the way.

Edit: in Firefox it's running equally fast

@JVanloofsvelt
Copy link
Contributor Author

If we could port SHA256Managed then we could exclude BC again.

@NicolasDorier
Copy link
Collaborator

I don't know if that's worth it, as I don't know if that's the real bottleneck.

@NicolasDorier
Copy link
Collaborator

@JVanloofsvelt can you reply to dotnet/runtime#40074 they say hash functions are supported on .net5.0

@JVanloofsvelt
Copy link
Contributor Author

@NicolasDorier Hi Nicolas, I verified it, and indeed hash functions seem to be supported.

The problem with NBitcoin is that for the HMACs it either uses a native HMAC with a native hash function, or it uses BC HMAC with BC hash function. I modified the code a little so that it can use BC HMAC with a native hash function.

My changes allow me to switch on compilation symbol "NO_BC" again. Furthermore, I can drop "NONATIVEHASH" which I had switched on earlier. This is a win.

I will create another pull request.

@JVanloofsvelt
Copy link
Contributor Author

JVanloofsvelt commented Jan 30, 2021

Pull request: #965

Use following compilation symbols to get a build that works in Blazor wasm:
NO_NATIVE_HMACSHA512;NO_NATIVESHA1;NO_NATIVE_RFC2898_HMACSHA512;NOCUSTOMSSLVALIDATION;NO_NATIVERIPEMD160;NETCORE;HAS_SPAN;NO_BC

The gist being that you need to add NO_NATIVE_HMACSHA512 and NO_NATIVE_RFC2898_HMACSHA512.

@NicolasDorier
Copy link
Collaborator

@JVanloofsvelt have you also check if the perfs were better?

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

No branches or pull requests

2 participants