eof: Fix EOF builtin names unintentionally reserved outside of EOF

[rodiazet]

Resolves: #15672

[nikola-matic]

Are any of these implemented in EOF, i.e. callable, like ext* calls?

[rodiazet]

They are not callable from yul level, but legacy jump and jumpi are reserved but also not callable from yul. So I assumed they should be reserved also.

[cameel]

Looks like for some reason a clash with a builtin name is a fatal error. There's no good reason for that. I submitted a PR to fix that: #15712. When it's merged, you'll be able to put all the EOF builtins in a single test.

[rodiazet]

Worth noting that when I changed the test to version without object and code section. The test results looks differently. Not sure if it's expected behavior.

yulSyntaxTests/eof/auxdataloadn_reserved_in_eof.yul: FAIL
  Contract:
    {
        function auxdataloadn() {}
    }
    // ====
    // bytecodeFormat: >=EOFv1

  Expected result:
    ParserError 5568: (41-53): Cannot use builtin function name "auxdataloadn" as identifier name.
    ParserError 8143: (41-53): Expected keyword "data" or "object" or "}".
  Obtained result:
    ParserError 5568: (15-27): Cannot use builtin function name "auxdataloadn" as identifier name.

[cameel]

Yeah, it looks fine.

The Yul parser is split into two parts: the main one which parses code blocks (and produces ASTs) and a smaller one that only handles the object structure (and produces Object tree).

What was originally happening here was that the object parser would run the code parser to parse a code block, which in turn would hit a fatal error (resulting in an exception). The object parser would probably catch that error but not stop and instead try to parse another nested object or code block at that point. This would produce the second error.

Now that you dropped the object, the object parser is no longer involved and you only get the first error. #15712 would have the same effect in this particular case but via a different route - by making the first error non-fatal and letting the code parser continue until the end of the code block, leaving the object parser at the right spot to parse the next thing.

[cameel]

You could use hasOpcode() to avoid repeating these:

EVMVersion::prague().hasOpcode(_instr, 1) && !EVMVersion::prague().hasOpcode(_instr, std::nullopt)

With an extra assert that EOF version is 1 and EVM version is <= Prague because this will need to be adjusted once new versions are out (if they and add/remove instructions).

[cameel]

BTW, validateInstructions() should have such an assert too, because it hard-coded EOF version to 1 and we can easily forget to update it once we have EOF v2.

[cameel]

A more general way to check this would be:

if (resolveBuiltinFunction(
	funCall.functionName,
	EVMDialect::strictAssemblyForEVMObjects(EVMVersion::prague(), 1)
))

And that also with an assert that EOF version <= 1 and EVM version <= Prague.

And a comment that strictAssemblyForEVMObjects() is expected to be the dialect that contains all the possible builtins. Or maybe we should just check both it and strictAssemblyForEVM() to be safe.

[cameel]

Also, I think it would fit better in validateInstructions() (the overload that takes a string). Just before return false, when we know we're not dealing with an instruction.

[cameel]

Suggested change

	"The \"{}\" instruction is only available in EOF.",
	"Builtin function \"{}\" is only available in EOF.",