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

feat: BLS keystore improvement #467

Merged
merged 6 commits into from
Feb 5, 2025
Merged

Conversation

gitferry
Copy link
Member

@gitferry gitferry commented Feb 3, 2025

Closes #336

wnjoon and others added 4 commits January 15, 2025 16:38
## Description

This PR was created from epic #336, separates BLS key from
priv_validator_key.json, and applies ERC2335.

There are still issues in CI that are being resolved, but I'm uploading
a PR to first discuss the changed structure and ideas.

### ERC2335 structure

- Use
[keystorev4](github.com/wealdtech/go-eth2-wallet-encryptor-keystorev4)
with [prysm](https://github.com/prysmaticlabs/prysm) as reference
- Add ERC2335 encryption/decryption and password generation functions

### Change WrappedFilePV to include BLS structure and use cometbft's
FilePV directly

```go
type WrappedFilePVKey struct {
	CometPVKey privval.FilePVKey
	BlsPVKey   BlsPVKey
}
```

### BLS separation

- At the time of `babylond init`, only priv_validator_key.json is
created.
- `babylond create-bls-key` creates a BLS key with the password entered
as the prompt and encrypts it into an ERC2335 structure.
- The generated BLS key in ERC2335 format is saved as `bls_key.json`,
and the password is saved as `bls_password.txt`.

An example of a `bls_key.json` is below:

```json
{
  "crypto": {
    "checksum": {
      "function": "sha256",
      "message": "d291102754ece0a5ab9d4c3c75f452069d192df6c90d616e466d7e7884f41b3a",
      "params": {}
    },
    "cipher": {
      "function": "aes-128-ctr",
      "message": "e638d3f04b803c3ec934c31749ed20c1ffcd48ab481b4fad6b303aa2e7ff991c",
      "params": {
        "iv": "473e315746ec9997f0d0e423f73e2d3d"
      }
    },
    "kdf": {
      "function": "pbkdf2",
      "message": "",
      "params": {
        "c": 262144,
        "dklen": 32,
        "prf": "hmac-sha256",
        "salt": "3f1b139f3663082f582acc6d48fd9bf8caf4ea9c4e51c6ef75c9bc5b5c04ee9e"
      }
    }
  },
  "version": 4,
  "uuid": "",
  "path": "",
  "pubkey": "a28b189e76e8d61461811eec22bc8d5c926bf1276afde5344a3b9d3a2769a1a61ea42471a867d4811a85640dfde8fed702d81bcc9f56494ec65471151fac6e3c262541245f1b16c9e22000eef2142c43f6ad5ca8c31fc16081261cc54d3f034c",
  "description": "bbn196xcr99y5eehdg6lr38u8re0j6xeq0twup5rl4"
}
```

An example of a `priv_validator_key.json` with BLS separated is below:

```json
{
  "address": "3141C6C8EF5FDAE49931AE2F9EB295F5C9E2212A",
  "pub_key": {
    "type": "tendermint/PubKeyEd25519",
    "value": "CwCehdhn8DmcVTKn4I1RHzlmur5LbEZxSEr+zvf7Ifo="
  },
  "priv_key": {
    "type": "tendermint/PrivKeyEd25519",
    "value": "HIHCMsU9+J3FjEU7gooSed5moKRjar6TfLa3Nb1CSCULAJ6F2GfwOZxVMqfgjVEfOWa6vktsRnFISv7O9/sh+g=="
  }
}
```

### Change DelegatorAddress to be stored in the description field of
ERC2335 structure instead of storing it in `priv_validator_key.json`.

- DelegatorAddress is stored in the description field of the
`bls_key.json` file in erc2335 format.

```go
type BlsPVKey struct {
	PubKey  bls12381.PublicKey  `json:"bls_pub_key"`
	PrivKey bls12381.PrivateKey `json:"bls_priv_key"`
	DelegatorAddress string
        // ...
}
```

## Test script

$ ./babylond init my-node --chain-id test-chain-1

$ ./babylond keys add validator1 --keyring-backend test

$ ./babylond add-genesis-account $(./babylond keys show validator1 -a
--keyring-backend test) 2000000000000ubbn,2000000000000stake

$ ./babylond gentx validator1 1500000000000stake --chain-id test-chain-1
--keyring-backend test --fees 400ubbn

$ ./babylond collect-gentxs

$ ./babylond create-bls-key $(./babylond keys show validator1 -a
--keyring-backend test)

$ ./babylond gen-helpers create-bls

After the create-bls command, json file starting with the prefix
"gen-bls" will be created in {home}/.babylond/config.

> example:
gen-bls-bbnvaloper196xcr99y5eehdg6lr38u8re0j6xeq0twpr77n5.json

Copy data in the created file to the 'checkpointing.genesis_keys' field
in genesis.json.

Here is an example after inserting data to 'checkpointing.genesis_keys'
field in genesis.json

```json
"checkpointing": {
      "genesis_keys": [
        {
          "validator_address": "bbnvaloper196xcr99y5eehdg6lr38u8re0j6xeq0twpr77n5",
          "bls_key": {
            "pubkey": "oosYnnbo1hRhgR7sIryNXJJr8Sdq/eU0SjudOidpoaYepCRxqGfUgRqFZA396P7XAtgbzJ9WSU7GVHEVH6xuPCYlQSRfGxbJ4iAA7vIULEP2rVyowx/BYIEmHMVNPwNM",
            "pop": {
              "ed25519_sig": "9YrVS1/KN0Fq6po0JbWirX+0enhxiShqxVw+lQFn22qam/2KaZXXzNMsOOOBoyQ9ZSD7ARc/TtohYBbhLIF/Cg==",
              "bls_sig": "pRAZUAH2ZDPZqgoiPTsFYTYndBctJXoY1lvN5GsWNNbPDYyIeXihaS28pqauHCcy"
            }
          },
          "val_pubkey": {
            "key": "CwCehdhn8DmcVTKn4I1RHzlmur5LbEZxSEr+zvf7Ifo="
          }
        }
      ]
    },
```

## Subsequent tasks

- Ensure the BLS key file is created atomically with
priv_validator_key.json
- BLS key non-validation logic for non-validator
- Remove DelegatorAddress from the `BlsPV` structure and obtain it from
`GetValidatorByConsAddr()` function of the Epoch(Staking) module.
- ~~Add the option to use the `--bls-password` flag when creating the
BLS key~~
[1f14b31](1f14b31)
- Modify not to keep `WrappedFilePV` since LastSignState is not used

```go
type WrappedFilePV struct {
	Key           WrappedFilePVKey
	LastSignState privval.FilePVLastSignState
}
```
…add migration cmd

This PR includes the following changes:

**1. Remove delegator address in WrappedFilePV:**

- Removed Delegator Address field from WrappedFilePV. Delegator Address
can be obtained from kvStore of x/checkpointing module via bls public
key.
- Simplify the structure of WrappedFilePV.

**2. Addition of InitCmd**:

- InitCmd has been updated to generate both Comet and BLS keys through
babylond init.
- The --bls-password flag allows you to specify a BLS password. If the
flag is not provided, the password will be requested interactively via a
prompt.

**3. BLS Key File Migration**:

- The babylond migrate-bls-key command allows the
priv_validator_key.json file from previous versions to be converted into
the separated key file structure used in the current version.
- If the priv_validator_key.json file does not exist or already contains
only Comet key information (indicating it is in the latest format), no
migration will occur.

**4. Fix for `e2e-run-upgrade-v1` Error**:

- Resolved an issue in e2e-run-upgrade-v1 during e2e testing caused by
differences in the key file structures between the current version and
the images pulled from Docker Hub for pre-upgrade containers.
- Introduced a new build tag, e2e_upgrade, applied when building the e2e
Docker image. This ensures that `babylond start` checks the
priv_validator_key.json file, and if it is in an older format, it is
automatically migrated.

And we suggest a new discussion about lightweight BlsSigner interface
with remove FilePVKey from comet in WrappedFilePV.

- When BLS public key is known, the validator address required for Vote
extensions can be obtained using the kvStore of the `x/checkpointing`
module.
- Therefore, the BlsSigner interface can be fully implemented in
BlsPVKey, and the FilePV from comet is not needed.
- In cases where CometBFT uses a PrivValidator that does not rely on
FilePV (e.g., Horcrux or other SignerClient), the FilePV will not be
present. Note that remote signers are currently not supported.
- [comment for
reference](b-harvest#8 (comment))
- Please consider whether this proposal is necessary and give us your
opinion.
## Description

This PR includes:
- Removal of the dependency on FilePV in Comet
- Addition of the `GetValAddr` function to retrieve the validator
address corresponding to the BLS key
- Move BLS functions from `privval` to `app/signer` since BLS is no
longer associated with PV
- Support for a remote signer

### Removal of dependency on FilePV of comet 

Previously, the method retrieved values from the FilePV structure, which
was created using the locally stored `priv_validator_key.json` during
`ExtendVote`. As a result, `ExtendVote` could not be performed when the
`priv_validator_key.json` was not available locally, such as in cases
using a remote signer.

To address this, we have removed the dependency on FilePV during
`ExtendVote` and added the `GetValAddr` function to the checkpointing
module, which returns the validator address corresponding to the BLS
key.

The public key corresponding to the validator address can be obtained
from the epoch module, which manages the validator list. In other words,
the information previously required from `priv_validator_key.json` for
vote extension can now be derived through the BLS key, eliminating the
need for FilePV.

Additionally, in the mainnet operation code, FilePV generated from
`priv_validator_key.json` is not used during app creation. Therefore, we
have modified the process to inject only BlsSigner at the time of app
creation.

For testing, we implemented a one-time Ed25519-based private key and
modified the Node structure to return the private key generated during
InitChain.

In conclusion, the WrappedFilePV structure has been removed.

### Support of remote signer 

For testing, we used [tmkms](https://github.com/iqlusioninc/tmkms) and
softsign. Here is a test scenario:

**1. Create priv_validator_key.json file via `babylond init`**

```sh
$ babylond keys add validator1 --keyring-backend test
```

**2. Set up and start `tmkms`**

```sh
#  First, move to working directory for tmkms and init
$ tmkms init config
$ tmkms softsign keygen ./config/secrets/secret_connection_key

# When init babylon, priv_validator_key.json is created to default path, <home>/.babylond.config. 
# Copy created priv_validator_key.json into working directory
$ cp ~/.babylond/config/priv_validator_key.json config/secret/

# Import the private validator key into tmkms 
$ tmkms softsign import ~/.babylond/config/priv_validator_key.json config/secrets/priv_validator_key
```

And modify `tmkms.toml`

```toml
# Tendermint KMS configuration file

## Chain Configuration

### Cosmos Hub Network

[[chain]]
id = "test-chain-1"
key_format = { type = "bech32", account_key_prefix = "bbnpub", consensus_key_prefix = "bbnvalconspub" }
state_file = "${tmkms_working_directory}/config/state/priv_validator_state.json"

## Signing Provider Configuration

### Software-based Signer Configuration

[[providers.softsign]]
chain_ids = ["test-chain-1"]
key_type = "consensus"
path = "${tmkms_working_directory}/config/secrets/priv_validator_key"

## Validator Configuration

[[validator]]
chain_id = "test-chain-1"
addr = "tcp://127.0.0.1:26659"
secret_key = "${tmkms_working_directory}/config/secrets/secret_connection_key"
protocol_version = "v0.34"
reconnect = true
```

Start `tmkms` 

```sh
$ tmkms start -c config/tmkms.toml
```

Since Babylon is not running yet, error logs may occur.

```sh
2025-01-31T07:44:23.266272Z  INFO tmkms::commands::start: tmkms 0.14.0 starting up...
2025-01-31T07:44:23.267372Z  INFO tmkms::keyring: [keyring:softsign] added consensus Ed25519 key: bbnvalconspub1zcjduepqugxyehru62jtl2wjkwapcs0yuld7sv76u29yss4lw4t2ge62adaqqpepmm
2025-01-31T07:44:23.267512Z  INFO tmkms::connection::tcp: KMS node ID: 0f286d871c04ccd86fa8f6eaf48d3094cf62ad4e
2025-01-31T07:44:23.268826Z ERROR tmkms::client: [test-chain-1@tcp://127.0.0.1:26659] I/O error: Connection refused (os error 61)
```

**3. Create account and set up `genesis.json`**

```sh
$ babylond keys add validator1 --keyring-backend test
$ babylond add-genesis-account $(./babylond keys show validator1 -a --keyring-backend test) 2000000000000ubbn,2000000000000stake
$ babylond gentx validator1 1500000000000stake --chain-id test-chain-1 --keyring-backend test --fees 400ubbn
$ babylond collect-gentxs
$ babylond gen-helpers create-bls $(./babylond keys show validator1 -a --keyring-backend test)
```

Add the newly created `gen-bls-*.json` contents to the
`checkpointing.genensis_key` field in `genesis.json`, located in
`.babylond/config`. Here is an example in `genesis.json`

```json
"checkpointing": {
  "genesis_keys": [
    // Start of gen-bls-*.json file
    {
      "validator_address": "bbnvaloper14gdwjrs7j55wy8f5sazhm4l7m9e2vpjgs7sxmh",
      "bls_key": {
        "pubkey": "oJMUZFWxyK9BdgNBr01SZhgXPIYmxhXrnQArEpDsYKFE8k5+R1KlUS6JyV3pkpsjCDCUOFYVV+Bc0IkbQaoTPXlVYrileOW3n3AnvCJH6B+vVvRPrJWyLjFFAZ8CH40u",
        "pop": {
          "ed25519_sig": "yxEyuVNUSz0Y6qM85LSGyliWOPAFlGF/BnSvOzrhKb/X4DHhEVyg0us/J+Jay54tR8GaJr3RefRZyxGsz+N5Dg==",
          "bls_sig": "lHgX9bNntjj1ifP1hckirp7Slo1P0ei+1uRtPAKasvIJkIf3A4P+lHrd0ZLibsNM"
        }
      },
      "val_pubkey": {
        "key": "4gxM3HzSpL+p0rO6HEHk59voM9riikhCv3VWpGdK63o="
      }
    }
   // End of gen-bls-*.json file
  ]
}
``` 

**4. Change `config.toml` of Babylon to use remote signer**

Comment out the existing entry that uses a local file, and write the
priv_validator_laddr entry as follows:

```toml
# Path to the JSON file containing the private key to use as a validator in the consensus protocol
# priv_validator_key_file = "config/priv_validator_key.json"

# Path to the JSON file containing the last sign state of a validator
# priv_validator_state_file = "data/priv_validator_state.json"

# TCP or UNIX socket address for CometBFT to listen on for
# connections from an external PrivValidator process
priv_validator_laddr = "tcp://0.0.0.0:26659"
```

**5. Remove `priv_validator_key.json` from `.babylond/config`**

After creating the `gen-bls-*.json` files, there is no longer a need for
the `priv_validator_key.json` file in `.babylond/config`.

**6. Start Babylon**

```sh
$ babylond start
```

```sh
4:51PM INF starting node with ABCI CometBFT in-process module=server
4:51PM INF service start impl=multiAppConn module=proxy msg="Starting multiAppConn service"
4:51PM INF service start connection=query impl=localClient module=abci-client msg="Starting localClient service"
4:51PM INF service start connection=snapshot impl=localClient module=abci-client msg="Starting localClient service"
4:51PM INF service start connection=mempool impl=localClient module=abci-client msg="Starting localClient service"
4:51PM INF service start connection=consensus impl=localClient module=abci-client msg="Starting localClient service"
4:51PM INF service start impl=EventBus module=events msg="Starting EventBus service"
4:51PM INF service start impl=PubSub module=pubsub msg="Starting PubSub service"
4:51PM INF service start impl=IndexerService module=txindex msg="Starting IndexerService service"
4:51PM INF service start impl=SignerListenerEndpoint module=privval msg="Starting SignerListenerEndpoint service"
4:51PM INF SignerListener: Blocking for connection module=privval
4:51PM INF SignerListener: Listening for new connection module=privval
4:51PM INF SignerListener: Connected module=privval
4:51PM INF SignerListener: Listening for new connection module=privval
```

---------

Co-authored-by: dongsam <[email protected]>
@gitferry gitferry force-pushed the feat/bls-keystore-improvement branch from 04d0ccc to 3df2c9f Compare February 3, 2025 11:03
@gitferry gitferry marked this pull request as ready for review February 3, 2025 12:29
@@ -86,6 +86,11 @@ else
BUILD_TAGS += mainnet
endif

# Handles the inclusion of e2e upgrade in binary
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can remove this part already, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

ohh, this is upgrade of bls keys, not software upgrade, my bad

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we should rename it as bls_upgrade but I am still not sure if it should be a build flag 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

@RafilxTenfen

The e2e_upgrade build tag is used for the e2e_upgrade_v1 test.

When this build tag is active, the previous version’s priv_validator_key.json file is automatically migrated to the updated structure, where the comet and BLS keys are separated. This ensures compatibility with the new key file format. code,

This migration is necessary because containers for pre-upgrade versions are pulled from Docker Hub, Without this adjustment, errors occur due to the outdated key file structure not being applied in the previous version.

That is the reason why the build tag is named e2e_upgrade.

// LoadBls returns a Bls after loading the erc2335 type of structure
// from the file and decrypt it using a password.
func LoadBls(keyFilePath, passwordFilePath string) *Bls {
passwordBytes, err := os.ReadFile(passwordFilePath)
Copy link
Contributor

Choose a reason for hiding this comment

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

it is possible to make it empty password? like no password

Copy link
Contributor

@wnjoon wnjoon Feb 4, 2025

Choose a reason for hiding this comment

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

@RafilxTenfen

Currently, the logic for generating the BLS key is as follows:

  • If the bls-password flag is set but left empty, an error occurs.
  • If the bls-password flag is not used, a password prompt appears, allowing the user to enter a password. At this point, an empty value ("") can be entered.
  • Add: If the no-bls-password flag is set, it automatically saves an empty value to bls_password.txt.

Copy link
Contributor

Choose a reason for hiding this comment

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

@RafilxTenfen

Applied in 99b9d3d

New PR created: #471

PrivateKey []byte `json:"privateKey"`
PeerId string `json:"peerId"`
IsValidator bool `json:"isValidator"`
CometPrivKey cmtcrypto.PrivKey `json:"cometPrivKey"`
Copy link
Contributor

Choose a reason for hiding this comment

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

having an interface here might be difficult to parse as json.Unmarshal at chain initialization

err = json.Unmarshal(configBz, &valConfig)

We should probably use primitive types

Copy link
Contributor

Choose a reason for hiding this comment

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

@RafilxTenfen

Applied in 99b9d3d

New PR created: #471

// BlsSigner is an interface for signing BLS messages
type BlsSigner interface {
SignMsgWithBls(msg []byte) (bls12381.Signature, error)
GetBlsPubkey() (bls12381.PublicKey, error)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
GetBlsPubkey() (bls12381.PublicKey, error)
BlsPubkey() (bls12381.PublicKey, error)

Copy link
Contributor

Choose a reason for hiding this comment

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

@RafilxTenfen

Applied in 99b9d3d

New PR created: #471

Copy link
Contributor

@RafilxTenfen RafilxTenfen left a comment

Choose a reason for hiding this comment

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

A few comments, but in general looks good to me

I was able to start a local chain and migrate bls, but how do I know if my node is using the correct key?

Should we have a document with instructions how to migrate the bls keys?

@wnjoon
Copy link
Contributor

wnjoon commented Feb 4, 2025

@RafilxTenfen

This migration is aimed at separating the BLS keys, previously included in the existing priv_validator_key.json, into a separate file. This means that the keys used by the node will remain unchanged.

If you need documentation on the migration process, please let me know. Below is a summary:

Summary

Description

Previously, the priv_validator_key.json file, created locally during the chain initialization process, stored both the BLS keys and the FilePV of Comet.

Due to this mixed structure, ExtendVote became dependent on the local priv_validator_key.json file. As a result, it was unusable in environments where the priv_validator_key.json file was not stored locally, such as with a remote signer.

To address this issue, we introduced the migrate-bls-key command. This command extracts the BLS key from priv_validator_key.json and stores it in a separate file, ensuring that priv_validator_key.json retains only the FilePV content generated during Comet’s initialization process.

Since the BLS key is stored in a file formatted according to ERC-2335, a password is required during the migration process. This password is stored in a separate file.

Requirements

First, we assume that the node is operating normally.

If you run a node for the first time with a new version, the babylond init command will create a priv_validator_key.json file that stores Comet’s FilePV, along with a separate file for the BLS key.

Additionally, the node must be running a previous version that does not use a separate file structure. If you are already using a newer version that creates a separate file structure, running the migrate-bls-key command will result in the following error:

failed to load previous version of priv_validator_key.json in ...

Process of migration

The migration will proceed in the following order:

1. the separation feature would be inserted in a new rc release
2. validator stops the current node
3. validator download the new release
4. validator run the migration cmd
5. restart the node and voting should be resumed

How to test migration-bls-key

1. Start the node using the babylond version before changing.

./babylond init my-node --chain-id test-chain-1
./babylond keys add validator1 --keyring-backend test
./babylond add-genesis-account $(./babylond keys show validator1 -a --keyring-backend test) 2000000000000ubbn,2000000000000stake
./babylond gentx validator1 1500000000000stake --chain-id test-chain-1 --keyring-backend test --fees 400ubbn
./babylond collect-gentxs
./babylond create-bls-key $(./babylond keys show validator1 -a --keyring-backend test)
./babylond gen-helpers create-bls
# copy content of gen-bls-x.json into checkpointing.genesis_keys field in genesis.json
$ ./babylond start

2. Stop the running node.

3. Execute migration from the changed version of babylond.

Before calling migration, check if the priv_validator_key.json file storing the two types of keys exists inside <home>/.babylond/config.

$ ./babylond migrate-bls-key

4. Enter the password you will use to encrypt the bls key into the erc2335 structure.

5. Verify that the bls_key.json and bls_password.txt files are correctly created in <home>/.babylond/config.

6. Verify that priv_validator_key.json contains only keys for comet.

7. Restart the node using changed version of babylond.

$ ./babylond start

Error cases

  • If priv_validator_key.json does not exist when calling migration
  • If priv_validator_key.json does not satisfy the previous version of a structure containing comet and bls keys.

@gitferry gitferry force-pushed the feat/bls-keystore-improvement branch from 1bbe3f4 to 74a6e39 Compare February 4, 2025 08:19
@gitferry
Copy link
Member Author

gitferry commented Feb 4, 2025

@RafilxTenfen I addressed conflicts with main mostly from #457. Could you review the changes?

@gitferry gitferry force-pushed the feat/bls-keystore-improvement branch from 74a6e39 to 5373a59 Compare February 4, 2025 08:37
This PR incorporates additional requests from issue #467.

Changes include:

- Adding the `no-bls-password` flag to commands related to BLS key
generation
- Applying a primitive type to the `CometPrivKey` field in the Node
struct
- Renaming a method in the `BlsSigner` interface
@gitferry
Copy link
Member Author

gitferry commented Feb 4, 2025

I could not perform all the steps from old version babylond transitioned to this PR due to some breaking changes in between. For example, after setting up with v1.0.0-rc.4, I migrated the key and got errors with babylond start:

babylond start
failed to load latest version: version of store zoneconcierge mismatch root store's version; expected 1 got 0; new stores should be added using StoreUpgrades

@RafilxTenfen Do you have any clues?

@KonradStaniec
Copy link
Collaborator

I could not perform all the steps from old version babylond transitioned to this PR due to some breaking changes in between. For example, after setting up with v1.0.0-rc.4, I migrated the key and got errors with babylond start:

I think you will able to make the compatibility test only after back porting this pr to v1. As on main , zoneconcierge module exists while on v1 it does not.

}
cmd.PrintErrf("successfully created bls key in %q.\n", privValKeyFilePath)

ck, err := appsigner.LoadConsensusKey(clientCtx.HomeDir)
Copy link
Contributor

Choose a reason for hiding this comment

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

Here we should basically check if the user already have the key setup, if it has there is no need to export, if there is no bls key set, export it

Copy link
Contributor

Choose a reason for hiding this comment

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

@RafilxTenfen

The BLS key is generated during babylond init.
Therefore, we can assume that the BLS key always exists when gentx is called.

Copy link
Contributor

Choose a reason for hiding this comment

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

so we should probably remove it here the call appsigner.ExportGenBls(
@gitferry

Copy link
Contributor

Choose a reason for hiding this comment

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

After offline discussing with @gitferry I understood

gen-bls-bbnvaloper1w60pd30puqlvma943fe5jcxsurlyfxzv3cxpvr.json can be created only by babylond gentx

bls_key.json can be created by bablyond init or babylond create-bls-key

no need to remove ExportGenBls all good to merge

Copy link
Contributor

@RafilxTenfen RafilxTenfen left a comment

Choose a reason for hiding this comment

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

LGTM, very nice docs @wnjoon 🎉
Only one comment during gentx where it might not even be necessary if the bls key is created during init

@gitferry gitferry merged commit d065cdd into main Feb 5, 2025
28 checks passed
@gitferry gitferry deleted the feat/bls-keystore-improvement branch February 5, 2025 02:18
gitferry added a commit that referenced this pull request Feb 5, 2025
Closes #336

---------

Co-authored-by: wonjoon <[email protected]>
Co-authored-by: dongsam <[email protected]>
gitferry added a commit that referenced this pull request Feb 5, 2025
This PR is backport from #467.

## Summary

### Description

Previously, the `priv_validator_key.json` file, created locally during
the chain initialization process, stored both the BLS keys and the
FilePV of Comet.

Due to this mixed structure, ExtendVote became dependent on the local
`priv_validator_key.json` file. As a result, it was unusable in
environments where the `priv_validator_key.json` file was not stored
locally, such as with a remote signer.

To address this issue, we introduced the migrate-bls-key command. This
command extracts the BLS key from `priv_validator_key.json` and stores
it in a separate file, ensuring that `priv_validator_key.json` retains
only the FilePV content generated during Comet’s initialization process.

Since the BLS key is stored in a file formatted according to ERC-2335, a
password is required during the migration process. This password is
stored in a separate file.

### Requirements

First, we assume that the node is operating normally.

If you run a node for the first time with a new version, the `babylond
init` command will create a `priv_validator_key.json` file that stores
Comet’s FilePV, along with a separate file for the BLS key.

Additionally, the node must be running a previous version that does not
use a separate file structure. If you are already using a newer version
that creates a separate file structure, running the `migrate-bls-key`
command will result in the following error:

```sh
failed to load previous version of priv_validator_key.json in ...
```

### Test migration-bls-key

#### 1. Setup and start the node using the `babylond` version, e.g.,
`v1.0.0-rc.4`.

```sh
$ babylond init my-node --chain-id test-chain-1
$ babylond keys add validator1 --keyring-backend test
$ babylond add-genesis-account $(babylond keys show validator1 -a --keyring-backend test) 2000000000000ubbn,2000000000000stake
$ babylond gentx validator1 1500000000000stake --chain-id test-chain-1 --keyring-backend test --fees 400ubbn
$ babylond collect-gentxs
$ babylond create-bls-key $(babylond keys show validator1 -a --keyring-backend test)
$ babylond gen-helpers create-bls
# copy content of gen-bls-x.json into checkpointing.genesis_keys field in genesis.json
$ babylond start
```

#### 2. Stop the running node.
#### 3. Execute migration from the changed version of `babylond`.

Before calling migration, check if the `priv_validator_key.json` file
storing the two types of keys exists inside `<home>/.babylond/config`.

```sh
$ babylond migrate-bls-key
```

#### 4. Enter the password you will use to encrypt the bls key into the
erc2335 structure.
#### 5. Verify that the `bls_key.json` and `bls_password.txt` files are
correctly created in `<home>/.babylond/config`.
#### 6. Verify that `priv_validator_key.json` contains only keys for
comet.
#### 7. Restart the node using changed version of `babylond`.

```sh
$ babylond start
```

#### Error cases
- If `priv_validator_key.json` does not exist when calling migration
- If `priv_validator_key.json` does not satisfy the previous version of
a structure containing comet and bls keys.

Co-authored-by: wonjoon <[email protected]>
Co-authored-by: dongsam <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Separate BLS key from priv_validator_key.json
4 participants