Allow multiple push statement for OP_RETURN

[dkatzan]

Currently, when checking if a script is isNullDataScript, it is expliciclty checks for OP_RETURN or OP_RETURN followed by a single push which happens here https://github.com/btcsuite/btcd/blame/master/txscript/standard.go#L498

how ever since bitcoin v.12.0, the restriction of having only a single push was lifted, and there can now be multiple push data, and the limit on the output size is now enforced on the entire script

Would it be possible to add support for this?

[kcalvinalvin]

Seems ok at a glance.

Are you guys using https://github.com/btcsuite/btcd/blob/09ba026580f4041bae30d732f26fa1a2036af003/txscript/script.go#L103-L10?

[dkatzan]

Hi @kcalvinalvin , thx for the quick reply

We are, but IIUC, this test should pass

func TestScriptType(t *testing.T) {
	s, err := hex.DecodeString("6a5d0f160100e6a233fc078088a5a9a30700")
	require.NoError(t, err)
	isNull := txscript.IsNullData(s)
	require.True(t, isNull)
}

but it in fact fails
I believe the issue is this script start with 6a which is OP_RETURN followed by 5d which is OP_13 and then some data

when reaching here

btcd/txscript/standard.go

Line 523 in 09ba026

return tokenizer.Next() && tokenizer.Done() &&

, and calling Next of the ScriptTokenizer, op.length of OP_13 is 1, so t.offset is set to 1

btcd/txscript/tokenizer.go

Line 79 in 09ba026

t.offset++

which ends up returning false on tokenizer.Done here

as a side note, it also contains this check
len(tokenizer.Data()) <= MaxDataCarrierSize
which I believe should be
len(tokenizer.Data()) <= MaxDataCarrierSize + 3

please lmk if I missunderstood something?

[Roasbeef]

@dkatzan if you want to fix this, then you'll need to add a new loop to continue parsing the pushes to validate that they're all minimal, etc. The final statement in that function has tokenizer.Done() which helps to assert that there's only a single push.

[dkatzan]

Hi @Roasbeef and @kcalvinalvin , I made an attempt at fixing the issue
#2309