Enforce that `ChanelSigner::pubkeys` is only called once
What changed, and why it matters
This commit only changes a test helper used during development. It adds an internal guard so that a mock signer in the test suite panics if its public keys are fetched more than once. It does not change production code, network behavior, or wallet security. There is no indication this fixes a real vulnerability.
No action required. Treat as a routine test-hardening change. If reviewing a release that includes this commit, verify separately whether the preceding `ChannelSigner::pubkeys` API change has any production implications, but this commit itself is not a security patch.
Security signals we found
No production code changed
Test-only file under `src/util/test_channel_signer.rs`
Commit message describes a design/test invariant, not a vulnerability
No CVE, advisory, or vendor security disclosure referenced
No externally reported incident or researcher attribution present
Evidence from the diff
The patch modifies lightning/src/util/test_channel_signer.rs, a test-only TestChannelSigner. It removes the derived Clone, implements a manual Clone that asserts have_fetched_pubkeys is false, and adds an atomic flag checked in ChannelSigner::pubkeys so the test signer panics on a second call. The commit message frames this as enforcing a design invariant for ChannelSigner::pubkeys callers, not as a security fix.
Changed components
lightning/src/util/test_channel_signer.rsInspect captured patch +30 / −2
diff --git a/lightning/src/util/test_channel_signer.rs b/lightning/src/util/test_channel_signer.rs
index 652c1ff..bad00f6 100644
--- a/lightning/src/util/test_channel_signer.rs
+++ b/lightning/src/util/test_channel_signer.rs
@@ -24,7 +24,9 @@ use crate::prelude::*;
#[cfg(any(test, feature = "_test_utils"))]
use crate::sync::MutexGuard;
use crate::sync::{Arc, Mutex};
+
use core::cmp;
+use core::sync::atomic::{AtomicBool, Ordering};
use bitcoin::hashes::Hash;
use bitcoin::sighash;
@@ -68,13 +70,31 @@ pub const INITIAL_REVOKED_COMMITMENT_NUMBER: u64 = 1 << 48;
///
/// Note that before we do so we should ensure its serialization format has backwards- and
/// forwards-compatibility prefix/suffixes!
-#[derive(Clone)]
pub struct TestChannelSigner {
pub inner: DynSigner,
/// Channel state used for policy enforcement
pub state: Arc<Mutex<EnforcementState>>,
pub disable_revocation_policy_check: bool,
pub disable_all_state_policy_checks: bool,
+ have_fetched_pubkeys: AtomicBool,
+}
+
+impl Clone for TestChannelSigner {
+ fn clone(&self) -> Self {
+ // Generally, a signer should only ever be cloned when a ChannelMonitor is cloned (which
+ // doesn't fetch the pubkeys at all). This isn't really a critical test, but if it
+ // it ever does fail we should make sure the clone is hapening in a sensible place.
+ assert!(!self.have_fetched_pubkeys.load(Ordering::Acquire));
+ Self {
+ inner: self.inner.clone(),
+ state: Arc::clone(&self.state),
+ disable_revocation_policy_check: self.disable_revocation_policy_check,
+ disable_all_state_policy_checks: self.disable_all_state_policy_checks,
+ // In some tests we clone a `ChannelMonitor` multiple times, so have to initialize with
+ // `!have_fetched_pubkeys` to ensure the above assertion passes.
+ have_fetched_pubkeys: AtomicBool::new(false),
+ }
+ }
}
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
@@ -129,6 +149,7 @@ impl TestChannelSigner {
state,
disable_revocation_policy_check: false,
disable_all_state_policy_checks: false,
+ have_fetched_pubkeys: AtomicBool::new(false),
}
}
@@ -141,7 +162,13 @@ impl TestChannelSigner {
inner: DynSigner, state: Arc<Mutex<EnforcementState>>,
disable_revocation_policy_check: bool, disable_all_state_policy_checks: bool,
) -> Self {
- Self { inner, state, disable_revocation_policy_check, disable_all_state_policy_checks }
+ Self {
+ inner,
+ state,
+ disable_revocation_policy_check,
+ disable_all_state_policy_checks,
+ have_fetched_pubkeys: AtomicBool::new(false),
+ }
}
#[cfg(any(test, feature = "_test_utils"))]
@@ -222,6 +249,7 @@ impl ChannelSigner for TestChannelSigner {
}
fn pubkeys(&self, secp_ctx: &Secp256k1<secp256k1::All>) -> ChannelPublicKeys {
+ assert!(!self.have_fetched_pubkeys.swap(true, Ordering::AcqRel));
self.inner.pubkeys(secp_ctx)
}
Why this scored 17/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.