Skip to content

Commit 47ca19d

Browse files
committed
Remove the async_signing cfg flag
Now that the core features required for `async_signing` are in place, we can go ahead and expose it publicly (rather than behind a a `cfg`-flag). We still don't have full async support for `get_per_commitment_point`, but only one case in channel reconnection remains. The overall logic may still have some hiccups, but its been in use in production at a major LDK user for some time now. Thus, it doesn't really make sense to hide behind a `cfg`-flag, even if the feature is only 99% complete. Further, the new paths exposed are very restricted to signing operations that run async, so the risk for existing users should be incredibly low.
1 parent d1e94bd commit 47ca19d

File tree

6 files changed

+26
-65
lines changed

6 files changed

+26
-65
lines changed

Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,6 @@ check-cfg = [
6161
"cfg(ldk_bench)",
6262
"cfg(ldk_test_vectors)",
6363
"cfg(taproot)",
64-
"cfg(async_signing)",
6564
"cfg(require_route_graph_test)",
6665
"cfg(splicing)",
6766
"cfg(async_payments)",

ci/ci-tests.sh

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,8 +162,6 @@ fi
162162
echo -e "\n\nTest cfg-flag builds"
163163
RUSTFLAGS="--cfg=taproot" cargo test --verbose --color always -p lightning
164164
[ "$CI_MINIMIZE_DISK_USAGE" != "" ] && cargo clean
165-
RUSTFLAGS="--cfg=async_signing" cargo test --verbose --color always -p lightning
166-
[ "$CI_MINIMIZE_DISK_USAGE" != "" ] && cargo clean
167165
RUSTFLAGS="--cfg=splicing" cargo test --verbose --color always -p lightning
168166
[ "$CI_MINIMIZE_DISK_USAGE" != "" ] && cargo clean
169167
RUSTFLAGS="--cfg=async_payments" cargo test --verbose --color always -p lightning

lightning/src/ln/channel.rs

Lines changed: 24 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -906,7 +906,6 @@ pub(super) struct MonitorRestoreUpdates {
906906
}
907907

908908
/// The return value of `signer_maybe_unblocked`
909-
#[allow(unused)]
910909
pub(super) struct SignerResumeUpdates {
911910
pub commitment_update: Option<msgs::CommitmentUpdate>,
912911
pub revoke_and_ack: Option<msgs::RevokeAndACK>,
@@ -3959,13 +3958,8 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
39593958
log_trace!(logger, "Counterparty commitment signature available for funding_signed message; clearing signer_pending_funding");
39603959
self.signer_pending_funding = false;
39613960
} else if signature.is_none() {
3962-
#[cfg(not(async_signing))] {
3963-
panic!("Failed to get signature for funding_signed");
3964-
}
3965-
#[cfg(async_signing)] {
3966-
log_trace!(logger, "Counterparty commitment signature not available for funding_signed message; setting signer_pending_funding");
3967-
self.signer_pending_funding = true;
3968-
}
3961+
log_trace!(logger, "Counterparty commitment signature not available for funding_signed message; setting signer_pending_funding");
3962+
self.signer_pending_funding = true;
39693963
}
39703964

39713965
signature.map(|(signature, _)| msgs::FundingSigned {
@@ -6114,7 +6108,6 @@ impl<SP: Deref> Channel<SP> where
61146108

61156109
/// Indicates that the signer may have some signatures for us, so we should retry if we're
61166110
/// blocked.
6117-
#[cfg(async_signing)]
61186111
pub fn signer_maybe_unblocked<L: Deref>(&mut self, logger: &L) -> SignerResumeUpdates where L::Target: Logger {
61196112
if !self.holder_commitment_point.is_available() {
61206113
log_trace!(logger, "Attempting to update holder per-commitment point...");
@@ -6231,21 +6224,16 @@ impl<SP: Deref> Channel<SP> where
62316224
&self.context.channel_id(), self.holder_commitment_point.transaction_number(),
62326225
self.holder_commitment_point.transaction_number() + 2);
62336226
}
6234-
#[cfg(not(async_signing))] {
6235-
panic!("Holder commitment point and per commitment secret must be available when generating revoke_and_ack");
6236-
}
6237-
#[cfg(async_signing)] {
6238-
// Technically if we're at HolderCommitmentPoint::PendingNext,
6239-
// we have a commitment point ready to send in an RAA, however we
6240-
// choose to wait since if we send RAA now, we could get another
6241-
// CS before we have any commitment point available. Blocking our
6242-
// RAA here is a convenient way to make sure that post-funding
6243-
// we're only ever waiting on one commitment point at a time.
6244-
log_trace!(logger, "Last revoke-and-ack pending in channel {} for sequence {} because the next per-commitment point is not available",
6245-
&self.context.channel_id(), self.holder_commitment_point.transaction_number());
6246-
self.context.signer_pending_revoke_and_ack = true;
6247-
None
6248-
}
6227+
// Technically if we're at HolderCommitmentPoint::PendingNext,
6228+
// we have a commitment point ready to send in an RAA, however we
6229+
// choose to wait since if we send RAA now, we could get another
6230+
// CS before we have any commitment point available. Blocking our
6231+
// RAA here is a convenient way to make sure that post-funding
6232+
// we're only ever waiting on one commitment point at a time.
6233+
log_trace!(logger, "Last revoke-and-ack pending in channel {} for sequence {} because the next per-commitment point is not available",
6234+
&self.context.channel_id(), self.holder_commitment_point.transaction_number());
6235+
self.context.signer_pending_revoke_and_ack = true;
6236+
None
62496237
}
62506238

62516239
/// Gets the last commitment update for immediate sending to our peer.
@@ -6316,16 +6304,11 @@ impl<SP: Deref> Channel<SP> where
63166304
}
63176305
update
63186306
} else {
6319-
#[cfg(not(async_signing))] {
6320-
panic!("Failed to get signature for new commitment state");
6321-
}
6322-
#[cfg(async_signing)] {
6323-
if !self.context.signer_pending_commitment_update {
6324-
log_trace!(logger, "Commitment update awaiting signer: setting signer_pending_commitment_update");
6325-
self.context.signer_pending_commitment_update = true;
6326-
}
6327-
return Err(());
6307+
if !self.context.signer_pending_commitment_update {
6308+
log_trace!(logger, "Commitment update awaiting signer: setting signer_pending_commitment_update");
6309+
self.context.signer_pending_commitment_update = true;
63286310
}
6311+
return Err(());
63296312
};
63306313
Ok(msgs::CommitmentUpdate {
63316314
update_add_htlcs, update_fulfill_htlcs, update_fail_htlcs, update_fail_malformed_htlcs, update_fee,
@@ -8362,13 +8345,8 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
83628345
log_trace!(logger, "Counterparty commitment signature ready for funding_created message: clearing signer_pending_funding");
83638346
self.context.signer_pending_funding = false;
83648347
} else if signature.is_none() {
8365-
#[cfg(not(async_signing))] {
8366-
panic!("Failed to get signature for new funding creation");
8367-
}
8368-
#[cfg(async_signing)] {
8369-
log_trace!(logger, "funding_created awaiting signer; setting signer_pending_funding");
8370-
self.context.signer_pending_funding = true;
8371-
}
8348+
log_trace!(logger, "funding_created awaiting signer; setting signer_pending_funding");
8349+
self.context.signer_pending_funding = true;
83728350
};
83738351

83748352
signature.map(|signature| msgs::FundingCreated {
@@ -8467,14 +8445,9 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
84678445
holder_commitment_point.current_point()
84688446
},
84698447
_ => {
8470-
#[cfg(not(async_signing))] {
8471-
panic!("Failed getting commitment point for open_channel message");
8472-
}
8473-
#[cfg(async_signing)] {
8474-
log_trace!(_logger, "Unable to generate open_channel message, waiting for commitment point");
8475-
self.signer_pending_open_channel = true;
8476-
return None;
8477-
}
8448+
log_trace!(_logger, "Unable to generate open_channel message, waiting for commitment point");
8449+
self.signer_pending_open_channel = true;
8450+
return None;
84788451
}
84798452
};
84808453
let keys = self.context.get_holder_pubkeys();
@@ -8562,7 +8535,6 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
85628535

85638536
/// Indicates that the signer may have some signatures for us, so we should retry if we're
85648537
/// blocked.
8565-
#[cfg(async_signing)]
85668538
pub fn signer_maybe_unblocked<L: Deref>(
85678539
&mut self, chain_hash: ChainHash, logger: &L
85688540
) -> (Option<msgs::OpenChannel>, Option<msgs::FundingCreated>) where L::Target: Logger {
@@ -8723,14 +8695,9 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
87238695
holder_commitment_point.current_point()
87248696
},
87258697
_ => {
8726-
#[cfg(not(async_signing))] {
8727-
panic!("Failed getting commitment point for accept_channel message");
8728-
}
8729-
#[cfg(async_signing)] {
8730-
log_trace!(_logger, "Unable to generate accept_channel message, waiting for commitment point");
8731-
self.signer_pending_accept_channel = true;
8732-
return None;
8733-
}
8698+
log_trace!(_logger, "Unable to generate accept_channel message, waiting for commitment point");
8699+
self.signer_pending_accept_channel = true;
8700+
return None;
87348701
}
87358702
};
87368703
let keys = self.context.get_holder_pubkeys();
@@ -8833,7 +8800,6 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
88338800

88348801
/// Indicates that the signer may have some signatures for us, so we should retry if we're
88358802
/// blocked.
8836-
#[allow(unused)]
88378803
pub fn signer_maybe_unblocked<L: Deref>(
88388804
&mut self, logger: &L
88398805
) -> Option<msgs::AcceptChannel> where L::Target: Logger {

lightning/src/ln/channelmanager.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9512,7 +9512,6 @@ where
95129512
/// attempted in every channel, or in the specifically provided channel.
95139513
///
95149514
/// [`ChannelSigner`]: crate::sign::ChannelSigner
9515-
#[cfg(async_signing)]
95169515
pub fn signer_unblocked(&self, channel_opt: Option<(PublicKey, ChannelId)>) {
95179516
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
95189517

lightning/src/ln/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ mod monitor_tests;
8585
#[cfg(test)]
8686
#[allow(unused_mut)]
8787
mod shutdown_tests;
88-
#[cfg(all(test, async_signing))]
88+
#[cfg(test)]
8989
#[allow(unused_mut)]
9090
mod async_signer_tests;
9191
#[cfg(test)]

lightning/src/sign/mod.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -728,8 +728,7 @@ pub trait ChannelSigner {
728728
/// Note that the commitment number starts at `(1 << 48) - 1` and counts backwards.
729729
///
730730
/// If the signer returns `Err`, then the user is responsible for either force-closing the channel
731-
/// or calling `ChannelManager::signer_unblocked` (this method is only available when the
732-
/// `async_signing` cfg flag is enabled) once the signature is ready.
731+
/// or calling `ChannelManager::signer_unblocked` once the signature is ready.
733732
///
734733
// TODO: link to `signer_unblocked` once the cfg flag is removed
735734
fn get_per_commitment_point(

0 commit comments

Comments
 (0)