search

prysmaticlabs / prysm

Go implementation of Ethereum proof of stake
https://www.offchainlabs.com
GNU General Public License v3.0
3.45k stars 984 forks source link

Outdated EIP2333 version #7763

Closed CarlBeek closed 3 years ago

CarlBeek commented 3 years ago

Outdated EIP2333 version

Description

A few months ago, EIP 2333 was updated so that hkdf_mod_r matched the KeyGen function update in the BLS specs. Prysm is still using the old version.

🔬 Minimal Reproduction

Try run the EIP's tests against your implementation.

The fix

Update hkdf_mod_r to the new version with the while loop as per EIP2333 and run it against the given test vectors:

1. salt = "BLS-SIG-KEYGEN-SALT-"
2. SK = 0
3. while SK == 0:
4.     salt = H(salt)
5.     PRK = HKDF-Extract(salt, IKM || I2OSP(0, 1))
6.     OKM = HKDF-Expand(PRK, key_info || I2OSP(L, 2), L)
7.     SK = OS2IP(OKM) mod r
8. return SK
rauljordan commented 3 years ago

Hi @CarlBeek we just tested this out in #7783 and are compliant with the tests. The reason there might be a misunderstanding is because we vendored in an old dependency to support old users of the Medalla testnet

https://github.com/prysmaticlabs/prysm/blob/d3ca9985eb9c00d1d7f1fc69b1c48bc6c54f5c7e/validator/keymanager/derived/keymanager.go#L491

We have a tracking issue to remove this old dependency before Prysm's mainnet release here #7752, thanks for bringing this up as now we have included the tests into our repo