[Issue #32] Add BTCPay Server Support #46

raphBTC · 2019-07-02T21:33:14Z

This PR enables support for BTCPay Server.

Description

Support new connection strings
Allow connection without certificate
Add Alert Dialog

Motivation and Context

Resolve issue: #32

How Has This Been Tested?

Set up a BTCPay Server instance to test the various connection strings/QR code.
If anyone has an instance running, would be happy for additional tests 👍

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the Contribution document.
I have added tests to cover my changes.
All new and existing tests passed.

raphBTC · 2019-07-02T21:34:11Z

app/src/main/java/zapsolutions/zap/connection/btcPay/BTCPayConfiguration.java

@@ -0,0 +1,49 @@
+package zapsolutions.zap.connection.btcPay;
+
+public class BTCPayConfiguration {


I've only added the fields that we currently have in use.

"certificateThumbprint" and "ssl" should maybe also included.
If I understood it correct, having ssl=false would mean falling back to certificateThumbprint, which actually is still ssl, as it uses LNDs generated sslCertificate instead of letsencryt one.
This case would have to be implemented in the other files as well though.
@rockstardev can you maybe comment on this? Did I understood that correct and is this case acutally important or does it not happen anyway?

app/src/main/res/values-de/strings.xml

app/src/main/java/zapsolutions/zap/util/UserGuardian.java

app/src/main/java/zapsolutions/zap/setup/ConnectRemoteNodeActivity.java

# Conflicts: # app/src/main/java/zapsolutions/zap/connection/LndConnection.java # app/src/main/java/zapsolutions/zap/setup/ConnectRemoteNodeActivity.java # app/src/main/res/values-de/strings.xml # app/src/main/res/values/strings.xml

raphBTC · 2019-07-07T11:19:10Z

app/src/main/res/values/styles.xml

@@ -33,7 +33,7 @@


    <!-- Theme for the QR-Code scanner activity. -->
-    <style name="AppOverlayTheme" parent="@style/Theme.AppCompat.Light">
+    <style name="AppOverlayTheme" parent="@style/Theme.AppCompat">


@michaelWuensch The light theme converted the text in the checkbox to black on my site.

michaelWuensch · 2019-07-07T12:31:12Z

app/src/main/res/values-de/strings.xml

@@ -239,6 +239,8 @@
    <string name="error_connection_server_unreachable">Zeitüberschreitung.\nDer Server ist nicht erreichbar.\n\n- Bitte stelle sicher, dass du eine gute Internetverbindung hast.\n\n- Überprüfe ob der Server läuft und unter folgender IP-Adresse erreichbar ist:\n\n%1$s</string>
    <string name="error_connection_lnd_unavailable">Die Verbindung zum LND kann nicht hergestellt werden.\n\n- Stelle sicher, dass der LND deamon auf dem Server läuft.\n\n- Stelle sicher, dass Port forwarding aktiviert ist. (Port: %1$s)\n\n- Stelle sicher, dass das Zertifikat gültig ist.</string>
    <string name="error_connection_wallet_locked">Bitte entsperre die Wallet.</string>
+    <string name="error_connection_btcpay_invalid_json">Die BTCPay Server Konfiguration hat eine ungültige JSON syntax.</string>
+    <string name="error_connection_btcpay_invalid_config">Die BTCPay Server Konfiguration enthält keine Konfiguration für BTC GRPC.</string>


"Server Konfiguration" should be "Serverkonfiguration"

Those Germans and their concatenated nouns 😄

app/src/test/java/zapsolutions/zap/BtcPayConfigurationParserTest.java

michaelWuensch · 2019-07-07T14:08:54Z

app/src/main/java/zapsolutions/zap/connection/LndConnection.java

@@ -59,7 +60,7 @@ private void readSavedConnectionInfo() {
        mConnectionInfo = connectionInfo.split(";");
        ZapLog.debug(LOG_TAG, connectionInfo);

-        if (mConnectionInfo[2].equals("null")) {
+        if (mConnectionInfo[2].equals(BTCPayConfig.NO_CERT)) {
            // BTC PAY (no cert, hex encoded macaroon)
            mMacaroon = new MacaroonCallCredential(mConnectionInfo[3]);
        } else {


In the else part the macaroon actually has to be created like this:

// LND Connect String macaroonBase64UrlString = mConnectionInfo[3]; if (mConnectionInfo[2].equals("null")){ mMacaroon = new MacaroonCallCredential(mConnectionInfo[3]); } else { byte[] macaroonBytes = BaseEncoding.base64Url().decode(macaroonBase64UrlString); String macaroon = BaseEncoding.base16().encode(macaroonBytes); mMacaroon = new MacaroonCallCredential(macaroon); }

This covers the case when we want to connect to a BtcPay with an lndconnect string.
In the future we will have to make sure that all our parsers already encode the macaroon in the same manner, so on the device the connection information is saved in the same format, no matter where it came from.
But this would require to reconnect the node after an update within the app.
One of the next things will probably Biometrics which will also require a reconnect. That's why I would wait with this refactor, so we need to do this only once.

Yes I agree, we should try to move the persisted config to a general format.
Biometrics are optional, so they would only require a reconnect for people that will use them, right?

What we could do is to support a legacy version for a few releases and with time stop supporting it, e.g. when the user base drops to a certain percentage of that version.

R.M added 4 commits July 2, 2019 23:09

Add Models and Resources

9fc3577

Add Logic

e421c22

Add Tests

18932ff

Adapt ReadMe

91a2faa

raphBTC requested review from rockstardev and michaelWuensch July 2, 2019 21:33

raphBTC self-assigned this Jul 2, 2019

raphBTC commented Jul 2, 2019

View reviewed changes

michaelWuensch reviewed Jul 3, 2019

View reviewed changes

app/src/main/res/values-de/strings.xml Outdated Show resolved Hide resolved

michaelWuensch reviewed Jul 3, 2019

View reviewed changes

app/src/main/java/zapsolutions/zap/util/UserGuardian.java Show resolved Hide resolved

Update guardian message

a23826f

michaelWuensch reviewed Jul 5, 2019

View reviewed changes

app/src/main/java/zapsolutions/zap/setup/ConnectRemoteNodeActivity.java Outdated Show resolved Hide resolved

michaelWuensch reviewed Jul 5, 2019

View reviewed changes

app/src/main/java/zapsolutions/zap/setup/ConnectRemoteNodeActivity.java Outdated Show resolved Hide resolved

R.M added 2 commits July 7, 2019 10:00

Merge master

0bae994

# Conflicts: # app/src/main/java/zapsolutions/zap/connection/LndConnection.java # app/src/main/java/zapsolutions/zap/setup/ConnectRemoteNodeActivity.java # app/src/main/res/values-de/strings.xml # app/src/main/res/values/strings.xml

Consolidate configs, parsing and error handling

dd8a9a3

raphBTC commented Jul 7, 2019

View reviewed changes

Update comments

a12f5cd

michaelWuensch reviewed Jul 7, 2019

View reviewed changes

R.M added 4 commits July 7, 2019 18:28

Feedback

6d093bb

Merge branch 'master' into feature/32-btcpay

0358d1d

Update Error Message

c064741

Big L

7ad10cf

michaelWuensch approved these changes Jul 7, 2019

View reviewed changes

raphBTC merged commit e925e72 into master Jul 7, 2019

raphBTC deleted the feature/32-btcpay branch July 7, 2019 17:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Issue #32] Add BTCPay Server Support #46

[Issue #32] Add BTCPay Server Support #46

raphBTC commented Jul 2, 2019 •

edited

Loading

raphBTC Jul 2, 2019 •

edited

Loading

michaelWuensch Jul 3, 2019

raphBTC Jul 7, 2019

michaelWuensch Jul 7, 2019

raphBTC Jul 7, 2019

michaelWuensch Jul 7, 2019

raphBTC Jul 7, 2019

		@@ -0,0 +1,49 @@
		package zapsolutions.zap.connection.btcPay;

		public class BTCPayConfiguration {

[Issue #32] Add BTCPay Server Support #46

[Issue #32] Add BTCPay Server Support #46

Conversation

raphBTC commented Jul 2, 2019 • edited Loading

Description

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

raphBTC Jul 2, 2019 • edited Loading

Choose a reason for hiding this comment

michaelWuensch Jul 3, 2019

Choose a reason for hiding this comment

raphBTC Jul 7, 2019

Choose a reason for hiding this comment

michaelWuensch Jul 7, 2019

Choose a reason for hiding this comment

raphBTC Jul 7, 2019

Choose a reason for hiding this comment

michaelWuensch Jul 7, 2019

Choose a reason for hiding this comment

raphBTC Jul 7, 2019

Choose a reason for hiding this comment

raphBTC commented Jul 2, 2019 •

edited

Loading

raphBTC Jul 2, 2019 •

edited

Loading