Skip to content

Commit

Permalink
cfg-gate async signing logic
Browse files Browse the repository at this point in the history
We are intending to release without having completed our async
signing logic, which sadly means we need to cfg-gate it to ensure
we restore the previous state of panicking on signer errors, rather
than putting us in a stuck state with no way to recover.

Here we add a new `async_signing` cfg flag and use it to gate all
the new logic from #2558 effectively reverting commits
1da2929 through
014a336.
  • Loading branch information
TheBlueMatt committed Dec 13, 2023
1 parent 304224e commit a866ba7
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 17 deletions.
2 changes: 2 additions & 0 deletions ci/check-cfg-flags.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ def check_cfg_tag(cfg):
pass
elif cfg == "taproot":
pass
elif cfg == "async_signing":
pass
elif cfg == "require_route_graph_test":
pass
else:
Expand Down
5 changes: 2 additions & 3 deletions ci/ci-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,6 @@ if [ -f "$(which arm-none-eabi-gcc)" ]; then
popd
fi

echo -e "\n\nTest Taproot builds"
pushd lightning
echo -e "\n\nTest cfg-flag builds"
RUSTFLAGS="$RUSTFLAGS --cfg=taproot" cargo test --verbose --color always -p lightning
popd
RUSTFLAGS="$RUSTFLAGS --cfg=async_signing" cargo test --verbose --color always -p lightning
37 changes: 26 additions & 11 deletions lightning/src/ln/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2434,8 +2434,13 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
.ok();

if funding_signed.is_none() {
log_trace!(logger, "Counterparty commitment signature not available for funding_signed message; setting signer_pending_funding");
self.signer_pending_funding = true;
#[cfg(not(async_signing))] {
panic!("Failed to get signature for funding_signed");
}
#[cfg(async_signing)] {
log_trace!(logger, "Counterparty commitment signature not available for funding_signed message; setting signer_pending_funding");
self.signer_pending_funding = true;
}
} else if self.signer_pending_funding {
log_trace!(logger, "Counterparty commitment signature available for funding_signed message; clearing signer_pending_funding");
self.signer_pending_funding = false;
Expand Down Expand Up @@ -4259,7 +4264,7 @@ impl<SP: Deref> Channel<SP> where

/// Indicates that the signer may have some signatures for us, so we should retry if we're
/// blocked.
#[allow(unused)]
#[cfg(async_signing)]
pub fn signer_maybe_unblocked<L: Deref>(&mut self, logger: &L) -> SignerResumeUpdates where L::Target: Logger {
let commitment_update = if self.context.signer_pending_commitment_update {
self.get_last_commitment_update_for_send(logger).ok()
Expand Down Expand Up @@ -4363,11 +4368,16 @@ impl<SP: Deref> Channel<SP> where
}
update
} else {
if !self.context.signer_pending_commitment_update {
log_trace!(logger, "Commitment update awaiting signer: setting signer_pending_commitment_update");
self.context.signer_pending_commitment_update = true;
#[cfg(not(async_signing))] {
panic!("Failed to get signature for new commitment state");
}
#[cfg(async_signing)] {
if !self.context.signer_pending_commitment_update {
log_trace!(logger, "Commitment update awaiting signer: setting signer_pending_commitment_update");
self.context.signer_pending_commitment_update = true;
}
return Err(());
}
return Err(());
};
Ok(msgs::CommitmentUpdate {
update_add_htlcs, update_fulfill_htlcs, update_fail_htlcs, update_fail_malformed_htlcs, update_fee,
Expand Down Expand Up @@ -6448,9 +6458,14 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {

let funding_created = self.get_funding_created_msg(logger);
if funding_created.is_none() {
if !self.context.signer_pending_funding {
log_trace!(logger, "funding_created awaiting signer; setting signer_pending_funding");
self.context.signer_pending_funding = true;
#[cfg(not(async_signing))] {
panic!("Failed to get signature for new funding creation");
}
#[cfg(async_signing)] {
if !self.context.signer_pending_funding {
log_trace!(logger, "funding_created awaiting signer; setting signer_pending_funding");
self.context.signer_pending_funding = true;
}
}
}

Expand Down Expand Up @@ -6796,7 +6811,7 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {

/// Indicates that the signer may have some signatures for us, so we should retry if we're
/// blocked.
#[allow(unused)]
#[cfg(async_signing)]
pub fn signer_maybe_unblocked<L: Deref>(&mut self, logger: &L) -> Option<msgs::FundingCreated> where L::Target: Logger {
if self.context.signer_pending_funding && self.context.is_outbound() {
log_trace!(logger, "Signer unblocked a funding_created");
Expand Down
3 changes: 1 addition & 2 deletions lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7322,8 +7322,7 @@ where
/// attempted in every channel, or in the specifically provided channel.
///
/// [`ChannelSigner`]: crate::sign::ChannelSigner
#[cfg(test)] // This is only implemented for one signer method, and should be private until we
// actually finish implementing it fully.
#[cfg(async_signing)]
pub fn signer_unblocked(&self, channel_opt: Option<(PublicKey, ChannelId)>) {
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);

Expand Down
2 changes: 1 addition & 1 deletion lightning/src/ln/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ mod monitor_tests;
#[cfg(test)]
#[allow(unused_mut)]
mod shutdown_tests;
#[cfg(test)]
#[cfg(all(test, async_signing))]
#[allow(unused_mut)]
mod async_signer_tests;

Expand Down

0 comments on commit a866ba7

Please sign in to comment.