smite-ir: generate compliant shutdown scripts

[morehouse]

Helps the fuzzer choose BOLT 2 compliant shutdown scripts so that deeper paths can be reached in the open_channel->accept_channel flow.

This change in combination with #57 are required to get past open_channel message validation in LDK.

Ref: #5 (Milestone 1)

[NishantBansal2003]

LGTM 👍

[NishantBansal2003]

I don't see legacy P2PKH or P2SH mentioned in BOLT 2 (https://github.com/lightning/bolts/blob/master/02-peer-protocol.md#requirements-31)

However, from the code, I can see that CLN, LDK, and Eclair accept them, but LND does not (is it fine to have it like this?)

[ekzyis]

bolt02 mentions this:

The scriptpubkey forms include only standard segwit forms accepted by the Bitcoin network, which ensures the resulting transaction will propagate to miners. However old nodes may send non-segwit scripts, which may be accepted for backwards-compatibility (with a caveat to force-close if this output doesn't meet dust relay requirements).

So I think it's for backward compatibility and LND started to drop it

[morehouse]

Yes, those types have been removed from the spec. But since the legacy code exists in multiple implementations, I figured we should exercise that code.

[ekzyis]

Just one typo, LGTM otherwise

[ekzyis]

bolt02 mentions this:

The scriptpubkey forms include only standard segwit forms accepted by the Bitcoin network, which ensures the resulting transaction will propagate to miners. However old nodes may send non-segwit scripts, which may be accepted for backwards-compatibility (with a caveat to force-close if this output doesn't meet dust relay requirements).

So I think it's for backward compatibility and LND started to drop it

[ekzyis]

nit: I verified that it's also okay to send upfront_shutdown_script when we did not signal option_upfront_shutdown_script during init exchange.

I see that this is documented for ShutdownScriptVariant:

accepted regardless of feature negotiation.

Could make sense to also add a small note here?

diff --git a/smite-scenarios/src/executor.rs b/smite-scenarios/src/executor.rs
index f05481a..b0e541e 100644
--- a/smite-scenarios/src/executor.rs
+++ b/smite-scenarios/src/executor.rs
@@ -330,7 +330,8 @@ fn build_open_channel(
         tlvs: OpenChannelTlvs {
             // Always send the TLV: a zero-length value is the BOLT 2 opt-out
             // signal when option_upfront_shutdown_script is negotiated.
-            // Omitting it is a protocol violation in that case.
+            // Omitting it is a protocol violation in that case. Including if
+            // not negotiated is not.
             upfront_shutdown_script: Some(resolve_bytes(variables, inputs[18])?.to_vec()),
             channel_type: nonempty_or_none(resolve_features(variables, inputs[19])?),
         },

[morehouse]

Done.

[ekzyis]

Suggested change

	/// `OP_RETURN_MAX_DATA_LEN..=OP_RETURN_MAX_DATA_LEN` bytes.
	/// `OP_RETURN_MIN_DATA_LEN..=OP_RETURN_MAX_DATA_LEN` bytes.

[morehouse]

Good catch! Done.

[ekzyis]

note: #51 adds the bitcoin crate. We could then use bitcoin::opcodes here in a follow-up PR. But it would require casting OpCode to u8 or updating our types to use OpCode instead of u8.