Make "crypto" module less dependent on 3rd party libraries (ease Android integration)

[serso]

This is primarily needed to ease its integration on Android.
Wallet and WalletFile were moved out of "crypto" to "core" as "core" seems to be a better place for them (this move was also needed to remove "fasterxml" dep).

The only one change that has to be done to the project now in order for "crypto" being compatible with Android is substituting the bouncycastle lib by the spongycastle, like this: serso@18bd32c

[codecov]

Codecov Report

Merging #618 into release/4.0 will decrease coverage by 0.15%.
The diff coverage is 80%.

@@                Coverage Diff                @@
##             release/4.0     #618      +/-   ##
=================================================
- Coverage          77.42%   77.26%   -0.16%     
+ Complexity          1853     1852       -1     
=================================================
  Files                241      241              
  Lines               6814     6819       +5     
  Branches            1012     1013       +1     
=================================================
- Hits                5276     5269       -7     
- Misses              1290     1300      +10     
- Partials             248      250       +2

Impacted Files	Coverage Δ	Complexity Δ
core/src/main/java/org/web3j/crypto/Wallet.java	83.69% <ø> (ø)	13 <0> (?)
...re/src/main/java/org/web3j/crypto/WalletUtils.java	98.14% <ø> (ø)	23 <0> (?)
...c/main/java/org/web3j/crypto/Bip44WalletUtils.java	95.23% <ø> (ø)	7 <0> (?)
...ore/src/main/java/org/web3j/crypto/WalletFile.java	39.03% <ø> (ø)	10 <0> (?)
crypto/src/main/java/org/web3j/crypto/Sign.java	67.64% <100%> (ø)	19 <1> (ø)	⬇️
crypto/src/main/java/org/web3j/crypto/Keys.java	90.9% <66.66%> (-4.1%)	16 <3> (+2)
.../src/main/java/org/web3j/crypto/MnemonicUtils.java	81.3% <87.5%> (+2.06%)	36 <2> (+1)	⬆️
...n/java/org/web3j/protocol/core/filters/Filter.java	53.12% <0%> (-14.07%)	8% <0%> (-2%)
core/src/main/java/org/web3j/utils/Flowables.java	84% <0%> (-4%)	10% <0%> (-1%)
...org/web3j/protocol/websocket/WebSocketService.java	85.26% <0%> (-1.06%)	50% <0%> (-1%)

---

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 cf66e64...9baa147. Read the comment docs.

[serso]

@conor10 how likely this change to hit the master? I can fix codecov checks if needed (though the changes are pretty simple)

[conor10]

I'm all for simplifying the crypto module - I really didn't like that I had to include those extra dependencies for the wallet file support.

However, is slf4j that significant a burden for Android devs? I spoke previously with some other Android devs about this - see #130

[conor10]

Is there any way we can use slf4j still? This is the one part of the PR I'm not keen on.

[conor10]

@serso any thoughts on the above?

[serso]

@conor10, I'm terribly sorry for the late response. I'm on vacation now and will return to the issue when I'm back (hopefully next week)

[serso]

Hi @conor10, I'm back. I have reverted the commit that removed slf4j dependency from crypto module. It seems that it's possible to use SLF4J on Android via https://www.slf4j.org/android/

[serso]

Ping :)

[serso]

Up

[conor10]

Hey @serso, apologies for the delay - I've been very snowed the last few months. The good news is that the ECF has given us funding, and @snazha-blkio is going to be helping maintain the project. My thinking is that this is a good candidate for the 4.0 milestone which will be going out before Devcon4.

[conor10]

I'd also like us to get publishing snapshot builds so that people have something to work with prior to official releases.

[serso]

@conor10 👍
I wonder if it would be possible to disconnect crypto from Bouncy Castle (which is a cryptographic library for Java). The problem with the Bouncy Castle is that it doesn't work well on Android and Spongy Castle has to be used instead (I unfortunately don't remember what exactly is broken). To workaround that now we have to apply a commit on the top rewriting all the imports (see serso@e53fc9c) in order to build an Android compatible jar.
Another solution could be to add an interface, say CryptoProvider, and default implementations (one for the BouncyCastle and another for the Spongy Castle) which can be used depending on the target platform.

Let me know if you agree with this proposal. I can help implementing it.

[conor10]

@serso we use SpongyCastle in the web3j-android branch (which is Java 1.6 compatible). Have you been working with the Android release or regular Java 8 web3j version?

[serso]

@conor10 I've been integrating the regular Java 8 version to an Android app. I saw the android branch but it looked stale.

Also, I believe branching is not a viable way to support multiple platforms as it requires a lot of maintenance work (especially painful to merge conflicts in moved code). A better way would be to abstract the platform details (in this case it's a crypto-provider implementation) and let the platforms decide what to use.

Regarding Java 1.6 and Android: Android has limited Java 8 support. Many common features work out of the box. I don't think Java 1.6 should be a requirement in web3j for Android.

[conor10]

Awesome - we'll get this in 4.0. Thanks @serso.

[conor10]

@serso I'd love to stop having to maintain the Android branch, but previously it seemed that this would exclude a significant number of devices from being able to use the library (this was 18 months ago or so). Is this still the case?

I want to ensure great Android support for the library's user base, but I don't have the Android platform expertise to know what the best strategy is here.

@ligi may have some opinions here too. But it would be great to get your views specifically on ensuring maximum device support.

Thanks!

[snazha-blkio]

Hi @serso

Could you please rebase from release/4.0? There have been some significant changes that have caused a break in this.

Specifically, this will result in the introduction of a circular dependency between core and crypto.

Thanks

[serso]

@snazha-blkio done. I had to move Bip44WalletUtils to core as well.

[serso]

@serso I'd love to stop having to maintain the Android branch, but previously it seemed that this would exclude a significant number of devices from being able to use the library (this was 18 months ago or so). Is this still the case?

@conor10 I'm not sure I'm following. If we talk about Java 8 language features supported on Android (lambdas, method references, etc.) then no devices are affected as these features are de-sugared (converted to Java 6). If we talk about Java 8 APIs that are not supported on older Android platforms (java.util.function, java.util.stream, etc.) then, yes, such APIs shouldn't be used in web3j. However, it's relatively easy to use pre-Java 8 alternatives (e.g. Guava) instead of such APIs. A simple lint check can also be added to restrict their usage.

Please check out this article about Java 8 and Android.

[iikirilov]

@serso I'd love to stop having to maintain the Android branch, but previously it seemed that this would exclude a significant number of devices from being able to use the library (this was 18 months ago or so). Is this still the case?

@conor10 I'm not sure I'm following. If we talk about Java 8 language features supported on Android (lambdas, method references, etc.) then no devices are affected as these features are de-sugared (converted to Java 6). If we talk about Java 8 APIs that are not supported on older Android platforms (java.util.function, java.util.stream, etc.) then, yes, such APIs shouldn't be used in web3j. However, it's relatively easy to use pre-Java 8 alternatives (e.g. Guava) instead of such APIs. A simple lint check can also be added to restrict their usage.

Please check out this article about Java 8 and Android.

Your comment is related to #769 .

[snazha-blkio]

Thanks @serso for your contribution!