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(crypto): add blst #20296

Merged
merged 13 commits into from
Jun 11, 2024
Merged

feat(crypto): add blst #20296

merged 13 commits into from
Jun 11, 2024

Conversation

tac0turtle
Copy link
Member

@tac0turtle tac0turtle commented May 6, 2024

Description

This pr adds bls signatures to the Cosmos SDK to be used for validator accounts


Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • included the correct type prefix in the PR title
  • confirmed ! in the type prefix if API or client breaking change
  • targeted the correct branch (see PR Targeting)
  • provided a link to the relevant issue or specification
  • reviewed "Files changed" and left comments if necessary
  • included the necessary unit and integration tests
  • added a changelog entry to CHANGELOG.md
  • updated the relevant documentation or specification, including comments for documenting Go code
  • confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic, API design and naming, documentation is accurate, tests and test coverage

Summary by CodeRabbit

  • New Features

    • Added support for BLS12-381 cryptographic keys, including public and private key functionalities.
    • Introduced new command flags for consensus key algorithms.
  • Updates

    • Enhanced InitializeNodeValidatorFiles function to support multiple key types.
    • Updated various components to register and handle BLS12-381 key types.

Copy link
Contributor

coderabbitai bot commented May 6, 2024

Warning

Rate limit exceeded

@tac0turtle has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 31 minutes and 53 seconds before requesting another review.

How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

Commits

Files that changed from the base of the PR and between d4d5120 and 16c8720.

Walkthrough

The changes introduce support for BLS12-381 cryptographic keys across various parts of the codebase. This includes registration of BLS12-381 keys with different codecs, adding constants, and implementing key operations. Additionally, the InitializeNodeValidatorFiles function is updated to support various key types, and new command flags are introduced to handle consensus key algorithms.

Changes

File Path Change Summary
Makefile Adds conditional block for blst in COSMOS_BUILD_OPTIONS.
crypto/codec/amino.go Registers bls12_381 public and private keys using legacy.Amino codec.
crypto/codec/cmt.go Introduces support for bls12_381 public key type in conversion functions.
crypto/codec/proto.go Adds support for BLS12-381 cryptographic keys and registers implementations.
crypto/hd/algo.go Adds Bls12_381Type constant.
crypto/keys/bls12_381/const.go Defines constants related to BLS12-381 key operations.
crypto/keys/bls12_381/key.go Adds functionality for BLS12-381 private and public keys.
crypto/keys/bls12_381/key_cgo.go Introduces functionality for BLS12-381 private and public keys using CGO.
crypto/keys/ed25519/ed25519.go Updates keyType to KeyType.
crypto/keys/multisig/codec.go Adds import for bls12_381 and registers bls12_381.PubKey with AminoCdc.
simapp/simd/cmd/testnet.go Updates genutil.InitializeNodeValidatorFiles function call.
testutil/network/network.go Modifies New function to include ed25519.PrivKeyName argument.
x/genutil/client/cli/collect.go Adds FlagConsensusKeyAlgo to command flags.
x/genutil/client/cli/gentx.go Introduces consensusKey variable and FlagConsensusKeyAlgo.
x/genutil/client/cli/init.go Reorders imports and adds new imports.
x/genutil/client/cli/init_test.go Adds import for ed25519 and modifies function call in TestInitNodeValidatorFiles.
x/genutil/utils.go Adds support for different key types in InitializeNodeValidatorFiles functions.
x/genutil/utils_test.go Modifies InitializeNodeValidatorFilesFromMnemonic function call.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot added C:CLI C:x/genutil genutil module issues Type: Build labels May 6, 2024
@mergify mergify bot mentioned this pull request May 6, 2024
12 tasks
Makefile Outdated Show resolved Hide resolved
Co-authored-by: Devon Bear <[email protected]>
crypto/keys/bls12_381/key.go Dismissed Show dismissed Hide dismissed
@tac0turtle tac0turtle marked this pull request as ready for review June 4, 2024 14:56
@tac0turtle tac0turtle requested a review from a team as a code owner June 4, 2024 14:56
@tac0turtle tac0turtle requested a review from itsdevbear June 4, 2024 14:56
@@ -53,7 +53,7 @@ func TestInitializeNodeValidatorFilesFromMnemonic(t *testing.T) {
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
_, _, err := InitializeNodeValidatorFilesFromMnemonic(cfg, tt.mnemonic)
_, _, err := InitializeNodeValidatorFilesFromMnemonic(cfg, tt.mnemonic, "ed25519")

if tt.expError {
Copy link
Contributor

Choose a reason for hiding this comment

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

utACK, will give'r a rip in around an hour

Copy link
Contributor

github-actions bot commented Jun 4, 2024

@tac0turtle your pull request is missing a changelog!

@itsdevbear
Copy link
Contributor

@tac0turtle tested, compiles and runs on 5 node network

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 8

Outside diff range and nitpick comments (10)
crypto/codec/cmt.go (1)

9-9: Add a comment explaining the purpose of the new imports for clarity.

x/genutil/utils.go (1)

Line range hint 55-107: Enhanced InitializeNodeValidatorFilesFromMnemonic to support BLS12-381 keys, but missing mnemonic support for BLS keys needs addressing.

- // TODO: need to add support for getting from mnemonic in Comet.
+ // TODO: Implement mnemonic support for BLS12-381 keys.
testutil/network/network.go (8)

Line range hint 94-94: The type ValidatorI is referenced multiple times but is not defined in this file or imported. This could lead to compilation errors.

Please ensure that ValidatorI is correctly defined or imported.

Also applies to: 227-227, 698-698, 699-699


Line range hint 276-276: The type Validator is used but not defined or imported in this file, which could cause compilation issues.

Please define or import the Validator type appropriately.

Also applies to: 323-323, 577-577


Line range hint 315-315: The type NetworkI is referenced but not defined or imported, which could lead to compilation errors.

Please ensure that NetworkI is correctly defined or imported.


Line range hint 74-74: The function FreeTCPAddr is used but not defined or imported in this file. This could cause runtime errors if not addressed.

Please define or import the FreeTCPAddr function.


Line range hint 487-487: The function writeFile is called but not defined or imported in this file, which could lead to runtime errors.

Please ensure that the writeFile function is correctly defined or imported.

Also applies to: 550-550


Line range hint 594-594: The function initGenFiles is used but not defined or imported in this file. This could cause runtime errors if not addressed.

Please define or import the initGenFiles function.


Line range hint 598-598: The function collectGenFiles is referenced but not defined or imported, which could lead to runtime errors.

Please ensure that collectGenFiles is correctly defined or imported.


Line range hint 605-605: The function startInProcess is used but not defined or imported in this file. This could cause runtime errors if not addressed.

Please define or import the startInProcess function.

Review details

Configuration used: .coderabbit.yml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 0ba7906 and d4d5120.

Files ignored due to path filters (2)
  • crypto/keys/bls12_381/keys.pb.go is excluded by !**/*.pb.go
  • go.mod is excluded by !**/*.mod
Files selected for processing (18)
  • Makefile (1 hunks)
  • crypto/codec/amino.go (3 hunks)
  • crypto/codec/cmt.go (3 hunks)
  • crypto/codec/proto.go (2 hunks)
  • crypto/hd/algo.go (1 hunks)
  • crypto/keys/bls12_381/const.go (1 hunks)
  • crypto/keys/bls12_381/key.go (1 hunks)
  • crypto/keys/bls12_381/key_cgo.go (1 hunks)
  • crypto/keys/ed25519/ed25519.go (3 hunks)
  • crypto/keys/multisig/codec.go (2 hunks)
  • simapp/simd/cmd/testnet.go (1 hunks)
  • testutil/network/network.go (2 hunks)
  • x/genutil/client/cli/collect.go (2 hunks)
  • x/genutil/client/cli/gentx.go (2 hunks)
  • x/genutil/client/cli/init.go (4 hunks)
  • x/genutil/client/cli/init_test.go (2 hunks)
  • x/genutil/utils.go (3 hunks)
  • x/genutil/utils_test.go (1 hunks)
Files not reviewed due to errors (3)
  • x/genutil/utils_test.go (no review received)
  • x/genutil/client/cli/collect.go (no review received)
  • crypto/hd/algo.go (no review received)
Files skipped from review due to trivial changes (2)
  • Makefile
  • crypto/keys/bls12_381/const.go
Additional context used
Path-based instructions (16)
crypto/keys/multisig/codec.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

crypto/codec/proto.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

crypto/codec/amino.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

x/genutil/utils_test.go (2)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.


Pattern **/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"

x/genutil/client/cli/collect.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

crypto/hd/algo.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

crypto/codec/cmt.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

x/genutil/utils.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

crypto/keys/bls12_381/key.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

crypto/keys/bls12_381/key_cgo.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

x/genutil/client/cli/init.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

crypto/keys/ed25519/ed25519.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

x/genutil/client/cli/gentx.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

x/genutil/client/cli/init_test.go (2)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.


Pattern **/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"

simapp/simd/cmd/testnet.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

testutil/network/network.go (1)

Pattern **/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.

golangci-lint
crypto/keys/multisig/codec.go

33-33: undefined: LegacyAminoPubKey (typecheck)

crypto/codec/amino.go

6-6: File is not gci-ed with --skip-generated -s standard -s default -s prefix(cosmossdk.io) -s prefix(github.com/cosmos/cosmos-sdk) --custom-order (gci)

x/genutil/utils_test.go

21-21: undefined: ExportGenesisFileWithTime (typecheck)


56-56: undefined: InitializeNodeValidatorFilesFromMnemonic (typecheck)

x/genutil/client/cli/collect.go

29-29: undefined: FlagConsensusKeyAlgo (typecheck)


50-50: undefined: newPrintInfo (typecheck)


60-60: undefined: displayInfo (typecheck)


63-63: undefined: FlagConsensusKeyAlgo (typecheck)

crypto/hd/algo.go

56-56: undefined: ComputeMastersFromSeed (typecheck)


60-60: undefined: DerivePrivateKeyForPath (typecheck)

crypto/codec/cmt.go

8-8: File is not gci-ed with --skip-generated -s standard -s default -s prefix(cosmossdk.io) -s prefix(github.com/cosmos/cosmos-sdk) --custom-order (gci)

crypto/keys/bls12_381/key.go

23-23: undefined: PrivKey (typecheck)


24-24: undefined: PrivKey (typecheck)


28-28: undefined: PrivKey (typecheck)


33-33: undefined: PrivKey (typecheck)


38-38: undefined: PrivKey (typecheck)


44-44: undefined: PrivKey (typecheck)


49-49: undefined: PrivKey (typecheck)


54-54: undefined: PrivKey (typecheck)


60-60: undefined: PrivKey (typecheck)


65-65: undefined: PrivKey (typecheck)


70-70: undefined: PrivKey (typecheck)


80-80: undefined: PrivKey (typecheck)


87-87: undefined: PrivKey (typecheck)


99-99: undefined: PubKey (typecheck)


104-104: undefined: PubKey (typecheck)


109-109: undefined: PubKey (typecheck)


114-114: undefined: PubKey (typecheck)


119-119: undefined: PubKey (typecheck)


124-124: undefined: PubKey (typecheck)


129-129: undefined: PubKey (typecheck)


55-55: undefined: KeyType (typecheck)


71-71: undefined: PrivKeySize (typecheck)


120-120: undefined: KeyType (typecheck)

crypto/keys/bls12_381/key_cgo.go

29-29: undefined: PrivKey (typecheck)


30-30: undefined: PrivKey (typecheck)


34-34: undefined: PrivKey (typecheck)


43-43: undefined: PrivKey (typecheck)


49-49: undefined: PrivKey (typecheck)


55-55: undefined: PrivKey (typecheck)


65-65: undefined: PrivKey (typecheck)


70-70: undefined: PrivKey (typecheck)


76-76: undefined: PrivKey (typecheck)


92-92: undefined: PrivKey (typecheck)


97-97: undefined: PrivKey (typecheck)


107-107: undefined: PrivKey (typecheck)


114-114: undefined: PrivKey (typecheck)


126-126: undefined: PubKey (typecheck)


131-131: undefined: PubKey (typecheck)


140-140: undefined: PubKey (typecheck)


164-164: undefined: PubKey (typecheck)


169-169: undefined: PubKey (typecheck)


174-174: undefined: PubKey (typecheck)


179-179: undefined: PubKey (typecheck)


35-35: undefined: bls12381.SecretKeyFromBytes (typecheck)


37-37: undefined: PrivKey (typecheck)


44-44: undefined: bls12381.RandKey (typecheck)


45-45: undefined: PrivKey (typecheck)


56-56: undefined: bls12381.SecretKeyFromBytes (typecheck)


61-61: undefined: PubKey (typecheck)


71-71: undefined: keyType (typecheck)


77-77: undefined: bls12381.SecretKeyFromBytes (typecheck)


82-82: undefined: MaxMsgLen (typecheck)


98-98: undefined: PrivKeySize (typecheck)


132-132: undefined: bls12381.PublicKeyFromBytes (typecheck)


133-133: undefined: PubKeySize (typecheck)


141-141: undefined: SignatureLength (typecheck)


145-145: undefined: bls12381.PublicKeyFromBytes (typecheck)


150-150: undefined: MaxMsgLen (typecheck)


155-155: undefined: bls12381.VerifySignature (typecheck)


170-170: undefined: keyType (typecheck)

x/genutil/client/cli/init.go

11-11: File is not gci-ed with --skip-generated -s standard -s default -s prefix(cosmossdk.io) -s prefix(github.com/cosmos/cosmos-sdk) --custom-order (gci)


14-14: File is not gci-ed with --skip-generated -s standard -s default -s prefix(cosmossdk.io) -s prefix(github.com/cosmos/cosmos-sdk) --custom-order (gci)


23-23: File is not gci-ed with --skip-generated -s standard -s default -s prefix(cosmossdk.io) -s prefix(github.com/cosmos/cosmos-sdk) --custom-order (gci)

crypto/keys/ed25519/ed25519.go

41-41: undefined: PrivKey (typecheck)


42-42: undefined: PrivKey (typecheck)


46-46: undefined: PrivKey (typecheck)


57-57: undefined: PrivKey (typecheck)


64-64: undefined: PrivKey (typecheck)


86-86: undefined: PrivKey (typecheck)


94-94: undefined: PrivKey (typecheck)


99-99: undefined: PrivKey (typecheck)


104-104: undefined: PrivKey (typecheck)


114-114: undefined: PrivKey (typecheck)


121-121: undefined: PrivKey (typecheck)


129-129: undefined: PrivKey (typecheck)


134-134: undefined: PrivKey (typecheck)


150-150: undefined: PrivKey (typecheck)


159-159: undefined: PubKey (typecheck)


160-160: undefined: PubKey (typecheck)


166-166: undefined: PubKey (typecheck)


176-176: undefined: PubKey (typecheck)


180-180: undefined: PubKey (typecheck)


191-191: undefined: PubKey (typecheck)


195-195: undefined: PubKey (typecheck)


199-199: undefined: PubKey (typecheck)


208-208: undefined: PubKey (typecheck)


213-213: undefined: PubKey (typecheck)


223-223: undefined: PubKey (typecheck)


230-230: undefined: PubKey (typecheck)


81-81: undefined: PubKey (typecheck)


142-142: undefined: PrivKey (typecheck)


153-153: undefined: PrivKey (typecheck)

x/genutil/client/cli/gentx.go

65-65: undefined: FlagConsensusKeyAlgo (typecheck)


220-220: undefined: FlagConsensusKeyAlgo (typecheck)

testutil/network/network.go

94-94: undefined: ValidatorI (typecheck)


276-276: undefined: Validator (typecheck)


315-315: undefined: NetworkI (typecheck)


698-698: undefined: ValidatorI (typecheck)


74-74: undefined: FreeTCPAddr (typecheck)


227-227: undefined: ValidatorI (typecheck)


323-323: undefined: Validator (typecheck)


487-487: undefined: writeFile (typecheck)


550-550: undefined: writeFile (typecheck)


577-577: undefined: Validator (typecheck)


594-594: undefined: initGenFiles (typecheck)


598-598: undefined: collectGenFiles (typecheck)


605-605: undefined: startInProcess (typecheck)


699-699: undefined: ValidatorI (typecheck)

GitHub Check: CodeQL
crypto/keys/bls12_381/key.go

[warning] 105-105: Panic in BeginBock or EndBlock consensus methods
Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt

Additional comments not posted (19)
crypto/keys/multisig/codec.go (2)

7-7: Import of bls12_381 correctly added to support new cryptographic functionality.


31-32: Registration of bls12_381.PubKey correctly added.

crypto/codec/proto.go (2)

6-6: Correctly added import and registration for bls12_381.PubKey.

Also applies to: 20-20


27-27: Correctly added registration for bls12_381.PrivKey.

crypto/codec/amino.go (2)

7-7: Correctly added import and registration for bls12_381.PubKey.

Also applies to: 24-24


35-35: Correctly added registration for bls12_381.PrivKey.

crypto/codec/cmt.go (2)

27-30: Successfully integrated BLS12-381 public key support in FromCmtProtoPublicKey.


51-56: Successfully integrated BLS12-381 public key support in ToCmtProtoPublicKey.

x/genutil/utils.go (1)

49-50: Ensure that the InitializeNodeValidatorFiles function correctly delegates to the mnemonic-based initialization.

crypto/keys/bls12_381/key_cgo.go (1)

33-62: Proper implementation of PrivKey methods using CGO and BLS12-381 library. Ensure that error handling is robust.

Tools
golangci-lint

34-34: undefined: PrivKey (typecheck)


43-43: undefined: PrivKey (typecheck)


49-49: undefined: PrivKey (typecheck)


55-55: undefined: PrivKey (typecheck)


35-35: undefined: bls12381.SecretKeyFromBytes (typecheck)


37-37: undefined: PrivKey (typecheck)


44-44: undefined: bls12381.RandKey (typecheck)


45-45: undefined: PrivKey (typecheck)


56-56: undefined: bls12381.SecretKeyFromBytes (typecheck)


61-61: undefined: PubKey (typecheck)

x/genutil/client/cli/init.go (1)

113-116: Proper handling of the FlagConsensusKeyAlgo flag with appropriate error wrapping.

crypto/keys/ed25519/ed25519.go (3)

37-37: The constant KeyType is correctly defined for the ed25519 key type.


95-95: Correctly returns the key type for PrivKey.


196-196: Correctly returns the key type for PubKey.

x/genutil/client/cli/gentx.go (1)

220-220: The FlagConsensusKeyAlgo is undefined, which will cause a compilation error.

Tools
golangci-lint

220-220: undefined: FlagConsensusKeyAlgo (typecheck)

x/genutil/client/cli/init_test.go (1)

25-25: The import of ed25519 is added but not used in the visible code. Ensure it's necessary or remove it to keep the code clean.

Verification successful

The import of ed25519 is indeed used in the code, specifically in the call to genutil.InitializeNodeValidatorFiles.

  • x/genutil/client/cli/init_test.go: ed25519 is used in the genutil.InitializeNodeValidatorFiles function call.
Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the usage of `ed25519` in `init_test.go`.

# Test: Search for the usage of `ed25519`. Expect: At least one usage.
rg --type go --files-with-matches 'ed25519' x/genutil/client/cli/init_test.go

Length of output: 111



Script:

#!/bin/bash
# Description: Verify the specific usage of `ed25519` in `init_test.go`.

# Test: Search for the specific usage of `ed25519` in the code.
rg 'ed25519' x/genutil/client/cli/init_test.go

Length of output: 184

simapp/simd/cmd/testnet.go (1)

295-295: The function genutil.InitializeNodeValidatorFiles is called with args.algo which is correctly retrieved from command flags. Ensure that all algorithms used are supported and correctly implemented.

Verification successful

The InitializeNodeValidatorFilesFromMnemonic function in x/genutil/utils.go supports the following algorithms:

  • ed25519
  • bls12_381

The default algorithm used when no valid mnemonic is provided is ed25519. The bls12_381 algorithm does not support mnemonic-based key generation and will return an error if attempted.

  • x/genutil/utils.go:
    • InitializeNodeValidatorFilesFromMnemonic supports ed25519 and bls12_381.
Scripts executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify the supported algorithms in `InitializeNodeValidatorFiles`.

# Test: Search for the implementation details of supported algorithms. Expect: Details on supported algorithms.
ast-grep --lang go --pattern $'func InitializeNodeValidatorFiles($_, $_) {
  $$$
  switch $_ {
    case "ed25519":
      $$$
    case "secp256k1":
      $$$
    case $_:
      $$$
  }
  $$$
}' crypto/keys/*.go

Length of output: 276



Script:

#!/bin/bash
# Description: Locate the implementation of `InitializeNodeValidatorFiles` in the codebase.

# Search for the definition of `InitializeNodeValidatorFiles` function.
ast-grep --lang go --pattern 'func InitializeNodeValidatorFiles($_, $_) { $$$ }'

Length of output: 82



Script:

#!/bin/bash
# Description: Locate the implementation of `InitializeNodeValidatorFiles` in the codebase.

# Search for the definition of `InitializeNodeValidatorFiles` function across the entire codebase.
rg 'func InitializeNodeValidatorFiles' 

Length of output: 358



Script:

#!/bin/bash
# Description: Extract the implementation details of `InitializeNodeValidatorFiles` to verify supported algorithms.

# Extract the implementation of `InitializeNodeValidatorFiles` function from `x/genutil/utils.go`.
ast-grep --lang go --pattern 'func InitializeNodeValidatorFiles($_, $_) { $$$ }' x/genutil/utils.go

Length of output: 101



Script:

#!/bin/bash
# Description: Extract the implementation details of `InitializeNodeValidatorFiles` to verify supported algorithms.

# Search for the function definition and surrounding lines in `x/genutil/utils.go`.
rg -A 20 'func InitializeNodeValidatorFiles' x/genutil/utils.go

Length of output: 1292



Script:

#!/bin/bash
# Description: Extract the complete implementation of `InitializeNodeValidatorFilesFromMnemonic` to verify supported algorithms.

# Search for the complete implementation of `InitializeNodeValidatorFilesFromMnemonic` function in `x/genutil/utils.go`.
rg -A 50 'func InitializeNodeValidatorFilesFromMnemonic' x/genutil/utils.go

Length of output: 1645

testutil/network/network.go (2)

52-52: The import of ed25519 is added to support the new cryptographic functionality.


450-450: The function genutil.InitializeNodeValidatorFilesFromMnemonic is updated to include ed25519.PrivKeyName as an argument. This aligns with the PR's objective to integrate BLS and other cryptographic enhancements.

@@ -62,7 +62,12 @@ $ %s gentx my-key-name 1000000stake --home=/path/to/home/dir --keyring-backend=o
}
cdc := clientCtx.Codec

nodeID, valPubKey, err := genutil.InitializeNodeValidatorFiles(config)
consensusKey, err := cmd.Flags().GetString(FlagConsensusKeyAlgo)
Copy link
Contributor

Choose a reason for hiding this comment

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

The FlagConsensusKeyAlgo is undefined, which will cause a compilation error.

+ const FlagConsensusKeyAlgo = "consensus-key-algo"
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
consensusKey, err := cmd.Flags().GetString(FlagConsensusKeyAlgo)
const FlagConsensusKeyAlgo = "consensus-key-algo"
consensusKey, err := cmd.Flags().GetString(FlagConsensusKeyAlgo)
Tools
golangci-lint

65-65: undefined: FlagConsensusKeyAlgo (typecheck)

@@ -27,6 +28,8 @@ func init() {
sr25519.PubKeyName)
AminoCdc.RegisterConcrete(&secp256k1.PubKey{},
secp256k1.PubKeyName)
AminoCdc.RegisterConcrete(&bls12_381.PubKey{},
bls12_381.PubKeyName)
AminoCdc.RegisterConcrete(&LegacyAminoPubKey{},
Copy link
Contributor

Choose a reason for hiding this comment

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

Undefined reference to LegacyAminoPubKey.

- AminoCdc.RegisterConcrete(&LegacyAminoPubKey{}, PubKeyAminoRoute)
+ // TODO: Define LegacyAminoPubKey or correct the reference if it's a typo

Committable suggestion was skipped due to low confidence.

Tools
golangci-lint

33-33: undefined: LegacyAminoPubKey (typecheck)

@@ -4,7 +4,7 @@ import (
"github.com/cometbft/cometbft/crypto/sr25519"

"cosmossdk.io/core/legacy"
Copy link
Contributor

Choose a reason for hiding this comment

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

Fix import ordering as per project conventions.

- import (
-	"github.com/cometbft/cometbft/crypto/sr25519"
-	"cosmossdk.io/core/legacy"
-	bls12_381 "github.com/cosmos/cosmos-sdk/crypto/keys/bls12_381"
-	"github.com/cosmos/cosmos-sdk/crypto/keys/ed25519"
-	kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig"
-	"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
-	cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
- )
+ import (
+	"cosmossdk.io/core/legacy"
+	cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
+	bls12_381 "github.com/cosmos/cosmos-sdk/crypto/keys/bls12_381"
+	"github.com/cosmos/cosmos-sdk/crypto/keys/ed25519"
+	kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig"
+	"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
+	"github.com/cometbft/cometbft/crypto/sr25519"
+ )
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
"cosmossdk.io/core/legacy"
import (
"cosmossdk.io/core/legacy"
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
bls12_381 "github.com/cosmos/cosmos-sdk/crypto/keys/bls12_381"
"github.com/cosmos/cosmos-sdk/crypto/keys/ed25519"
kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig"
"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
"github.com/cometbft/cometbft/crypto/sr25519"
)
Tools
golangci-lint

6-6: File is not gci-ed with --skip-generated -s standard -s default -s prefix(cosmossdk.io) -s prefix(github.com/cosmos/cosmos-sdk) --custom-order (gci)

@@ -6,7 +6,7 @@ import (
"github.com/cometbft/cometbft/crypto/encoding"

"cosmossdk.io/errors"
Copy link
Contributor

Choose a reason for hiding this comment

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

Reorder imports according to the Golang style guide.

import (
	cmtprotocrypto "github.com/cometbft/cometbft/api/cometbft/crypto/v1"
	cmtcrypto "github.com/cometbft/cometbft/crypto"
	"github.com/cometbft/cometbft/crypto/encoding"
	"cosmossdk.io/errors"
	bls12_381 "github.com/cosmos/cosmos-sdk/crypto/keys/bls12_381"
	"github.com/cosmos/cosmos-sdk/crypto/keys/ed25519"
	"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
	cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
)
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
"cosmossdk.io/errors"
import (
cmtprotocrypto "github.com/cometbft/cometbft/api/cometbft/crypto/v1"
cmtcrypto "github.com/cometbft/cometbft/crypto"
"github.com/cometbft/cometbft/crypto/encoding"
"cosmossdk.io/errors"
bls12_381 "github.com/cosmos/cosmos-sdk/crypto/keys/bls12_381"
"github.com/cosmos/cosmos-sdk/crypto/keys/ed25519"
"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
)
Tools
golangci-lint

8-8: File is not gci-ed with --skip-generated -s standard -s default -s prefix(cosmossdk.io) -s prefix(github.com/cosmos/cosmos-sdk) --custom-order (gci)

Comment on lines +104 to +111
func (pubKey PubKey) Address() crypto.Address {
panic("not implemented, build flags are required to use bls12_381 keys")
}

// VerifySignature verifies the given signature.
func (pubKey PubKey) VerifySignature(msg, sig []byte) bool {
panic("not implemented, build flags are required to use bls12_381 keys")
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The panic in Address method of PubKey could cause a chain halt in consensus methods. This needs to be handled more gracefully.

- panic("not implemented, build flags are required to use bls12_381 keys")
+ if len(pubKey.Key) == 0 {
+     return nil, errors.New("public key not initialized")
+ }

Committable suggestion was skipped due to low confidence.

Tools
golangci-lint

104-104: undefined: PubKey (typecheck)


109-109: undefined: PubKey (typecheck)

GitHub Check: CodeQL

[warning] 105-105: Panic in BeginBock or EndBlock consensus methods
Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt

Comment on lines +28 to +62
func NewPrivateKeyFromBytes(bz []byte) (PrivKey, error) {
panic("not implemented, build flags are required to use bls12_381 keys")
}

// GenPrivKey generates a new key.
func GenPrivKey() (PrivKey, error) {
panic("not implemented, build flags are required to use bls12_381 keys")
}

// Bytes returns the byte representation of the Key.
func (privKey PrivKey) Bytes() []byte {
panic("not implemented, build flags are required to use bls12_381 keys")
}

// PubKey returns the private key's public key. If the privkey is not valid
// it returns a nil value.
func (privKey PrivKey) PubKey() cryptotypes.PubKey {
panic("not implemented, build flags are required to use bls12_381 keys")
}

// Equals returns true if two keys are equal and false otherwise.
func (privKey PrivKey) Equals(other cryptotypes.LedgerPrivKey) bool {
panic("not implemented, build flags are required to use bls12_381 keys")
}

// Type returns the type.
func (PrivKey) Type() string {
return KeyType
}

// Sign signs the given byte array. If msg is larger than
// MaxMsgLen, SHA256 sum will be signed instead of the raw bytes.
func (privKey PrivKey) Sign(msg []byte) ([]byte, error) {
panic("not implemented, build flags are required to use bls12_381 keys")
}
Copy link
Contributor

Choose a reason for hiding this comment

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

All methods in PrivKey are not implemented and require build flags. This needs to be addressed for full functionality.

+ // TODO: Implement these methods or ensure appropriate build flags are set.
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
func NewPrivateKeyFromBytes(bz []byte) (PrivKey, error) {
panic("not implemented, build flags are required to use bls12_381 keys")
}
// GenPrivKey generates a new key.
func GenPrivKey() (PrivKey, error) {
panic("not implemented, build flags are required to use bls12_381 keys")
}
// Bytes returns the byte representation of the Key.
func (privKey PrivKey) Bytes() []byte {
panic("not implemented, build flags are required to use bls12_381 keys")
}
// PubKey returns the private key's public key. If the privkey is not valid
// it returns a nil value.
func (privKey PrivKey) PubKey() cryptotypes.PubKey {
panic("not implemented, build flags are required to use bls12_381 keys")
}
// Equals returns true if two keys are equal and false otherwise.
func (privKey PrivKey) Equals(other cryptotypes.LedgerPrivKey) bool {
panic("not implemented, build flags are required to use bls12_381 keys")
}
// Type returns the type.
func (PrivKey) Type() string {
return KeyType
}
// Sign signs the given byte array. If msg is larger than
// MaxMsgLen, SHA256 sum will be signed instead of the raw bytes.
func (privKey PrivKey) Sign(msg []byte) ([]byte, error) {
panic("not implemented, build flags are required to use bls12_381 keys")
}
func NewPrivateKeyFromBytes(bz []byte) (PrivKey, error) {
panic("not implemented, build flags are required to use bls12_381 keys")
}
// GenPrivKey generates a new key.
func GenPrivKey() (PrivKey, error) {
panic("not implemented, build flags are required to use bls12_381 keys")
}
// Bytes returns the byte representation of the Key.
func (privKey PrivKey) Bytes() []byte {
panic("not implemented, build flags are required to use bls12_381 keys")
}
// PubKey returns the private key's public key. If the privkey is not valid
// it returns a nil value.
func (privKey PrivKey) PubKey() cryptotypes.PubKey {
panic("not implemented, build flags are required to use bls12_381 keys")
}
// Equals returns true if two keys are equal and false otherwise.
func (privKey PrivKey) Equals(other cryptotypes.LedgerPrivKey) bool {
panic("not implemented, build flags are required to use bls12_381 keys")
}
// Type returns the type.
func (PrivKey) Type() string {
return KeyType
}
// Sign signs the given byte array. If msg is larger than
// MaxMsgLen, SHA256 sum will be signed instead of the raw bytes.
func (privKey PrivKey) Sign(msg []byte) ([]byte, error) {
panic("not implemented, build flags are required to use bls12_381 keys")
}
// TODO: Implement these methods or ensure appropriate build flags are set.
Tools
golangci-lint

28-28: undefined: PrivKey (typecheck)


33-33: undefined: PrivKey (typecheck)


38-38: undefined: PrivKey (typecheck)


44-44: undefined: PrivKey (typecheck)


49-49: undefined: PrivKey (typecheck)


54-54: undefined: PrivKey (typecheck)


60-60: undefined: PrivKey (typecheck)


55-55: undefined: KeyType (typecheck)

Comment on lines +128 to +137
// Address returns the address of the key.
//
// The function will panic if the public key is invalid.
func (pubKey PubKey) Address() crypto.Address {
pk, _ := bls12381.PublicKeyFromBytes(pubKey.Key)
if len(pk.Marshal()) != PubKeySize {
panic("pubkey is incorrect size")
}
return crypto.Address(tmhash.SumTruncated(pubKey.Key))
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The Address method in PubKey should handle potential errors from PublicKeyFromBytes more gracefully instead of panicking.

- panic("pubkey is incorrect size")
+ if len(pk.Marshal()) != PubKeySize {
+     return nil, errors.New("public key is incorrect size")
+ }

Committable suggestion was skipped due to low confidence.

Tools
golangci-lint

131-131: undefined: PubKey (typecheck)


132-132: undefined: bls12381.PublicKeyFromBytes (typecheck)


133-133: undefined: PubKeySize (typecheck)

cmttypes "github.com/cometbft/cometbft/types"
"github.com/cosmos/go-bip39"
"github.com/spf13/cobra"

errorsmod "cosmossdk.io/errors"
Copy link
Contributor

Choose a reason for hiding this comment

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

Reorder imports according to the Golang style guide.

import (
	"bufio"
	"encoding/json"
	"errors"
	"fmt"
	"os"
	"path/filepath"
	errorsmod "cosmossdk.io/errors"
	"cosmossdk.io/math/unsafe"
	cfg "github.com/cometbft/cometbft/config"
	cmttypes "github.com/cometbft/cometbft/types"
	"github.com/cosmos/cosmos-sdk/client"
	"github.com/cosmos/cosmos-sdk/client/flags"
	"github.com/cosmos/cosmos-sdk/client/input"
	sdk "github.com/cosmos/cosmos-sdk/types"
	"github.com/cosmos/cosmos-sdk/types/module"
	"github.com/cosmos/cosmos-sdk/version"
	"github.com/cosmos/cosmos-sdk/x/genutil"
	"github.com/cosmos/cosmos-sdk/x/genutil/types"
	"github.com/cosmos/go-bip39"
	"github.com/spf13/cobra"
)

Also applies to: 14-14, 23-23

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
errorsmod "cosmossdk.io/errors"
import (
"bufio"
"encoding/json"
"errors"
"fmt"
"os"
"path/filepath"
errorsmod "cosmossdk.io/errors"
"cosmossdk.io/math/unsafe"
cfg "github.com/cometbft/cometbft/config"
cmttypes "github.com/cometbft/cometbft/types"
"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/flags"
"github.com/cosmos/cosmos-sdk/client/input"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/module"
"github.com/cosmos/cosmos-sdk/version"
"github.com/cosmos/cosmos-sdk/x/genutil"
"github.com/cosmos/cosmos-sdk/x/genutil/types"
"github.com/cosmos/go-bip39"
"github.com/spf13/cobra"
)
Tools
golangci-lint

11-11: File is not gci-ed with --skip-generated -s standard -s default -s prefix(cosmossdk.io) -s prefix(github.com/cosmos/cosmos-sdk) --custom-order (gci)

Copy link
Contributor

@akhilkumarpilli akhilkumarpilli left a comment

Choose a reason for hiding this comment

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

utACK, left small nit

x/genutil/client/cli/gentx.go Outdated Show resolved Hide resolved
Copy link
Contributor

@testinginprod testinginprod left a comment

Choose a reason for hiding this comment

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

LGTM

@tac0turtle tac0turtle added this pull request to the merge queue Jun 11, 2024
Merged via the queue into main with commit 05ff7a7 Jun 11, 2024
63 of 65 checks passed
@tac0turtle tac0turtle deleted the bera/blst-sdk branch June 11, 2024 17:20
alpe added a commit that referenced this pull request Jun 12, 2024
* main:
  refactor(x/auth): Fix system test (#20531)
  feat(crypto): add blst (#20296)
  docs: Update high level overview and introduction (#20535)
  refactor(x/core): remove test (#20624)
  feat(x/mint)!: Replace InflationCalculationFn with MintFn + simple epoch minting (#20363)
  docs: remove duplicate words (#20622)
  feat: prep for errors v2 (#20539)
  chore: reduce default inflation (#20606)
  refactor(store): add miss defer (#20602)
  chore: use comet api pkg instead of comet alias (#20614)
  chore: write gentx info to cmd.ErrOrStderr (#20616)
  docs: ADR 073: update to accepted and add to README.md (#20619)
  chore(proto): change future extracted modules version from v1.0.0 to v0.2.0 (#20600)
  fix: remove some duplicate words (#20605)
  feat(stf): port simappv2 changes (#20587)
  chore: bring patch changelogs to main (#20599)
kocubinski added a commit that referenced this pull request Jun 14, 2024
Copy link
Member

@julienrbrt julienrbrt left a comment

Choose a reason for hiding this comment

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

Feels like we need an API break changelog for some calls, and a feature changelog.

@@ -44,13 +46,13 @@ func ExportGenesisFileWithTime(genFile, chainID string, validators []cmttypes.Ge
}

// InitializeNodeValidatorFiles creates private validator and p2p configuration files.
func InitializeNodeValidatorFiles(config *cfg.Config) (nodeID string, valPubKey cryptotypes.PubKey, err error) {
return InitializeNodeValidatorFilesFromMnemonic(config, "")
func InitializeNodeValidatorFiles(config *cfg.Config, keyType string) (nodeID string, valPubKey cryptotypes.PubKey, err error) {
Copy link
Member

Choose a reason for hiding this comment

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

can we get changelogs for this?

@julienrbrt
Copy link
Member

@tac0turtle your pull request is missing a changelog!

bot was right, pls changelog

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

Successfully merging this pull request may close these issues.

6 participants