Change get_per_commitment_point to return result type

Includes simple changes to test util signers and tests, as well as
handling the error case for get_per_commitment_point in
HolderCommitmentPoint. This leaves an `.expect("TODO")` in places
that will be addressed in upcoming commits.

Author: alecchendev

lightning/src/ln/channel.rs

@@ -977,10 +977,12 @@ impl HolderCommitmentPoint where {
 		where SP::Target: SignerProvider, L::Target: Logger
 	{
 		if let HolderCommitmentPoint::Uninitialized { transaction_number } = self {
-			let current = signer.as_ref().get_per_commitment_point(*transaction_number, secp_ctx); // TODO
-			log_trace!(logger, "Retrieved current per-commitment point {}", transaction_number);
-			*self = HolderCommitmentPoint::PendingNext { transaction_number: *transaction_number, current };
-			// TODO: handle error case when get_per_commitment_point becomes async
+			if let Ok(current) = signer.as_ref().get_per_commitment_point(*transaction_number, secp_ctx) {
+				log_trace!(logger, "Retrieved initial per-commitment point {}", transaction_number);
+				*self = HolderCommitmentPoint::PendingNext { transaction_number: *transaction_number, current };
+			} else {
+				log_trace!(logger, "Initial per-commitment point {} is pending", transaction_number);
+			}
 		}
 
 		if let HolderCommitmentPoint::Available { transaction_number, next, .. } = self {
@@ -991,10 +993,12 @@ impl HolderCommitmentPoint where {
 		}
 
 		if let HolderCommitmentPoint::PendingNext { transaction_number, current } = self {
-			let next = signer.as_ref().get_per_commitment_point(*transaction_number - 1, secp_ctx); // TODO
-			log_trace!(logger, "Retrieved next per-commitment point {}", *transaction_number - 1);
-			*self = HolderCommitmentPoint::Available { transaction_number: *transaction_number, current: *current, next };
-			// TODO: handle error case when get_per_commitment_point becomes async
+			if let Ok(next) = signer.as_ref().get_per_commitment_point(*transaction_number - 1, secp_ctx) {
+				log_trace!(logger, "Retrieved next per-commitment point {}", *transaction_number - 1);
+				*self = HolderCommitmentPoint::Available { transaction_number: *transaction_number, current: *current, next };
+			} else {
+				log_trace!(logger, "Next per-commitment point {} is pending", transaction_number);
+			}
 		}
 	}
 }
@@ -5373,7 +5377,7 @@ impl<SP: Deref> Channel<SP> where
 
 		let our_commitment_transaction = INITIAL_COMMITMENT_NUMBER - self.context.holder_commitment_point.transaction_number() - 1;
 		if msg.next_remote_commitment_number > 0 {
-			let expected_point = self.context.holder_signer.as_ref().get_per_commitment_point(INITIAL_COMMITMENT_NUMBER - msg.next_remote_commitment_number + 1, &self.context.secp_ctx);
+			let expected_point = self.context.holder_signer.as_ref().get_per_commitment_point(INITIAL_COMMITMENT_NUMBER - msg.next_remote_commitment_number + 1, &self.context.secp_ctx).expect("TODO");
 			let given_secret = SecretKey::from_slice(&msg.your_last_per_commitment_secret)
 				.map_err(|_| ChannelError::Close("Peer sent a garbage channel_reestablish with unparseable secret key".to_owned()))?;
 			if expected_point != PublicKey::from_secret_key(&self.context.secp_ctx, &given_secret) {

lightning/src/ln/functional_tests.rs

@@ -741,7 +741,7 @@ fn test_update_fee_that_funder_cannot_afford() {
 		(pubkeys.revocation_basepoint, pubkeys.htlc_basepoint,
 		 pubkeys.funding_pubkey)
 	};
-	let (remote_delayed_payment_basepoint, remote_htlc_basepoint,remote_point, remote_funding) = {
+	let (remote_delayed_payment_basepoint, remote_htlc_basepoint, remote_point, remote_funding) = {
 		let per_peer_state = nodes[1].node.per_peer_state.read().unwrap();
 		let chan_lock = per_peer_state.get(&nodes[0].node.get_our_node_id()).unwrap().lock().unwrap();
 		let remote_chan = chan_lock.channel_by_id.get(&chan.2).map(
@@ -750,7 +750,7 @@ fn test_update_fee_that_funder_cannot_afford() {
 		let chan_signer = remote_chan.get_signer();
 		let pubkeys = chan_signer.as_ref().pubkeys();
 		(pubkeys.delayed_payment_basepoint, pubkeys.htlc_basepoint,
-		 chan_signer.as_ref().get_per_commitment_point(INITIAL_COMMITMENT_NUMBER - 1, &secp_ctx),
+		 chan_signer.as_ref().get_per_commitment_point(INITIAL_COMMITMENT_NUMBER - 1, &secp_ctx).unwrap(),
 		 pubkeys.funding_pubkey)
 	};
 
@@ -1474,7 +1474,7 @@ fn test_fee_spike_violation_fails_htlc() {
 		let pubkeys = chan_signer.as_ref().pubkeys();
 		(pubkeys.revocation_basepoint, pubkeys.htlc_basepoint,
 		 chan_signer.as_ref().release_commitment_secret(INITIAL_COMMITMENT_NUMBER),
-		 chan_signer.as_ref().get_per_commitment_point(INITIAL_COMMITMENT_NUMBER - 2, &secp_ctx),
+		 chan_signer.as_ref().get_per_commitment_point(INITIAL_COMMITMENT_NUMBER - 2, &secp_ctx).unwrap(),
 		 chan_signer.as_ref().pubkeys().funding_pubkey)
 	};
 	let (remote_delayed_payment_basepoint, remote_htlc_basepoint, remote_point, remote_funding) = {
@@ -1486,7 +1486,7 @@ fn test_fee_spike_violation_fails_htlc() {
 		let chan_signer = remote_chan.get_signer();
 		let pubkeys = chan_signer.as_ref().pubkeys();
 		(pubkeys.delayed_payment_basepoint, pubkeys.htlc_basepoint,
-		 chan_signer.as_ref().get_per_commitment_point(INITIAL_COMMITMENT_NUMBER - 1, &secp_ctx),
+		 chan_signer.as_ref().get_per_commitment_point(INITIAL_COMMITMENT_NUMBER - 1, &secp_ctx).unwrap(),
 		 chan_signer.as_ref().pubkeys().funding_pubkey)
 	};
 

lightning/src/sign/mod.rs

@@ -729,8 +729,11 @@ pub trait ChannelSigner {
 	/// Gets the per-commitment point for a specific commitment number
 	///
 	/// Note that the commitment number starts at `(1 << 48) - 1` and counts backwards.
+	///
+	/// If the signer returns `Err`, then the user is responsible for either force-closing the channel
+	/// or retrying once the signature is ready.
 	fn get_per_commitment_point(&self, idx: u64, secp_ctx: &Secp256k1<secp256k1::All>)
-		-> PublicKey;
+		-> Result<PublicKey, ()>;
 
 	/// Gets the commitment secret for a specific commitment number as part of the revocation process
 	///
@@ -1343,11 +1346,11 @@ impl EntropySource for InMemorySigner {
 impl ChannelSigner for InMemorySigner {
 	fn get_per_commitment_point(
 		&self, idx: u64, secp_ctx: &Secp256k1<secp256k1::All>,
-	) -> PublicKey {
+	) -> Result<PublicKey, ()> {
 		let commitment_secret =
 			SecretKey::from_slice(&chan_utils::build_commitment_secret(&self.commitment_seed, idx))
 				.unwrap();
-		PublicKey::from_secret_key(secp_ctx, &commitment_secret)
+		Ok(PublicKey::from_secret_key(secp_ctx, &commitment_secret))
 	}
 
 	fn release_commitment_secret(&self, idx: u64) -> [u8; 32] {

lightning/src/util/test_channel_signer.rs

@@ -126,7 +126,9 @@ impl TestChannelSigner {
 }
 
 impl ChannelSigner for TestChannelSigner {
-	fn get_per_commitment_point(&self, idx: u64, secp_ctx: &Secp256k1<secp256k1::All>) -> PublicKey {
+	fn get_per_commitment_point(&self, idx: u64, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<PublicKey, ()> {
+		// TODO: implement a mask in EnforcementState to let you test signatures being
+		// unavailable
 		self.inner.get_per_commitment_point(idx, secp_ctx)
 	}