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

Remove support for composite (BLS) key type #817

Merged

Conversation

torao
Copy link
Contributor

@torao torao commented Dec 1, 2022

Description

This PR removes the support for composite from --priv_key_type option of init CLI command in relation to #808.

  • fixed to be an error when unsupported key types are specified.
  • correct duplicated default value (see below) and wording in help messages.
  • added test case: ed25519 → success, composite → failure, lamport → failure

Note that this removal is limited to the LBM layer and composite keys in Ostracon can still be used. The only subcommand covered is init which generates the private key for setup.

With this modification, only ed25519 key type can be specified in LBM, but the --priv_key_type option is retained for future use.

closes: #808

Motivation and context

The composite (BLS) keys are experimental on Ostracon side and are therefore not intended for formal use in the current LBM implementation. However, since the key type is passed unchecked to Ostracon's library at node creation, it's possible to create a key of type compsite, which will subsequently fail to load. For this reason, LBM requires composite keys to be an error.

How has this been tested?

Two new unit tests have been introduced.

Screenshots (if appropriate):

simd init --help

Checklist:

  • I followed the contributing guidelines and code of conduct.
  • I have added a relevant changelog to CHANGELOG.md
  • I have added tests to cover my changes.
  • I have updated the documentation accordingly.
  • I have updated API documentation client/docs/swagger-ui/swagger.yaml

…init CLI command

Note that this removal is limited to the LBM layer and composite keys in Ostracon can still be used.

* fixed to be error when unsupported key types are specified.
* fixed duplicated default values in help messages.
* added test case: ed25519 → success, composite → failure, lamport → failure
@codecov
Copy link

codecov bot commented Dec 1, 2022

Codecov Report

Merging #817 (c523e52) into main (ffb28cd) will increase coverage by 0.06%.
The diff coverage is 77.77%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #817      +/-   ##
==========================================
+ Coverage   61.75%   61.82%   +0.06%     
==========================================
  Files         883      883              
  Lines      100381   100384       +3     
==========================================
+ Hits        61988    62059      +71     
+ Misses      34775    34695      -80     
- Partials     3618     3630      +12     
Impacted Files Coverage Δ
x/genutil/client/cli/collect.go 0.00% <0.00%> (ø)
client/flags/flags.go 89.83% <100.00%> (+69.49%) ⬆️
x/genutil/client/cli/init.go 69.14% <100.00%> (+1.01%) ⬆️
x/token/msgs.go 47.90% <0.00%> (-3.43%) ⬇️
crypto/keys/internal/ecdsa/privkey.go 81.13% <0.00%> (-1.89%) ⬇️
x/wasm/keeper/keeper.go 86.16% <0.00%> (-0.37%) ⬇️
x/collection/msgs.go 53.57% <0.00%> (+5.95%) ⬆️

* add changelog
* add uncovered test case
* correct import sequence
@torao torao force-pushed the fix/remove_composite_key_type_support_in_init branch from 4f64d48 to 10cd59c Compare December 1, 2022 08:43
@torao torao changed the title Remove support for composite (BLS) type Remove support for composite (BLS) key type Dec 1, 2022
@torao torao marked this pull request as ready for review December 1, 2022 09:16
@torao torao self-assigned this Dec 1, 2022
@torao torao requested review from 0Tech and zemyblue December 1, 2022 10:06
Copy link
Collaborator

@0Tech 0Tech left a comment

Choose a reason for hiding this comment

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

LGTM

x/genutil/client/cli/init_test.go Outdated Show resolved Hide resolved
torao added 2 commits December 7, 2022 14:08
…b.com:torao/lbm-sdk into fix/remove_composite_key_type_support_in_init
@torao torao merged commit 22edb12 into Finschia:main Dec 12, 2022
zemyblue added a commit to zemyblue/finschia-sdk that referenced this pull request Jan 12, 2023
* main: (30 commits)
  chore(deps): Bump actions/cache from 3.2.2 to 3.2.3 (Finschia#860)
  chore(deps): Bump golang.org/x/crypto from 0.4.0 to 0.5.0 (Finschia#854)
  feat: Remove `x/wasm` module (Finschia#850)
  refactor: Remove useless stub BeginBlock/EndBlock methods (Finschia#853)
  feat: enable querying based on mempool status (only gRPC) (Finschia#840)
  feat(x/foundation): remove unnecessary gov-mint feature (Finschia#848)
  chore(deps): Bump actions/cache from 3.2.1 to 3.2.2 (Finschia#845)
  chore(deps): Bump github.com/mattn/go-isatty from 0.0.16 to 0.0.17 (Finschia#847)
  chore(deps): Bump actions/cache from 3.2.0 to 3.2.1 (Finschia#841)
  fix: apply foundation audit (Finschia#834)
  chore(deps): Bump actions/setup-go from 3.4.0 to 3.5.0 (Finschia#831)
  chore(deps): Bump actions/cache from 3.0.11 to 3.2.0 (Finschia#839)
  ci: automate release process (Finschia#829)
  chore(deps): Bump github.com/prometheus/common from 0.37.0 to 0.39.0 (Finschia#832)
  chore(deps): Bump goreleaser/goreleaser-action from 3 to 4 (Finschia#830)
  chore(deps): Bump github.com/magiconair/properties from 1.8.6 to 1.8.7 (Finschia#826)
  chore(deps): Bump technote-space/get-diff-action from 6.1.1 to 6.1.2 (Finschia#822)
  chore(deps): Bump golang.org/x/crypto from 0.2.0 to 0.4.0 (Finschia#828)
  feat: Get validator pubkey considering KMS (Finschia#821)
  Remove support for composite (BLS) key type (Finschia#817)
  ...

# Conflicts:
#	x/foundation/msgs.go
#	x/wasm/lbmtypes/codec.go
#	x/wasm/types/codec.go
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.

"--priv_key_type composite" is not supported
3 participants