Request: Script::is_tapscript

[stevenroose]

The Script::to_v1_p2th method says "assuming that the script is a tapscript". It would be nice if we could actually check that.

[junderw]

1. Iterate through all instructions, caching the previous two instructions.
2. If OP_CHECKMULTISIG* (0xae or 0xaf) is seen, fail.
3. If OP_CHECKSIG(VERIFY) is seen, make sure the pubkey is 32 length, and the signature is 64 or 65 length.
4. If OP_SIGADD is seen, make sure the signature is 64 or 65 length.

Since we don't have the entire witness stack, we can't even try to calculate the sigop budget, so that's about all we can do.

[junderw]

So the question really boils down to:

If we can't guarantee it will work, why not place the burden 100% on the caller?

We'd assume anyone messing with custom scripts will test with regtest and testnet first.

[junderw]

Or maybe instead of a bool we could have a 3 option enum, Tapscript, NotTapscript, Inconclusive... etc.

A large majority of real non-toy scripts should have a CHECKSIG, and looking at the pubkey size can tell us most of the time surely.

[junderw]

Maybe something along these lines? After writing it out, it looks very hairy, and after considering the fact that there might be op codes in between adding and dropping things, so the pubkey etc. won't necessarily be pushed immediately before the CHECKSIG etc.........

I am not sure if this is worth adding now, tbh.

Here's the code I had so far in case someone wants to pick up where I left off.

/// Detect whether this Script is possible a tapscript.
///
/// This can be inconclusive if mixed signals are given
/// (ie. SIGADD and CHECKMULTISIG are both used)
#[inline]
pub fn is_tapscript(&self) -> IsTapscript {
    // Most previous pushdata size if pushdata, None if OP
    let mut tail_1 = None;
    // Second to most previous pushdata size if pushdata, None if OP
    let mut tail_2 = None;
    let mut yes_count = 0;
    let mut no_count = 0;
    // Provably invalid scripts should be inconclusive
    for inst in self.instructions() {
        match inst.expect("Not requiring minimal push") {
            Instruction::Op(op) => {
                match op {
                    OP_CHECKMULTISIG | OP_CHECKMULTISIGVERIFY => no_count += 1,
                    OP_CHECKSIGADD => {
                        yes_count += 1;
                        match tail_1 {
                            Some(s) if s != 64 && s != 65 => {
                                no_count += 1;
                            }
                            _ => {}
                        };
                    }
                    OP_CHECKSIG | OP_CHECKSIGVERIFY => {
                        match tail_2 {
                            Some(s) if s != 64 && s != 65 => {
                                no_count += 1;
                            }
                            Some(_) => {
                                yes_count += 1;
                            }
                            None => {}
                        };
                        match tail_1 {
                            Some(s) if s == 32 => {
                                yes_count += 1;
                            }
                            Some(_) => {
                                no_count += 1;
                            }
                            None => {}
                        };
                    }
                    _ => {}
                }
                tail_2 = tail_1.take();
                tail_1 = None;
            }
            Instruction::PushBytes(bytes) => {
                tail_2 = tail_1.take();
                tail_1 = Some(bytes.len());
            }
        }
    }
    match (yes_count, no_count) {
        (0, i) if i > 0 => IsTapscript::No,
        (i, 0) if i > 0 => IsTapscript::Yes,
        _ => IsTapscript::Inconclusive,
    }
}

[sanket1729]

I don't think there is nice general way of doing. As @junderw mentioned, I am also of the opinion that we shouldn't do it.

[junderw]

I am also of the opinion that we should do it.

Did you mean shouldn't?

[sanket1729]

yes, sorry about that. I meant should not .

[stevenroose]

But.. What does the witness have to do with the script itself being valid tapscript or not? I mean, what the user inputs in the witness doesn't change the script itself.

From @junderw 's first post, it seems that a script is_valid_tapscript iff it doesn't have OP_CHECKMULTISIG* opcodes?

[stevenroose]

Hmm, I start to kinda agree, but for different reasons.

First of all the argument about what's in the witness I think is false, the witness doesn't change the script itself, whether or not it's a valid script. Witness is input.

I had made this implementation:

    pub fn is_valid_tapscript(&self) -> bool {
        self.instructions().all(|i| match i {
            Ok(Instruction::Op(op)) if op == OP_CHECKMULTISIG => false,
            Ok(Instruction::Op(op)) if op == OP_CHECKMULTISIGVERIFY => false,
            _ => true,
        })
    }

But looking into the bips, I realize that this is just the differentiator between tapscript and legacy script. But there are various parameters that make a script invalid also in legacy script. Like, we have many of illegal opcodes that if you use them, renders the script invalid. So in order to implement an is_valid_tapscript, we would need a regular is_valid and that seems like quite a non-trivial task.

So I suppose I agree that this is not a good idea, goes slightly too deep into consensus.

[apoelstra]

Agreed, this sort of script analysis is what rust-miniscript is for.