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.

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

MULDIV instruction #5000

Merged
merged 5 commits into from
Jul 13, 2022
Merged

MULDIV instruction #5000

merged 5 commits into from
Jul 13, 2022

Conversation

hrkrshnn
Copy link
Member

Introduce a new instruction, MULDIV(x, y, z), to perform ((x * y) / z) % 2**256 in 512-bit precision. z = 0 is a special case for (x * y) / 2**256.

@@ -0,0 +1,147 @@
---
eip: nnn
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
eip: nnn
eip: 4998

Since we try to discourage sniping "fancy" EIP numbers, and this PR was opened immediately after a spam bot posted issue 4999, I'm reassigning to a less special number.

Copy link
Contributor

Choose a reason for hiding this comment

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

@SamWilsn PR was merged as EIP-5000?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, that was @lightclient's decision.

title: MULDIV instruction
description:
author: Harikrishnan Mulackal (@hrkrshnn), Alex Beregszaszi (@axic), Paweł Bylica (@chfast)
discussions-to:
Copy link
Member

Choose a reason for hiding this comment

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

Missing link to FEM.

---
eip: nnn
title: MULDIV instruction
description:
Copy link
Member

Choose a reason for hiding this comment

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

Missing description.

EIPS/eip-draft-muldiv.md Outdated Show resolved Hide resolved
@eth-bot
Copy link
Collaborator

eth-bot commented Jul 13, 2022

All tests passed; auto-merging...

(pass) eip-5000.md

classification
updateEIP
  • passed!

@hrkrshnn hrkrshnn marked this pull request as ready for review July 13, 2022 13:41
EIPS/eip-5000.md Outdated Show resolved Hide resolved
Copy link
Member

@lightclient lightclient left a comment

Choose a reason for hiding this comment

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

LGTM

@eth-bot eth-bot enabled auto-merge (squash) July 13, 2022 13:58
@axic axic closed this Jul 13, 2022
auto-merge was automatically disabled July 13, 2022 14:16

Pull request was closed

@axic axic reopened this Jul 13, 2022
@eth-bot eth-bot enabled auto-merge (squash) July 13, 2022 14:17
@eth-bot eth-bot merged commit 6077576 into ethereum:master Jul 13, 2022
MicahZoltu added a commit that referenced this pull request Jul 14, 2022
This appears to be authored by someone actively attacking the EIP numbering system.  Not only did they appear to number snipe, but they blatantly ignored editor number assignment and picked the number they wanted anyway.  It was merged, but I suspect the editor who approved it just missed the assignment comment because it was marked by GitHub as "resolved".

Timeline:
* At around 18 minutes after the hour, a spammer created issue #4998.  This has since been deleted as spam.
* At around 19 minutes after the hour, a spammer created issue #4999.  This has since been deleted as spam.
* At around 19 minutes after the hour, @hrkrshnn created PR #5000.
* An hour and 7 minutes later [Sam reviewed it](#5000 (review)) and assigned number 4998 with a comment indicating why 5000 wasn't assigned.
* ~18 days later (with no updates from the author) [lightclient reviewed it](#5000 (review))
* ~Over two months later (still no updates from the author) [lightclient reviewed it again](#5000 (review))
* About an hour after that the author made the [first change](edaa3fb) to the PR, in which they ignored Sam's assigned number and took 5000 anyway.
* Review proceeded from there as normal and was eventually merged.

----

We have a serious spam problem on this repository and it is a huge editor time sink.  At best, the author of this PR wasn't the spammer but they were running a number squatting bot and they blatantly ignored editor number assignment.  At worst they are also the spammer and part of a serious problem.

I propose we delete this EIP and either ban the author from the repository for this behavior.  The blatant disregard for an editor's number assignment without discussion combined with the blatant number sniping makes me classify this as an active attack.

----

Also, since an editor is a co-author on this EIP I think this case should be taken *far* more seriously than normal.  Editors should be held to a far higher standard than regular drive-by authors and while I am not suggesting @axic played an active role in this attack (they may not have known about it), I think that a strong show of intolerance against malicious behavior here is necessary due to this association as we cannot be seen to be giving preferential treatment to editors.
@MicahZoltu MicahZoltu mentioned this pull request Jul 14, 2022
Copy link

@MDStone1993420 MDStone1993420 left a comment

Choose a reason for hiding this comment

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

@axic axic deleted the muldiv branch July 17, 2022 12:15
nachomazzara pushed a commit to nachomazzara/EIPs that referenced this pull request Jan 13, 2023
* Add MULDIV draft

* Fixed the number and a typo

* Add description and discussions-to

* Fixed a trailing whitespace

Co-authored-by: Alex Beregszaszi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants