Fix remote-hsmd CI (#12)

* adding g++, protobuf, grpc++
* add libgrpc++-dev to .travis.yml
* wget remotesigner.proto
* added libprotobuf-dev
* fixed compilation warnings/errors
* use Get instead of [] in protobuf seq
* fixed include sort order
* added protobuf generated files to skip nogen checks
* fixed gensrc pattern
* added libstdc++6
* fixed python spacing
* added specified inclusion guards
* renamed c++ header extensions to hpp to avoid c checks
* guard satoshi amount calculations
* libstdc++-6-dev
* added `ln -s /usr/lib/x86_64-linux-gnu/libstdc++.so.6 /usr/lib/x86_64-linux-gnu/libstdc++.so`

Author: ksedgwic

.travis.yml

@@ -5,7 +5,8 @@ notifications:
   email: false
 
 before_install:
-  sudo apt-get install -y libsqlite3-dev cppcheck valgrind gcc-4.8 gettext libc6-dev-armhf-cross libc6-dev-arm64-cross gcc-aarch64-linux-gnu gcc-arm-linux-gnueabihf qemu-user-static binfmt-support libsecp256k1-dev
+  - sudo apt-get install -y libsqlite3-dev cppcheck valgrind gcc-4.8 gettext libc6-dev-armhf-cross libc6-dev-arm64-cross gcc-aarch64-linux-gnu gcc-arm-linux-gnueabihf qemu-user-static binfmt-support libsecp256k1-dev g++ protobuf-compiler protobuf-c-compiler libprotobuf-dev protobuf-compiler-grpc libgrpc++-dev libstdc++-6-dev
+  - sudo ln -s /usr/lib/x86_64-linux-gnu/libstdc++.so.6 /usr/lib/x86_64-linux-gnu/libstdc++.so
 
 env:
   - ARCH=64 SOURCE_CHECK_ONLY=true COPTFLAGS="-O3"

.travis/build.sh

@@ -45,6 +45,8 @@ pip3 install --user -U --quiet --progress-bar off \
      pytest-sentry \
      pytest-rerunfailures
 
+wget -q --directory-prefix=contrib/remote_hsmd https://gitlab.com/lightning-signer/rust-lightning-signer/-/raw/master/src/server/remotesigner.proto
+    
 echo "Configuration which is going to be built:"
 echo -en 'travis_fold:start:script.1\\r'
 ./configure CC="$CC"

contrib/pyln-testing/pyln/testing/utils.py

@@ -73,6 +73,7 @@ def env(name, default=None):
 TIMEOUT = int(env("TIMEOUT", 180 if SLOW_MACHINE else 60))
 SUBDAEMON = env("SUBDAEMON", "")
 
+
 def wait_for(success, timeout=TIMEOUT):
     start_time = time.time()
     interval = 0.25

contrib/remote_hsmd/Makefile

@@ -60,8 +60,8 @@ RMTHSMD_COMMON_OBJS :=				\
 	common/withdraw_tx.o
 
 # For checking
-LIGHTNINGD_RMTHSM_ALLSRC_NOGEN := $(filter-out contrib/remote_hsmd/gen_%, $(LIGHTNINGD_RMTHSM_SRC) $(LIGHTNINGD_RMTHSM_SRC))
-LIGHTNINGD_RMTHSM_ALLHEADERS_NOGEN := $(filter-out contrib/remote_hsmd/gen_%, $(LIGHTNINGD_RMTHSM_HEADERS))
+LIGHTNINGD_RMTHSM_ALLSRC_NOGEN := $(filter-out contrib/remote_hsmd/gen_% contrib/remote_hsmd/remotesigner.%, $(LIGHTNINGD_RMTHSM_SRC) $(LIGHTNINGD_RMTHSM_SRC))
+LIGHTNINGD_RMTHSM_ALLHEADERS_NOGEN := $(filter-out contrib/remote_hsmd/gen_% contrib/remote_hsmd/remotesigner.%, $(LIGHTNINGD_RMTHSM_HEADERS))
 
 $(LIGHTNINGD_RMTHSM_OBJS): $(LIGHTNINGD_HEADERS)
 

contrib/remote_hsmd/NOTES.md

@@ -83,7 +83,6 @@ rust-lightning-signer
 
     cargo run --bin server |& tee log3
 
-
 Signing Formats
 ```
 rust-lightning  c-lightning     rust-lightning-signer
@@ -94,13 +93,3 @@ p2shwpkh        p2sh-p2wpkh     P2SH_P2WPKH
 p2wsh
 p2shwsh
 ```
-
-
-Failing tests after removing seed from hsmd:
-```
-    export THETEST=tests/test_misc.py::test_blockchaintrack
-    export THETEST=tests/test_misc.py::test_new_node_is_mainnet
-    export THETEST=tests/test_misc.py::test_getsharedsecret
-    export THETEST=tests/test_wallet.py::test_hsm_secret_encryption
-    export THETEST=tests/test_wallet.py::test_hsmtool_secret_decryption
-```

contrib/remote_hsmd/capabilities.h

@@ -1,5 +1,5 @@
-#ifndef LIGHTNING_HSMD_CAPABILITIES_H
-#define LIGHTNING_HSMD_CAPABILITIES_H
+#ifndef LIGHTNING_CONTRIB_REMOTE_HSMD_CAPABILITIES_H
+#define LIGHTNING_CONTRIB_REMOTE_HSMD_CAPABILITIES_H
 
 #define HSM_CAP_ECDH 1
 #define HSM_CAP_SIGN_GOSSIP 2
@@ -9,4 +9,4 @@
 #define HSM_CAP_SIGN_CLOSING_TX 32
 
 #define HSM_CAP_MASTER 1024
-#endif /* LIGHTNING_HSMD_CAPABILITIES_H */
+#endif /* LIGHTNING_CONTRIB_REMOTE_HSMD_CAPABILITIES_H */

contrib/remote_hsmd/dump.cc

@@ -1,5 +1,4 @@
-#include <iostream>
-#include <sstream>
+#include "contrib/remote_hsmd/dump.hpp"
 
 extern "C" {
 #include <bitcoin/chainparams.h>
@@ -11,11 +10,13 @@ extern "C" {
 #include <common/status.h>
 #include <common/utils.h>
 #include <common/utxo.h>
-#include <secp256k1_recovery.h>
+}
+#include <iostream>
+#include <sstream>
+extern "C" {
 #include <wally_bip32.h>
 }
 
-#include "contrib/remote_hsmd/dump.h"
 
 using std::ostringstream;
 using std::string;

contrib/remote_hsmd/dump.h renamed to contrib/remote_hsmd/dump.hpp

@@ -1,3 +1,10 @@
+#ifndef LIGHTNING_CONTRIB_REMOTE_HSMD_DUMP_H
+#define LIGHTNING_CONTRIB_REMOTE_HSMD_DUMP_H
+
+extern "C" {
+#include <ccan/short_types/short_types.h>
+#include <secp256k1_recovery.h>
+}
 #include <string>
 
 std::string dump_hex(const void *vptr, size_t sz);
@@ -26,3 +33,5 @@ std::string dump_wally_tx_outputs(const struct wally_tx_output *outputs,
 std::string dump_wally_tx(const struct wally_tx *wtx);
 std::string dump_wally_psbt(const struct wally_psbt *psbt);
 std::string dump_tx(const struct bitcoin_tx *tx);
+
+#endif /* LIGHTNING_CONTRIB_REMOTE_HSMD_DUMP_H */

contrib/remote_hsmd/hsmd.c

@@ -38,12 +38,12 @@
 #include <common/utils.h>
 #include <common/version.h>
 #include <common/withdraw_tx.h>
+/*~ All gen_ files are autogenerated; in this case by tools/generate-wire.py */
+#include <contrib/remote_hsmd/gen_hsm_wire.h>
+#include <contrib/remote_hsmd/proxy.hpp>
 #include <errno.h>
 #include <fcntl.h>
 #include <hsmd/capabilities.h>
-/*~ All gen_ files are autogenerated; in this case by tools/generate-wire.py */
-#include <contrib/remote_hsmd/gen_hsm_wire.h>
-#include <contrib/remote_hsmd/proxy.h>
 #include <inttypes.h>
 #include <secp256k1_ecdh.h>
 #include <sodium.h>

contrib/remote_hsmd/proxy.cc

@@ -1,19 +1,14 @@
 /* This needs to be first */
 #define __STDC_FORMAT_MACROS
-#include <inttypes.h>
-
-#include <sys/types.h>	/* These two only needed for sleep() and getpid() */
-#include <unistd.h>
-
-#include <iostream>
-#include <sstream>
-
-#include <grpc++/grpc++.h>
 
+#include "contrib/remote_hsmd/dump.hpp"
+#include "contrib/remote_hsmd/proxy.hpp"
+#include "contrib/remote_hsmd/remotesigner.grpc.pb.h"
+#include "contrib/remote_hsmd/remotesigner.pb.h"
 extern "C" {
 #include <bitcoin/chainparams.h>
-#include <bitcoin/psbt.h>
 #include <bitcoin/privkey.h>
+#include <bitcoin/psbt.h>
 #include <bitcoin/short_channel_id.h>
 #include <bitcoin/tx.h>
 #include <common/derive_basepoints.h>
@@ -22,15 +17,20 @@ extern "C" {
 #include <common/status.h>
 #include <common/utils.h>
 #include <common/utxo.h>
+}
+#include <grpc++/grpc++.h>
+#include <inttypes.h>
+#include <iostream>
+extern "C" {
 #include <secp256k1_recovery.h>
+}
+#include <sstream>
+#include <sys/types.h>	/* These two only needed for sleep() and getpid() */
+#include <unistd.h>
+extern "C" {
 #include <wally_bip32.h>
 }
 
-#include "contrib/remote_hsmd/remotesigner.pb.h"
-#include "contrib/remote_hsmd/remotesigner.grpc.pb.h"
-
-#include "contrib/remote_hsmd/dump.h"
-#include "contrib/remote_hsmd/proxy.h"
 
 using std::cerr;
 using std::endl;
@@ -269,7 +269,7 @@ void unmarshal_witnesses(RepeatedPtrField<Witness> const &wits, u8 ****o_wits)
 		owits = tal_arrz(tmpctx, u8**, nwits);
 		for (size_t ii = 0; ii < nwits; ++ii) {
 			owits[ii] = tal_arrz(owits, u8*, 2);
-			Witness const &wit = wits[ii];
+			Witness const &wit = wits.Get(ii);
 			const string &sig = wit.signature().data();
 			const string &pubkey = wit.pubkey().data();
 			owits[ii][0] = tal_arr(owits[ii], u8, sig.size());
@@ -915,7 +915,7 @@ proxy_stat proxy_handle_get_channel_basepoints(
 		unmarshal_pubkey(bps.delayed_payment(),
 				 &o_basepoints->delayed_payment);
 		unmarshal_pubkey(bps.funding_pubkey(), o_funding_pubkey);
-		status_debug("%s:%d %s self_id=%s",
+		status_debug("%s:%d %s self_id=%s basepoints=%s pubkey=%s",
 			     __FILE__, __LINE__, __FUNCTION__,
 			     dump_node_id(&self_id).c_str(),
 			     dump_basepoints(o_basepoints).c_str(),

contrib/remote_hsmd/proxy.h renamed to contrib/remote_hsmd/proxy.hpp

@@ -1,8 +1,12 @@
+#ifndef LIGHTNING_CONTRIB_REMOTE_HSMD_PROXY_H
+#define LIGHTNING_CONTRIB_REMOTE_HSMD_PROXY_H
+
 #ifdef __cplusplus
 extern "C" {
 #endif
 
 #include <ccan/short_types/short_types.h>
+#include <common/hash_u5.h>
 
 enum proxy_status {
 	/* SUCCESS */
@@ -187,3 +191,5 @@ void print_tx(char const *tag, struct bitcoin_tx const *tx);
 #ifdef __cplusplus
 } /* extern C */
 #endif
+
+#endif /* LIGHTNING_CONTRIB_REMOTE_HSMD_PROXY_H */

hsmd/hsmd.c

@@ -1473,11 +1473,11 @@ static struct io_plan *handle_new_channel(struct io_conn *conn,
 
 static bool mem_is_zero(const void *mem, size_t len)
 {
-    size_t i;
-    for (i = 0; i < len; ++i)
-        if (((const unsigned char *)mem)[i])
-            return false;
-    return true;
+	size_t i;
+	for (i = 0; i < len; ++i)
+		if (((const unsigned char *)mem)[i])
+			return false;
+	return true;
 }
 
 /*~ This is used to provide all unchanging public channel parameters. */
@@ -1497,6 +1497,7 @@ static struct io_plan *handle_ready_channel(struct io_conn *conn,
 	u16 remote_to_self_delay;
 	u8 *remote_shutdown_script;
 	bool option_static_remotekey;
+	struct amount_msat value_msat;
 
 	if (!fromwire_hsm_ready_channel(tmpctx, msg_in, &is_outbound,
 					&channel_value, &push_value, &funding_txid,
@@ -1510,8 +1511,9 @@ static struct io_plan *handle_ready_channel(struct io_conn *conn,
 		return bad_req(conn, c, msg_in);
 
 	/* Fail fast if any values are obviously uninitialized. */
-	assert(channel_value.satoshis > 0);
-	assert(push_value.millisatoshis / 1000 <= channel_value.satoshis);
+	assert(amount_sat_greater(channel_value, AMOUNT_SAT(0)));
+	assert(amount_sat_to_msat(&value_msat, channel_value));
+	assert(amount_msat_less_eq(push_value, value_msat));
 	assert(!mem_is_zero(&funding_txid, sizeof(funding_txid)));
 	assert(local_to_self_delay > 0);
 	assert(!mem_is_zero(&remote_basepoints, sizeof(remote_basepoints)));