Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

EIP-4200: Add RJUMPV #6056

Merged
merged 3 commits into from
Dec 2, 2022
Merged

EIP-4200: Add RJUMPV #6056

merged 3 commits into from
Dec 2, 2022

Conversation

axic
Copy link
Member

@axic axic commented Nov 28, 2022

No description provided.

@github-actions github-actions bot added c-update Modifies an existing proposal s-review This EIP is in Review t-core labels Nov 28, 2022
@eth-bot
Copy link
Collaborator

eth-bot commented Nov 28, 2022

All tests passed; auto-merging...

(pass) eip-4200.md

classification
updateEIP
  • passed!

axic
axic commented Nov 28, 2022
View reviewed changes
EIPS/eip-4200.md Outdated
2. `RJUMPI relative_offset` (0x5d), pops a value (`condition`) from the stack, and sets the `PC` to `PC_post_instruction + ((condition == 0) ? 0 : relative_offset)`.
1. `RJUMP relative_offset` sets the `PC` to `PC_post_instruction + relative_offset`.
2. `RJUMPI relative_offset` pops a value (`condition`) from the stack, and sets the `PC` to `PC_post_instruction + ((condition == 0) ? 0 : relative_offset)`.
3. `RJUMPV count relative_offset+` pops a value (`case`) from the stack, and sets the `PC` to `PC_post_instruction + ((case >= count) ? 0 : relative_offset[case])`.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has some interesting properties:

  1. In the current version with fallthrough on out of bounds: RJUMPV 0 offset will work like an inverted-RJUMPI. It will jump to the offset if and only if the case is 0, otherwise fall through.
  2. If the we choose to require the default target offset (= instead of fallthrough, the first immediate is the "default target" offset): RJUMPV 0 offset would be identical to RJUMP.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(1) seems nice

@gumb0
Copy link
Member

gumb0 commented Nov 30, 2022

Abstract needs update

gumb0
gumb0 reviewed Nov 30, 2022
View reviewed changes
EIPS/eip-4200.md

The immediate argument `relative_offset` is encoded as a 16-bit **signed** (two's-complement) big-endian value. Under `PC_post_instruction` we mean the `PC` position after the entire immediate value.

We also extend the validation algorithm of [EIP-3670](./eip-3670.md) to verify that each `RJUMP`/`RJUMPI` has a `relative_offset` pointing to an instruction. This means it cannot point to an immediate data of `PUSHn`/`RJUMP`/`RJUMPI`. It cannot point outside of code bounds. It is allowed to point to a `JUMPDEST`, but is not required to.
The immediate encoding of `RJUMPV` is more special: the 8-bit `count` value incremented by 1 determines the number of `relative_offset` values following. Based on this it can be seen that the encoding of `RJUMPV` must have at least one `relative_offset` and thus it will take at minimum 4 bytes. Furthermore, the `case >= count` condition falling through means that in many use cases one would place the *default* path following the `RJUMPV` instruction.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why incremented by 1?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess to avoid empty jump table.

RJUMPV 0 could be a no-op or rejected at validation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess to avoid empty jump table.

Yes.

RJUMPV 0 could be a no-op or rejected at validation.

Yes that's an option.

Copy link
Member

@lightclient lightclient Dec 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason to not just reject count=0? Seems better to avoid adding things in the background.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the current description count is always +1d, so this removes the need for a special case of "reject count 0". We can go either way.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can also keep count=0 as yet another NOP-style instruction if that's better.

gumb0
gumb0 reviewed Nov 30, 2022
View reviewed changes
EIPS/eip-4200.md Outdated Show resolved Hide resolved
chfast
chfast reviewed Dec 1, 2022
View reviewed changes
EIPS/eip-4200.md Outdated Show resolved Hide resolved
chfast
chfast reviewed Dec 1, 2022
View reviewed changes
EIPS/eip-4200.md

If the code is valid EOF1:

1. `RJUMP relative_offset` (0x5c), sets the `PC` to `PC_post_instruction + relative_offset`.
2. `RJUMPI relative_offset` (0x5d), pops a value (`condition`) from the stack, and sets the `PC` to `PC_post_instruction + ((condition == 0) ? 0 : relative_offset)`.
1. `RJUMP relative_offset` sets the `PC` to `PC_post_instruction + relative_offset`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot find where this PC_post_instruction is defined.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On L52.

gumb0
gumb0 reviewed Dec 2, 2022
View reviewed changes
EIPS/eip-4200.md Outdated Show resolved Hide resolved
gumb0
gumb0 previously approved these changes Dec 2, 2022
View reviewed changes
@axic axic marked this pull request as ready for review December 2, 2022 14:01
@axic axic requested a review from eth-bot as a code owner December 2, 2022 14:01
@eth-bot eth-bot enabled auto-merge (squash) December 2, 2022 14:02
@eth-bot eth-bot merged commit 319fa66 into ethereum:master Dec 2, 2022
@axic axic deleted the eip-4200-rjumpv branch December 2, 2022 14:06
Woodpile37 added a commit to Woodpile37/EIPs that referenced this pull request Nov 11, 2023
@Woodpile37
build(deps-dev): bump axios from 1.5.1 to 1.6.1 in /assets/eip-7007 (#…
021cd0a 
…190)

Bumps [axios](https://github.com/axios/axios) from 1.5.1 to 1.6.1.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a
href="https://github.com/axios/axios/releases">axios's
releases</a>.</em></p>
<blockquote>
<h2>Release v1.6.1</h2>
<h2>Release notes:</h2>
<h3>Bug Fixes</h3>
<ul>
<li><strong>formdata:</strong> fixed content-type header normalization
for non-standard browser environments; (<a
href="https://redirect.github.com/axios/axios/issues/6056">#6056</a>)
(<a
href="https://github.com/axios/axios/commit/dd465ab22bbfa262c6567be6574bf46a057d5288">dd465ab</a>)</li>
<li><strong>platform:</strong> fixed emulated browser detection in
node.js environment; (<a
href="https://redirect.github.com/axios/axios/issues/6055">#6055</a>)
(<a
href="https://github.com/axios/axios/commit/3dc8369e505e32a4e12c22f154c55fd63ac67fbb">3dc8369</a>)</li>
</ul>
<h3>Contributors to this release</h3>
<ul>
<li><!-- raw HTML omitted --> <a
href="https://github.com/DigitalBrainJS" title="+432/-65
([ethereum#6059](axios/axios#6059)
[ethereum#6056](axios/axios#6056)
[ethereum#6055](axios/axios#6055) )">Dmitriy
Mozgovoy</a></li>
<li><!-- raw HTML omitted --> <a href="https://github.com/meyfa"
title="+5/-2 ([ethereum#5835](axios/axios#5835)
)">Fabian Meyer</a></li>
</ul>
<h2>Release v1.6.0</h2>
<h2>Release notes:</h2>
<h3>Bug Fixes</h3>
<ul>
<li><strong>CSRF:</strong> fixed CSRF vulnerability CVE-2023-45857 (<a
href="https://redirect.github.com/axios/axios/issues/6028">#6028</a>)
(<a
href="https://github.com/axios/axios/commit/96ee232bd3ee4de2e657333d4d2191cd389e14d0">96ee232</a>)</li>
<li><strong>dns:</strong> fixed lookup function decorator to work
properly in node v20; (<a
href="https://redirect.github.com/axios/axios/issues/6011">#6011</a>)
(<a
href="https://github.com/axios/axios/commit/5aaff532a6b820bb9ab6a8cd0f77131b47e2adb8">5aaff53</a>)</li>
<li><strong>types:</strong> fix AxiosHeaders types; (<a
href="https://redirect.github.com/axios/axios/issues/5931">#5931</a>)
(<a
href="https://github.com/axios/axios/commit/a1c8ad008b3c13d53e135bbd0862587fb9d3fc09">a1c8ad0</a>)</li>
</ul>
<h3>PRs</h3>
<ul>
<li>CVE 2023 45857 ( <a
href="https://api.github.com/repos/axios/axios/pulls/6028">#6028</a>
)</li>
</ul>
<pre><code>
⚠️ Critical vulnerability fix. See
https://security.snyk.io/vuln/SNYK-JS-AXIOS-6032459
</code></pre>
<h3>Contributors to this release</h3>
<ul>
<li><!-- raw HTML omitted --> <a
href="https://github.com/DigitalBrainJS" title="+449/-114
([ethereum#6032](axios/axios#6032)
[ethereum#6021](axios/axios#6021)
[ethereum#6011](axios/axios#6011)
[ethereum#5932](axios/axios#5932)
[ethereum#5931](axios/axios#5931) )">Dmitriy
Mozgovoy</a></li>
<li><!-- raw HTML omitted --> <a
href="https://github.com/valentin-panov" title="+4/-4
([ethereum#6028](axios/axios#6028) )">Valentin
Panov</a></li>
<li><!-- raw HTML omitted --> <a href="https://github.com/therealrinku"
title="+1/-1 ([ethereum#5889](axios/axios#5889)
)">Rinku Chaudhari</a></li>
</ul>
</blockquote>
</details>
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a
href="https://github.com/axios/axios/blob/v1.x/CHANGELOG.md">axios's
changelog</a>.</em></p>
<blockquote>
<h2><a
href="https://github.com/axios/axios/compare/v1.6.0...v1.6.1">1.6.1</a>
(2023-11-08)</h2>
<h3>Bug Fixes</h3>
<ul>
<li><strong>formdata:</strong> fixed content-type header normalization
for non-standard browser environments; (<a
href="https://redirect.github.com/axios/axios/issues/6056">#6056</a>)
(<a
href="https://github.com/axios/axios/commit/dd465ab22bbfa262c6567be6574bf46a057d5288">dd465ab</a>)</li>
<li><strong>platform:</strong> fixed emulated browser detection in
node.js environment; (<a
href="https://redirect.github.com/axios/axios/issues/6055">#6055</a>)
(<a
href="https://github.com/axios/axios/commit/3dc8369e505e32a4e12c22f154c55fd63ac67fbb">3dc8369</a>)</li>
</ul>
<h3>Contributors to this release</h3>
<ul>
<li><!-- raw HTML omitted --> <a
href="https://github.com/DigitalBrainJS" title="+432/-65
([ethereum#6059](axios/axios#6059)
[ethereum#6056](axios/axios#6056)
[ethereum#6055](axios/axios#6055) )">Dmitriy
Mozgovoy</a></li>
<li><!-- raw HTML omitted --> <a href="https://github.com/meyfa"
title="+5/-2 ([ethereum#5835](axios/axios#5835)
)">Fabian Meyer</a></li>
</ul>
<h1><a
href="https://github.com/axios/axios/compare/v1.5.1...v1.6.0">1.6.0</a>
(2023-10-26)</h1>
<h3>Bug Fixes</h3>
<ul>
<li><strong>CSRF:</strong> fixed CSRF vulnerability CVE-2023-45857 (<a
href="https://redirect.github.com/axios/axios/issues/6028">#6028</a>)
(<a
href="https://github.com/axios/axios/commit/96ee232bd3ee4de2e657333d4d2191cd389e14d0">96ee232</a>)</li>
<li><strong>dns:</strong> fixed lookup function decorator to work
properly in node v20; (<a
href="https://redirect.github.com/axios/axios/issues/6011">#6011</a>)
(<a
href="https://github.com/axios/axios/commit/5aaff532a6b820bb9ab6a8cd0f77131b47e2adb8">5aaff53</a>)</li>
<li><strong>types:</strong> fix AxiosHeaders types; (<a
href="https://redirect.github.com/axios/axios/issues/5931">#5931</a>)
(<a
href="https://github.com/axios/axios/commit/a1c8ad008b3c13d53e135bbd0862587fb9d3fc09">a1c8ad0</a>)</li>
</ul>
<h3>PRs</h3>
<ul>
<li>CVE 2023 45857 ( <a
href="https://api.github.com/repos/axios/axios/pulls/6028">#6028</a>
)</li>
</ul>
<pre><code>
⚠️ Critical vulnerability fix. See
https://security.snyk.io/vuln/SNYK-JS-AXIOS-6032459
</code></pre>
<h3>Contributors to this release</h3>
<ul>
<li><!-- raw HTML omitted --> <a
href="https://github.com/DigitalBrainJS" title="+449/-114
([ethereum#6032](axios/axios#6032)
[ethereum#6021](axios/axios#6021)
[ethereum#6011](axios/axios#6011)
[ethereum#5932](axios/axios#5932)
[ethereum#5931](axios/axios#5931) )">Dmitriy
Mozgovoy</a></li>
<li><!-- raw HTML omitted --> <a
href="https://github.com/valentin-panov" title="+4/-4
([ethereum#6028](axios/axios#6028) )">Valentin
Panov</a></li>
<li><!-- raw HTML omitted --> <a href="https://github.com/therealrinku"
title="+1/-1 ([ethereum#5889](axios/axios#5889)
)">Rinku Chaudhari</a></li>
</ul>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="https://github.com/axios/axios/commit/f6d2cf9763bfa124f15c2dc6a5d5d5d9d3e26169"><code>f6d2cf9</code></a>
chore(ci): fix publish action content permission; (<a
href="https://redirect.github.com/axios/axios/issues/6061">#6061</a>)</li>
<li><a
href="https://github.com/axios/axios/commit/a22f4b918a71a4d4caa57ff23d8247eac93765de"><code>a22f4b9</code></a>
chore(release): v1.6.1 (<a
href="https://redirect.github.com/axios/axios/issues/6060">#6060</a>)</li>
<li><a
href="https://github.com/axios/axios/commit/cb8bb2beb215a94a29f19b0d66ab05d32b390230"><code>cb8bb2b</code></a>
chore(ci): Publish to NPM with provenance (<a
href="https://redirect.github.com/axios/axios/issues/5835">#5835</a>)</li>
<li><a
href="https://github.com/axios/axios/commit/37cbf9214a1140d25c2c1a5ff097666c96721d6a"><code>37cbf92</code></a>
chore(ci): added labeling and notification for published PRs; (<a
href="https://redirect.github.com/axios/axios/issues/6059">#6059</a>)</li>
<li><a
href="https://github.com/axios/axios/commit/dd465ab22bbfa262c6567be6574bf46a057d5288"><code>dd465ab</code></a>
fix(formdata): fixed content-type header normalization for non-standard
brows...</li>
<li><a
href="https://github.com/axios/axios/commit/3dc8369e505e32a4e12c22f154c55fd63ac67fbb"><code>3dc8369</code></a>
fix(platform): fixed emulated browser detection in node.js environment;
(<a
href="https://redirect.github.com/axios/axios/issues/6055">#6055</a>)</li>
<li><a
href="https://github.com/axios/axios/commit/f7adacdbaa569281253c8cfc623ad3f4dc909c60"><code>f7adacd</code></a>
chore(release): v1.6.0 (<a
href="https://redirect.github.com/axios/axios/issues/6031">#6031</a>)</li>
<li><a
href="https://github.com/axios/axios/commit/9917e67cbb6c157382863bad8c741de58e3f3c2b"><code>9917e67</code></a>
chore(ci): fix release-it arg; (<a
href="https://redirect.github.com/axios/axios/issues/6032">#6032</a>)</li>
<li><a
href="https://github.com/axios/axios/commit/96ee232bd3ee4de2e657333d4d2191cd389e14d0"><code>96ee232</code></a>
fix(CSRF): fixed CSRF vulnerability CVE-2023-45857 (<a
href="https://redirect.github.com/axios/axios/issues/6028">#6028</a>)</li>
<li><a
href="https://github.com/axios/axios/commit/7d45ab2e2ad6e59f5475e39afd4b286b1f393fc0"><code>7d45ab2</code></a>
chore(tests): fixed tests to pass in node v19 and v20 with
<code>keep-alive</code> enabl...</li>
<li>Additional commits viewable in <a
href="https://github.com/axios/axios/compare/v1.5.1...v1.6.1">compare
view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=axios&package-manager=npm_and_yarn&previous-version=1.5.1&new-version=1.6.1)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)
You can disable automated security fix PRs for this repo from the
[Security Alerts
page](https://github.com/Woodpile37/EIPs/network/alerts).

</details>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lightclient lightclient lightclient left review comments

@chfast chfast chfast left review comments

@eth-bot eth-bot eth-bot approved these changes

@gumb0 gumb0 gumb0 left review comments

Assignees
No one assigned
Labels
c-update Modifies an existing proposal s-review This EIP is in Review t-core
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@axic @eth-bot @gumb0 @chfast @lightclient