-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
Create bigint_modexp.md #198
Conversation
EIPS/bigint_modexp.md
Outdated
|
||
<length_of_BASE> <length_of_EXPONENT> <length_of_MODULUS> <BASE> <EXPONENT> <MODULUS> | ||
|
||
Where every length is a 32-byte left-padded integer representing the number of bytes to be taken up by the next value. Call data is assumed to be infinitely right-padded with zero bytes, and excess data is ignored. Consumes `floor(max(length_of_MODULUS, length_of_BASE) ** 2 * max(length_of_EXPONENT, 1) / GQUADDIVISOR)` gas, and if there is enough gas, returns an output `(BASE**EXPONENT) % MODULUS` as a byte array with the same length as the modulus. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be safe assuming that it is unlikely to have calls consuming gigabytes of data?
If yes, how about making each length field 32 bits wide (or even less):
00000001
00000020
00000020
03
fffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc2e
fffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc2f
I understand we have 0 gas for 0 bytes in the payload, but does that make any sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"representing the number of bytes to be taken up by the next value" <- I don't think this is correct anymore. It used to be like that in the initial drafts, but not anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Zasss..
I like 32 bytes more for two reasons:
|
We could also pack them into a single 32 byte slot, that would make it easier. That would only need multiplication (2 times) and a single mstore. If however the motivation is to make it easy on ABI encoding, then I would change the gas calculation not to be based on supplied length, but based on first byte set. That would allow sending each field as a multiple of 32 bytes. Your example would become:
This way it is up to the caller's implementation to decide which is more beneficial: allocating more memory or using more instructions to pack them tight. |
Please make compliant with EIP-0 (eg, add a header with number/status etc), renumber to EIP 198 (or the issue number), update in response to any comments, and tag with |
@vbuterin What is the value of |
EIPS/bigint_modexp.md
Outdated
fffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc2e | ||
fffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc2f | ||
|
||
Would be parsed as a base of 0, exponent of `2**256 - 2**32 - 978` and modulus of `2**256 - 2**32 - 978`, and so would return 0. Notice how if the length_of_BASE is 0, then it does not interpret _any_ data as the base, instead immediately interpreting the next 32 bytes as length_of_EXPONENT. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
length_of_EXPONENT
-> EXPONENT
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? BASE
is followed by length_of_EXPONENT
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
L9: <length_of_BASE> <length_of_EXPONENT> <length_of_MODULUS> <BASE> <EXPONENT> <MODULUS>
Since length_of_BASE
is zero, it does not interpret any data as the base. The following 32 bytes (after the zero-length BASE
) are the EXPONENT
, not the length_of_EXPONENT
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, modulus is interpreted as 2**256 - 2**32 - 977
You're right. I was not following the format change in the last commit.
|
EIPS/bigint_modexp.md
Outdated
fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe | ||
fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffd | ||
|
||
Would parse a base length of 0, a modulus length of 32, and an exponent length of `2**256 - 1`, where the base is empty, the modulus is `2**256 - 2` and the exponent is `(2**256 - 3) * 256**(2**256 - 33)` (yes, that's a really big number). It would then immediately fail, as it's not possible to provide enough gas to make that computation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is base of length 0, an exponent of length 32 and modulus of length 2**256 - 1
.
The exponent is 2**256 - 2
, the modulus is (2**256 - 3) * 256**(2**256 - 33)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems correct. I apply this change.
Please update to follow the template here and rename to |
I support for paying only for significant bytes in the provided data (the same way it is done for exponent of EXP). If the field lengths are known at the "compile" time in most of the use cases, can we represent all lengths in a single word, with 32-bit reserved per field length?
|
This PR is meant to replace #101, but I'm not seeing big int addition, subtraction, multiplication or division in this PR. Are they going to be added or not? This affects the choice of addresses for the following precompiled contracts. |
What is recommended for doing bigint division now that the division precompile was removed? Currently I am getting the reciprocal of the divisor using the Newton-Raphson method and multiplying with the dividend using this contract. Is that sufficient? |
Division is incredibly slow - but there is an alternative for some cases. Modular reduction (and therefore exponentiation) for any cryptographic scheme operating over a public prime group (Schnorr, ECC, ElGamal) can be massively sped up when a prime is chosen that is close to a power of two (e.g. 2^255 - 19, 2^3072 - 77405 etc). The algorithm is really simple: https://gist.github.com/tristanhoy/01450b820864aa7205587581c7b12099 Additionally, if a set of recommended public primes are provided, the gas costs for exponentiation will be very predictable (unlike the product of two secret primes). |
@vbuterin do you mind if I append
? |
Because ethereum/EIPs#198 supersedes ethereum/EIPs#101 and the new version does not contain these.
@Arachnid You nominated this as an "EIPs that should be merged". Can you please share your notes on that here? |
This pull request is referenced by #609, which has already been merged. As such, it needs to either be finalised and merged, or the dependency in the existing EIP needs to be removed. |
Rename bigint_modexp.md to eip-198.md, add new prologue and copyrigt notice
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:
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:
|
* Create bigint_modexp.md * Update bigint_modexp.md * Update bigint_modexp.md * Update bigint_modexp.md * Update bigint_modexp.md * Update bigint_modexp.md * Update bigint_modexp.md * Update bigint_modexp.md * Update bigint_modexp.md * Update bigint_modexp.md * Update bigint_modexp.md * Update bigint_modexp.md * Update bigint_modexp.md * Fixing an example after the specification change https://github.com/ethereum/EIPs/pull/198/files#r106236783 * Fix one example after a format change https://github.com/ethereum/EIPs/pull/198/files#r109660742 * Rename bigint_modexp.md to eip-198.md, add new prologue and copyright notice.
Good projeck |
Precompile for bigint modular exponentiation. Meant to supersede #101.