Expose node ids as UniFFI objects

Reject malformed binding inputs in a fallible constructor before they
reach graph lookups. Also expose safe conversion to and from public keys
while keeping Rust graph APIs on the upstream type.

Co-Authored-By: HAL 9000

Author: tnull

CHANGELOG.md

@@ -14,6 +14,9 @@
 - The language bindings now expose `NodeAlias` as an object instead of a string alias. Node aliases
   must be passed through its fallible constructor, which returns `NodeError::InvalidNodeAlias` for
   invalid input.
+- The language bindings now expose `NodeId` as an object instead of a string alias. Node ids must
+  be passed through its fallible constructor, which returns `NodeError::InvalidNodeId` for invalid
+  input.
 - `generate_entropy_mnemonic` has been removed. Use `bip39::Mnemonic::generate` in Rust and
   `Mnemonic::generate` in the language bindings instead.
 - Migrating between storage backends does not preserve the relative creation order of

bindings/ldk_node.udl

@@ -388,8 +388,7 @@ typedef interface SocketAddress;
 
 typedef interface PublicKey;
 
-[Custom]
-typedef string NodeId;
+typedef interface NodeId;
 
 [Custom]
 typedef string Address;

bindings/python/src/ldk_node/test_ldk_node.py

@@ -309,6 +309,38 @@ def test_node_alias_object(self):
         self.assertIsInstance(error.exception, NodeError.InvalidNodeAlias)
 
 
+class TestNodeId(unittest.TestCase):
+    def test_node_id_object(self):
+        self.assertTrue(
+            hasattr(bindings.NodeId, "from_str"),
+            "NodeId should be exposed as an object",
+        )
+
+        node_id_str = (
+            "02eec7245d6b7d2ccb30380bfbe2a3648cd7a942653f5aa340edcea1f283686619"
+        )
+        node_id = bindings.NodeId.from_str(node_id_str)
+
+        self.assertIsInstance(node_id, bindings.NodeId)
+        self.assertEqual(str(node_id), node_id_str)
+        self.assertEqual(node_id.as_bytes(), bytes.fromhex(node_id_str))
+
+        public_key = bindings.PublicKey.from_str(node_id_str)
+        self.assertEqual(bindings.NodeId.from_public_key(public_key), node_id)
+        self.assertEqual(node_id.as_public_key(), public_key)
+
+        with self.assertRaises(NodeError) as error:
+            bindings.NodeId.from_str("invalid")
+
+        self.assertIsInstance(error.exception, NodeError.InvalidNodeId)
+
+        invalid_public_key_node_id = bindings.NodeId.from_str("00" * 33)
+        with self.assertRaises(NodeError) as error:
+            invalid_public_key_node_id.as_public_key()
+
+        self.assertIsInstance(error.exception, NodeError.InvalidNodeId)
+
+
 class TestLdkNode(unittest.TestCase):
     def setUp(self):
         bitcoin_cli("createwallet ldk_node_test")

src/ffi/types.rs

@@ -38,8 +38,8 @@ use lightning::offers::payer_proof::{
 use lightning::offers::refund::Refund as LdkRefund;
 use lightning::offers::static_invoice::StaticInvoice as LdkStaticInvoice;
 use lightning::onion_message::dns_resolution::HumanReadableName as LdkHumanReadableName;
-use lightning::routing::gossip::NodeAlias as LdkNodeAlias;
-pub use lightning::routing::gossip::{NodeId, RoutingFees};
+pub use lightning::routing::gossip::RoutingFees;
+use lightning::routing::gossip::{NodeAlias as LdkNodeAlias, NodeId as LdkNodeId};
 pub use lightning::routing::router::RouteParametersConfig;
 use lightning::util::persist::PageToken as LdkPageToken;
 use lightning::util::ser::{Readable, RequiredWrapper, Writeable, Writer};
@@ -242,19 +242,71 @@ impl ReadablePublicKey for RequiredWrapper<Arc<PublicKey>> {
 	}
 }
 
-uniffi::custom_type!(NodeId, String, {
-	remote,
-	try_lift: |val| {
-		if let Ok(key) = NodeId::from_str(&val) {
-			return Ok(key);
-		}
+/// A compressed public key identifying a node in the network graph.
+#[derive(Debug, Clone, PartialEq, Eq, uniffi::Object)]
+#[uniffi::export(Debug, Display, Eq)]
+pub struct NodeId {
+	pub(crate) inner: LdkNodeId,
+}
 
-		Err(Error::InvalidNodeId.into())
-	},
-	lower: |obj| {
-		obj.to_string()
-	},
-});
+#[uniffi::export]
+impl NodeId {
+	/// Constructs a node id from its serialized representation.
+	#[uniffi::constructor]
+	pub fn from_str(node_id_str: &str) -> Result<Self, Error> {
+		node_id_str.parse()
+	}
+
+	/// Constructs a node id from a public key.
+	#[uniffi::constructor]
+	pub fn from_public_key(public_key: Arc<PublicKey>) -> Self {
+		LdkNodeId::from_pubkey(public_key.as_ref().as_ref()).into()
+	}
+
+	/// Returns the serialized node id.
+	pub fn as_bytes(&self) -> Vec<u8> {
+		self.inner.as_slice().to_vec()
+	}
+
+	/// Returns the node id as a public key.
+	pub fn as_public_key(&self) -> Result<Arc<PublicKey>, Error> {
+		self.inner.as_pubkey().map(PublicKey::from).map(Arc::new).map_err(|_| Error::InvalidNodeId)
+	}
+}
+
+impl FromStr for NodeId {
+	type Err = Error;
+
+	fn from_str(node_id_str: &str) -> Result<Self, Self::Err> {
+		node_id_str.parse::<LdkNodeId>().map(Self::from).map_err(|_| Error::InvalidNodeId)
+	}
+}
+
+impl From<LdkNodeId> for NodeId {
+	fn from(inner: LdkNodeId) -> Self {
+		Self { inner }
+	}
+}
+
+impl Deref for NodeId {
+	type Target = LdkNodeId;
+
+	fn deref(&self) -> &Self::Target {
+		&self.inner
+	}
+}
+
+impl AsRef<LdkNodeId> for NodeId {
+	fn as_ref(&self) -> &LdkNodeId {
+		self.deref()
+	}
+}
+
+impl std::fmt::Display for NodeId {
+	fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+		write!(f, "{}", self.inner)
+	}
+}
 
 uniffi::custom_type!(Address, String, {
 	remote,

src/graph.rs

@@ -11,14 +11,21 @@ use std::sync::Arc;
 
 #[cfg(feature = "uniffi")]
 type SocketAddress = Arc<crate::ffi::SocketAddress>;
-use lightning::routing::gossip::NodeId;
+#[cfg(not(feature = "uniffi"))]
+use lightning::routing::gossip::NodeId as LdkNodeId;
 #[cfg(feature = "uniffi")]
 use lightning::routing::gossip::RoutingFees;
 #[cfg(not(feature = "uniffi"))]
 use lightning::routing::gossip::{ChannelInfo, NodeInfo};
 
+use crate::ffi::{maybe_deref, maybe_wrap};
 use crate::types::Graph;
 
+#[cfg(not(feature = "uniffi"))]
+type NodeId = LdkNodeId;
+#[cfg(feature = "uniffi")]
+type NodeId = Arc<crate::ffi::NodeId>;
+
 /// Represents the network as nodes and channels between them.
 #[cfg_attr(feature = "uniffi", derive(uniffi::Object))]
 pub struct NetworkGraph {
@@ -45,12 +52,12 @@ impl NetworkGraph {
 
 	/// Returns the list of nodes in the graph
 	pub fn list_nodes(&self) -> Vec<NodeId> {
-		self.inner.read_only().nodes().unordered_keys().map(|n| *n).collect()
+		self.inner.read_only().nodes().unordered_keys().map(|n| maybe_wrap(*n)).collect()
 	}
 
 	/// Returns information on a node with the given id.
 	pub fn node(&self, node_id: &NodeId) -> Option<NodeInfo> {
-		self.inner.read_only().nodes().get(node_id).cloned().map(|n| n.into())
+		self.inner.read_only().nodes().get(maybe_deref(node_id)).cloned().map(|n| n.into())
 	}
 }
 
@@ -78,9 +85,9 @@ pub struct ChannelInfo {
 impl From<lightning::routing::gossip::ChannelInfo> for ChannelInfo {
 	fn from(value: lightning::routing::gossip::ChannelInfo) -> Self {
 		Self {
-			node_one: value.node_one,
+			node_one: maybe_wrap(value.node_one),
 			one_to_two: value.one_to_two.map(|u| u.into()),
-			node_two: value.node_two,
+			node_two: maybe_wrap(value.node_two),
 			two_to_one: value.two_to_one.map(|u| u.into()),
 			capacity_sats: value.capacity_sats,
 		}