Add check_valid for SendPaymentCommand

src/fiber/graph.rs

@@ -449,9 +449,9 @@ where
         payment_request: SendPaymentCommand,
     ) -> Result<Vec<OnionInfo>, GraphError> {
         let source = self.get_source_pubkey();
-        let target = payment_request.target_pubkey;
-        let amount = payment_request.amount;
-        let payment_hash = payment_request.payment_hash;
+        let (target, amount, payment_hash) = payment_request
+            .check_valid()
+            .map_err(|e| GraphError::Other(format!("payment request is invalid {:?}", e)))?;
         let invoice = payment_request
             .invoice
             .map(|x| x.parse::<CkbInvoice>().unwrap());
@@ -460,14 +460,10 @@ where
             .and_then(|x| x.hash_algorithm().copied())
             .unwrap_or_default();
 
-        if let Some(invoice) = invoice {
-            if *invoice.payment_hash() != payment_hash {
-                return Err(GraphError::Amount(
-                    "payment hash in the invoice does not match the payment hash in the command"
-                        .to_string(),
-                ));
-            }
-        }
+        info!(
+            "build_route source: {:?} target: {:?} amount: {:?}, payment_hash: {:?}",
+            source, target, amount, payment_hash
+        );
 
         let max_fee_amount = payment_request.max_fee_amount.unwrap_or(1000);
         let route = self.find_route(source, target, amount, max_fee_amount)?;
@@ -738,7 +734,7 @@ impl PaymentSession {
     }
 
     pub fn payment_hash(&self) -> Hash256 {
-        self.command.payment_hash
+        self.command.payment_hash()
     }
 }
 
@@ -1177,9 +1173,9 @@ mod tests {
         let node3 = network.keys[3];
         // Test build route from node1 to node3
         let route = network.graph.build_route(SendPaymentCommand {
-            target_pubkey: node3.into(),
-            amount: 100,
-            payment_hash: Hash256::default(),
+            target_pubkey: Some(node3.into()),
+            amount: Some(100),
+            payment_hash: Some(Hash256::default()),
             invoice: None,
             final_cltv_delta: Some(100),
             timeout: Some(10),
@@ -1212,9 +1208,9 @@ mod tests {
 
         // Test build route from node1 to node3 with amount exceeding max_htlc_value
         let route = network.graph.build_route(SendPaymentCommand {
-            target_pubkey: node3.into(),
-            amount: 100, // Exceeds max_htlc_value of 50
-            payment_hash: Hash256::default(),
+            target_pubkey: Some(node3.into()),
+            amount: Some(100), // Exceeds max_htlc_value of 50
+            payment_hash: Some(Hash256::default()),
             invoice: None,
             final_cltv_delta: Some(100),
             timeout: Some(10),
@@ -1234,9 +1230,9 @@ mod tests {
 
         // Test build route from node1 to node3 with amount below min_htlc_value
         let route = network.graph.build_route(SendPaymentCommand {
-            target_pubkey: node3.into(),
-            amount: 10, // Below min_htlc_value of 50
-            payment_hash: Hash256::default(),
+            target_pubkey: Some(node3.into()),
+            amount: Some(10), // Below min_htlc_value of 50
+            payment_hash: Some(Hash256::default()),
             invoice: None,
             final_cltv_delta: Some(100),
             timeout: Some(10),

src/fiber/network.rs

@@ -67,7 +67,7 @@ use crate::fiber::channel::{
 };
 use crate::fiber::graph::{ChannelInfo, NodeInfo, PaymentSession};
 use crate::fiber::types::{secp256k1_instance, FiberChannelMessage, OnionPacket, TxSignatures};
-use crate::invoice::InvoiceStore;
+use crate::invoice::{CkbInvoice, InvoiceStore};
 use crate::{unwrap_or_return, Error};
 
 pub const FIBER_PROTOCOL_ID: ProtocolId = ProtocolId::new(42);
@@ -193,31 +193,81 @@ pub struct OpenChannelCommand {
 #[derive(Clone, Debug, Serialize, Deserialize)]
 pub struct SendPaymentCommand {
     // the identifier of the payment target
-    pub target_pubkey: Pubkey,
-
+    pub target_pubkey: Option<Pubkey>,
     // the amount of the payment
-    pub amount: u128,
-
+    pub amount: Option<u128>,
     // The hash to use within the payment's HTLC
     // FIXME: this should be optional when AMP is enabled
-    pub payment_hash: Hash256,
-
-    // The CLTV delta from the current height that should be used to set the timelock for the final hop
-    pub final_cltv_delta: Option<u64>,
-
+    pub payment_hash: Option<Hash256>,
     // the encoded invoice to send to the recipient
     pub invoice: Option<String>,
-
+    // The CLTV delta from the current height that should be used to set the timelock for the final hop
+    pub final_cltv_delta: Option<u64>,
     // the payment timeout in seconds, if the payment is not completed within this time, it will be cancelled
     pub timeout: Option<u64>,
-
     // the maximum fee amounts in shannons that the sender is willing to pay, default is 1000 shannons CKB.
     pub max_fee_amount: Option<u128>,
-
     // max parts for the payment, only used for multi-part payments
     pub max_parts: Option<u64>,
 }
 
+impl SendPaymentCommand {
+    pub fn check_valid(&self) -> Result<(Pubkey, u128, Hash256), String> {
+        let invoice = self
+            .invoice
+            .as_ref()
+            .map(|invoice| invoice.parse::<CkbInvoice>())
+            .transpose()
+            .map_err(|_| "invoice is invalid".to_string())?;
+
+        fn validate_field<T: PartialEq + Clone>(
+            field: Option<T>,
+            invoice_field: Option<T>,
+            field_name: &str,
+        ) -> Result<T, String> {
+            match (field, invoice_field) {
+                (Some(f), Some(i)) => {
+                    if f != i {
+                        return Err(format!("{} does not match the invoice", field_name));
+                    }
+                    Ok(f)
+                }
+                (Some(f), None) => Ok(f),
+                (None, Some(i)) => Ok(i),
+                (None, None) => Err(format!("{} is missing", field_name)),
+            }
+        }
+
+        let target = validate_field(
+            self.target_pubkey,
+            invoice
+                .as_ref()
+                .and_then(|i| i.payee_pub_key().cloned().map(|k| Pubkey::from(k))),
+            "target_pubkey",
+        )?;
+
+        let amount = validate_field(
+            self.amount,
+            invoice.as_ref().and_then(|i| i.amount()),
+            "amount",
+        )?;
+
+        let payment_hash = validate_field(
+            self.payment_hash,
+            invoice.as_ref().map(|i| i.payment_hash().clone()),
+            "payment_hash",
+        )?;
+
+        Ok((target, amount, payment_hash))
+    }
+
+    pub fn payment_hash(&self) -> Hash256 {
+        let (_target, _amount, payment_hash) =
+            self.check_valid().expect("valid SendPaymentCommand");
+        payment_hash
+    }
+}
+
 #[derive(Debug)]
 pub struct AcceptChannelCommand {
     pub temp_channel_id: Hash256,
@@ -1332,10 +1382,15 @@ where
                 let _ = reply.send(signature);
             }
             NetworkActorCommand::SendPayment(payment_request, reply) => {
-                let payment_hash = payment_request.payment_hash;
-                let res = self.on_send_payment(myself, payment_request).await;
-                info!("send_payment res: {:?}", res);
-                let _ = reply.send(Ok(SendPaymentResponse { payment_hash }));
+                match self.on_send_payment(myself, payment_request).await {
+                    Ok(payment_hash) => {
+                        let _ = reply.send(Ok(SendPaymentResponse { payment_hash }));
+                    }
+                    Err(e) => {
+                        error!("Failed to send payment: {:?}", e);
+                        let _ = reply.send(Err(e.to_string()));
+                    }
+                }
             }
             NetworkActorCommand::BroadcastLocalInfo(kind) => match kind {
                 LocalInfoKind::NodeAnnouncement => {

src/invoice/invoice_impl.rs

@@ -7,22 +7,19 @@ use crate::fiber::serde_utils::U128Hex;
 use crate::fiber::types::Hash256;
 use crate::invoice::InvoiceError;
 use bech32::{encode, u5, FromBase32, ToBase32, Variant, WriteBase32};
-use bitcoin::hashes::sha256::Hash as Sha256;
-use bitcoin::hashes::Hash;
-use bitcoin::{
-    key::Secp256k1,
-    secp256k1::{
-        self,
-        ecdsa::{RecoverableSignature, RecoveryId},
-        Message, PublicKey,
-    },
-};
+use bitcoin::hashes::{sha256::Hash as Sha256, Hash as _};
 use ckb_types::{
     packed::{Byte, Script},
     prelude::{Pack, Unpack},
 };
 use core::time::Duration;
 use molecule::prelude::{Builder, Entity};
+use secp256k1::{
+    self,
+    ecdsa::{RecoverableSignature, RecoveryId},
+    Message, PublicKey, Secp256k1,
+};
+
 use serde::{Deserialize, Serialize};
 use serde_with::serde_as;
 use std::{cmp::Ordering, str::FromStr};
@@ -182,7 +179,7 @@ impl CkbInvoice {
             .or(recovered_pub_key.as_ref())
             .expect("One is always present");
 
-        let hash = Message::from_slice(&self.hash()[..])
+        let hash = Message::from_digest_slice(&self.hash()[..])
             .expect("Hash is 32 bytes long, same as MESSAGE_SIZE");
 
         let secp_context = Secp256k1::new();
@@ -199,7 +196,7 @@ impl CkbInvoice {
         F: FnOnce(&Message) -> RecoverableSignature,
     {
         let hash = self.hash();
-        let message = Message::from_slice(&hash).unwrap();
+        let message = Message::from_digest_slice(&hash).unwrap();
         let signature = sign_function(&message);
         self.signature = Some(InvoiceSignature(signature));
         self.check_signature()?;
@@ -208,7 +205,7 @@ impl CkbInvoice {
 
     /// Recovers the public key used for signing the invoice from the recoverable signature.
     pub fn recover_payee_pub_key(&self) -> Result<PublicKey, secp256k1::Error> {
-        let hash = Message::from_slice(&self.hash()[..])
+        let hash = Message::from_digest_slice(&self.hash()[..])
             .expect("Hash is 32 bytes long, same as MESSAGE_SIZE");
 
         let res = secp256k1::Secp256k1::new()
@@ -730,28 +727,25 @@ impl TryFrom<gen_invoice::RawInvoiceData> for InvoiceData {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use bitcoin::{
-        key::{KeyPair, Secp256k1},
-        secp256k1::SecretKey,
-    };
     use ckb_hash::blake2b_256;
+    use secp256k1::{Keypair, SecretKey};
     use std::time::{SystemTime, UNIX_EPOCH};
 
     fn gen_rand_public_key() -> PublicKey {
         let secp = Secp256k1::new();
-        let key_pair = KeyPair::new(&secp, &mut rand::thread_rng());
+        let key_pair = Keypair::new(&secp, &mut rand::thread_rng());
         PublicKey::from_keypair(&key_pair)
     }
 
     fn gen_rand_private_key() -> SecretKey {
         let secp = Secp256k1::new();
-        let key_pair = KeyPair::new(&secp, &mut rand::thread_rng());
+        let key_pair = Keypair::new(&secp, &mut rand::thread_rng());
         SecretKey::from_keypair(&key_pair)
     }
 
     fn gen_rand_keypair() -> (PublicKey, SecretKey) {
         let secp = Secp256k1::new();
-        let key_pair = KeyPair::new(&secp, &mut rand::thread_rng());
+        let key_pair = Keypair::new(&secp, &mut rand::thread_rng());
         (
             PublicKey::from_keypair(&key_pair),
             SecretKey::from_keypair(&key_pair),
@@ -790,8 +784,10 @@ mod tests {
     #[test]
     fn test_signature() {
         let private_key = gen_rand_private_key();
-        let signature = Secp256k1::new()
-            .sign_ecdsa_recoverable(&Message::from_slice(&[0u8; 32]).unwrap(), &private_key);
+        let signature = Secp256k1::new().sign_ecdsa_recoverable(
+            &Message::from_digest_slice(&[0u8; 32]).unwrap(),
+            &private_key,
+        );
         let signature = InvoiceSignature(signature);
         let base32 = signature.to_base32();
         assert_eq!(base32.len(), SIGNATURE_U5_SIZE);
@@ -870,8 +866,10 @@ mod tests {
     #[test]
     fn test_invoice_bc32m_not_same() {
         let private_key = gen_rand_private_key();
-        let signature = Secp256k1::new()
-            .sign_ecdsa_recoverable(&Message::from_slice(&[0u8; 32]).unwrap(), &private_key);
+        let signature = Secp256k1::new().sign_ecdsa_recoverable(
+            &Message::from_digest_slice(&[0u8; 32]).unwrap(),
+            &private_key,
+        );
         let invoice = CkbInvoice {
             currency: Currency::Fibb,
             amount: Some(1280),

src/rpc/channel.rs

@@ -173,15 +173,15 @@ pub struct UpdateChannelParams {
 #[derive(Clone, Debug, Serialize, Deserialize)]
 pub struct SendPaymentCommandParams {
     // the identifier of the payment target
-    pub target_pubkey: Pubkey,
+    pub target_pubkey: Option<Pubkey>,
 
     // the amount of the payment
-    #[serde_as(as = "U128Hex")]
-    pub amount: u128,
+    #[serde_as(as = "Option<U128Hex>")]
+    pub amount: Option<u128>,
 
     // The hash to use within the payment's HTLC
     // FIXME: this should be optional when AMP is enabled
-    pub payment_hash: Hash256,
+    pub payment_hash: Option<Hash256>,
 
     // The CLTV delta from the current height that should be used to set the timelock for the final hop
     #[serde_as(as = "Option<U64Hex>")]