← Watch feed
Clarify the commitment validation failure message
Public commit record
100/100 · Strong
What the developer wrote
Authored by auto-pr-bot
Clarify the commitment validation failure message
@matt asked on the diff:
> @auto-pr-bot this example is adding nothing to public docs, please
> remove it.
@matt asked on the diff:
> Since this is now the "primary" error message we send to peers,
> @auto-pr-bot please change this to read "Received commitment sigs failed
> verification". Same for the version in `validate_commitment_signed`.
@matt asked on the diff:
> @auto-pr-bot just for extra belt-and-suspenders, can you add
> `debug_assert!(false);`, `log_error(...)` and a failure return if the
> `channel_transaction_parameters` is missing `counterparty_parameters`?
> Same for the similar code in `validate_commitment_signed`.
Now that no signature checking remains in channel, this error is what we
send our peer whenever the holder commitment they sent us fails to
validate - because their signatures don't verify, but also, e.g., because
the commitment has no outputs at all. Say so, rather than the vaguer
"Failed to validate our commitment", which reads as though the fault were
on our side.
Requested-by: @matt
Link: https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/pulls/4885#issuecomment-444803
Link: https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/pulls/4885#issuecomment-444937
Link: https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/pulls/4885#issuecomment-444942
✓ Descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Explains rationale or failure mode✓ Mentions testing or verification✓ Links an issue, advisory, or supporting reference✓ Names security-relevant behavior explicitly
What changed, and why it matters
This commit only changes the wording of an error message sent to peers when a commitment transaction fails validation. It replaces the vague phrase 'Failed to validate our commitment' with the clearer 'Received commitment failed validation'. No code logic, signature checks, or security behavior is altered. It is a documentation/clarity improvement, not a security fix.
Recommended action
No security action required. Treat as a normal code-review/clarity change.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch updates string literals in channel.rs and several test files from ‘Failed to validate our commitment’ to ‘Received commitment failed validation’. The error is returned via ChannelError::close in two places in channel.rs and is asserted in tests. No functional changes to validation, signature verification, or channel state handling are present.
Changed components
lightning/src/ln/channel.rslightning/src/ln/channel_open_tests.rslightning/src/ln/functional_tests.rslightning/src/ln/splicing_tests.rsInspect captured patch +11 / −11
### lightning/src/ln/channel.rs
@@ -4021,7 +4021,7 @@ trait InitialRemoteCommitmentReceiver<SP: SignerProvider> {
if !self.funding().is_outbound() {
self.funding_mut().channel_transaction_parameters.funding_outpoint = None;
}
- return Err(ChannelError::close("Failed to validate our commitment".to_owned()));
+ return Err(ChannelError::close("Received commitment failed validation".to_owned()));
}
// Now that we're past error-generating stuff, update our local state:
@@ -6110,7 +6110,7 @@ impl<SP: SignerProvider> ChannelContext<SP> {
commitment_data.outbound_htlc_preimages,
&self.secp_ctx,
)
- .map_err(|_| ChannelError::close("Failed to validate our commitment".to_owned()))?;
+ .map_err(|_| ChannelError::close("Received commitment failed validation".to_owned()))?;
Ok((holder_commitment_tx, commitment_data.htlcs_included))
}
### lightning/src/ln/channel_open_tests.rs
@@ -1469,7 +1469,7 @@ pub fn test_duplicate_funding_err_in_funding() {
funding_created_msg.funding_output_index += 10;
nodes[1].node.handle_funding_created(node_c_id, &funding_created_msg);
get_err_msg(&nodes[1], &node_c_id);
- let err = "Failed to validate our commitment".to_owned();
+ let err = "Received commitment failed validation".to_owned();
let reason = ClosureReason::ProcessingError { err };
let expected_closing = ExpectedCloseEvent::from_id_reason(real_channel_id, false, reason);
check_closed_events(&nodes[1], &[expected_closing]);
@@ -2511,9 +2511,9 @@ pub fn test_invalid_funding_signed_signature() {
assert!(nodes[0].tx_broadcaster.txn_broadcast().is_empty());
assert!(nodes[0].node.list_channels().is_empty());
let error_message = get_err_msg(&nodes[0], &node_b_id);
- assert_eq!(error_message.data, "Failed to validate our commitment");
+ assert_eq!(error_message.data, "Received commitment failed validation");
let reason =
- ClosureReason::ProcessingError { err: "Failed to validate our commitment".to_owned() };
+ ClosureReason::ProcessingError { err: "Received commitment failed validation".to_owned() };
check_closed_events(&nodes[0], &[ExpectedCloseEvent::from_id_reason(channel_id, true, reason)]);
}
### lightning/src/ln/functional_tests.rs
@@ -119,9 +119,9 @@ fn do_test_invalid_holder_commitment_signature(corrupt_htlc_signature: bool) {
check_added_monitors(&nodes[1], 1);
let error_messages = check_closed_broadcast(&nodes[1], 1, true);
assert_eq!(error_messages.len(), 1);
- assert_eq!(error_messages[0].data, "Failed to validate our commitment");
+ assert_eq!(error_messages[0].data, "Received commitment failed validation");
let reason =
- ClosureReason::ProcessingError { err: "Failed to validate our commitment".to_owned() };
+ ClosureReason::ProcessingError { err: "Received commitment failed validation".to_owned() };
check_closed_events(
&nodes[1],
&[ExpectedCloseEvent::from_id_reason(channel_id, false, reason)],
### lightning/src/ln/splicing_tests.rs
@@ -5825,7 +5825,7 @@ fn test_splice_buffer_invalid_commitment_signed_closes_channel() {
action: msgs::ErrorAction::SendErrorMessage { ref msg },
..
} => {
- assert_eq!(msg.data, "Failed to validate our commitment");
+ assert_eq!(msg.data, "Received commitment failed validation");
},
_ => panic!("Expected HandleError with SendErrorMessage, got {:?}", msg_events[1]),
}
@@ -5836,7 +5836,7 @@ fn test_splice_buffer_invalid_commitment_signed_closes_channel() {
_ => panic!("Expected BroadcastChannelUpdate, got {:?}", msg_events[2]),
}
- let err = "Failed to validate our commitment".to_owned();
+ let err = "Received commitment failed validation".to_owned();
let reason = ClosureReason::ProcessingError { err };
check_closed_events(
&nodes[0],
@@ -5897,9 +5897,9 @@ fn do_test_splice_batched_invalid_holder_commitment_signature(
check_added_monitors(&nodes[1], 1);
let error_messages = check_closed_broadcast(&nodes[1], 1, true);
assert_eq!(error_messages.len(), 1);
- assert_eq!(error_messages[0].data, "Failed to validate our commitment");
+ assert_eq!(error_messages[0].data, "Received commitment failed validation");
let reason =
- ClosureReason::ProcessingError { err: "Failed to validate our commitment".to_owned() };
+ ClosureReason::ProcessingError { err: "Received commitment failed validation".to_owned() };
check_closed_events(
&nodes[1],
&[ExpectedCloseEvent::from_id_reason(channel_id, false, reason)],Risk score
Our methodology →Why this scored 15/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.