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

Ecmul Precompile #1829

Merged
merged 10 commits into from
Mar 7, 2019
Merged

Conversation

recmo
Copy link
Contributor

@recmo recmo commented Mar 6, 2019

No description provided.

discussions-to: https://ethereum-magicians.org/t/ewasm-precompile-for-general-elliptic-curve-math/2581
status: Draft
type: Core
replaces: EIP-196 (in some cases), EIP 665, EIP 1108 (partially), https://github.com/ethereum/EIPs/issues/603
Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately this can only use an EIP number and that must be merged (cannot be an unmerged draft). For #603 I'd suggest you refer to it in a "References" or "Prior art" section.

Copy link
Member

Choose a reason for hiding this comment

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

I actually don't think the replaces field is appropriate here. Perhaps it is as field not applicable to the EIP process we're following.

@@ -0,0 +1,151 @@
---
eip: To be assigned
Copy link
Member

Choose a reason for hiding this comment

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

You can change this to 1829 and also rename the file.

status: Draft
type: Core
replaces: EIP-196 (in some cases), EIP 665, EIP 1108 (partially), https://github.com/ethereum/EIPs/issues/603
author: Remco Bloemen <[email protected]>
Copy link
Member

Choose a reason for hiding this comment

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

Could order these fields the same way the template has it?

MUL_GAS = ...
```

The total gas cost is `BASE_GAS` plus `ADD_GAS` for each $s_i$ that is $1$ and `MUL_GAS` for each $s_i > 1$ ($s_0 = 0$ is free).
Copy link
Member

Choose a reason for hiding this comment

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

Not sure markdown renders $..$ but you could use backticks instead?

@recmo
Copy link
Contributor Author

recmo commented Mar 6, 2019

@axic Thank you! I believe I have addressed all the comments.

@recmo
Copy link
Contributor Author

recmo commented Mar 6, 2019

The Travis error does not seem related to the PR.

@nicksavers nicksavers merged commit b501ba9 into ethereum:master Mar 7, 2019
@vbuterin
Copy link
Contributor

The thing that has always stopped me from proposing a similar thing is, what happens if someone submits a non-prime modulus? There are different ways to implement elliptic curve addition and multiplication, and in general I don't expect that the different libraries have been tested to ensure that they fail in the same cases when the modulus ends up being composite, and this seems like a large and deep potential source of consensus failures. Unfortunately primality tests that take into account weird edge cases like Carmichael numbers but are also clean and efficient don't really exist.

@recmo
Copy link
Contributor Author

recmo commented Mar 14, 2019

@vbuterin

EDIT: Response moved to the discussion thread to keep the conversation in one place: https://ethereum-magicians.org/t/precompile-for-general-elliptic-curve-linear-combinations/2581/14?u=recmo

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.

4 participants