EIPs
EIPs copied to clipboard
eip-2315 example bytecode fix
When opening a pull request to submit a new EIP, please use the suggested template: https://github.com/ethereum/EIPs/blob/master/eip-template.md
We have a GitHub bot that automatically merges some PRs. It will merge yours immediately if certain criteria are met:
- The PR edits only existing draft PRs.
- The build passes.
- Your GitHub username or email address is listed in the 'author' header of all affected PRs, inside
. - If matching on email address, the email address is the one publicly listed on your GitHub profile.
All tests passed; auto-merging...
(pass) eip-2315.md
| classification |
|---|
updateEIP |
- passed!
Right, my bad.
On another note: the immediate is defined as uint16, so should not be referenced to as '-1'. Also, there needs to be not one but two bytes of immediate data (four nibbles instead of one) . So 0xffff, 65535 rather than -1.
On another note: the immediate is defined as uint16, so should not be referenced to as '-1'.
I believe that this case demonstrates that it should be rather defined as signed int16 - to be consistent with EIP 4200. I raised the relevant question here: https://ethereum-magicians.org/t/eip-2315-simple-subroutines-for-the-evm/3941/137
Also, there needs to be not one but two bytes of immediate data (four nibbles instead of one) . So 0xffff, 65535 rather than -1.
You are right about 0xffff. And similar mistake is for RJUMP as it should be int16. See EIP 4200: The immediate argument relative_offset is encoded as a 16-bit signed (two’s-complement) big-endian value.
I guess the resulting bytecode should be: 0x5c00045e5fffff
I'm getting ready to travel, so am out of time to look at this, but you folks seem to have it under control, thanks. EIP-4200 came much, much later than this proposal and I'm not surprised that I failed to fully harmonize them.
On another note: the immediate is defined as uint16, so should not be referenced to as '-1'.
I believe that this case demonstrates that it should be rather defined as signed int16 - to be consistent with EIP 4200. I raised the relevant question here: https://ethereum-magicians.org/t/eip-2315-simple-subroutines-for-the-evm/3941/137
At line 76 the spec already says the immediate is signed:
The
relative_offsetis relative to the currentPC. The address is encoded as a two-byte, twos-complement signed integer, stored MSB-first.
At line 76 the spec already says the immediate is signed:
And at line 69 it says otherwise. It should be int16. Or just removed, as it is redundant.
EDIT: I removed the type declaration. The text is sufficient.
I have updated the PR to reflect the conclusions here.
@gcolvin PR merge conflict was corrected, I do not know how to resolve Auto Label server error.
This should just need an approval from one of the listed authors.
I don't really feel comfortable being listed as one. This eip has changed a lot.
I don't really feel comfortable being listed as one. This eip has changed a lot.
I just made the approval, but it doesn't seem to be merging. I think another PR to remove you from the author list would be better given the troubles we are having with this merge.
From my point of view the basic facility hasn't really changed: it's one instruction to call a subroutine and one to return, using a return stack. The changes to the actual opcode specifications are just the move to immediate arguments, (which we always wanted), relative offsets, (to harmonize with RJUMP) and the removal of BEGINSUB, (which was not in the original proposal anyway, and I regretted adding). The validation I took from EIP-615 and added long ago. I moved it a separate EIP for a while, but it seemed better to have it all in one place. The rest is expanded motivation and rationale, not specification.
So I've vacillated on whether to change numbers, as all of the analysis and discussion so far remains relevant, and the number and the concept are pretty well-known by now. But I'd have no strong objections to renumbering and/or splitting up this proposal if you prefer.
If it's the proposal itself you don't support I'd like to discuss it with you privately first, if you don't mind.
EDIT:
Besides, the Test Cases section we are discussing here remains primarily your work. That alone rates coauthorship.
Thank you so very much! I appreciate all you do! I will be sending blessings of residual significance.
On Fri, Oct 28, 2022, 12:59 PM Sam Wilson @.***> wrote:
Merged #5718 https://github.com/ethereum/EIPs/pull/5718 into master.
— Reply to this email directly, view it on GitHub https://github.com/ethereum/EIPs/pull/5718#event-7693671416, or unsubscribe https://github.com/notifications/unsubscribe-auth/A3H6XK5UDVNALV644NC4SSLWFQIBJANCNFSM6AAAAAAQVVASV4 . You are receiving this because you are subscribed to this thread.Message ID: @.***>