Separate BLS key from priv_validator_key.json

[gitferry]

Should consider utilizing some keyring for bls keys storage like ethereum bls key store - https://github.com/ethereum/ercs/blob/master/ERCS/erc-2335.md, or most recent versions of comet bft have support for bls keys - https://github.com/cometbft/cometbft/tree/main/crypto/bls12381

[gitferry]

This should be done in two steps:

1. Naively separate BLS key from priv_validator_key.json as currently it is embeded with comet's ed25519 key for consensus.
2. Store the BLS key securely following https://github.com/ethereum/ercs/blob/master/ERCS/erc-2335.md
3. ensure the BLS key file is created atomically with priv_validator_key.json and remove create-bls-key cmd

[gitferry]

Some context of why and how BLS key is used can be found here.

The BLS key should be treated as important as the ed25519 key used in Comet consens. Currently it is embeded in the priv_validator_key.json and validators should use babylond create-bls-key to generate the BLS key and bond with the ed25519 key generated in pior. However, cosmos tooling (e.g., tmkms and horcrux) cannot recognize the bls key in the priv_validator_key.json, so now we decided to separate it from priv_validator_key.json

In order for secure management of the BLS key, after separation, the BLS key should be kept following standard proposed by Ethereum https://github.com/ethereum/ercs/blob/master/ERCS/erc-2335.md so that it can work with related toolings.

[dongsam]

Hi, I am an engineer at B-Harvest. Our team plans to work on this issue, and we are considering proceeding as follows. If there are any issues or alternative suggestions, I’d appreciate it if you could share them.

1. Preserve priv_validator_key.json in its original form for compatibility with consensus-related logic and tools

• Modify WrappedFilePVKey to ensure it does not overwrite the existing priv_validator_key.json.

{
-  "acc_address": "bbn17m08...",
  "address": "78C5FF6E441E3...",
  "pub_key": {
    "type": "tendermint/PubKeyEd25519",
    "value": "IAnqptCTD3pUKhnt...="
  },
  "priv_key": {
    "type": "tendermint/PrivKeyEd25519",
    "value": "ZnDpyEjrv7Iyat5trmM...=="
  },
-  "bls_pub_key": "r2Q3kAeky1A2GqYAt...",
-  "bls_priv_key": "WeSnvWnqBLN...="
}

2. Implement ERC-2335

• Store and manage the separated BLS-related keys and fields in compliance with the ERC-2335 standard.
• Since Prysm keystores (an Ethereum 2.0 consensus client) implements BLS and ERC-2335-based consensus signing, we can reference their flow and code.
• Internally, go-eth2-wallet-encryptor-keystorev4/ is used to implement ERC-2335.
  Reference: source code.
• Plan to align with ERC-2335’s standards and encryption methods while modifying related configurations, commands, logic, and documentation to accommodate the addition of passwords.
• Let us know if there are other tools or repositories dependent on the legacy BLS method so we can address them together.
• Performing 1 and 2 together in a single PR seems to minimize redundant code changes.

---

Consideration / Questions

A. Removing DelegatorAddress from WrappedFilePVKey

• Before proceeding with the above, we need to consider how to store the remaining DelegatorAddress from WrappedFilePVKey in compliance with the ERC-2335 format, as CometBFT and BLS-related fields are excluded.

• However, a review of the code suggests that DelegatorAddress is not essential for BLS signing. It appears to be primarily used in GetBLSSignerAddress() of BlsSigner.

• Since WrappedFilePVKey already contains validator consensus public key information, and the validator corresponding to the consensus public key can be obtained via the GetValidatorByConsAddr function of the Epoch(Staking) module, it seems unnecessary to retain DelegatorAddress.

• Here is a pseudo-diff code I quickly prepared for reference. I would appreciate your feedback.

   		// 1. check if itself is the validator as the BLS sig is only signed
   		// when the node itself is a validator
  -		signer := k.GetBLSSignerAddress()
  +		valConsAddr := k.GetValConsAddress()
   		curValSet := k.GetValidatorSet(ctx, epoch.EpochNumber)
  -		_, _, err := curValSet.FindValidatorWithIndex(signer)
  +		val, err := k.GetValidatorByConsAddr(ctx, valConsAddr)
  +		if err != nil {
  +			panic(fmt.Errorf("the BLS signer's consensus address %s is not in the validator set", valConsAddr.String()))
  +		}
  +		signer, _ := sdk.ValAddressFromBech32(val.GetOperator())
  +		_, _, err = curValSet.FindValidatorWithIndex(signer)
   		if err != nil {
   			// NOTE: this indicates programmatic error because ExtendVote
   			// should not be invoked if the validator is not in the
  

B. Consider migration flow

Should we focus only on new users, or also consider migration flows for users who have already integrated legacy priv_validator_key.json with BLS (e.g., in existing testnets, genesis)?

C. Random password

• For the ERC-2335-based BLS key file, a password is required to decrypt it.
• For the mentioned 3

“3. ensure the BLS key file is created atomically with priv_validator_key.json and remove create-bls-key cmd”

• would it work to generate a random password and save it in a separate file alongside the key file?
• As a reference, Prysm generates a separate file for passwords to enable automatic unlocking.

[gitferry]

Hey @dongsam, sorry for the late reply. Thanks for taking this issue. The two steps approaching this sounds good to me. And I agree that combining 1 and 2 in one PR avoids most redundant code changes but I in general I prefer to have smaller PRs if possible as it is more friendly for review. Let's see how we can split it well.

Some relies to coments/questions:

A. Removing DelegatorAddress from WrappedFilePVKey

The WrappedFilePVKey is adapted from cometbft/privval/file.go to include BLS key management. And you are right, the DelegatorAddress is the account address of the validator and used for checking if the validator is in the active validator set. Acutally, I was thinking maybe this check is unnecessary because calling ExtendVote already indicates this fact.

I would be ideal if we have embed cometbft's FilePVKey into WrappedFilePV so that we can reuse the code from cometbft to manage the original ed25519 key for consensus. Like the following:

// WrappedFilePVKey wraps FilePVKey with BLS keys.
type WrappedFilePVKey struct {
	CometPVKey       privval.FilePVKey `json:"comet_pv_key"`
	BLSPVKey         erc2335.BLSPVKey  `json:bls_pv_key`
}

B. Consider migration flow

We probably don't need migration. Let's focus on new users for now.

C. Random password

would it work to generate a random password and save it in a separate file alongside the key file?

Storing the password in a separate file sounds good to me if it is part of ERC-2335. But maybe I was missing something, why it has to be a random password? Can it be a prompt asking user to specify the password when creating the BLS key?

[dongsam]

Hi @gitferry, Thanks for your reply.
I’ll work on it, and if the PR gets too large, I’ll try to split it as much as possible to make it easier to review.

The WrappedFilePV structure you mentioned looks good. I’ll work on implementing it in a way that reuses the cometBFT’s FilePVKey type as much as possible.

The reason I brought up the random password was a concern about breaking atomicity the

3. ensure the BLS key file is created atomically with priv_validator_key.json

when no password is specified via flags like --password. However, if it’s okay to prompt asking for the password, then there’s no need to use a random password. I’ll proceed with that approach. Thanks.

[gitferry]

Thanks @dongsam.

ensure the BLS key file is created atomically with priv_validator_key.json

A note about the above: this is a secondary requirement. Ideally, we want the creation of BLS keys to be transparent to the user. That is, when the user creates the ed25519 key (i.e., priv_validator_key.json), the BLS key is generated automatically (the cmd of create-bls-key is for backup).

[gitferry]

Some notes from offline discussion:

• BLS separation (1 and 2) delivered by the next week (Week 1) including necessary tests
• BLS key migration from priv_validator_key.json to two separate files should start right after BLS separation is finished and be delivered by Week 2
• (3) is secondary and can discuss timeline after previous deliverables

[wnjoon]

After the meeting, I created the structures of WrappedFilePVKey, CometPVKey, and BLSPVKey respectively.

// WrappedFilePVKey wraps FilePVKey with BLS keys.
type WrappedFilePVKey struct {
	CometPVKey       privval.FilePVKey `json:"comet_pv_key"`
	BLSPVKey         erc2335.BLSPVKey  `json:bls_pv_key`
}

type CometPVKey struct {
	Address          types.Address       `json:"address"`
	PubKey           cmtcrypto.PubKey    `json:"pub_key"`
	PrivKey          cmtcrypto.PrivKey   `json:"priv_key"`
}

type BLSPVKey struct {
	BlsPubKey        bls12381.PublicKey  `json:"bls_pub_key"`
	BlsPrivKey       bls12381.PrivateKey `json:"bls_priv_key"`
}

If the BLSPVKey structure is the same as ERC2335, the user creates BlsPrivKey using a password every time for signing.

How about creating a .json file for the key according to the ERC2335 structure, but configuring the BLSKey structure of WrappedFilePVKey as the BlsPubKey and BlsPrivKey in the same structure as the existing structure?

The BLS key to be created as a .json file will follow the ERC2335 structure as below. In other words, we want to call the Save() function with a separate structure.

type BLSFileKey struct {
	Crypto struct {
		KDF struct {
			Function string `json:"function"` // "pbkdf2"
			Params struct {
				Dklen int    `json:"dklen"`  // 32
				C     int    `json:"c"`      // 262144
				Prf   string `json:"prf"`    // "hmac-sha256"
				Salt  string `json:"salt"`
			} `json:"params"`
			Message string `json:"message"`
		} `json:"kdf"`
		Checksum struct {
			Function string                 `json:"function"` // "sha256"
			Params   map[string]interface{} `json:"params"`
			Message  string                 `json:"message"`
		} `json:"checksum"`
		Cipher struct {
			Function string `json:"function"` // "aes-128-ctr"
			Params struct {
				IV string `json:"iv"`
			} `json:"params"`
			Message string `json:"message"`
		} `json:"cipher"`
	} `json:"crypto"`
	Description string `json:"description"` // "BLS-12381 private key"
	Pubkey     string `json:"pubkey"`      // hex-encoded public key
	Path       string `json:"path"`        // "m/12381/60/0/0"
	UUID       string `json:"uuid"`        // random UUID
	Version    int    `json:"version"`     // 4
	
	filePath   string
}

[gitferry]

For CometPVKey, can we reuse cometbft's FilePVKey?

If the BLSPVKey structure is the same as ERC2335, the user creates BlsPrivKey using a password every time for signing.
How about creating a .json file for the key according to the ERC2335 structure, but configuring the BLSKey structure of WrappedFilePVKey as the BlsPubKey and BlsPrivKey in the same structure as the existing structure?

This is to load the private key into memory once during initiation, right? I wouldn't mind but how does Ethereum handle this situation? Eth validators also need automatic signing in consensus, right?

The BLS key to be created as a .json file will follow the ERC2335 structure as below. In other words, we want to call the Save() function with a separate structure.

Yeah, I would imagine we will have an ERC2335 like this

[wnjoon]

@gitferry

I have a question during refactoring.

func InitializeNodeValidatorFilesFromMnemonic(config *cfg.Config, mnemonic string, addr sdk.AccAddress) (nodeID string, valKeys *privval.ValidatorKeys, err error) {
	//...
	var filePV *privval.WrappedFilePV
	if len(mnemonic) == 0 {
		filePV = privval.LoadOrGenWrappedFilePV(pvKeyFile, pvStateFile)
	} else {
		privKey := cmted25519.GenPrivKeyFromSecret([]byte(mnemonic))
		blsPrivKey := bls12381.GenPrivKeyFromSecret([]byte(mnemonic))
		filePV = privval.NewWrappedFilePV(privKey, blsPrivKey, pvKeyFile, pvStateFile)
	}
	filePV.SetAccAddress(addr)
        //...
}

In the above code, the mnemonic value is commonly used when creating ed25519 and bls keys. I wonder if this is intentional code or if it is lightly written code for a test environment.

[dongsam]

@gitferry

This is to load the private key into memory once during initiation, right? I wouldn't mind but how does Ethereum handle this situation? Eth validators also need automatic signing in consensus, right?

Yes, similarly, Ethereum Consensus client Prysm, which uses ERC-2335 for key storage and BLS signing, also caches the decrypted BLS key in memory for faster signing. [code]

// Initialize public and secret key caches that are used to speed up the functions
// FetchValidatingPublicKeys and Sign
func (km *Keymanager) initializeKeysCachesFromKeystore() error {
	lock.Lock()
	defer lock.Unlock()
	count := len(km.accountsStore.PrivateKeys)
	orderedPublicKeys = make([][fieldparams.BLSPubkeyLength]byte, count)
	secretKeysCache = make(map[[fieldparams.BLSPubkeyLength]byte]bls.SecretKey, count)
	for i, publicKey := range km.accountsStore.PublicKeys {
		publicKey48 := bytesutil.ToBytes48(publicKey)
		orderedPublicKeys[i] = publicKey48
		secretKey, err := bls.SecretKeyFromBytes(km.accountsStore.PrivateKeys[i])
		if err != nil {
			return errors.Wrap(err, "failed to initialize keys caches from account keystore")
		}
		secretKeysCache[publicKey48] = secretKey
	}
	return nil
}

[gitferry]

@wnjoon hey, tbh I can't remember but I think that's for testing purpose

[gitferry]

@dongsam thanks! This sounds good to me

[gitferry]

@dongsam Please ensure node cannot be started if either the BLS key file or priv_validator.key.json is not setup or not well formatted

[dongsam]

@gitferry
Yes, I already heard from a team member about the situation in the testnet.

As part of the basic logic of cometBFT, whether it’s a non-validator node or a validator node, the priv_validator_key.json is automatically generated if it’s not already set up. Therefore, if the priv_validator_key.json is properly in place, I think the start condition should be determined based on the presence or absence of the BLS key. However, if it is automatically generated under condition (3), it might pose a risk. What do you think?

What about allowing the key to be explicitly set up during the init or create-bls-key commands, where it takes a password to generate it automatically, and ensuring that it doesn’t get auto-generated during the start command? If the BLS key is missing, it should fail to start with an error.

Should non-validator nodes that are not validators and do not participate in signing also be subject to the same conditions for BLS key validation?
c.c @wnjoon

[gitferry]

As part of the basic logic of cometBFT, whether it’s a non-validator node or a validator node, the priv_validator_key.json is automatically generated if it’s not already set up. Therefore, if the priv_validator_key.json is properly in place, I think the start condition should be determined based on the presence or absence of the BLS key. However, if it is automatically generated under condition #336 (comment), it might pose a risk. What do you think?

I think the current status in the testnet is that, people may start running the node before setting up the BLS keys so that the keys were not loaded properly.

What about allowing the key to be explicitly set up during the init or create-bls-key commands, where it takes a password to generate it automatically, and ensuring that it doesn’t get auto-generated during the start command? If the BLS key is missing, it should fail to start with an error.

It would be ideal if the BLS keys can be generated in the init command in a separate file along with priv_validator_key.json. And before babylon node is started, it should check whether the key files are there and well formatted.

Should non-validator nodes that are not validators and do not participate in signing also be subject to the same conditions for BLS key validation?

Technically we should follow how non-validator nodes are treaded against priv_validator_key.json. I think priv_validator_key.json will be generated either way, right?

[dongsam]

@gitferry

It would be ideal if the BLS keys can be generated in the init command in a separate file along with priv_validator_key.json. And before babylon node is started, it should check whether the key files are there and well formatted.

Got it, we’re planning to split it into two PRs.

• The first PR will separate the BLS key from priv_validator_key and store it in ERC2335 format while ensuring it still works with the current instructions.
• The second PR will remove the requirement to include the DelegatorAddress in the existing BLS key file format, allowing BLS key generation during the init stage without needing a separate create-bls-key command. Once the second PR is completed, the approach you mentioned will be fully implemented.

Technically we should follow how non-validator nodes are treaded against priv_validator_key.json. I think priv_validator_key.json will be generated either way, right?

Yes, the current cometBFT code priv_validator_key.json is automatically generated for non-validator nodes as well. The BLS key can follow the same approach.

However, since ERC2335 requires a password, non-validator nodes would need to go through a password input step even if they don’t immediately use it, which could slightly impact the UX.

[gitferry]

The two PRs sounds good to me!

However, since ERC2335 requires a password, non-validator nodes would need to go through a password input step even if they don’t immediately use it, which could slightly impact the UX.

Ah, if so, we probably don't need non-validator to go through this, but how do we know if the node is a validator? Is it specified explicitly by any flag or config when starting the node?

[dongsam]

Currently, there is no explicit flag to indicate whether a node is a validator. To determine this clearly, we would need to get validator info from the staking module to check if the consensus public key of the current node is registered as a validator. However, this requires accessing the state, which can only be done after the app initialization is complete, making the process a bit more complex.

For the first PR, we’ll skip validating whether the node is a validator and instead check the presence and format of the BLS key for all nodes. In the second PR, we’ll refactor and explore better ways to verify if a node is a validator.

[gitferry]

@dongsam Sounds good to me. Thanks for the explanation!

[dongsam]

We opened the first PR #396 as a draft in case there might be some waiting. We haven’t fully verified the CI pass yet, so there’s still some debugging to do on the breaking parts. However, the code structure, overall direction, and main logic are fairly finalized. Feel free to take a look and leave comments if you’d like. Anyway, once the CI passes, we’ll update the PR to R4R and formally request your review.