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

EIP 665 - Add precompile for Ed25519 #949

Merged
merged 7 commits into from
Mar 28, 2018
Merged

Conversation

oberstet
Copy link
Contributor

See also this PR requesting the same #665

The PR here is more complete and takes up the feedback of @pirapira

EIPS/eip-665.md Outdated
The proposal adds a new precompiled function with the following signature

```
ed25519verify(bytes32 m, bytes32 pk, bytes32 s1, bytes32 s2) returns (uint8)
Copy link
Contributor

Choose a reason for hiding this comment

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

Existing precompiles do not use function selectors (for example, the RSA precompile). I think it'd be a good idea to remain consistent with that, and specify this in terms of call data.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure what you mean, could you expand? Note that m is supposed to be the message to be signed, and is needed by the actual function body.

Copy link
Contributor

Choose a reason for hiding this comment

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

Take a look at http://eips.ethereum.org/EIPS/eip-196 for instance - the precompile takes call data that is the direct encoding of the parameters, with no function selector (eg, it's not a Solidity-standard multi-function contract). All precompiles so far are like this; it'd be a good idea to retain consistency here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, got it. Thanks for pointing me in the right direction! So I believe the changes I pushed should address that.

@Arachnid
Copy link
Contributor

This is a courtesy notice to let you know that the format for EIPs has been modified slightly. If you want your draft merged, you will need to make some small changes to how your EIP is formatted:

  • Frontmatter is now contained between lines with only a triple dash ('---')
  • Headers in the frontmatter are now lowercase.

If your PR is editing an existing EIP rather than creating a new one, this has already been done for you, and you need only rebase your PR.

In addition, a continuous build has been setup, which will check your PR against the rules for EIP formatting automatically once you update your PR. This build ensures all required headers are present, as well as performing a number of other checks.

Please rebase your PR against the latest master, and edit your PR to use the above format for frontmatter. For convenience, here's a sample header you can copy and adapt:

---
eip: <num>
title: <title>
author: <author>
type: [Standards Track|Informational|Meta]
category: [Core|Networking|Interface|ERC] (for type: Standards Track only)
status: Draft
created: <date>
---

@gcolvin gcolvin merged commit 28da87d into ethereum:master Mar 28, 2018
@oberstet
Copy link
Contributor Author

@gcolvin Awesome, thx for merging!

As I am new here, may I ask for a tip? What is the best / acceptable way to promote or support this EIP?

  • Should I come up with an implementation PR for say go-ethereum?
  • Should I go to Ethereum forums/IRC and nudge people to support this EIP, "organizing support"?
  • Anything else? Did I miss some docs?

Thanks again!

@Arachnid
Copy link
Contributor

A good first step would be to ask @Souptacular to add this to the agenda for the next All Core Devs, and join the meeting to describe your EIP.

Being able to show a POC implementation on one or more clients wouldn't hurt.

@oberstet
Copy link
Contributor Author

@Arachnid thanks again for your hints! Will follow your advice.

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.

3 participants