Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch to use NetAddress for peer addresses #85

Closed

Conversation

tnull
Copy link
Collaborator

@tnull tnull commented May 3, 2023

Fixes #11, based on #84.

We so far waited for lightningdevkit/rust-lightning#2056 to be resolved. However, as it didn't happen in time for LDK 0.0.115, we now switch our peer info to use a newtype around NetAddress so that we won't have to break serialization compatibility when the upstream changes become available post-0.1.

@tnull tnull force-pushed the 2023-05-switch-to-netaddress branch from ba818f2 to 29a7c9a Compare May 3, 2023 12:46
@tnull tnull mentioned this pull request May 3, 2023
47 tasks
@tnull tnull force-pushed the 2023-05-switch-to-netaddress branch 2 times, most recently from 04032ec to 38c5707 Compare May 9, 2023 07:59
@tnull
Copy link
Collaborator Author

tnull commented May 9, 2023

Rebased on #84 after #25 had been merged. Added NetAddress bindings support.

@tnull tnull force-pushed the 2023-05-switch-to-netaddress branch from 38c5707 to 3042d2b Compare May 10, 2023 07:32
@tnull
Copy link
Collaborator Author

tnull commented May 10, 2023

Rebased on main after #84 landed.

@tnull
Copy link
Collaborator Author

tnull commented May 12, 2023

Rebased after #56 has been merged.

@tnull
Copy link
Collaborator Author

tnull commented May 18, 2023

Rebased on main.

@tnull tnull force-pushed the 2023-05-switch-to-netaddress branch from abfda94 to 8f9229c Compare May 19, 2023 12:10
Copy link

@wpaulino wpaulino left a comment

Choose a reason for hiding this comment

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

FYI your commits are unsigned.

src/types.rs Outdated Show resolved Hide resolved
src/types.rs Outdated Show resolved Hide resolved
src/types.rs Outdated Show resolved Hide resolved
src/types.rs Outdated Show resolved Hide resolved
src/types.rs Outdated Show resolved Hide resolved
src/types.rs Outdated Show resolved Hide resolved
src/lib.rs Show resolved Hide resolved
@tnull tnull force-pushed the 2023-05-switch-to-netaddress branch from 8f9229c to a87508c Compare May 21, 2023 06:26
@tnull
Copy link
Collaborator Author

tnull commented May 21, 2023

FYI your commits are unsigned.

Thanks, they are actually all signed, just to a different user ID. Not exactly sure why Github is showing as unverified, it knows both email addresses... Will need to look into that.

Copy link

@wpaulino wpaulino left a comment

Choose a reason for hiding this comment

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

Feel free to squash.

src/lib.rs Outdated Show resolved Hide resolved
@tnull tnull force-pushed the 2023-05-switch-to-netaddress branch 3 times, most recently from 9780c83 to 67f4811 Compare May 23, 2023 07:58
@tnull
Copy link
Collaborator Author

tnull commented May 23, 2023

Squashed commits and included the following changes:

diff --git a/bindings/ldk_node.udl b/bindings/ldk_node.udl
index d31865a..b767de0 100644
--- a/bindings/ldk_node.udl
+++ b/bindings/ldk_node.udl
@@ -74,5 +74,4 @@ enum NodeError {
        "InvoiceCreationFailed",
        "PaymentFailed",
-       "PeerInfoNotFound",
        "ChannelCreationFailed",
        "ChannelClosingFailed",
diff --git a/src/error.rs b/src/error.rs
index 7fdd9d6..1d59fc0 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -16,6 +16,4 @@ pub enum Error {
        /// An attempted payment has failed.
        PaymentFailed,
-       /// A given peer info could not be found.
-       PeerInfoNotFound,
        /// A channel could not be opened.
        ChannelCreationFailed,
@@ -71,5 +69,4 @@ impl fmt::Display for Error {
                        Self::InvoiceCreationFailed => write!(f, "Failed to create invoice."),
                        Self::PaymentFailed => write!(f, "Failed to send the given payment."),
-                       Self::PeerInfoNotFound => write!(f, "Failed to resolve the given peer information."),
                        Self::ChannelCreationFailed => write!(f, "Failed to create channel."),
                        Self::ChannelClosingFailed => write!(f, "Failed to close channel."),
diff --git a/src/lib.rs b/src/lib.rs
index 0b8e7eb..75e74cf 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -1608,5 +1608,8 @@ async fn do_connect_peer(
        let socket_addr = addr
                .to_socket_addrs()
-               .map_err(|_| Error::PeerInfoNotFound)?
+               .map_err(|e| {
+                       log_error!(logger, "Failed to resolve network address: {}", e);
+                       Error::InvalidNetAddress
+               })?
                .next()
                .ok_or(Error::ConnectionFailed)?;

tnull added 3 commits May 23, 2023 13:29
While we're still blocked on upstream changes, we now switch our peer info
to use a newtype around `NetAddress` so that we won't have to break
serialization compatibility when the upstream changes becom available
post-0.1.
@tnull tnull force-pushed the 2023-05-switch-to-netaddress branch from 67f4811 to 63f3105 Compare May 23, 2023 11:29
src/lib.rs Outdated Show resolved Hide resolved
@tnull
Copy link
Collaborator Author

tnull commented May 23, 2023

Closing manually as the merge somehow didn't close this.

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.

2 participants