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

cli: drop support for binary keys, use wallets #2357

Merged
merged 1 commit into from
May 24, 2023
Merged

Conversation

roman-khimov
Copy link
Member

Wallets were introduced way back when in 0.27.5 and that's the way keys should be stored. We've had enough transition period for people to adjust to using them. Binary keys are dangerous and inconvenient (NEP-6 is common for all of the Neo tooling, everyone knows how to use it), therefore we shouldn't provide support for them.

Related to #2136.

@roman-khimov roman-khimov added neofs-cli NeoFS CLI application issues security Affects security labels May 24, 2023
@roman-khimov roman-khimov added this to the v0.37.0 milestone May 24, 2023
@roman-khimov roman-khimov requested review from 532910 and aprasolova May 24, 2023 08:09
Copy link
Contributor

@notimetoname notimetoname left a comment

Choose a reason for hiding this comment

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

How about filling the "Updating..." section? CLIs can be used in scripts.

@codecov
Copy link

codecov bot commented May 24, 2023

Codecov Report

Merging #2357 (d1eb8cb) into master (0e29c77) will increase coverage by 0.00%.
The diff coverage is 83.33%.

❗ Current head d1eb8cb differs from pull request most recent head 79ce05c. Consider uploading reports for the commit 79ce05c to get more accurate results

@@           Coverage Diff           @@
##           master    #2357   +/-   ##
=======================================
  Coverage   29.93%   29.93%           
=======================================
  Files         398      398           
  Lines       29743    29742    -1     
=======================================
+ Hits         8903     8904    +1     
+ Misses      20122    20120    -2     
  Partials      718      718           
Impacted Files Coverage Δ
cmd/neofs-cli/internal/key/wallet.go 93.10% <ø> (ø)
cmd/neofs-node/morph.go 0.00% <0.00%> (ø)
pkg/innerring/innerring.go 0.00% <0.00%> (ø)
cmd/neofs-node/config/morph/config.go 95.83% <85.71%> (ø)
cmd/neofs-cli/internal/key/raw.go 58.33% <100.00%> (-4.63%) ⬇️
pkg/innerring/config.go 97.70% <100.00%> (+0.01%) ⬆️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@@ -35,6 +36,12 @@ Changelog for NeoFS Node
- `morph.switch_interval` IR and SN config value is not used anymore.
- `morph.rpc_endpoint` SN config value and `morph.endpoint.client` IR config value has been deprecated and will be
removed with the next minor release. Use `morph.endpoints` for both instead (NOTE: it does not have priorities now).
- If you're using binary keys with neofs-cli (`-w`), convert them to proper
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, could you duplicate this manual to README or any place where it'd be easy to find?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it's only relevant for this upgrade and careful admins read this documentation when they perform upgrade. And frankly I don't expect to see anyone still using these binaries keys, they're not even easy to create now (everyone just goes neo-go wallet create route).

Wallets were introduced way back when in 0.27.5 and that's the way keys should
be stored. We've had enough transition period for people to adjust to using
them. Binary keys are dangerous and inconvenient (NEP-6 is common for all of
the Neo tooling, everyone knows how to use it), therefore we shouldn't provide
support for them.

Related to #2136.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
@roman-khimov roman-khimov merged commit 9ab8291 into master May 24, 2023
@roman-khimov roman-khimov deleted the drop-binary-keys branch May 24, 2023 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
neofs-cli NeoFS CLI application issues security Affects security
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants