-
Notifications
You must be signed in to change notification settings - Fork 21k
crypto(experiment): Add modexp abstraction plus GMP implementation (based off of #32174) #32187
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
base: master
Are you sure you want to change the base?
Conversation
Co-authored-by: Felix Lange <[email protected]>
- **macOS**: `brew install gmp` | ||
- **Windows**: See [GMP Windows builds](https://gmplib.org/) | ||
|
||
> TODO: There is an alternative branch, where we compile from source however this is slightly messier (though it allows us to pin to a specific commit like we do for libsecp256k1) |
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.
The other way to install gmp would be to download the gmp repo, compile it locally and then package the static lib. I have a repo that does it like that here for reference: https://github.com/kevaundray/go-gmp/tree/master?tab=readme-ov-file#building-from-source
Its not as nice as just apt install libgmp-dev and it requires someone to explicitly call a script to compile the gmp source code vs using apt/brew/dnf -- it does give more control over the GMP that gets included with geth though and the compile flags that are used
|
||
## License | ||
|
||
TODO -- whatever go-ethereum does. |
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.
TODO
// Special case: zero modulus | ||
allZero := true | ||
for _, b := range mod { | ||
if b != 0 { | ||
allZero = false | ||
break | ||
} | ||
} | ||
if allZero { | ||
return []byte{}, nil | ||
} |
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.
Haven't checked how GMP handles modulus == 0 -- if it does, then can remove this if its faster for gmp to do it
crypto/modexp/gmp/gmp.go
Outdated
// Keep the slices alive until after the C call completes | ||
runtime.KeepAlive(base) | ||
runtime.KeepAlive(exp) | ||
runtime.KeepAlive(mod) | ||
runtime.KeepAlive(result) |
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.
Common pattern to stop the slices being deallocated
} | ||
|
||
// Perform modular exponentiation | ||
mpz_powm(result_mpz, base_mpz, exp_mpz, mod_mpz); |
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.
Theres a powm_ui method for short exponents -- might want to benchmark to see if its worth it
if (*result_len < needed) { | ||
mpz_clears(base_mpz, exp_mpz, mod_mpz, result_mpz, NULL); | ||
return -2; | ||
} |
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 should never trigger since the golang code allocated enough bytes to hold the modulus
import ( | ||
"github.com/ethereum/go-ethereum/crypto/modexp/gmp" | ||
) | ||
|
||
// ModExp performs modular exponentiation on byte arrays | ||
// result = base^exp mod mod | ||
// This uses the gmp implementation by default. | ||
// To use bigint implementation, import crypto/modexp/bigint directly. | ||
func ModExp(base, exp, mod []byte) ([]byte, error) { | ||
return gmp.ModExp(base, exp, mod) | ||
} |
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.
Similar API to bn254, can switch to big.Int by changing the import
This PR is based off of #32184 so the diff is larger than it seems and supersedes #32174
There are some optimizations that have not been transferred over from #32174 -- these are likely fine, since we mainly want to see the diff with gmp.