Allow multiple push statement for OP_RETURN
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?
Seems ok at a glance.
Are you guys using https://github.com/btcsuite/btcd/blob/09ba026580f4041bae30d732f26fa1a2036af003/txscript/script.go#L103-L10?
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 https://github.com/btcsuite/btcd/blob/09ba026580f4041bae30d732f26fa1a2036af003/txscript/standard.go#L523, and calling Next of the ScriptTokenizer, op.length of OP_13 is 1, so t.offset is set to 1 https://github.com/btcsuite/btcd/blob/09ba026580f4041bae30d732f26fa1a2036af003/txscript/tokenizer.go#L79
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?
@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.
Hi @Roasbeef and @kcalvinalvin , I made an attempt at fixing the issue https://github.com/btcsuite/btcd/pull/2309