smite: add is_standard_shutdown_script helper - #186
Conversation
f33051b to
22217d9
Compare
NishantBansal2003
left a comment
There was a problem hiding this comment.
Thanks! I was about to add this as a follow-up to #185, but it looks like I don’t have to now
| /// Feature bits that widen the set of standard `shutdown` scriptpubkeys. | ||
| #[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] | ||
| pub struct ShutdownScriptFeatures { | ||
| /// Additionally permits witness program versions 1..=16 with a 2..=40 byte | ||
| /// program. | ||
| pub option_shutdown_anysegwit: bool, | ||
| /// Additionally permits a single-push `OP_RETURN` script. | ||
| pub option_simple_close: bool, | ||
| } |
There was a problem hiding this comment.
I think this can be removed/simplified once #192 gets merged, so we can just use just use: negotiated_features.supports_feature(Features::OPTION_SHUTDOWN_ANYSEGWIT) or negotiated_features.supports_feature(Features::OPTION_SIMPLE_CLOSE)
There was a problem hiding this comment.
Sounds good, I will review #192 and then rebase this PR after merge
|
|
||
| /// Returns `true` if `spk` is a BOLT 2 `option_simple_close` `OP_RETURN` | ||
| /// script: `OP_RETURN` followed by a single data push of 6 to 80 bytes. | ||
| fn is_simple_close_op_return(spk: &[u8]) -> bool { |
There was a problem hiding this comment.
nit: using the bitcoin crate would make this more understandable/consistent. See: https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/src/commit/384e0d61305b24f5c6cdebe59c2c8f89b015d7b0/lightning/src/ln/script.rs#L135
22217d9 to
220b1bf
Compare
BOLT-02 specifies sender requirements for shutdown scripts. They must be witness v0 (P2WPKH, P2WSH) or following features must be negotiated: * `option_shutdown_anysegwit`: witness v1-v16 with a 2..=40 byte program * `option_simple_close`: `OP_RETURN` with a single minimal data push of 6..=80 bytes Receivers may accept legacy scripts (P2PKH, P2SH), but we reject them since we're judging the sender's output. This applies to the `shutdown` and `closing_complete` messages, and the `upfront_shutdown_script` TLV in the `open_channel`, `open_channel2`, `accept_channel` and `accept_channel2` messages. This commit adds a helper to catch targets that don't comply with the spec.
220b1bf to
847c2f1
Compare
From the commit message:
As per the note I added to the code, I'm not sure if the fuzzer should also reject legacy scripts, since a target must not send them.update: decided to reject them, see discussionI haven't wired this into existing code or #163 yet, but I thought the introduction of the helper might be worthwile to review itself, especially considering the question wrt legacy scripts.