-
Notifications
You must be signed in to change notification settings - Fork 21
smite: add is_standard_shutdown_script helper #186
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
ekzyis
wants to merge
1
commit into
lnfuzz:master
Choose a base branch
from
ekzyis:is-standard-shutdown-script
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -1,6 +1,11 @@ | ||||||||
| //! BOLT 2 shutdown message. | ||||||||
|
|
||||||||
| use bitcoin::opcodes::all::OP_RETURN; | ||||||||
| use bitcoin::script::Instruction; | ||||||||
| use bitcoin::{Script, WitnessVersion}; | ||||||||
|
|
||||||||
| use super::BoltError; | ||||||||
| use super::Features; | ||||||||
| use super::types::ChannelId; | ||||||||
| use super::wire::WireFormat; | ||||||||
|
|
||||||||
|
|
@@ -51,11 +56,78 @@ impl Shutdown { | |||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| /// Returns `true` if `spk` is a standard shutdown scriptpubkey per BOLT 2: P2WPKH or P2WSH. | ||||||||
| /// Negotiated `features` widen the accepted set. | ||||||||
| /// | ||||||||
| /// Legacy P2PKH/P2SH are rejected. A receiver may accept them for backward compatibility, but this | ||||||||
| /// oracle judges the sender's output. | ||||||||
| #[must_use] | ||||||||
| pub fn is_standard_shutdown_script(spk: &[u8], features: &Features) -> bool { | ||||||||
| let script = Script::from_bytes(spk); | ||||||||
| let witness_v0 = script.is_p2wpkh() || script.is_p2wsh(); | ||||||||
| let anysegwit = features.supports_feature(Features::OPTION_SHUTDOWN_ANYSEGWIT) | ||||||||
| && matches!(script.witness_version(), Some(v) if v != WitnessVersion::V0); | ||||||||
| let simple_close = features.supports_feature(Features::OPTION_SIMPLE_CLOSE) | ||||||||
| && is_simple_close_op_return(script); | ||||||||
| witness_v0 || anysegwit || simple_close | ||||||||
| } | ||||||||
|
|
||||||||
| /// Returns `true` if `spk` is a shutdown scriptpubkey a receiver may accept per | ||||||||
| /// BOLT 2. | ||||||||
| /// | ||||||||
| /// This includes the standard shutdown scriptpubkey forms, as well as legacy | ||||||||
| /// P2PKH/P2SH outputs accepted for backward compatibility. | ||||||||
| #[must_use] | ||||||||
| pub fn is_acceptable_shutdown_script(spk: &[u8], features: &Features) -> bool { | ||||||||
| let script = Script::from_bytes(spk); | ||||||||
| is_standard_shutdown_script(spk, features) || script.is_p2pkh() || script.is_p2sh() | ||||||||
| } | ||||||||
|
Comment on lines
+81
to
+84
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should add tests for |
||||||||
|
|
||||||||
| /// Returns `true` if `script` is a BOLT 2 `option_simple_close` `OP_RETURN` script: `OP_RETURN` | ||||||||
| /// followed by a single minimal push of 6..=80 bytes. | ||||||||
| /// | ||||||||
| /// A non-minimal push here would be `OP_PUSHDATA1` used for a payload of fewer than 76 bytes. | ||||||||
| fn is_simple_close_op_return(script: &Script) -> bool { | ||||||||
| let mut instrs = script.instructions_minimal(); | ||||||||
| if !matches!(instrs.next(), Some(Ok(Instruction::Op(op))) if op == OP_RETURN) { | ||||||||
| return false; | ||||||||
| } | ||||||||
| match instrs.next() { | ||||||||
| // matches any minimal push | ||||||||
| Some(Ok(Instruction::PushBytes(bytes))) => { | ||||||||
| (6..=80).contains(&bytes.len()) && instrs.next().is_none() | ||||||||
| } | ||||||||
| _ => false, | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| #[cfg(test)] | ||||||||
| mod tests { | ||||||||
| use bitcoin::opcodes::all::{ | ||||||||
| OP_CHECKSIG, OP_DUP, OP_EQUAL, OP_EQUALVERIFY, OP_HASH160, OP_PUSHBYTES_0, OP_PUSHBYTES_1, | ||||||||
| OP_PUSHBYTES_20, OP_PUSHBYTES_21, OP_PUSHBYTES_32, OP_PUSHDATA1, OP_PUSHNUM_1, | ||||||||
| OP_PUSHNUM_16, OP_RETURN, | ||||||||
| }; | ||||||||
|
|
||||||||
| use super::super::CHANNEL_ID_SIZE; | ||||||||
| use super::*; | ||||||||
|
|
||||||||
| fn no_features() -> Features { | ||||||||
| Features::new() | ||||||||
| } | ||||||||
| fn with_anysegwit() -> Features { | ||||||||
| Features::from_bits(&[Features::OPTION_SHUTDOWN_ANYSEGWIT]) | ||||||||
| } | ||||||||
| fn with_simple_close() -> Features { | ||||||||
| Features::from_bits(&[Features::OPTION_SIMPLE_CLOSE]) | ||||||||
| } | ||||||||
| fn all_shutdown_features() -> Features { | ||||||||
| Features::from_bits(&[ | ||||||||
| Features::OPTION_SHUTDOWN_ANYSEGWIT, | ||||||||
| Features::OPTION_SIMPLE_CLOSE, | ||||||||
| ]) | ||||||||
| } | ||||||||
|
|
||||||||
| #[test] | ||||||||
| fn shutdown_for_channel() { | ||||||||
| let channel_id = ChannelId::new([0x42; CHANNEL_ID_SIZE]); | ||||||||
|
|
@@ -151,4 +223,191 @@ mod tests { | |||||||
| assert_eq!(original, decoded); | ||||||||
| assert!(decoded.scriptpubkey.is_empty()); | ||||||||
| } | ||||||||
|
|
||||||||
| #[test] | ||||||||
| fn is_standard_shutdown_script_accepts_witness_v0() { | ||||||||
| // Witness v0 forms are accepted regardless of features. | ||||||||
| let v0 = OP_PUSHBYTES_0.to_u8(); | ||||||||
| let mut p2wpkh = vec![v0, OP_PUSHBYTES_20.to_u8()]; | ||||||||
| p2wpkh.extend_from_slice(&[0x33; 20]); | ||||||||
| assert!(is_standard_shutdown_script(&p2wpkh, &no_features())); | ||||||||
| assert!(is_standard_shutdown_script( | ||||||||
| &p2wpkh, | ||||||||
| &all_shutdown_features() | ||||||||
| )); | ||||||||
|
|
||||||||
| let mut p2wsh = vec![v0, OP_PUSHBYTES_32.to_u8()]; | ||||||||
| p2wsh.extend_from_slice(&[0x44; 32]); | ||||||||
| assert!(is_standard_shutdown_script(&p2wsh, &no_features())); | ||||||||
| assert!(is_standard_shutdown_script( | ||||||||
| &p2wsh, | ||||||||
| &all_shutdown_features() | ||||||||
| )); | ||||||||
| } | ||||||||
|
|
||||||||
| #[test] | ||||||||
| fn is_standard_shutdown_script_accepts_anysegwit() { | ||||||||
|
ekzyis marked this conversation as resolved.
|
||||||||
| // BOLT 2 anysegwit: witness versions 1..=16 with a 2..=40 byte program. | ||||||||
| for v in OP_PUSHNUM_1.to_u8()..=OP_PUSHNUM_16.to_u8() { | ||||||||
| for len in [2u8, 20, 40] { | ||||||||
| // For 1..=75, the push opcode byte equals the pushed length. | ||||||||
| let mut anysegwit = vec![v, len]; | ||||||||
| anysegwit.extend_from_slice(&vec![0x00; usize::from(len)]); | ||||||||
| // Gated on option_shutdown_anysegwit; option_simple_close alone doesn't help. | ||||||||
| assert!(!is_standard_shutdown_script(&anysegwit, &no_features())); | ||||||||
| assert!(!is_standard_shutdown_script( | ||||||||
| &anysegwit, | ||||||||
| &with_simple_close() | ||||||||
| )); | ||||||||
| assert!(is_standard_shutdown_script(&anysegwit, &with_anysegwit())); | ||||||||
| assert!(is_standard_shutdown_script( | ||||||||
| &anysegwit, | ||||||||
| &all_shutdown_features() | ||||||||
| )); | ||||||||
| } | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| #[test] | ||||||||
| fn is_standard_shutdown_script_accepts_simple_close_op_return() { | ||||||||
| // OP_RETURN + a single direct push of 6..=75 bytes. | ||||||||
| for len in [6u8, 40, 75] { | ||||||||
| let mut spk = vec![OP_RETURN.to_u8(), len]; | ||||||||
| spk.extend_from_slice(&vec![0xab; usize::from(len)]); | ||||||||
| // Gated on option_simple_close; option_shutdown_anysegwit alone doesn't help. | ||||||||
| assert!(!is_standard_shutdown_script(&spk, &no_features())); | ||||||||
| assert!(!is_standard_shutdown_script(&spk, &with_anysegwit())); | ||||||||
| assert!(is_standard_shutdown_script(&spk, &with_simple_close())); | ||||||||
| assert!(is_standard_shutdown_script(&spk, &all_shutdown_features())); | ||||||||
| } | ||||||||
|
|
||||||||
| // OP_RETURN + OP_PUSHDATA1 + a single push of 76..=80 bytes. | ||||||||
| for len in [76u8, 80] { | ||||||||
| let mut spk = vec![OP_RETURN.to_u8(), OP_PUSHDATA1.to_u8(), len]; | ||||||||
| spk.extend_from_slice(&vec![0xab; usize::from(len)]); | ||||||||
| assert!(!is_standard_shutdown_script(&spk, &no_features())); | ||||||||
| assert!(is_standard_shutdown_script(&spk, &with_simple_close())); | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| #[test] | ||||||||
| fn is_standard_shutdown_script_rejects_legacy() { | ||||||||
| // Legacy P2PKH/P2SH are non-standard even with all features negotiated. | ||||||||
| // | ||||||||
| // TODO: Oracle verification depends on if we're verifying a target's message or our own | ||||||||
| // message. Legacy scripts MAY be accepted by receivers, but MUST NOT be sent. | ||||||||
|
Comment on lines
+296
to
+298
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| let mut p2pkh = vec![OP_DUP.to_u8(), OP_HASH160.to_u8(), OP_PUSHBYTES_20.to_u8()]; | ||||||||
| p2pkh.extend_from_slice(&[0x11; 20]); | ||||||||
| p2pkh.extend_from_slice(&[OP_EQUALVERIFY.to_u8(), OP_CHECKSIG.to_u8()]); | ||||||||
| assert!(!is_standard_shutdown_script( | ||||||||
| &p2pkh, | ||||||||
| &all_shutdown_features() | ||||||||
| )); | ||||||||
|
|
||||||||
| let mut p2sh = vec![OP_HASH160.to_u8(), OP_PUSHBYTES_20.to_u8()]; | ||||||||
| p2sh.extend_from_slice(&[0x22; 20]); | ||||||||
| p2sh.push(OP_EQUAL.to_u8()); | ||||||||
| assert!(!is_standard_shutdown_script( | ||||||||
| &p2sh, | ||||||||
| &all_shutdown_features() | ||||||||
| )); | ||||||||
| } | ||||||||
|
|
||||||||
| #[test] | ||||||||
| fn is_standard_shutdown_script_rejects_invalid_segwit() { | ||||||||
| // Witness version 0 with a non-{20,32} program length | ||||||||
| let v0 = OP_PUSHBYTES_0.to_u8(); | ||||||||
| let mut witness_v0_invalid_prog_length_spk = vec![v0, OP_PUSHBYTES_21.to_u8()]; | ||||||||
| witness_v0_invalid_prog_length_spk.extend_from_slice(&[0x00; 21]); | ||||||||
| assert!(!is_standard_shutdown_script( | ||||||||
| &witness_v0_invalid_prog_length_spk, | ||||||||
| &all_shutdown_features() | ||||||||
| )); | ||||||||
|
|
||||||||
| // Witness version 1 with a program length just outside 2..=40 | ||||||||
| for len in [1u8, 41] { | ||||||||
| let mut witness_v1_invalid_prog_length_spk = vec![OP_PUSHNUM_1.to_u8(), len]; | ||||||||
| witness_v1_invalid_prog_length_spk.extend_from_slice(&vec![0x00; usize::from(len)]); | ||||||||
| assert!(!is_standard_shutdown_script( | ||||||||
| &witness_v1_invalid_prog_length_spk, | ||||||||
| &all_shutdown_features() | ||||||||
| )); | ||||||||
| } | ||||||||
|
|
||||||||
| // Length prefix disagrees with the actual program length | ||||||||
| let mut witness_v0_invalid_length_prefix_spk = vec![v0, OP_PUSHBYTES_20.to_u8()]; | ||||||||
| witness_v0_invalid_length_prefix_spk.extend_from_slice(&[0x00; 19]); | ||||||||
| assert!(!is_standard_shutdown_script( | ||||||||
| &witness_v0_invalid_length_prefix_spk, | ||||||||
| &all_shutdown_features() | ||||||||
| )); | ||||||||
| } | ||||||||
|
|
||||||||
| #[test] | ||||||||
| fn is_standard_shutdown_script_rejects_invalid_simple_close_op_return() { | ||||||||
| // Rejected even with option_simple_close negotiated. | ||||||||
|
|
||||||||
| // Bare OP_RETURN with no push. | ||||||||
| assert!(!is_standard_shutdown_script( | ||||||||
| &[OP_RETURN.to_u8()], | ||||||||
| &all_shutdown_features() | ||||||||
| )); | ||||||||
|
|
||||||||
| // Direct push below the 6-byte minimum. | ||||||||
| let mut too_short = vec![OP_RETURN.to_u8(), 5]; | ||||||||
| too_short.extend_from_slice(&[0xab; 5]); | ||||||||
| assert!(!is_standard_shutdown_script( | ||||||||
| &too_short, | ||||||||
| &all_shutdown_features() | ||||||||
| )); | ||||||||
|
|
||||||||
| // Push length disagrees with the trailing data (claims 6, has 5). | ||||||||
| let mut len_mismatch = vec![OP_RETURN.to_u8(), 6]; | ||||||||
| len_mismatch.extend_from_slice(&[0xab; 5]); | ||||||||
| assert!(!is_standard_shutdown_script( | ||||||||
| &len_mismatch, | ||||||||
| &all_shutdown_features() | ||||||||
| )); | ||||||||
|
|
||||||||
| // Extra bytes after the single push (multiple pushes not allowed). | ||||||||
| let mut trailing = vec![OP_RETURN.to_u8(), 6]; | ||||||||
| trailing.extend_from_slice(&[0xab; 6]); | ||||||||
| trailing.extend_from_slice(&[OP_PUSHBYTES_1.to_u8(), 0xff]); | ||||||||
| assert!(!is_standard_shutdown_script( | ||||||||
| &trailing, | ||||||||
| &all_shutdown_features() | ||||||||
| )); | ||||||||
|
|
||||||||
| // OP_PUSHDATA1 with a length above the 80-byte maximum. | ||||||||
| let mut too_long = vec![OP_RETURN.to_u8(), OP_PUSHDATA1.to_u8(), 81]; | ||||||||
| too_long.extend_from_slice(&[0xab; 81]); | ||||||||
| assert!(!is_standard_shutdown_script( | ||||||||
| &too_long, | ||||||||
| &all_shutdown_features() | ||||||||
| )); | ||||||||
|
|
||||||||
| // Non-minimal push: OP_PUSHDATA1 used for less than 76 bytes. | ||||||||
| let mut non_minimal = vec![OP_RETURN.to_u8(), OP_PUSHDATA1.to_u8(), 75]; | ||||||||
| non_minimal.extend_from_slice(&[0xab; 75]); | ||||||||
| assert!(!is_standard_shutdown_script( | ||||||||
| &non_minimal, | ||||||||
| &all_shutdown_features() | ||||||||
| )); | ||||||||
| } | ||||||||
|
|
||||||||
| #[test] | ||||||||
| fn is_standard_shutdown_script_rejects_other() { | ||||||||
| // Malformed scripts that are always rejected. | ||||||||
| let empty_spk = vec![]; | ||||||||
| assert!(!is_standard_shutdown_script( | ||||||||
| &empty_spk, | ||||||||
| &all_shutdown_features() | ||||||||
| )); | ||||||||
|
|
||||||||
| let random_spk = vec![0x00, 0x01, 0x02]; | ||||||||
| assert!(!is_standard_shutdown_script( | ||||||||
| &random_spk, | ||||||||
| &all_shutdown_features() | ||||||||
| )); | ||||||||
| } | ||||||||
| } | ||||||||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the oracle, we verify what the peer accepted (and whether it was valid), and then we check our side to ensure that the target is actually sending that.
So, when verifying a target-accepted
open_channel,I need it to pass even if it contains the legacy script. However, for our receivedaccept_channel, I need to ensure that it does not contain legacy scriptsThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't address this yet in 63fed11. I think we want to handle this in a similar way to
enum Side.