Merge pull request #688 from TheBlueMatt/2020-09-dev-docs

Add developer guideline notes for C/C++ bindings generation

Author: TheBlueMatt

.github/workflows/build.yml

@@ -17,16 +17,12 @@ jobs:
         include:
           - toolchain: stable
             build-net-tokio: true
-            build-bindings: true
           - toolchain: beta
             build-net-tokio: true
-            build-bindings: true
           - toolchain: 1.39.0
             build-net-tokio: true
-            build-bindings: true
             coverage: true
           - toolchain: 1.34.2
-            build-bindings: true
     runs-on: ubuntu-latest
     steps:
       - name: Checkout source code
@@ -43,9 +39,6 @@ jobs:
       - name: Build on Rust ${{ matrix.toolchain }}
         if: "! matrix.build-net-tokio"
         run: RUSTFLAGS="-C link-dead-code" cargo build --verbose  --color always -p lightning
-      - name: Build bindings on Rust ${{ matrix.toolchain }}
-        if: matrix.build-bindings
-        run: RUSTFLAGS="-C link-dead-code" cargo build --verbose  --color always -p lightning-c-bindings
       - name: Test on Rust ${{ matrix.toolchain }} with net-tokio
         if: matrix.build-net-tokio
         run: RUSTFLAGS="-C link-dead-code" cargo test --verbose --color always
@@ -127,6 +120,9 @@ jobs:
           apt-get -y install cargo valgrind lld git g++ clang
       - name: Checkout source code
         uses: actions/checkout@v2
+      - name: Sanity test bindings
+        working-directory: lightning-c-bindings
+        run: cargo check
       - name: Install cbindgen
         run: cargo install --force cbindgen
       - name: Rebuild bindings, and check the sample app builds + links

.gitignore

@@ -1,4 +1,7 @@
 /target/
+c-bindings-gen/target/
+lightning-c-bindings/target/
+lightning-c-bindings/a.out
 /hfuzz_target/
 /net-tokio/target/
 **/*.rs.bk

CONTRIBUTING.md

@@ -99,6 +99,17 @@ Fuzzing is heavily encouraged: you will find all related material under `fuzz/`
 
 Mutation testing is work-in-progress; any contribution there would be warmly welcomed.
 
+C/C++ Bindings
+--------------
+
+You can learn more about the C/C++ bindings that are made available by reading the
+[C/C++ Bindings README](lightning-c-bindings/README.md). If you are not using the C/C++ bindings,
+you likely don't need to worry about them, and during their early experimental phase we are not
+requiring that pull requests keep the bindings up to date (and, thus, pass the bindings_check CI
+run). If you wish to ensure your PR passes the bindings generation phase, you should run the
+`genbindings.sh` script in the top of the directory tree to generate, build, and test C bindings on
+your local system.
+
 Going further
 -------------
 

Cargo.toml

@@ -3,7 +3,6 @@
 members = [
     "lightning",
     "lightning-net-tokio",
-    "lightning-c-bindings",
 ]
 
 # Our tests do actual crypo and lots of work, the tradeoff for -O1 is well worth it

genbindings.sh

@@ -48,21 +48,21 @@ fi
 # Finally, sanity-check the generated C and C++ bindings with demo apps:
 
 # Naively run the C demo app:
-gcc -Wall -g -pthread demo.c ../target/debug/liblightning.a -ldl
+gcc -Wall -g -pthread demo.c target/debug/libldk.a -ldl
 ./a.out
 
 # And run the C++ demo app in valgrind to test memory model correctness and lack of leaks.
-g++ -std=c++11 -Wall -g -pthread demo.cpp -L../target/debug/ -llightning -ldl
+g++ -std=c++11 -Wall -g -pthread demo.cpp -Ltarget/debug/ -lldk -ldl
 if [ -x "`which valgrind`" ]; then
-	LD_LIBRARY_PATH=../target/debug/ valgrind --error-exitcode=4 --memcheck:leak-check=full --show-leak-kinds=all ./a.out
+	LD_LIBRARY_PATH=target/debug/ valgrind --error-exitcode=4 --memcheck:leak-check=full --show-leak-kinds=all ./a.out
 	echo
 else
 	echo "WARNING: Please install valgrind for more testing"
 fi
 
 # Test a statically-linked C++ version, tracking the resulting binary size and runtime
 # across debug, LTO, and cross-language LTO builds (using the same compiler each time).
-clang++ -std=c++11 -Wall -pthread demo.cpp ../target/debug/liblightning.a -ldl
+clang++ -std=c++11 -Wall -pthread demo.cpp target/debug/libldk.a -ldl
 ./a.out >/dev/null
 echo " C++ Bin size and runtime w/o optimization:"
 ls -lha a.out
@@ -75,19 +75,19 @@ if [ "$HOST_PLATFORM" = "host: x86_64-unknown-linux-gnu" ]; then
 		if [ -x "$(which clang-$LLVM_V)" ]; then
 			cargo +nightly clean
 			cargo +nightly rustc -Zbuild-std --target x86_64-unknown-linux-gnu -v -- -Zsanitizer=memory -Zsanitizer-memory-track-origins -Cforce-frame-pointers=yes
-			mv ../target/x86_64-unknown-linux-gnu/debug/liblightning.* ../target/debug/
+			mv target/x86_64-unknown-linux-gnu/debug/libldk.* target/debug/
 
 			# Sadly, std doesn't seem to compile into something that is memsan-safe as of Aug 2020,
 			# so we'll always fail, not to mention we may be linking against git rustc LLVM which
 			# may differ from clang-llvm, so just allow everything here to fail.
 			set +e
 
 			# First the C demo app...
-			clang-$LLVM_V -std=c++11 -fsanitize=memory -fsanitize-memory-track-origins -Wall -g -pthread demo.c ../target/debug/liblightning.a -ldl
+			clang-$LLVM_V -std=c++11 -fsanitize=memory -fsanitize-memory-track-origins -Wall -g -pthread demo.c target/debug/libldk.a -ldl
 			./a.out
 
 			# ...then the C++ demo app
-			clang++-$LLVM_V -std=c++11 -fsanitize=memory -fsanitize-memory-track-origins -Wall -g -pthread demo.cpp ../target/debug/liblightning.a -ldl
+			clang++-$LLVM_V -std=c++11 -fsanitize=memory -fsanitize-memory-track-origins -Wall -g -pthread demo.cpp target/debug/libldk.a -ldl
 			./a.out >/dev/null
 
 			# restore exit-on-failure
@@ -153,11 +153,11 @@ if [ "$HOST_PLATFORM" = "host: x86_64-unknown-linux-gnu" -o "$HOST_PLATFORM" = "
 		mv Cargo.toml.bk Cargo.toml
 
 		# First the C demo app...
-		$CLANG -fsanitize=address -Wall -g -pthread demo.c ../target/debug/liblightning.a -ldl
+		$CLANG -fsanitize=address -Wall -g -pthread demo.c target/debug/libldk.a -ldl
 		ASAN_OPTIONS='detect_leaks=1 detect_invalid_pointer_pairs=1 detect_stack_use_after_return=1' ./a.out
 
 		# ...then the C++ demo app
-		$CLANGPP -std=c++11 -fsanitize=address -Wall -g -pthread demo.cpp ../target/debug/liblightning.a -ldl
+		$CLANGPP -std=c++11 -fsanitize=address -Wall -g -pthread demo.cpp target/debug/libldk.a -ldl
 		ASAN_OPTIONS='detect_leaks=1 detect_invalid_pointer_pairs=1 detect_stack_use_after_return=1' ./a.out >/dev/null
 	else
 		echo "WARNING: Please install clang-$RUSTC_LLVM_V and clang++-$RUSTC_LLVM_V to build with address sanitizer"
@@ -168,7 +168,7 @@ fi
 
 # Now build with LTO on on both C++ and rust, but without cross-language LTO:
 cargo rustc -v --release -- -C lto
-clang++ -std=c++11 -Wall -flto -O2 -pthread demo.cpp ../target/release/liblightning.a -ldl
+clang++ -std=c++11 -Wall -flto -O2 -pthread demo.cpp target/release/libldk.a -ldl
 echo "C++ Bin size and runtime with only RL (LTO) optimized:"
 ls -lha a.out
 time ./a.out > /dev/null
@@ -180,7 +180,7 @@ if [ "$HOST_PLATFORM" != "host: x86_64-apple-darwin" -a "$CLANGPP" != "" ]; then
 	# packaging than simply shipping the rustup binaries (eg Debian should Just Work
 	# here).
 	cargo rustc -v --release -- -C linker-plugin-lto -C lto -C link-arg=-fuse-ld=lld
-	$CLANGPP -Wall -std=c++11 -flto -fuse-ld=lld -O2 -pthread demo.cpp ../target/release/liblightning.a -ldl
+	$CLANGPP -Wall -std=c++11 -flto -fuse-ld=lld -O2 -pthread demo.cpp target/release/libldk.a -ldl
 	echo "C++ Bin size and runtime with cross-language LTO:"
 	ls -lha a.out
 	time ./a.out > /dev/null

lightning-c-bindings/Cargo.toml

@@ -9,11 +9,16 @@ Utilities to fetch the chain from Bitcoin Core REST/RPC Interfaces and feed them
 """
 
 [lib]
-name = "lightning"
+name = "ldk"
 crate-type = ["staticlib"
 # Note that the following line is matched exactly by genbindings to turn off dylib creation
 ,"cdylib"]
 
 [dependencies]
 bitcoin = "0.24"
 lightning = { version = "0.0.11", path = "../lightning" }
+
+# We eventually want to join the root workspace, but for now, the bindings generation is
+# a bit brittle and we don't want to hold up other developers from making changes just
+# because they break the bindings
+[workspace]

lightning-c-bindings/README.md

@@ -222,6 +222,12 @@ These include:
 **It is highly recommended that you test any code which relies on the C (or C++) bindings in
 valgrind, AddressSanitizer, MemorySanitizer, or other similar tools to ensure correctness.**
 
+Process
+=======
+
+`genbindings.sh` is currently a catch-all script for bindings - it generates the latest Rust/C/C++
+code for bindings from the rust-lightning source code, builds it, and then runs various test apps.
+
 Note that after running `genbindings.sh`, if possible, the static lib in target/debug (eg
 target/debug/liblightning.a) will be linked with address sanitizer. In order to build against it,
 you will need to link with `clang` with `-fsanitize=address` with the same version of LLVM as