diff --git a/Cargo.lock b/Cargo.lock index cef221309..93fac6bbe 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1355,7 +1355,7 @@ checksum = "07e28edb80900c19c28f1072f2e8aeca7fa06b23cd4169cefe1af5aa3260783f" [[package]] name = "gl-cli" -version = "0.1.0" +version = "0.1.1" dependencies = [ "clap", "dirs", @@ -1506,6 +1506,16 @@ dependencies = [ "which 4.4.2", ] +[[package]] +name = "gl-util" +version = "0.1.0" +dependencies = [ + "bytes 1.10.1", + "serde", + "serde_json", + "tonic 0.11.0", +] + [[package]] name = "governor" version = "0.5.1" @@ -3483,15 +3493,15 @@ dependencies = [ [[package]] name = "rustix" -version = "1.0.7" +version = "1.0.8" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c71e83d6afe7ff64890ec6b71d6a69bb8a610ab78ce364b3352876bb4c801266" +checksum = "11181fbabf243db407ef8df94a6ce0b2f9a733bd8be4ad02b4eda9602296cac8" dependencies = [ "bitflags 2.9.1", "errno", "libc", "linux-raw-sys 0.9.4", - "windows-sys 0.59.0", + "windows-sys 0.60.2", ] [[package]] @@ -4098,7 +4108,7 @@ dependencies = [ "fastrand", "getrandom 0.3.3", "once_cell", - "rustix 1.0.7", + "rustix 1.0.8", "windows-sys 0.59.0", ] diff --git a/Cargo.toml b/Cargo.toml index 3227652be..d96aec8cf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,6 +18,7 @@ members = [ "libs/gl-plugin", "libs/gl-signerproxy", "libs/gl-cli", + "libs/gl-util", ] [workspace.dependencies] diff --git a/libs/gl-client-py/tests/test_plugin.py b/libs/gl-client-py/tests/test_plugin.py index ad8b3cd34..1a65cbad0 100644 --- a/libs/gl-client-py/tests/test_plugin.py +++ b/libs/gl-client-py/tests/test_plugin.py @@ -1,6 +1,7 @@ from gltesting.fixtures import * from pyln.testing.utils import wait_for from pyln import grpc as clnpb +import grpc import pytest import secrets from pathlib import Path @@ -92,7 +93,7 @@ def test_trampoline_pay(bitcoind, clients, node_factory): res = n1.trampoline_pay(inv["bolt11"], bytes.fromhex(l2.info["id"])) assert res - assert len(res.payment_hash) == 32 # There was a confusion about hex/bytes return. + assert len(res.payment_hash) == 32 # There was a confusion about hex/bytes return. l2.rpc.unsetchecks() @@ -124,7 +125,9 @@ def test_trampoline_pay(bitcoind, clients, node_factory): # calling `trampoline_pay` with an unkown tmrp_node_id must fail. with pytest.raises( - expected_exception=ValueError, match=r"Peer error: No such peer" + expected_exception=ValueError, + + match=f"Unknown peer {l3.info['id']}", ): res = n1.trampoline_pay(inv["bolt11"], bytes.fromhex(l3.info["id"])) @@ -134,13 +137,10 @@ def test_trampoline_pay(bitcoind, clients, node_factory): # trampoline payments must fail. with pytest.raises( expected_exception=ValueError, - match=r"Features \\\"[a-f0-9]+\\\" do not contain feature bit 427", + match="Peer doesn't suport trampoline payments", ): res = n1.trampoline_pay(inv["bolt11"], bytes.fromhex(l3.info["id"])) - res = n1.listpays() - print(f"LISTPAYS: {res}") - def test_trampoline_multi_htlc(bitcoind, clients, node_factory): c1 = clients.new() diff --git a/libs/gl-plugin/Cargo.toml b/libs/gl-plugin/Cargo.toml index cde38e96c..5ae621e60 100644 --- a/libs/gl-plugin/Cargo.toml +++ b/libs/gl-plugin/Cargo.toml @@ -23,6 +23,7 @@ cln-rpc = { workspace = true } env_logger = "^0.7.1" futures = "0.3" gl-client = { version = "^0.3.0", path = "../gl-client" } +gl-util = { version = "0.1", path = "../gl-util" } governor = { version = "0.5", default-features = false, features = ["std"] } hex = "0.4" hyper = "0.14.28" diff --git a/libs/gl-plugin/src/awaitables.rs b/libs/gl-plugin/src/awaitables.rs index 4fae28150..54738226d 100644 --- a/libs/gl-plugin/src/awaitables.rs +++ b/libs/gl-plugin/src/awaitables.rs @@ -20,8 +20,10 @@ const RPC_CALL_DELAY_MSEC: u64 = 250; #[derive(Debug, thiserror::Error)] pub enum Error { - #[error("Peer error: {0}")] - Peer(&'static str), + #[error("Unknown peer {0}")] + PeerUnknown(String), + #[error("Can't connect to peer {0}")] + PeerConnectionFailure(String), #[error("Channel error: {0}")] Channel(&'static str), #[error("RPC error: {0}")] @@ -270,7 +272,10 @@ async fn ensure_peer_connection( level: None, }) .await?; - let peer = res.peers.first().ok_or(Error::Peer("No such peer"))?; + let peer = res + .peers + .first() + .ok_or(Error::PeerUnknown(peer_id.to_string()))?; if !peer.connected { log::debug!("Peer {} is not connected, connecting", peer_id); @@ -282,7 +287,7 @@ async fn ensure_peer_connection( let res = rpc .call_typed(&req) .await - .map_err(|_| Error::Peer("unable to connect"))?; + .map_err(|_| Error::PeerConnectionFailure(peer_id.to_string()))?; log::debug!("Connect call to {} resulted in {:?}", peer_id, res); } diff --git a/libs/gl-plugin/src/lib.rs b/libs/gl-plugin/src/lib.rs index 9156b1e74..8a84006a8 100644 --- a/libs/gl-plugin/src/lib.rs +++ b/libs/gl-plugin/src/lib.rs @@ -18,7 +18,7 @@ pub mod responses; pub mod stager; pub mod storage; pub mod tlv; -mod tramp; +pub mod tramp; #[cfg(unix)] mod unix; diff --git a/libs/gl-plugin/src/node/mod.rs b/libs/gl-plugin/src/node/mod.rs index a05af6f03..a38320a25 100644 --- a/libs/gl-plugin/src/node/mod.rs +++ b/libs/gl-plugin/src/node/mod.rs @@ -555,16 +555,11 @@ impl Node for PluginNodeServer { &self, r: tonic::Request, ) -> Result, Status> { - match tramp::trampolinepay(r.into_inner(), self.rpc_path.clone()) + tramp::trampolinepay(r.into_inner(), self.rpc_path.clone()) .await - .map(|res| { - >::into( - res, - ) - }) { - Ok(res) => { - debug!("Trampoline payment successful"); - Ok(tonic::Response::new(pb::TrampolinePayResponse { + .map(cln_rpc::model::responses::PayResponse::into) + .map(|res: cln_grpc::pb::PayResponse| { + tonic::Response::new(pb::TrampolinePayResponse { payment_preimage: res.payment_preimage, payment_hash: res.payment_hash, created_at: res.created_at, @@ -572,13 +567,12 @@ impl Node for PluginNodeServer { amount_msat: res.amount_msat.unwrap_or_default().msat, amount_sent_msat: res.amount_sent_msat.unwrap_or_default().msat, destination: res.destination.unwrap_or_default(), - })) - } - Err(e) => { - debug!("Trampoline payment failed: {}", e); - Err(Status::new(Code::Unknown, e.to_string())) - } - } + }) + }) + .map_err(|err| { + debug!("Trampoline payment failed: {}", err); + err.into() + }) } } diff --git a/libs/gl-plugin/src/tramp.rs b/libs/gl-plugin/src/tramp.rs index 8028acfaf..d883e7f0d 100644 --- a/libs/gl-plugin/src/tramp.rs +++ b/libs/gl-plugin/src/tramp.rs @@ -1,13 +1,15 @@ -use crate::awaitables::{AwaitableChannel, AwaitablePeer}; +use crate::awaitables::{AwaitableChannel, AwaitablePeer, Error as AwaitablePeerError}; use crate::pb; -use anyhow::{anyhow, Result}; use cln_rpc::{ primitives::{Amount, PublicKey, ShortChannelId}, - ClnRpc, + ClnRpc, RpcError, }; use futures::{future::join_all, FutureExt}; +use gl_util::error::{ClnRpcError, ErrorCode, ErrorStatusConversionExt, GreenlightError}; use log::{debug, warn}; use serde::{Deserialize, Serialize}; +use serde_json::json; +use std::error::Error as StdError; use std::path::PathBuf; use std::str::FromStr; use std::{path::Path, time::Duration}; @@ -32,17 +34,258 @@ const PAY_UNPARSEABLE_ONION_CODE: i32 = 202; // How long do we wait for channels to re-establish? const AWAIT_CHANNELS_TIMEOUT_SEC: u64 = 20; +/// Converts Core Lightning RPC errors into trampoline errors. +/// +/// This conversion preserves the original error code and message, +/// and includes any additional error data as JSON in the error context. +impl From for TrampolineError { + fn from(value: RpcError) -> Self { + let mut c_err = Self::new( + TrampolineErrCode::RpcError(value.code.unwrap_or(-1)), + value.message, + ); + if let Some(data) = value.data { + let jdata = serde_json::to_string(&data) + .ok() + .unwrap_or_else(|| "could not convert cln rpc error to json".to_string()); + c_err = c_err.with_context(jdata); + } + c_err + } +} + +/// Error codes specific to trampoline routing operations. +/// +/// These error codes cover various failure scenarios in Lightning Network +/// trampoline routing, from configuration issues to payment failures. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] +pub enum TrampolineErrCode { + FeatureNotSupported, + InvalidNodeId, + NetworkError, + UnknownPeer, + MissingAmount, + AmbigousAmount, + MissingChannel, + InsufficientFunds, + InvalidInvoice, + PeerNodeFailure, + PaymentFailure, + PeerConnectionFailure, + MissingPreimage, + Internal, + RpcError(ClnRpcError), +} + +impl core::fmt::Display for TrampolineErrCode { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.code()) + } +} + +/// Maps trampoline error codes to numeric values and back. +/// +/// Error code allocation: +/// - 42701-42714: Reserved for trampoline-specific errors +/// - Other ranges: CLN RPC error codes (see comments in from_code) +impl ErrorCode for TrampolineErrCode { + fn code(&self) -> i32 { + match self { + Self::FeatureNotSupported => 42701, + Self::InvalidNodeId => 42702, + Self::NetworkError => 42703, + Self::UnknownPeer => 42704, + Self::MissingAmount => 42705, + Self::AmbigousAmount => 42706, + Self::MissingChannel => 42707, + Self::InsufficientFunds => 42708, + Self::InvalidInvoice => 42709, + Self::PeerNodeFailure => 42710, + Self::PaymentFailure => 42711, + Self::PeerConnectionFailure => 42712, + Self::MissingPreimage => 42713, + Self::Internal => 42714, + Self::RpcError(cln_err) => cln_err.code(), + } + } + + fn from_code(code: i32) -> Option { + match code { + 42701 => Some(Self::FeatureNotSupported), + 42702 => Some(Self::InvalidNodeId), + 42703 => Some(Self::NetworkError), + 42704 => Some(Self::UnknownPeer), + 42705 => Some(Self::MissingAmount), + 42706 => Some(Self::AmbigousAmount), + 42707 => Some(Self::MissingChannel), + 42708 => Some(Self::InsufficientFunds), + 42709 => Some(Self::InvalidInvoice), + 42710 => Some(Self::PeerNodeFailure), + 42711 => Some(Self::PeerNodeFailure), + 42712 => Some(Self::PeerConnectionFailure), + 42713 => Some(Self::MissingPreimage), + 42714 => Some(Self::Internal), + // Possible sendpay failure codes: + // -1: Catchall nonspecific error. + // 201: Already paid with this hash using different amount or destination. + // 202: Unparseable onion reply. + // 203: Permanent failure at destination. + // 204: Failure along route; retry a different route. + // 212: localinvreqid refers to an invalid, or used, local invoice_request. + // Possible waitsendpay error codes: + // -1: Catchall nonspecific error. + // 200: Timed out before the payment could complete. + // 202: Unparseable onion reply. + // 203: Permanent failure at destination. + // 204: Failure along route; retry a different route. + // 208: A payment for payment_hash was never made and there is nothing to wait for. + // 209: The payment already failed, but the reason for failure was not stored. This should only occur when querying failed payments on very old databases. + -1 | 200..204 | 208 | 209 | 212 => Some(Self::RpcError(code)), + _ => None, + } + } +} + +/// Maps trampoline error codes to appropriate gRPC status codes. +/// +/// The mapping follows gRPC best practices: +/// - Client errors (invalid input) → InvalidArgument +/// - Precondition failures → FailedPrecondition +/// - Server errors → Internal +/// - Unknown/unclassified errors → Unknown +impl ErrorStatusConversionExt for TrampolineErrCode { + fn status_code(&self) -> tonic::Code { + match self { + // Precondition failures: The operation was rejected because the + // system is not in a state required for the operation's execution + TrampolineErrCode::FeatureNotSupported + | TrampolineErrCode::UnknownPeer + | TrampolineErrCode::MissingAmount + | TrampolineErrCode::MissingChannel + | TrampolineErrCode::InsufficientFunds + | TrampolineErrCode::PeerConnectionFailure + | TrampolineErrCode::MissingPreimage => tonic::Code::FailedPrecondition, + + // Invalid arguments: Client specified an invalid argument + TrampolineErrCode::InvalidNodeId + | TrampolineErrCode::AmbigousAmount + | TrampolineErrCode::InvalidInvoice => tonic::Code::InvalidArgument, + + // Internal errors: Server-side errors + TrampolineErrCode::NetworkError + | TrampolineErrCode::PeerNodeFailure + | TrampolineErrCode::Internal => tonic::Code::Internal, + + // Unknown: Error type cannot be classified + TrampolineErrCode::PaymentFailure => tonic::Code::Unknown, + + // RPC errors are mapped to Unknown as they can represent various + // failure types + TrampolineErrCode::RpcError(_) => tonic::Code::Unknown, + } + } +} + +// We need to respect rusts orphan rule which does not let us use a simple +// type TrampolineError = GreenlightError, as this would +// not let us implement own methods on TrampolineError since it IS +// GreenlightError and therefore not defined in this crate. +// We can avoid to implement all these delegations of the trait methods and +// the builder methods (furhter down) in the future by providing a proc +// macro that derives all the boilerplate for us. +pub struct TrampolineError(pub GreenlightError); + +impl core::fmt::Display for TrampolineError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.0.fmt(f) + } +} + +impl From for tonic::Status { + fn from(value: TrampolineError) -> Self { + tonic::Status::from(value.0) + } +} + +impl From> for TrampolineError { + fn from(err: GreenlightError) -> Self { + TrampolineError(err) + } +} + +/// Type alias for convenience. +type Result = core::result::Result; + +/// TrampolineError Convenience Constructors +impl TrampolineError { + // Delegate builder methods, wrapping the result + fn new(code: TrampolineErrCode, message: impl Into) -> Self { + TrampolineError(GreenlightError::new(code, message)) + } + + pub fn with_hint(self, hint: impl Into) -> Self { + TrampolineError(self.0.with_hint(hint)) + } + + pub fn with_context(self, context: impl Into) -> Self { + TrampolineError(self.0.with_context(context)) + } + + pub fn with_source(self, source: impl StdError + Send + Sync + 'static) -> Self { + TrampolineError(self.0.with_source(source)) + } + + // Implement convenince constructors. + fn feature_not_supported(features: impl Into) -> Self { + Self::new( + TrampolineErrCode::FeatureNotSupported, + format!("Peer doesn't suport trampoline payments"), + ) + .with_hint("Use a peer node that supports trampoline pay (feat 427)") + .with_context(json!({"features": features.into()}).to_string()) + } + + /// Creates an error for invalid node ID format. + fn invalid_node_id(source: impl StdError + Send + Sync + 'static) -> Self { + TrampolineError::new( + TrampolineErrCode::InvalidNodeId, + format!("Got an invalid node id: {}", source.to_string()), + ) + .with_hint("A node id must be exactly 33 bytes (66 hex characters)") + .with_source(source) + } + + /// Creates a network error for Core Lightning connection failures. + fn network(reason: impl Into) -> Self { + TrampolineError::new( + TrampolineErrCode::NetworkError, + format!( + "Could not connect to core-lightning node: {}", + reason.into() + ), + ) + } + + /// Creates an internal error for unexpected failures. + fn internal(message: impl Into) -> Self { + let msg = message.into(); + debug!( + "Something unexpected happened during trampoline_pay: {}", + &msg + ); + TrampolineError::new(TrampolineErrCode::Internal, msg) + } +} + fn feature_guard(features: impl Into>, feature_bit: usize) -> Result<()> { let mut features = features.into(); features.reverse(); let byte_pos = feature_bit / 8; let bit_pos = feature_bit % 8; if byte_pos >= features.len() || (features[byte_pos] & (1 << bit_pos)) == 0 { - return Err(anyhow!( - "Features {:?} do not contain feature bit {}", - hex::encode(features), - feature_bit - )); + return Err(TrampolineError::feature_not_supported(hex::encode( + features, + ))); } Ok(()) } @@ -62,10 +305,13 @@ pub async fn trampolinepay( req: pb::TrampolinePayRequest, rpc_path: impl AsRef, ) -> Result { - let node_id = cln_rpc::primitives::PublicKey::from_slice(&req.trampoline_node_id[..])?; + let node_id = cln_rpc::primitives::PublicKey::from_slice(&req.trampoline_node_id[..]) + .map_err(TrampolineError::invalid_node_id)?; let hex_node_id = hex::encode(node_id.serialize()); - let mut rpc = ClnRpc::new(&rpc_path).await?; + let mut rpc = ClnRpc::new(&rpc_path) + .await + .map_err(|e| TrampolineError::network(e.to_string()))?; // Extract the amount from the bolt11 or use the set amount field // Return an error if there is a mismatch. @@ -102,7 +348,12 @@ pub async fn trampolinepay( let preimage = match resp.payment_preimage { Some(preimage) => preimage, - None => return Err(anyhow!("got completed payment part without preimage")), + None => { + return Err(TrampolineError::new( + TrampolineErrCode::MissingPreimage, + "got completed payment part without preimage", + )) + } }; return Ok(cln_rpc::model::responses::PayResponse { amount_msat: resp.amount_msat.unwrap_or(Amount::from_msat(0)), @@ -141,7 +392,23 @@ pub async fn trampolinepay( log::debug!("Await peer connection to {}", hex_node_id); AwaitablePeer::new(node_id, rpc_path.as_ref().to_path_buf()) .wait() - .await?; + .await + .map_err(|e| match e { + AwaitablePeerError::PeerUnknown(s) => TrampolineError::new( + TrampolineErrCode::UnknownPeer, + format!("Unknown peer {}", s), + ) + .with_hint("You need to be connected and share a channel with the trampoline node"), + AwaitablePeerError::PeerConnectionFailure(s) => TrampolineError::new( + TrampolineErrCode::PeerConnectionFailure, + format!("Can't connect to peer {}", s), + ), + AwaitablePeerError::Rpc(rpc_error) => rpc_error.into(), + _ => { + TrampolineError::internal("Got an unexpected error while awaiting peer connection") + .with_source(e) + } + })?; // Check if peer has signaled that they support forward trampoline pays: @@ -153,28 +420,43 @@ pub async fn trampolinepay( .await? .peers .first() - .ok_or_else(|| anyhow!("node with id {} is unknown", hex_node_id))? + .ok_or_else(|| { + TrampolineError::new( + TrampolineErrCode::UnknownPeer, + format!("Unknown peer node {}", hex_node_id), + ).with_hint("You need to have an active channel with the trampoline node before you can execute a trampoline payment") + })? .features .as_ref() .map(|feat| hex::decode(feat)) - .ok_or_else(|| anyhow!("Could not extract features from peer object."))??; + .ok_or_else(|| { + TrampolineError::internal("missing feature bits on listpeers response") + })? + .map_err(|e| { + TrampolineError::internal("could not parse feature bits from hex").with_source(e) + })?; feature_guard(features, TRAMPOLINE_FEATURE_BIT)?; let amount_msat = match (as_option(req.amount_msat), decoded.amount_msat) { (None, None) => { - return Err(anyhow!( - "Bolt11 does not contain an amount and no amount was set either" + return Err(TrampolineError::new( + TrampolineErrCode::MissingAmount, + "Missing amount") + .with_hint( + "If the invoice does not have a fixed amount you need to set the amount parameter" )); } (None, Some(amt)) => amt.msat(), (Some(amt), None) => amt, (Some(set_amt), Some(bolt11_amt)) => { if set_amt != bolt11_amt.msat() { - return Err(anyhow!( - "Bolt11 amount {}msat and given amount {}msat do not match.", - bolt11_amt.msat(), - set_amt, + return Err(TrampolineError::new( + TrampolineErrCode::AmbigousAmount, + format!("Invoice amount and the given amount don't match"), + ) + .with_hint( + "If the invoice has the amount set you don't need to set it as a parameter", )); } bolt11_amt.msat() @@ -188,7 +470,7 @@ pub async fn trampolinepay( * (as_option(req.maxfeepercent).unwrap_or(DEFAULT_OVERPAY_PERCENT) as f64 / 100 as f64); let amount_msat = amount_msat + overpay as u64; - debug!("overpay={}, total_amt={}", overpay, amount_msat); + debug!("overpay={}, total_amt={}", overpay as u64, amount_msat); let channels: Vec = rpc .call_typed(&cln_rpc::model::requests::ListpeerchannelsRequest { id: Some(node_id) }) @@ -236,7 +518,10 @@ pub async fn trampolinepay( // Check if we actually got a channel to the trampoline node. if channels.is_empty() { - return Err(anyhow!("Has no channels with trampoline node")); + return Err(TrampolineError::new( + TrampolineErrCode::MissingChannel, + "No active and usable channelt to trampoline node found", + ).with_hint("In order to execute a trampoline payment, you heed to share a channel with the trampoline node that has a usable outgoing balance")); } // Await and filter out re-established channels. @@ -265,10 +550,12 @@ pub async fn trampolinepay( { Some(alloc) => alloc, None => { - return Err(anyhow!( - "could not allocate enough funds across channels {}msat<{}msat", - channels.iter().map(|ch| ch.spendable_msat).sum::(), - amount_msat + return Err(TrampolineError::new( + TrampolineErrCode::InsufficientFunds, + format!( + "Insufficient funds, {}msat are required, current maximal available {}msat", + amount_msat, + channels.iter().map(|ch| ch.spendable_msat).sum::()), )); } } @@ -292,9 +579,7 @@ pub async fn trampolinepay( let mut part_id = if alloc.len() == 1 { 0 } else { 1 }; let group_id = max_group_id + 1; let mut handles: Vec< - tokio::task::JoinHandle< - std::result::Result, - >, + tokio::task::JoinHandle>, > = vec![]; for ch in &alloc { let bolt11 = req.bolt11.clone(); @@ -303,8 +588,24 @@ pub async fn trampolinepay( let scid = ch.channel.short_channel_id.clone(); let description = decoded.description.clone(); let payload_hex = payload_hex.clone(); - let mut rpc = ClnRpc::new(&rpc_path).await?; + let mut rpc = ClnRpc::new(&rpc_path) + .await + .map_err(|e| TrampolineError::network(e.to_string()))?; let handle = tokio::spawn(async move { + let payment_secret = decoded + .payment_secret + .map(|e| e[..].to_vec()) + .ok_or(TrampolineError::new( + TrampolineErrCode::InvalidInvoice, + "The invoice is invalid, missing payment secret", + ))? + .try_into() + .map_err(|e: anyhow::Error| { + TrampolineError::new( + TrampolineErrCode::InvalidInvoice, + format!("The invoice is invalid, {}", e.to_string()), + ) + })?; do_pay( &mut rpc, node_id, @@ -317,11 +618,7 @@ pub async fn trampolinepay( group_id, decoded.payment_hash, cln_rpc::primitives::Amount::from_msat(amount_msat), - decoded - .payment_secret - .map(|e| e[..].to_vec()) - .ok_or(anyhow!("missing payment secret"))? - .try_into()?, + payment_secret, payload_hex, as_option(req.maxdelay), ) @@ -334,7 +631,9 @@ pub async fn trampolinepay( let results = join_all(handles).await; let mut payment_preimage = None; for result in results { - let response = result??; + let response = result.map_err(|e| { + TrampolineError::internal("Failed to wait for all tasks to complete").with_source(e) + })??; if let Some(preimage) = response.payment_preimage { payment_preimage = Some(preimage); } @@ -353,7 +652,10 @@ pub async fn trampolinepay( payment_preimage, }) } else { - Err(anyhow!("missing payment_preimage")) + Err(TrampolineError::new( + TrampolineErrCode::PaymentFailure, + "Payment failed, missing payment preimage", + )) } } @@ -417,16 +719,24 @@ async fn do_pay( Err(e) => { if let Some(code) = e.code { if code == PAY_UNPARSEABLE_ONION_CODE { - return Err(anyhow!("trampoline payment failed by the server")); + return Err(TrampolineError::new( + TrampolineErrCode::PeerNodeFailure, + "Got unparsable onion code from peer", + )); } } else if e.message == PAY_UNPARSEABLE_ONION_MSG { - return Err(anyhow!("trampoline payment failed by the server")); + return Err(TrampolineError::new( + TrampolineErrCode::PeerNodeFailure, + "Got unparsable onion code from peer", + )); } Err(e.into()) } } } +// FIXME: Once the `assert_send` is removed we can return a `Vec` or an +// `Option>` instead of a result. async fn reestablished_channels( channels: Vec, node_id: PublicKey, @@ -434,9 +744,11 @@ async fn reestablished_channels( deadline: Instant, ) -> Result> { // Wait for channels to re-establish. + // FIXME: Seems that this is a left-over of the development process that + // ensures that the channels are "sendable", they are `Send`. crate::awaitables::assert_send(AwaitableChannel::new( node_id, - ShortChannelId::from_str("1x1x1")?, + ShortChannelId::from_str("1x1x1").map_err(|e| TrampolineError::internal(e.to_string()))?, rpc_path.clone(), )); let mut futures = Vec::new(); diff --git a/libs/gl-util/Cargo.toml b/libs/gl-util/Cargo.toml new file mode 100644 index 000000000..7f3632cde --- /dev/null +++ b/libs/gl-util/Cargo.toml @@ -0,0 +1,10 @@ +[package] +name = "gl-util" +version = "0.1.0" +edition = "2024" + +[dependencies] +bytes = "1.10" +serde = { version = "1.0", features = ["derive"] } +serde_json = "1" +tonic = "0.11" diff --git a/libs/gl-util/src/error.rs b/libs/gl-util/src/error.rs new file mode 100644 index 000000000..a9631c85b --- /dev/null +++ b/libs/gl-util/src/error.rs @@ -0,0 +1,341 @@ +//! # Greenlight Error Module +//! +//! This module provides a comprehensive error handling system for +//! greenlight. It features a generic error type that can be customized with +//! module- or crate-specific error codes, while maintaining compatibility +//! with gRPC status codes and providing rich error context. +use bytes::Bytes; +use core::error::Error as StdError; +use serde::{Deserialize, Serialize}; +use serde_json; +use std::sync::Arc; +use tonic; + +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct ParsingError(String); + +impl core::fmt::Display for ParsingError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "could not parse error: {}", self.0) + } +} + +impl StdError for ParsingError {} + +// Convenient macro for creating ParsingError +macro_rules! parsing_error { + ($($arg:tt)*) => { + ParsingError(format!($($arg)*)) + }; +} + +/// Trait for defining module-specific error codes. +/// +/// This trait should be implemented by enums that represent different error +/// categories in greenlight. The error codes should be unique integers that +/// can be serialized and transmitted over the network. +pub trait ErrorCode: core::fmt::Debug + core::fmt::Display + Clone + Send + Sync + 'static { + /// Returns the numeric error code for this error type. + /// + /// This code should be unique within your application and stable + /// across versions for backward compatibility. + fn code(&self) -> i32; + + /// Attempts to construct an error code from its numeric representation. + /// + /// Returns `None` if the code is not recognized. + fn from_code(code: i32) -> Option + where + Self: Sized; +} + +/// JSON structure for transmitting error details over gRPC. +/// +/// This structure is serialized into the `details` field of a +/// `tonic::Status` to provide structured error information that can be +/// parsed by clients. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct GrpcErrorDetails { + pub code: i32, + /// Optional hint to help users resolve the issue + #[serde(skip_serializing_if = "Option::is_none")] + pub hint: Option, +} + +/// Extracted error information from a gRPC status. +/// +/// This structure contains all the error information that was transmitted +/// over gRPC, including both the standard gRPC fields and our custom +/// structured error details. +#[derive(Debug, Clone)] +pub struct GrpcErrorInfo { + pub code: i32, + pub message: String, + pub hint: Option, + pub grpc_code: tonic::Code, +} + +/// Attempts to parse structured error information from a `tonic::Status`. +/// +/// This implementation expects the status details to contain a JSON-encoded +/// `GrpcErrorDetails` structure. If parsing fails, a `serde_json::Error` is +/// returned. +impl TryFrom for GrpcErrorInfo { + type Error = serde_json::Error; + + fn try_from(value: tonic::Status) -> std::result::Result { + let parsed = serde_json::from_slice::(&value.details())?; + Ok(GrpcErrorInfo { + code: parsed.code, + message: value.message().to_owned(), + hint: parsed.hint, + grpc_code: value.code(), + }) + } +} + +/// Extension trait for mapping error codes to gRPC status codes. +/// +/// This trait should be implemented alongside `ErrorCode` to define +/// how your greenlight-specific errors map to standard gRPC status codes. +pub trait ErrorStatusConversionExt: ErrorCode { + /// Maps this error to an appropriate gRPC status code. + /// + /// The returned status code should follow gRPC conventions: + /// See: https://grpc.io/docs/guides/status-codes/ + fn status_code(&self) -> tonic::Code; +} + +/// Generic error type that combines error codes with rich error context. +#[derive(Debug, Clone)] +pub struct GreenlightError { + /// Error code for categorization and programmatic handling + pub code: C, + /// User-facing error message + pub message: String, + /// Optional hint to help users resolve the issue + pub hint: Option, + /// Context for debugging + pub context: Option, + /// Source error chain for debugging + pub source: Option>, +} + +impl GreenlightError { + /// Creates a new error with the given code and message. + pub fn new(code: C, message: impl Into) -> Self { + Self { + code, + message: message.into(), + hint: None, + context: None, + source: None, + } + } + + /// Adds a hint to help users resolve the issue. + /// + /// Hints should provide actionable guidance for end users. + pub fn with_hint(mut self, hint: impl Into) -> Self { + self.hint = Some(hint.into()); + self + } + + /// Adds internal context for debugging. + /// + /// Context is meant for developers and should include information + /// about what the system was doing when the error occurred. + /// TODO: currently unarmed, but can be used to log errors in a + /// standardized way in the future. + pub fn with_context(mut self, context: impl Into) -> Self { + self.context = Some(context.into()); + self + } + + /// Adds a source error for error chaining. + /// + /// This is useful for preserving the original error that caused this error. + /// + pub fn with_source(mut self, source: impl StdError + Send + Sync + 'static) -> Self { + self.source = Some(Arc::new(source)); + self + } + + /// Adds a source error from an existing Arc. + /// + /// This is useful when forwarding errors that are already wrapped in an Arc, + /// avoiding unnecessary allocations. + pub fn with_source_arc(mut self, source: Arc) -> Self { + self.source = Some(source); + self + } + + /// Returns the numeric error code. + /// + /// This is a convenience method that calls `code()` on the error code. + pub fn code(&self) -> i32 { + self.code.code() + } + + /// Converts this error to use a different error code type. + /// + /// This is useful when errors need to be converted between different + /// modules or layers that use different error code enums. + pub fn map_code(self, new_code: T) -> GreenlightError { + GreenlightError { + code: new_code, + message: self.message, + hint: self.hint, + context: self.context, + source: self.source, + } + } +} + +/// Displays the error message. +impl core::fmt::Display for GreenlightError { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + write!(f, "{}", self.message) + } +} + +/// Implements the standard error trait for error chaining. +impl StdError for GreenlightError { + fn source(&self) -> Option<&(dyn StdError + 'static)> { + self.source + .as_ref() + .map(|e| e.as_ref() as &(dyn StdError + 'static)) + } +} + +/// Converts a `GreenlightError` into a `tonic::Status` for gRPC transmission. +/// +/// The error details are JSON-encoded and included in the status details +/// field. If JSON serialization fails, a fallback JSON string is used. +impl From> for tonic::Status { + fn from(value: GreenlightError) -> Self { + let code = value.code.status_code(); + let details: Bytes = serde_json::to_vec(&GrpcErrorDetails { + code: value.code(), + hint: value.hint.clone(), + }) + .unwrap_or_else(|_| { + // Fallback to simple JSON if serialization fails + // This ensures we always send valid JSON even if something goes wrong + format!( + "{{\"code\":{},\"message\":\"{}\"}}", + value.code(), + value.message, + ) + .into_bytes() + }) + .into(); + tonic::Status::with_details(code, value.message, details) + } +} + +/// Attempts to convert a `tonic::Status` back into a `GreenlightError`. +/// +/// This requires that: +/// 1. The status contains valid JSON details in the expected format +/// 2. The error code in the details can be mapped to a valid `C` variant +/// +/// Returns an `anyhow::Error` if parsing fails or the error code is unknown. +impl TryFrom for GreenlightError { + type Error = ParsingError; + + fn try_from(value: tonic::Status) -> std::result::Result { + let grpc_err: GrpcErrorInfo = value + .try_into() + .map_err(|e| parsing_error!("failed to convert Status into GrpcErrorInfo {}", e))?; + let code = C::from_code(grpc_err.code) + .ok_or_else(|| parsing_error!("unknown error code: {}", grpc_err.code))?; + Ok(Self { + code, + message: grpc_err.message, + hint: grpc_err.hint, + context: None, + source: None, + }) + } +} + +/// Type alias for Core Lightning RPC error codes. +/// +/// CLN uses specific numeric codes to indicate different types of failures +/// in payment operations. This type preserves the original error code +/// for debugging and logging purposes. +pub type ClnRpcError = i32; + +/// Implementation of `ErrorCode` for CLN RPC errors. +/// +/// This implementation treats all i32 values as valid error codes, +/// allowing us to preserve any error code returned by CLN without loss. +impl ErrorCode for ClnRpcError { + fn code(&self) -> i32 { + *self + } + + fn from_code(code: i32) -> Option + where + Self: Sized, + { + Some(code) + } +} + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn test_status_conversion() { + #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] + enum TestErrorCodes { + FailedPrecondition = 101, + NotFound = 202, + } + + impl ErrorCode for TestErrorCodes { + fn code(&self) -> i32 { + *self as i32 + } + + fn from_code(_code: i32) -> Option + where + Self: Sized, + { + unimplemented!() + } + } + + impl core::fmt::Display for TestErrorCodes { + fn fmt(&self, _f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + unimplemented!() + } + } + + impl ErrorStatusConversionExt for TestErrorCodes { + fn status_code(&self) -> tonic::Code { + match self { + TestErrorCodes::FailedPrecondition => tonic::Code::FailedPrecondition, + TestErrorCodes::NotFound => tonic::Code::NotFound, + } + } + } + + type TestError = GreenlightError; + + let t_err = TestError::new(TestErrorCodes::FailedPrecondition, "a failed precondition") + .with_hint("How to resolve it"); + let status: tonic::Status = t_err.clone().into(); + assert_eq!(status.message(), t_err.message); + + let mut details: serde_json::Value = serde_json::from_slice(status.details()).unwrap(); + assert_eq!( + details["code"].take(), + TestErrorCodes::FailedPrecondition.code() + ); + assert_eq!(details["hint"].take(), "How to resolve it"); + } +} diff --git a/libs/gl-util/src/lib.rs b/libs/gl-util/src/lib.rs new file mode 100644 index 000000000..a91e73517 --- /dev/null +++ b/libs/gl-util/src/lib.rs @@ -0,0 +1 @@ +pub mod error;