smite-ir: implement RecvFundingSigned operation#123
Conversation
Signed-off-by: Nishant Bansal <nishant.bansal.282003@gmail.com>
| /// The opener cannot afford the feerate for the commitment transaction. | ||
| #[error("opener cannot afford commitment fee for channel_id {0:?}")] | ||
| OpenerCannotAffordFee(ChannelId), |
There was a problem hiding this comment.
According to the spec, a correct target should reject this case immediately after receiving the invalid open_channel. So it seems that this check should really live in the handling of RecvAcceptChannel.
I think it would be a good future PR to implement an open_channel/accept_channel oracle that checks whether the target properly validates the channel parameters according to the spec.
There was a problem hiding this comment.
Yeah, ideally that’s the correct place to perform the check. However, we currently key the state by ChannelId, which is derived from the funding outpoint, and we don’t know the funding outpoint when processing RecvAcceptChannel. Even if we were to verify it manually at that stage, we’d still need funding_satoshis from the corresponding open_channel message to fully validate it. Because of that, we first need a way to map an accept_channel response back to its corresponding open_channel message. The verification in funding_signed is effectively doing that mapping indirectly, which is why I placed the check there.
But yeah, we could definitely move this validation to immediately after RecvAcceptChannel if we maintain that mapping explicitly. I think we can revisit this in the future and keep the current approach for the MVP?
There was a problem hiding this comment.
Sounds good. Maybe we could create a OpenAcceptChannelOracle object that we inform about every open_channel and accept_channel, and the oracle could manage the mapping itself via temporary_channel_id...
morehouse
left a comment
morehouse
left a comment
There was a problem hiding this comment.
Looks good, ready to squash.
Signed-off-by: Nishant Bansal <nishant.bansal.282003@gmail.com>
NishantBansal2003
force-pushed
the
recv-funding-signed
branch
from
8011e67 to
bf4a892
Compare
2 participants
This PR adds
RecvFundingSignedoperation, which receives afunding_signedmessage from the target and verifies that it is expected for the givenchannel_id. It also checks that afunding_signedmessage is not received when the opener cannot afford the feerate. Finally, it verifies that the commitment signature sent by the remote party is valid for the local commitment transaction.I also took this PR as an opportunity to address the comment in: #97 (comment), since otherwise there would be an inconsistency between the
Displayimplementations ofRecvFundingSignedandRecvAcceptChannel