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-1108 updated with the new gas costs #1452

Merged
merged 3 commits into from
Sep 26, 2018

Conversation

pdyraga
Copy link
Contributor

@pdyraga pdyraga commented Sep 26, 2018

We benchmarked Parity client and adjusted proposed new gas costs to the
results. (Parity is slower than Go-Ethereum)

  • proposed ECADD cost has been updated to 150
  • proposed ECMUL cost has been updated to 6 000
  • proposed pairing check cost has been updated to 28 300k + 35 450.

Pairing check cost has been scaled down by 2.82 factor, since Parity,
after the recent updates, is ~2.82 times faster for pairing check.

We benchmarked Parity client and adjusted proposed new gas costs to the
results. (Parity is slower than Go-Ethereum)

- proposed `ECADD` cost has been updated to 150
- proposed `ECMUL` cost has been updated to 6 000
- proposed pairing check cost has been updated to 28 300k + 35 450.

Pairing check cost has been scaled down by 2.82 factor, since Parity,
after the recent updates is `~2.82` times faster for pairing check.
EIPS/eip-1108.md Outdated
| Pairing check | `0x08` | 80 000k + 100 000<sup>[2]</sup>| 5 500k + 80 000 |
| `ECADD` | `0x06` | 500<sup>[1]</sup> | 150 |
| `ECMUL` | `0x07` | 40 000<sup>[1]</sup> | 6 000 |
| Pairing check | `0x08` | 80 000k + 100 000<sup>[2]</sup>| 28 300k + 35 450 |
Copy link
Member

Choose a reason for hiding this comment

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

Is this really 28300k = 28300000 gas ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a bit misleading but if you look at the EIP referenced in [2], the cost is expressed as:

The gas costs of the precompiled contract are 80 000 * k + 100 000, where k is the number of points or, equivalently, the length of the input divided by 192.

The k seems like a wrong choice of a letter but we did not want to move away from that notation to do not cause even more confusion.

Copy link
Member

Choose a reason for hiding this comment

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

To be clear the cost is expressed as 80 000 * k + 100 000. I'd suggest you use the same notation to avoid confusion, that is, do not omit the multiplication sign :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, yeah! My fault. Will fix it in a second...

@axic
Copy link
Member

axic commented Sep 26, 2018

@Shadowfiend can you please review and approve if you agree?

@eip-automerger
Copy link

Hi! I'm a bot, and I wanted to automerge your PR, but couldn't because of the following issue(s):

`k` represents the number of points we want to check the pairing for. The `80 000k` is confusing so we are altering the gas cost to `80 000 * k`
Copy link
Contributor

@Shadowfiend Shadowfiend left a comment

Choose a reason for hiding this comment

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

Looks good to me, benchmarks are pretty comprehensive as well and glad we can link to them from the EIP; that was a big missing component before.

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