Add bolt 2 spec checks for funding messages #190
Conversation
| @@ -1300,6 +1300,9 @@ impl Channel { | |||
| if self.channel_outbound { | |||
| return Err(HandleError{err: "Received funding_created for an outbound channel?", action: Some(msgs::ErrorAction::SendErrorMessage {msg: msgs::ErrorMessage {channel_id: self.channel_id, data: "Received funding_created for an outbound channel?".to_string()}})}); | |||
| } | |||
| if self.channel_id != msg.temporary_channel_id { | |||
There was a problem hiding this comment.
The method is called on self b/c this condition is true. So this if expression will never be false.
There was a problem hiding this comment.
I potentially have this wrong, but isnt msg the incoming message from the remote peer? Which means h=it could have it wrong
There was a problem hiding this comment.
channelmanager maintains a mapping from channel_id to channel struct, so if an unknown temporary_channel_id is received, it will be disregarded instead of this method being invoked.
There was a problem hiding this comment.
awesome, cwl then we dont have to check these.
| @@ -1352,6 +1355,9 @@ impl Channel { | |||
| self.cur_local_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER { | |||
| panic!("Should not have advanced channel commitment tx numbers prior to funding_created"); | |||
| } | |||
| if self.channel_monitor.funding_txo.clone().unwrap().0.to_channel_id() != msg.channel_id { | |||
| return Err(HandleError{err: "Received incorrect channel_id in funding_signed message", action: None}); | |||
There was a problem hiding this comment.
Given how things are structured it'd be very hard for someone to introduce a bug that will make this condition true, but if that ever happens it should be hard panic, not returning Err.
There was a problem hiding this comment.
I agree that it is very likely that it col be caused by a bug. But the remote could accidentally or intentionally send the incorrect id.
There was a problem hiding this comment.
ChannelManager handles an incorrect channel_id and channel_id collision.
There was a problem hiding this comment.
awesome, cwl then we dont have to check these.
|
closing this, as this only checks the ID, which is checked before this by the channel_mananger making this redundant. |
I added additional checks for the funding_created, funding_signed and funding_locked messages.
This is to ensure 100% coverage of BOLT2 spec. I added tests for the channel id, to ensure the received channel id is correct.
This is in relation to #129