feat: define initcode mode and require validation of RETURNCONTRACT #86
Conversation
spec/eof.md
Outdated
| @@ -316,10 +316,15 @@ The following instructions are introduced in EOF code: | |||
| - the first code section must have a type signature `(0, 0x80, max_stack_height)` (0 inputs non-returning function) | |||
| - `EOFCREATE` `initcontainer_index` must be less than `num_container_sections` | |||
| - `EOFCREATE` the subcontainer pointed to by `initcontainer_index` must have its `len(data_section)` equal `data_size`, i.e. data section content is exactly as the size declared in the header (see [Data section lifecycle](#data-section-lifecycle)) | |||
| - `EOFCREATE` the subcontainer pointed to by `initcontainer_index` must be an "initcode" subcontainer, that is, it *must* contain a `RETURNCONTRACT` instruction. | |||
There was a problem hiding this comment.
Can EOFCREATE point to a container which contains only REVERT or INVALID?
There was a problem hiding this comment.
hm, interesting! are there any implicit STOP instructions in EOF?
There was a problem hiding this comment.
hm, interesting! are there any implicit STOP instructions in EOF?
no, a terminating instruction or an unconditional jump is required at the end of each code section.
There was a problem hiding this comment.
in this case, maybe i should change the wording to:
the subcontainer pointed to by
initcontainer_indexmust be an "initcode" subcontainer, that is, it must not contain aRETURNorSTOPinstruction.
There was a problem hiding this comment.
the subcontainer pointed to by initcontainer_index must be an "initcode" subcontainer, that is, it must not contain a RETURN or STOP instruction.
I think this is too weak of a definition, in light of the conditions which are listed below. Also, this makes a REVERT-only subcontainer an "initcode" one, which is not natural IMO.
To be clear: I think the current wording is fine, we just need to be aware of how we treat containers which contain only REVERTs and INVALIDs. As of now, such container is invalid when pointed to by EOFCREATE and valid when pointed to by RETURNCONTRACT, and this rule is implicit. Maybe better to add a **Note** at the bottom to make it explicit?
There was a problem hiding this comment.
maybe three types of containers -- RETURN/STOP (0b01), RETURNCONTRACT (0b10) and none (0b00). 0b11 is invalid, but 0b00 can be referenced by either EOFCREATE or RETURNCONTRACT. i don't think an initcode which can only REVERT or INVALID would be very useful, but it might be used somehow for a signaling purpose (like there is information in the revertdata)? but in such a case you would just want to call the contract.
There was a problem hiding this comment.
speaking of signaling - should EOFCREATE and/or TXCREATE return status codes, like EXT*CALL?
There was a problem hiding this comment.
maybe three types of containers
This is now unclear to me - does your proposition introduce explicit types in the header or implicit based on the instructions? I thought implicit, without modification to the header
speaking of signaling - should EOFCREATE and/or TXCREATE return status codes, like EXT*CALL?
Both return new address (or zero on failure). I don't recall any discussion happening on this topic.
There was a problem hiding this comment.
This is now unclear to me - does your proposition introduce explicit types in the header or implicit based on the instructions? I thought implicit, without modification to the header
it's still implicit, but i was saying we could allow containers with only REVERT or INVALID (and no RETURN/STOP/RETURNCONTRACT) as initcode. i don't think it's very useful though, especially without signaling. (it essentially would be a way to call a subcontainer without actually deploying it.)
spec/eof.md
Outdated
| - `RETURNCONTRACT` `deploy_container_index` must be less than `num_container_sections` | ||
| - `RETURNCONTRACT` the subcontainer pointed to `deploy_container_index` must not be an "initcode" subcontainer, that is, it *must not* contain a `RETURNCONTRACT` instruction. |
There was a problem hiding this comment.
"not contain RETURNCONTRACT" is not the negation of "initcode" subcontainer definition from 2 lines above - that would be "may contain RETURN or STOP". I think trying to define "initcode" container or mode doesn't help, let's just list the rules (exact wording pending):
- EOFCREATE points to container without RETURN or STOP
- RETURNCONTRACT points to container without RETURNCONTRACT
and that's it, no? I think it covers all the situations described here, following the "lax" ruleset; that is, a container with only REVERT & INVALID is always good.
And actually there's a third one, which is missed here altogether (just realized it), I'm not sure it belongs better here or in the TXCREATE runtime spec:
- Top-level container from
initcodespointed to with TXCREATE is without RETURN or STOP
This leaves us with a potential rule to consider: what about subcontainers not pointed to at all? I would simply not allow them, as they're just dead code which may become "undead" on upgrades if we miss it.
- Container with a subcontainer not pointed to by either EOFCREATE or RETURNCONTRACT is invalid.
(lastly, albeit this can't be fixed in this PR, just need to remember to fix it in the future, in case #78 goes through:)
- Top-level container from tx
datain "Creation transaction (toisnil)" is without RETURN or STOP
There was a problem hiding this comment.
ok I'll make the changes and remove "initcode-mode"
- i can make the change in this PR
- i don't know how they could become "undead", but yea i think it is a good idea in general to ban dead code
There was a problem hiding this comment.
although i think it's helpful to have "initcode" as a term for discussion purposes, i'll update the wording and see what you think
|
It needs rebase now
|
RETURNCONTRACT to point to runtime code.
update TXCREATE rules massage the verbiage around runtime vs initcode mode
gumb0
force-pushed
the
initcode-validation
branch
from
46556d1
to
c2d0548
Compare
require EOFCREATE to point to initcode, and RETURNCONTRACT to point to runtime code.