Skip to content

Conversation

@TheBlueMatt
Copy link
Collaborator

No description provided.

Specifically, for the LDK `EventsProvider` trait, we can't
(trivially) implement the `process_pending_events` function,
defined as:
`fn process_pending_events<H: Deref>(&self, handler: H) where H::Target: EventHandler;`

We currently do not support implementing generic methods, as we'd
need to wrap them in a generic Rust-trait-to-C-trait conversion
utility, which we do not currently have. Thus, because users
almost certainly have no reason to implement the `EventsProvider`
themselves, its simpler to simply prevent the use of the C trait
struct as the Rust trait.

Concretely, this means just skipping the
`impl rustEventsProvider for CEventsProvider` block, which works
fine as no Rust functions take an `EventsProvider` as an argument.
@jkczyz
Copy link
Contributor

jkczyz commented Jun 1, 2021

Having difficulty running this on MacOS.

+ cd ..
+ mv lightning-c-bindings/src/c_types/mod.rs ./
+ mv lightning-c-bindings/src/bitcoin ./
+ git checkout lightning-c-bindings/src
+ git checkout lightning-c-bindings/include
++ git describe --tag --dirty
fatal: No names found, cannot describe anything.
+ BINDINGS_GIT=

@valentinewallace
Copy link

Having difficulty running this on MacOS.

+ cd ..
+ mv lightning-c-bindings/src/c_types/mod.rs ./
+ mv lightning-c-bindings/src/bitcoin ./
+ git checkout lightning-c-bindings/src
+ git checkout lightning-c-bindings/include
++ git describe --tag --dirty
fatal: No names found, cannot describe anything.
+ BINDINGS_GIT=

Getting the same error on #25

@jkczyz
Copy link
Contributor

jkczyz commented Jun 1, 2021

Couple errors in check_bindings related to EventHandler:

error[E0053]: method `process_pending_events` has an incompatible type for trait
    --> src/lightning/util/events.rs:1125:2
     |
1125 |     fn process_pending_events<H:core::ops::Deref>(&self, mut handler: crate::lightning::util::events::EventHandler) {
     |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected type parameter `H`, found struct `lightning::util::events::EventHandler`
     |
     = note: expected fn pointer `fn(&lightning::util::events::EventsProvider, H)`
                found fn pointer `fn(&lightning::util::events::EventsProvider, lightning::util::events::EventHandler)`

error[E0055]: reached the recursion limit while auto-dereferencing `lightning::util::events::EventHandler`
    --> src/lightning/util/events.rs:1126:56
     |
1126 |         (self.process_pending_events)(self.this_arg, handler.into())
     |                                                              ^^^^ deref recursion limit reached
     |
     = help: consider adding a `#![recursion_limit="256"]` attribute to your crate (`ldk`)

error: aborting due to 2 previous errors

@TheBlueMatt
Copy link
Collaborator Author

fatal: No names found, cannot describe anything.

Huh, do you need git fetch --tags?

Copy link
Contributor

@jkczyz jkczyz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, do you need git fetch --tags?

Same issue after running that.

let ref_type: syn::Type = syn::parse_quote!(&#path);
assert!(!types.write_to_c_conversion_new_var(w, &format_ident!("a"), &*i.self_ty, Some(&gen_types), false), "We don't support new var conversions when comparing equality");

writeln!(w, "\t// Note that we'd love to use std::collections::hash_map::DefaultHasher but its not in core").unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: it's

@TheBlueMatt
Copy link
Collaborator Author

Couple errors in check_bindings related to EventHandler:

Right, oops, just forgot to check in the latest bindings, should work with genbindings.

git fetch --tags --all

Maybe git fetch --tags --all?

@jkczyz
Copy link
Contributor

jkczyz commented Jun 1, 2021

Maybe git fetch --tags --all?

Yes, that worked.

@TheBlueMatt
Copy link
Collaborator Author

Squashed the fixup and re-bumped the git head. Will merge if CI passes:

$ git diff-tree -U1 71b69c5 123a003
diff --git a/lightning-c-bindings/Cargo.toml b/lightning-c-bindings/Cargo.toml
index c8e7c00..40d316e 100644
--- a/lightning-c-bindings/Cargo.toml
+++ b/lightning-c-bindings/Cargo.toml
@@ -20,5 +20,5 @@ secp256k1 = { version = "0.20.1", features = ["global-context-less-secure"] }
 # Note that the following line is matched by genbindings to update the path
-lightning = { git = "https://git.bitcoin.ninja/rust-lightning", rev = "27c05fdd7f7ac8c485dbeb64a65215ce9deae0df", features = ["allow_wallclock_use"] }
-lightning-persister = { git = "https://git.bitcoin.ninja/rust-lightning", rev = "27c05fdd7f7ac8c485dbeb64a65215ce9deae0df" }
-lightning-invoice = { git = "https://git.bitcoin.ninja/rust-lightning", rev = "27c05fdd7f7ac8c485dbeb64a65215ce9deae0df" }
+lightning = { git = "https://git.bitcoin.ninja/rust-lightning", rev = "3996eaab6e2eaf5fde9374b51d952c0edef5ea92", features = ["allow_wallclock_use"] }
+lightning-persister = { git = "https://git.bitcoin.ninja/rust-lightning", rev = "3996eaab6e2eaf5fde9374b51d952c0edef5ea92" }
+lightning-invoice = { git = "https://git.bitcoin.ninja/rust-lightning", rev = "3996eaab6e2eaf5fde9374b51d952c0edef5ea92" }
 
$

@TheBlueMatt TheBlueMatt merged commit 6658ad8 into lightningdevkit:main Jun 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants