Skip to content

Commit cf74c5b

Browse files
committed
Optionally disable all state-based policy checks in test signer
The signer we use in tests tracks the state of the channel and refuses to sign when the channel attempts an invalid state transition. In the next commit, however, we'll add an upgrade test which will fail these checks as the the state won't get copied from previous versions of LDK to this version. Thus, here, we add the ability to disable all state-based checks in the signer.
1 parent aa0cb5c commit cf74c5b

File tree

4 files changed

+59
-42
lines changed

4 files changed

+59
-42
lines changed

fuzz/src/chanmon_consistency.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,7 @@ impl SignerProvider for KeyProvider {
377377
);
378378
let revoked_commitment = self.make_enforcement_state_cell(keys.commitment_seed);
379379
let keys = DynSigner::new(keys);
380-
TestChannelSigner::new_with_revoked(keys, revoked_commitment, false)
380+
TestChannelSigner::new_with_revoked(keys, revoked_commitment, false, false)
381381
}
382382

383383
fn get_destination_script(&self, _channel_keys_id: [u8; 32]) -> Result<ScriptBuf, ()> {

fuzz/src/full_stack.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -457,7 +457,7 @@ impl SignerProvider for KeyProvider {
457457
f = key;
458458
let signer = InMemorySigner::new(&secp_ctx, a, b, c, d, e, f, keys_id, keys_id);
459459

460-
TestChannelSigner::new_with_revoked(DynSigner::new(signer), state, false)
460+
TestChannelSigner::new_with_revoked(DynSigner::new(signer), state, false, false)
461461
}
462462

463463
fn get_destination_script(&self, _channel_keys_id: [u8; 32]) -> Result<ScriptBuf, ()> {

lightning/src/util/test_channel_signer.rs

Lines changed: 52 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ pub struct TestChannelSigner {
7373
/// Channel state used for policy enforcement
7474
pub state: Arc<Mutex<EnforcementState>>,
7575
pub disable_revocation_policy_check: bool,
76+
pub disable_all_state_policy_checks: bool,
7677
}
7778

7879
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
@@ -122,7 +123,12 @@ impl TestChannelSigner {
122123
/// Construct an TestChannelSigner
123124
pub fn new(inner: DynSigner) -> Self {
124125
let state = Arc::new(Mutex::new(EnforcementState::new()));
125-
Self { inner, state, disable_revocation_policy_check: false }
126+
Self {
127+
inner,
128+
state,
129+
disable_revocation_policy_check: false,
130+
disable_all_state_policy_checks: false,
131+
}
126132
}
127133

128134
/// Construct an TestChannelSigner with externally managed storage
@@ -132,9 +138,9 @@ impl TestChannelSigner {
132138
/// here, usually by an implementation of KeysInterface.
133139
pub fn new_with_revoked(
134140
inner: DynSigner, state: Arc<Mutex<EnforcementState>>,
135-
disable_revocation_policy_check: bool,
141+
disable_revocation_policy_check: bool, disable_all_state_policy_checks: bool,
136142
) -> Self {
137-
Self { inner, state, disable_revocation_policy_check }
143+
Self { inner, state, disable_revocation_policy_check, disable_all_state_policy_checks }
138144
}
139145

140146
#[cfg(any(test, feature = "_test_utils"))]
@@ -174,12 +180,12 @@ impl ChannelSigner for TestChannelSigner {
174180
if !self.is_signer_available(SignerOp::ReleaseCommitmentSecret) {
175181
return Err(());
176182
}
177-
{
178-
let mut state = self.state.lock().unwrap();
183+
let mut state = self.state.lock().unwrap();
184+
if !self.disable_all_state_policy_checks {
179185
assert!(idx == state.last_holder_revoked_commitment || idx == state.last_holder_revoked_commitment - 1, "can only revoke the current or next unrevoked commitment - trying {}, last revoked {}", idx, state.last_holder_revoked_commitment);
180186
assert!(idx > state.last_holder_commitment, "cannot revoke the last holder commitment - attempted to revoke {} last commitment {}", idx, state.last_holder_commitment);
181-
state.last_holder_revoked_commitment = idx;
182187
}
188+
state.last_holder_revoked_commitment = idx;
183189
self.inner.release_commitment_secret(idx)
184190
}
185191

@@ -189,12 +195,14 @@ impl ChannelSigner for TestChannelSigner {
189195
) -> Result<(), ()> {
190196
let mut state = self.state.lock().unwrap();
191197
let idx = holder_tx.commitment_number();
192-
assert!(
193-
idx == state.last_holder_commitment || idx == state.last_holder_commitment - 1,
194-
"expecting to validate the current or next holder commitment - trying {}, current {}",
195-
idx,
196-
state.last_holder_commitment
197-
);
198+
if !self.disable_all_state_policy_checks {
199+
assert!(
200+
idx == state.last_holder_commitment || idx == state.last_holder_commitment - 1,
201+
"expecting to validate the current or next holder commitment - trying {}, current {}",
202+
idx,
203+
state.last_holder_commitment
204+
);
205+
}
198206
state.last_holder_commitment = idx;
199207
Ok(())
200208
}
@@ -205,7 +213,9 @@ impl ChannelSigner for TestChannelSigner {
205213
return Err(());
206214
}
207215
let mut state = self.state.lock().unwrap();
208-
assert!(idx == state.last_counterparty_revoked_commitment || idx == state.last_counterparty_revoked_commitment - 1, "expecting to validate the current or next counterparty revocation - trying {}, current {}", idx, state.last_counterparty_revoked_commitment);
216+
if !self.disable_all_state_policy_checks {
217+
assert!(idx == state.last_counterparty_revoked_commitment || idx == state.last_counterparty_revoked_commitment - 1, "expecting to validate the current or next counterparty revocation - trying {}, current {}", idx, state.last_counterparty_revoked_commitment);
218+
}
209219
state.last_counterparty_revoked_commitment = idx;
210220
Ok(())
211221
}
@@ -227,14 +237,14 @@ impl EcdsaChannelSigner for TestChannelSigner {
227237
) -> Result<(Signature, Vec<Signature>), ()> {
228238
self.verify_counterparty_commitment_tx(channel_parameters, commitment_tx, secp_ctx);
229239

230-
{
231-
#[cfg(test)]
232-
if !self.is_signer_available(SignerOp::SignCounterpartyCommitment) {
233-
return Err(());
234-
}
235-
let mut state = self.state.lock().unwrap();
236-
let actual_commitment_number = commitment_tx.commitment_number();
237-
let last_commitment_number = state.last_counterparty_commitment;
240+
#[cfg(test)]
241+
if !self.is_signer_available(SignerOp::SignCounterpartyCommitment) {
242+
return Err(());
243+
}
244+
let mut state = self.state.lock().unwrap();
245+
let actual_commitment_number = commitment_tx.commitment_number();
246+
let last_commitment_number = state.last_counterparty_commitment;
247+
if !self.disable_all_state_policy_checks {
238248
// These commitment numbers are backwards counting. We expect either the same as the previously encountered,
239249
// or the next one.
240250
assert!(
@@ -252,9 +262,9 @@ impl EcdsaChannelSigner for TestChannelSigner {
252262
actual_commitment_number,
253263
state.last_counterparty_revoked_commitment
254264
);
255-
state.last_counterparty_commitment =
256-
cmp::min(last_commitment_number, actual_commitment_number)
257265
}
266+
state.last_counterparty_commitment =
267+
cmp::min(last_commitment_number, actual_commitment_number);
258268

259269
Ok(self
260270
.inner
@@ -278,14 +288,16 @@ impl EcdsaChannelSigner for TestChannelSigner {
278288
}
279289
let trusted_tx =
280290
self.verify_holder_commitment_tx(channel_parameters, commitment_tx, secp_ctx);
281-
let state = self.state.lock().unwrap();
282-
let commitment_number = trusted_tx.commitment_number();
283-
if state.last_holder_revoked_commitment - 1 != commitment_number
284-
&& state.last_holder_revoked_commitment - 2 != commitment_number
285-
{
286-
if !self.disable_revocation_policy_check {
287-
panic!("can only sign the next two unrevoked commitment numbers, revoked={} vs requested={} for {}",
288-
state.last_holder_revoked_commitment, commitment_number, self.inner.channel_keys_id()[0])
291+
if !self.disable_all_state_policy_checks {
292+
let state = self.state.lock().unwrap();
293+
let commitment_number = trusted_tx.commitment_number();
294+
if state.last_holder_revoked_commitment - 1 != commitment_number
295+
&& state.last_holder_revoked_commitment - 2 != commitment_number
296+
{
297+
if !self.disable_revocation_policy_check {
298+
panic!("can only sign the next two unrevoked commitment numbers, revoked={} vs requested={} for {}",
299+
state.last_holder_revoked_commitment, commitment_number, self.inner.channel_keys_id()[0])
300+
}
289301
}
290302
}
291303
Ok(self.inner.sign_holder_commitment(channel_parameters, commitment_tx, secp_ctx).unwrap())
@@ -353,13 +365,15 @@ impl EcdsaChannelSigner for TestChannelSigner {
353365
if !self.is_signer_available(SignerOp::SignHolderHtlcTransaction) {
354366
return Err(());
355367
}
356-
let state = self.state.lock().unwrap();
357-
if state.last_holder_revoked_commitment - 1 != htlc_descriptor.per_commitment_number
358-
&& state.last_holder_revoked_commitment - 2 != htlc_descriptor.per_commitment_number
359-
{
360-
if !self.disable_revocation_policy_check {
361-
panic!("can only sign the next two unrevoked commitment numbers, revoked={} vs requested={} for {}",
362-
state.last_holder_revoked_commitment, htlc_descriptor.per_commitment_number, self.inner.channel_keys_id()[0])
368+
if !self.disable_all_state_policy_checks {
369+
let state = self.state.lock().unwrap();
370+
if state.last_holder_revoked_commitment - 1 != htlc_descriptor.per_commitment_number
371+
&& state.last_holder_revoked_commitment - 2 != htlc_descriptor.per_commitment_number
372+
{
373+
if !self.disable_revocation_policy_check {
374+
panic!("can only sign the next two unrevoked commitment numbers, revoked={} vs requested={} for {}",
375+
state.last_holder_revoked_commitment, htlc_descriptor.per_commitment_number, self.inner.channel_keys_id()[0])
376+
}
363377
}
364378
}
365379
assert_eq!(htlc_tx.input[input], htlc_descriptor.unsigned_tx_input());

lightning/src/util/test_utils.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1492,6 +1492,7 @@ pub struct TestKeysInterface {
14921492
pub backing: DynKeysInterface,
14931493
pub override_random_bytes: Mutex<Option<[u8; 32]>>,
14941494
pub disable_revocation_policy_check: bool,
1495+
pub disable_all_state_policy_checks: bool,
14951496
enforcement_states: Mutex<HashMap<[u8; 32], Arc<Mutex<EnforcementState>>>>,
14961497
expectations: Mutex<Option<VecDeque<OnGetShutdownScriptpubkey>>>,
14971498
pub unavailable_signers_ops: Mutex<HashMap<[u8; 32], HashSet<SignerOp>>>,
@@ -1552,8 +1553,9 @@ impl SignerProvider for TestKeysInterface {
15521553
fn derive_channel_signer(&self, channel_keys_id: [u8; 32]) -> TestChannelSigner {
15531554
let keys = self.backing.derive_channel_signer(channel_keys_id);
15541555
let state = self.make_enforcement_state_cell(keys.channel_keys_id());
1555-
let signer =
1556-
TestChannelSigner::new_with_revoked(keys, state, self.disable_revocation_policy_check);
1556+
let rev_checks = self.disable_revocation_policy_check;
1557+
let state_checks = self.disable_all_state_policy_checks;
1558+
let signer = TestChannelSigner::new_with_revoked(keys, state, rev_checks, state_checks);
15571559
#[cfg(test)]
15581560
if let Some(ops) = self.unavailable_signers_ops.lock().unwrap().get(&channel_keys_id) {
15591561
for &op in ops {
@@ -1621,6 +1623,7 @@ impl TestKeysInterface {
16211623
backing: DynKeysInterface::new(backing),
16221624
override_random_bytes: Mutex::new(None),
16231625
disable_revocation_policy_check: false,
1626+
disable_all_state_policy_checks: false,
16241627
enforcement_states: Mutex::new(new_hash_map()),
16251628
expectations: Mutex::new(None),
16261629
unavailable_signers_ops: Mutex::new(new_hash_map()),

0 commit comments

Comments
 (0)