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: remove dependency cometpv remote signer #11

Open
wants to merge 10 commits into
base: feat/bls-keystore-improvement
Choose a base branch
from

Conversation

wnjoon
Copy link
Collaborator

@wnjoon wnjoon commented Jan 25, 2025

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 and softsign. Here is a test scenario:

1. Create priv_validator_key.json file via babylond init

$ babylond keys add validator1 --keyring-backend test

2. Set up and start tmkms

#  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

# 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

$ tmkms start -c config/tmkms.toml

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

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

$ 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

"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:

# 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

$ babylond start
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

@wnjoon
Copy link
Collaborator Author

wnjoon commented Jan 25, 2025

Commit Summary b2c5b47

  • Removed the dependency on FilePV by importing the validator address and public key in priv_validator_key.json, required for ExtendVote, from the checkpointing and epoch modules.
  • Changed implementation of BlsSinger interface from WrappedFilePV to BlsPVKey
  • Changed the app to inject only BlsPVKey when starting up. This means the app will start normally even if priv_validator_key.json does not exist.
  • For specific commands (appExport, CmdWrappedCreateValidator), check if priv_validator_key.json exists locally.

@wnjoon
Copy link
Collaborator Author

wnjoon commented Jan 30, 2025

Commit Summary 93804c5

  • Remove duplicate codes, related to loading FilePV and BlsPV

@dongsam
Copy link
Member

dongsam commented Jan 31, 2025

@wnjoon Since the 2nd PR has been merged into upstream branch, please sync the base branch to the latest so that the already merged commits do not appear in this PR.

wnjoon pushed a commit that referenced this pull request Jan 31, 2025
@wnjoon wnjoon marked this pull request as ready for review January 31, 2025 02:33
@wnjoon wnjoon force-pushed the feat/remove-dependency-cometpv-remote-signer branch from de87472 to 848829e Compare January 31, 2025 05:29
privval/bls.go Outdated Show resolved Hide resolved
cmd/babylond/cmd/root.go Outdated Show resolved Hide resolved
privval/bls.go Outdated Show resolved Hide resolved
x/checkpointing/keeper/registration_state.go Outdated Show resolved Hide resolved
@wnjoon wnjoon requested a review from dongsam January 31, 2025 06:28
Copy link
Member

@dongsam dongsam left a comment

Choose a reason for hiding this comment

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

Idea/suggestion @wnjoon

  • FilePV dependency has been completely removed.
  • BLS is no longer associated with PV (private-validator.json).
  • To avoid confusion, any remaining occurrences of "PV" in struct names, function names, variables, and file paths should be removed.
  • Refactor the privval path and package to bls.
  • Refactor BlsPV* to Bls and BlsPVKey to BlsKey, along with related function and variable names.

@wnjoon
Copy link
Collaborator Author

wnjoon commented Feb 1, 2025

Commit Summary 94d674a

  • Moved bls package under app/signer
  • Changed app/signer to import with the name appsigner for consistency

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants