Rewrite of HttpClientImpl for okhttp3

[juan-cortes]

Update

The library allows us to not specify the content type and allow it to be inferred. The first version that was setting it to json was breaking for algorand since the payload is in binary format. Removing the MediaType.parse("application/json") part of the code and simply passing null allows the request to succeed and, as seen in the traces below, it sets the content type when needed.

Add accounts / Sync

• Bitcoin
• Tezos
• Algorand
• Cosmos
• Stellar

Send

• Bitcoin
• Tezos
• Algorand
• Cosmos
• Stellar

---

Following a gist from @gre this is a rewrite simplified version of the HttpClientImpl class based on the logic from react native itself. I'm particularly worried about breaking whatever this snippet from @haammar-ledger and @pollastri-pierre was doing, but perhaps we don't have to worry about that with this implementation was doing.

lib-ledger-core-react-native-bindings/android/src/main/java/com/ledger/reactnative/HttpClientImpl.java

Lines 61 to 63 in bfebc65

	if (!headers.containsKey("Content-Encoding") && !headers.containsValue("application/x-binary")) {
	connection.setRequestProperty( "Content-Encoding", "UTF-8");
	}

https://ledgerhq.atlassian.net/browse/LL-4389

[gagbo]

I guess that in the worst case setting the headers can be handled on libcore side (the http requests for the http client impl have setters to customize the headers), but in order to know if we're missing some header setters we'll have to test at least all c++-handled currencies.

For Cosmos, anything that doesn't silently drop requests (when too many requests come and fill the job queue) should be enough to fix the initial synchronization bug

[ghost]

@juan-cortes from my dusty memories, the code you mention has to do with Algorand, where the broadcasted tx payloads are in binary format and not json like many other coins.

For Algorand txs broadcasts we do set the "Content-Type=application/x-binary" header in the cpp code, however we don't need to set a Content-Encoding header, so the code you mention added a "Content-Encoding=UTF-8" which messed with the requests... don't recall how exactly but there was some issue.

If this patch in HttpClientImpl.java is not good maybe we can try to set a Content-Encoding header from libcore side, but I don't know if there's a dummy value we could use that wouldn't have any side effect...

(+1 that all C++ coins should be tested with this PR)

[gre]

let's try to provide tomorrow a testing build that QA can try out. make sure to be in sync with latest's master & let's not forget that we would need to test Cosmos Testnet too.

actually there is 2 stategies, either we test it all-in with the new cosmos work. or we test the PREVIOUS cosmos implementation (e.g. to confirm bug is fixed).
I tend to think we need to do both at least to confirm cosmos is indeed fixed.

[gagbo]

Closes #71 by the way

[gagbo]

Actually it looks like Content-Encoding: UTF-8 isn't really a standard or valid value, why was it needed ?

[gre]

@juan-cortes let's try to come back on a plan for this in next sprint.

[juan-cortes]

Summary of today's findings, my assumption that hardcoding the application/json header for the content type would break things turned out to be correct and it broke for several currencies, ie algorand, stellar. Setting it to null allows the okhttp library to decide which content type to use, and this worked out of the box for all except stellar, which needed to have the one set in the header to begin with.

Long story short, it all works right now, ready for QA as far as I'm concerned.
I've reset the branch to match master and only added the changes to this file since it was a mess.

cc @gre

[juan-cortes]

Breaks on libcore involved swaps.
Just happened to test that unknowingly, I will take a look since I don't see why it would be different for a swap, it's just a normal tx in the end 🤔