0.0.13 Bindings Updates #829

TheBlueMatt · 2021-03-06T00:22:20Z

Few minor things to get bindings working on the changes slated for 0.0.13 and one non-bindings function rename (the function name wait can't be used in Java, and while we could rename it on the Java end, adding more context in Rust didn't seem bad to me).

codecov · 2021-03-06T00:40:23Z

Codecov Report

Merging #829 (192d177) into main (b5d88a5) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #829   +/-   ##
=======================================
  Coverage   90.98%   90.98%           
=======================================
  Files          48       48           
  Lines       26538    26538           
=======================================
  Hits        24145    24145           
  Misses       2393     2393

Impacted Files	Coverage Δ
lightning/src/ln/channelmanager.rs	`85.47% <ø> (ø)`
lightning/src/ln/peer_handler.rs	`44.82% <ø> (ø)`
lightning/src/ln/functional_tests.rs	`96.89% <0.00%> (ø)`

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 b5d88a5...f1b4534. Read the comment docs.

jkczyz

Hmmm... getting some errors. Presumably I'll need to install something. Should the README be updated with instructions?

+ cargo rustc -v --target=wasm32-wasi -- -C embed-bitcode=yes
       Fresh cc v1.0.65
   Compiling bech32 v0.7.2
   Compiling bitcoin_hashes v0.9.4
     Running `rustc --crate-name bech32 /Users/jkczyz/.cargo/registry/src/i.8713187.xyz-1ecc6299db9ec823/bech32-0.7.2/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts --crate-type lib --emit=dep-info,metadata,link -C panic=abort -Cembed-bitcode=no -C debuginfo=2 -C metadata=92a5811ea903a31d -C extra-filename=-92a5811ea903a31d --out-dir /Users/jkczyz/src/rust-lightning-review/lightning-c-bindings/target/wasm32-wasi/debug/deps --target wasm32-wasi -L dependency=/Users/jkczyz/src/rust-lightning-review/lightning-c-bindings/target/wasm32-wasi/debug/deps -L dependency=/Users/jkczyz/src/rust-lightning-review/lightning-c-bindings/target/debug/deps --cap-lints allow`
     Running `rustc --crate-name bitcoin_hashes /Users/jkczyz/.cargo/registry/src/i.8713187.xyz-1ecc6299db9ec823/bitcoin_hashes-0.9.4/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts --crate-type lib --emit=dep-info,metadata,link -C panic=abort -Cembed-bitcode=no -C debuginfo=2 --cfg 'feature="default"' --cfg 'feature="std"' -C metadata=f887dca6f663bbeb -C extra-filename=-f887dca6f663bbeb --out-dir /Users/jkczyz/src/rust-lightning-review/lightning-c-bindings/target/wasm32-wasi/debug/deps --target wasm32-wasi -L dependency=/Users/jkczyz/src/rust-lightning-review/lightning-c-bindings/target/wasm32-wasi/debug/deps -L dependency=/Users/jkczyz/src/rust-lightning-review/lightning-c-bindings/target/debug/deps --cap-lints allow`
   Compiling secp256k1-sys v0.4.0 (https://github.com/TheBlueMatt/rust-secp256k1?rev=15a0d4195a20355f6b1e8f54c84eba56abc15cbd#15a0d419)
     Running `/Users/jkczyz/src/rust-lightning-review/lightning-c-bindings/target/debug/build/secp256k1-sys-26c8f0d1d2fb5878/build-script-build`
The following warnings were emitted during compilation:

warning: error: unable to create target: 'No available targets are compatible with this triple.'
warning: 1 error generated.

error: failed to run custom build command for `secp256k1-sys v0.4.0 (https://github.com/TheBlueMatt/rust-secp256k1?rev=15a0d4195a20355f6b1e8f54c84eba56abc15cbd#15a0d419)`

Caused by:
  process didn't exit successfully: `/Users/jkczyz/src/rust-lightning-review/lightning-c-bindings/target/debug/build/secp256k1-sys-26c8f0d1d2fb5878/build-script-build` (exit code: 1)
--- stdout
TARGET = Some("wasm32-wasi")
OPT_LEVEL = Some("0")
HOST = Some("x86_64-apple-darwin")
CC_wasm32-wasi = None
CC_wasm32_wasi = None
TARGET_CC = None
CC = None
CFLAGS_wasm32-wasi = None
CFLAGS_wasm32_wasi = None
TARGET_CFLAGS = None
CFLAGS = None
CRATE_CC_NO_DEFAULTS = None
DEBUG = Some("true")
CC_wasm32-wasi = None
CC_wasm32_wasi = None
TARGET_CC = None
CC = None
CFLAGS_wasm32-wasi = None
CFLAGS_wasm32_wasi = None
TARGET_CFLAGS = None
CFLAGS = None
CRATE_CC_NO_DEFAULTS = None
running: "clang" "-O0" "-ffunction-sections" "-fdata-sections" "-fPIC" "-g" "-fno-omit-frame-pointer" "--target=wasm32-wasi" "-I" "depend/secp256k1/" "-I" "depend/secp256k1/include" "-I" "depend/secp256k1/src" "-I" "wasm-sysroot" "-Wall" "-Wextra" "-DSECP256K1_BUILD=1" "-DENABLE_MODULE_ECDH=1" "-DENABLE_MODULE_SCHNORRSIG=1" "-DENABLE_MODULE_EXTRAKEYS=1" "-DECMULT_GEN_PREC_BITS=4" "-DUSE_NUM_NONE=1" "-DUSE_FIELD_INV_BUILTIN=1" "-DUSE_SCALAR_INV_BUILTIN=1" "-DECMULT_WINDOW_SIZE=15" "-DUSE_EXTERNAL_DEFAULT_CALLBACKS=1" "-DENABLE_MODULE_RECOVERY=1" "-o" "/Users/jkczyz/src/rust-lightning-review/lightning-c-bindings/target/wasm32-wasi/debug/build/secp256k1-sys-2767aebdf7cf1ca9/out/depend/secp256k1/contrib/lax_der_parsing.o" "-c" "depend/secp256k1/contrib/lax_der_parsing.c"
cargo:warning=error: unable to create target: 'No available targets are compatible with this triple.'
cargo:warning=1 error generated.
exit code: 1

--- stderr


error occurred: Command "clang" "-O0" "-ffunction-sections" "-fdata-sections" "-fPIC" "-g" "-fno-omit-frame-pointer" "--target=wasm32-wasi" "-I" "depend/secp256k1/" "-I" "depend/secp256k1/include" "-I" "depend/secp256k1/src" "-I" "wasm-sysroot" "-Wall" "-Wextra" "-DSECP256K1_BUILD=1" "-DENABLE_MODULE_ECDH=1" "-DENABLE_MODULE_SCHNORRSIG=1" "-DENABLE_MODULE_EXTRAKEYS=1" "-DECMULT_GEN_PREC_BITS=4" "-DUSE_NUM_NONE=1" "-DUSE_FIELD_INV_BUILTIN=1" "-DUSE_SCALAR_INV_BUILTIN=1" "-DECMULT_WINDOW_SIZE=15" "-DUSE_EXTERNAL_DEFAULT_CALLBACKS=1" "-DENABLE_MODULE_RECOVERY=1" "-o" "/Users/jkczyz/src/rust-lightning-review/lightning-c-bindings/target/wasm32-wasi/debug/build/secp256k1-sys-2767aebdf7cf1ca9/out/depend/secp256k1/contrib/lax_der_parsing.o" "-c" "depend/secp256k1/contrib/lax_der_parsing.c" with args "clang" did not execute successfully (status code exit code: 1).



warning: build failed, waiting for other jobs to finish...
error[E0463]: can't find crate for `std`
  |
  = note: the `wasm32-wasi` target may not be installed

error: aborting due to previous error

For more information about this error, try `rustc --explain E0463`.
error[E0463]: can't find crate for `std`
  |
  = note: the `wasm32-wasi` target may not be installed

error: aborting due to previous error

For more information about this error, try `rustc --explain E0463`.
error: build failed
+ echo 'WARNING: Failed to generate WASM LLVM-bitcode-embedded library'
WARNING: Failed to generate WASM LLVM-bitcode-embedded library
+ CARGO_PROFILE_RELEASE_LTO=true
+ cargo rustc -v --release --target=wasm32-wasi -- -C opt-level=s -C linker-plugin-lto -C lto
       Fresh cc v1.0.65
   Compiling bech32 v0.7.2
   Compiling bitcoin_hashes v0.9.4
     Running `rustc --crate-name bech32 /Users/jkczyz/.cargo/registry/src/i.8713187.xyz-1ecc6299db9ec823/bech32-0.7.2/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts --crate-type lib --emit=dep-info,metadata,link -C opt-level=3 -C panic=abort -Clinker-plugin-lto -C metadata=eddf55f3d9fa7cac -C extra-filename=-eddf55f3d9fa7cac --out-dir /Users/jkczyz/src/rust-lightning-review/lightning-c-bindings/target/wasm32-wasi/release/deps --target wasm32-wasi -L dependency=/Users/jkczyz/src/rust-lightning-review/lightning-c-bindings/target/wasm32-wasi/release/deps -L dependency=/Users/jkczyz/src/rust-lightning-review/lightning-c-bindings/target/release/deps --cap-lints allow`
   Compiling secp256k1-sys v0.4.0 (https://github.com/TheBlueMatt/rust-secp256k1?rev=15a0d4195a20355f6b1e8f54c84eba56abc15cbd#15a0d419)
     Running `rustc --crate-name bitcoin_hashes /Users/jkczyz/.cargo/registry/src/i.8713187.xyz-1ecc6299db9ec823/bitcoin_hashes-0.9.4/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts --crate-type lib --emit=dep-info,metadata,link -C opt-level=3 -C panic=abort -Clinker-plugin-lto --cfg 'feature="default"' --cfg 'feature="std"' -C metadata=baef145ce6ae43f7 -C extra-filename=-baef145ce6ae43f7 --out-dir /Users/jkczyz/src/rust-lightning-review/lightning-c-bindings/target/wasm32-wasi/release/deps --target wasm32-wasi -L dependency=/Users/jkczyz/src/rust-lightning-review/lightning-c-bindings/target/wasm32-wasi/release/deps -L dependency=/Users/jkczyz/src/rust-lightning-review/lightning-c-bindings/target/release/deps --cap-lints allow`
     Running `rustc --crate-name build_script_build /Users/jkczyz/.cargo/git/checkouts/rust-secp256k1-4eba1ec55b4aaf08/15a0d41/secp256k1-sys/build.rs --error-format=json --json=diagnostic-rendered-ansi --crate-type bin --emit=dep-info,link -C opt-level=3 -Cembed-bitcode=no --cfg 'feature="recovery"' --cfg 'feature="std"' -C metadata=913d5e27f89a4b0c -C extra-filename=-913d5e27f89a4b0c --out-dir /Users/jkczyz/src/rust-lightning-review/lightning-c-bindings/target/release/build/secp256k1-sys-913d5e27f89a4b0c -L dependency=/Users/jkczyz/src/rust-lightning-review/lightning-c-bindings/target/release/deps --extern cc=/Users/jkczyz/src/rust-lightning-review/lightning-c-bindings/target/release/deps/libcc-eb00dbf091a70778.rlib --cap-lints allow`
error[E0463]: can't find crate for `std`
  |
  = note: the `wasm32-wasi` target may not be installed

error: aborting due to previous error

For more information about this error, try `rustc --explain E0463`.
error: could not compile `bech32`.

Caused by:
  process didn't exit successfully: `rustc --crate-name bech32 /Users/jkczyz/.cargo/registry/src/i.8713187.xyz-1ecc6299db9ec823/bech32-0.7.2/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts --crate-type lib --emit=dep-info,metadata,link -C opt-level=3 -C panic=abort -Clinker-plugin-lto -C metadata=eddf55f3d9fa7cac -C extra-filename=-eddf55f3d9fa7cac --out-dir /Users/jkczyz/src/rust-lightning-review/lightning-c-bindings/target/wasm32-wasi/release/deps --target wasm32-wasi -L dependency=/Users/jkczyz/src/rust-lightning-review/lightning-c-bindings/target/wasm32-wasi/release/deps -L dependency=/Users/jkczyz/src/rust-lightning-review/lightning-c-bindings/target/release/deps --cap-lints allow` (exit code: 1)
warning: build failed, waiting for other jobs to finish...
error[E0463]: can't find crate for `std`
  |
  = note: the `wasm32-wasi` target may not be installed

error: aborting due to previous error

For more information about this error, try `rustc --explain E0463`.
error: build failed

lightning/src/ln/channelmanager.rs

lightning-c-bindings/demo.c

TheBlueMatt · 2021-03-06T16:28:37Z

Correct me if I'm wrong, but the error you got shouldn't be fatal? In any case, I added a check before calling the wasm cargo build's so you shouldn't see it anymore at all.

jkczyz

Correct me if I'm wrong, but the error you got shouldn't be fatal? In any case, I added a check before calling the wasm cargo build's so you shouldn't see it anymore at all.

Correct, not fatal, but now it is. :P Looks like the error might be misleading or we're checking for the wrong thing.

$ rustc --print target-list | grep wasm32-wasi
wasm32-wasi

lightning-c-bindings/demo.c

TheBlueMatt · 2021-03-07T03:12:34Z

Oops, right, its not a rustc issue, its a clang issue. I pushed a fix that works on linux but let me know how it looks on OSX (it should always fail unless you update clang to point to a locally-installed version that supports wasm32).

jkczyz · 2021-03-07T07:52:42Z

Oops, right, its not a rustc issue, its a clang issue. I pushed a fix that works on linux but let me know how it looks on OSX (it should always fail unless you update clang to point to a locally-installed version that supports wasm32).

All good now.

++ rustc --print target-list
++ grep wasm32-wasi
+ '[' wasm32-wasi '!=' '' ']'
+ echo 'int main() {}'
+ clang -nostdlib -o /dev/null --target=wasm32-wasi -Wl,--no-entry genbindings_wasm_test_file.c
+ echo 'Cannot build WASM lib as clang does not seem to support the wasm32-wasi target'
Cannot build WASM lib as clang does not seem to support the wasm32-wasi target
+ rm genbindings_wasm_test_file.c

`wait` doesn't capture enough of what's going on, but also Java Java doesn't accpet methods just called `wait`, as it conflicts with existing sync primitives on all Objects.

When the (somewhat anti-pattern) `impl Deref for X { type Target = X; .. }` is used, this avoids an infinite dereference exception trying to figure out what type to resolve `is_null` against.

Specifically, this is required for some paths which use `Option<msgs::Message>`.

This is largely useful for bindings, and the off-github discussion around lightningdevkit#814 concluded these should be pub, but the PR was not updated to capture this. Now that the bindings support generation for the structs, expose them.

TheBlueMatt · 2021-03-07T18:11:28Z

Squashed the bindings updates (with no change from the previous commit - $ git diff-tree -U3 45b21a6 f1b4534aad1b6d36be7312710681ca92c0479ae7).

The full non-bindings updates in this PR are:

$ git diff-tree -U3 b5d88a5422913a0a8950455c5f86764a042429d7 f1b4534aad1b6d36be7312710681ca92c0479ae7 lightning
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 6e46d79f..de0ce535 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -480,10 +480,11 @@ pub struct ChainParameters {
 }
 
 /// Whenever we release the `ChannelManager`'s `total_consistency_lock`, from read mode, it is
-/// desirable to notify any listeners on `wait_timeout`/`wait` that new updates are available for
-/// persistence. Therefore, this struct is responsible for locking the total consistency lock and,
-/// upon going out of scope, sending the aforementioned notification (since the lock being released
-/// indicates that the updates are ready for persistence).
+/// desirable to notify any listeners on `await_persistable_update_timeout`/
+/// `await_persistable_update` that new updates are available for persistence. Therefore, this
+/// struct is responsible for locking the total consistency lock and, upon going out of scope,
+/// sending the aforementioned notification (since the lock being released indicates that the
+/// updates are ready for persistence).
 struct PersistenceNotifierGuard<'a> {
 	persistence_notifier: &'a PersistenceNotifier,
 	// We hold onto this result so the lock doesn't get released immediately.
@@ -3407,17 +3408,19 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 	}
 
 	/// Blocks until ChannelManager needs to be persisted or a timeout is reached. It returns a bool
-	/// indicating whether persistence is necessary. Only one listener on `wait_timeout` is
-	/// guaranteed to be woken up.
+	/// indicating whether persistence is necessary. Only one listener on
+	/// `await_persistable_update` or `await_persistable_update_timeout` is guaranteed to be woken
+	/// up.
 	/// Note that the feature `allow_wallclock_use` must be enabled to use this function.
 	#[cfg(any(test, feature = "allow_wallclock_use"))]
-	pub fn wait_timeout(&self, max_wait: Duration) -> bool {
+	pub fn await_persistable_update_timeout(&self, max_wait: Duration) -> bool {
 		self.persistence_notifier.wait_timeout(max_wait)
 	}
 
-	/// Blocks until ChannelManager needs to be persisted. Only one listener on `wait` is
-	/// guaranteed to be woken up.
-	pub fn wait(&self) {
+	/// Blocks until ChannelManager needs to be persisted. Only one listener on
+	/// `await_persistable_update` or `await_persistable_update_timeout` is guaranteed to be woken
+	/// up.
+	pub fn await_persistable_update(&self) {
 		self.persistence_notifier.wait()
 	}
 
@@ -3669,7 +3672,7 @@ impl<Signer: Sign, M: Deref + Sync + Send, T: Deref + Sync + Send, K: Deref + Sy
 }
 
 /// Used to signal to the ChannelManager persister that the manager needs to be re-persisted to
-/// disk/backups, through `wait_timeout` and `wait`.
+/// disk/backups, through `await_persistable_update_timeout` and `await_persistable_update`.
 struct PersistenceNotifier {
 	/// Users won't access the persistence_lock directly, but rather wait on its bool using
 	/// `wait_timeout` and `wait`.
diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs
index d3747632..db8bb429 100644
--- a/lightning/src/ln/peer_handler.rs
+++ b/lightning/src/ln/peer_handler.rs
@@ -42,7 +42,7 @@ use bitcoin::hashes::{HashEngine, Hash};
 
 /// A dummy struct which implements `RoutingMessageHandler` without storing any routing information
 /// or doing any processing. You can provide one of these as the route_handler in a MessageHandler.
-struct IgnoringMessageHandler{}
+pub struct IgnoringMessageHandler{}
 impl MessageSendEventsProvider for IgnoringMessageHandler {
 	fn get_and_clear_pending_msg_events(&self) -> Vec<MessageSendEvent> { Vec::new() }
 }
@@ -67,7 +67,7 @@ impl Deref for IgnoringMessageHandler {
 
 /// A dummy struct which implements `ChannelMessageHandler` without having any channels.
 /// You can provide one of these as the route_handler in a MessageHandler.
-struct ErroringMessageHandler {
+pub struct ErroringMessageHandler {
 	message_queue: Mutex<Vec<MessageSendEvent>>
 }
 impl ErroringMessageHandler {

valentinewallace

✔️ genbindings works
✔️ wait rename seems reasonable, didn't miss any renames
✔️ pub peer_handler.rs changes seem reasonable

[bindings] Support converting Network to rust, update variants

6742343

TheBlueMatt added this to the 0.0.13 milestone Mar 6, 2021

TheBlueMatt force-pushed the 2021-03-0.0.13-bindings branch from 8fd57fe to ef889b1 Compare March 6, 2021 00:23

jkczyz reviewed Mar 6, 2021

View reviewed changes

lightning/src/ln/channelmanager.rs Show resolved Hide resolved

lightning/src/ln/channelmanager.rs Outdated Show resolved Hide resolved

lightning-c-bindings/demo.c Show resolved Hide resolved

TheBlueMatt force-pushed the 2021-03-0.0.13-bindings branch from ef889b1 to 98e791f Compare March 6, 2021 16:27

jkczyz reviewed Mar 6, 2021

View reviewed changes

lightning-c-bindings/demo.c Outdated Show resolved Hide resolved

TheBlueMatt force-pushed the 2021-03-0.0.13-bindings branch from 98e791f to 45b21a6 Compare March 7, 2021 03:12

jkczyz approved these changes Mar 7, 2021

View reviewed changes

TheBlueMatt added 8 commits March 7, 2021 13:06

Change ChannelManager::wait to be more descriptive

578f8b7

`wait` doesn't capture enough of what's going on, but also Java Java doesn't accpet methods just called `wait`, as it conflicts with existing sync primitives on all Objects.

[bindings] Avoid double-use'ing traits by writing out the path

bfa95cd

[bindings] Be explicit when calling pointer.is_null()

6b4c8e4

When the (somewhat anti-pattern) `impl Deref for X { type Target = X; .. }` is used, this avoids an infinite dereference exception trying to figure out what type to resolve `is_null` against.

[bindings] Handle ::-containing paths in generics

f7f7500

Specifically, this is required for some paths which use `Option<msgs::Message>`.

[bindings] Update demos for new upstream API

33f511d

[bindings] Check for a wasm32 target before compiling in genbindings

14441a0

Auto-generated bindings updates

f1b4534

TheBlueMatt force-pushed the 2021-03-0.0.13-bindings branch from 45b21a6 to f1b4534 Compare March 7, 2021 18:11

valentinewallace approved these changes Mar 7, 2021

View reviewed changes

TheBlueMatt merged commit c42ea50 into lightningdevkit:main Mar 7, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

0.0.13 Bindings Updates #829

0.0.13 Bindings Updates #829

Uh oh!

TheBlueMatt commented Mar 6, 2021

Uh oh!

codecov bot commented Mar 6, 2021 •

edited

Loading

Uh oh!

jkczyz left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TheBlueMatt commented Mar 6, 2021

Uh oh!

jkczyz left a comment

Uh oh!

Uh oh!

TheBlueMatt commented Mar 7, 2021

Uh oh!

jkczyz commented Mar 7, 2021

Uh oh!

TheBlueMatt commented Mar 7, 2021

Uh oh!

valentinewallace left a comment

Uh oh!

Uh oh!

0.0.13 Bindings Updates #829

0.0.13 Bindings Updates #829

Uh oh!

Conversation

TheBlueMatt commented Mar 6, 2021

Uh oh!

codecov bot commented Mar 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jkczyz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TheBlueMatt commented Mar 6, 2021

Uh oh!

jkczyz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

TheBlueMatt commented Mar 7, 2021

Uh oh!

jkczyz commented Mar 7, 2021

Uh oh!

TheBlueMatt commented Mar 7, 2021

Uh oh!

valentinewallace left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Mar 6, 2021 •

edited

Loading