CLI/keys: define default supported keyring algorithms

[gsora]

Description

This PR adds a way for developers to specify a suite of algorithms to be used in keyring instances instantiated with keyring.New().

While New() already has ways of achieving something similar doing so in CLI commands, in a generic enough way, is very hard.

We address this by providing package-level settings in keyring to define a default SigningAlgoList, automatically used in every keyring initialization phase.

This PR streamlines also the keys add command execution flow by using the Keyring included in the CLI context instead of instantiating a new one.

closes: #8803

---

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

• Targeted PR against correct branch (see CONTRIBUTING.md)
• Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
• Code follows the module structure standards.
• Wrote unit and integration tests
• Updated relevant documentation (docs/) or specification (x/<module>/spec/)
• Added relevant godoc comments.
• Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
• Re-reviewed Files changed in the Github PR explorer
• Review Codecov Report in the comment section below once CI passes

[codecov]

Codecov Report

Merging #8825 (3090574) into master (1fddce7) will decrease coverage by 0.00%.
The diff coverage is 44.44%.

@@            Coverage Diff             @@
##           master    #8825      +/-   ##
==========================================
- Coverage   59.24%   59.23%   -0.01%     
==========================================
  Files         571      571              
  Lines       31807    31800       -7     
==========================================
- Hits        18844    18837       -7     
- Misses      10760    10761       +1     
+ Partials     2203     2202       -1     

Impacted Files	Coverage Δ
client/context.go	57.00% <0.00%> (-1.09%)	⬇️
client/cmd.go	57.26% <100.00%> (ø)
client/keys/add.go	62.27% <100.00%> (-0.80%)	⬇️

[alessio]

ACKed

[orijbot]

Benchmark beginning. Status page: https://github.orijtech.com/benchmark/status?commit=7d230ad05e1514cc4e7a1d2175eada87fd5dde9e

[orijbot]

Benchmark finished. See the result: https://github.orijtech.com/benchmark/result?id=0033cb024854468eb46b7623454cb42c

[robert-zaremba]

Thanks for the PR. Please don't use globals for controlling a constructor behavior.

BTW: I was also looking at this code last week, and felt a need to make a simapp keys list-algos command: #8886

[orijbot]

Benchmark beginning. Status page: https://github.orijtech.com/benchmark/status?commit=1c573ddfeebd11607cdab08e7085bc27a361bbbb

[orijbot]

Benchmark beginning. Status page: https://github.orijtech.com/benchmark/status?commit=ae4c2b8323951e0ec843620c528cf571228b27c9

[orijbot]

Benchmark finished. See the result: https://github.orijtech.com/benchmark/result?id=5d842bc99b774e178f8f3568bf103097

[robert-zaremba]

1. Tests needs to be udpated (it's still using the old, failing logic)
2. Context obfuscates the flow, and makes it hard to see how the parameters are passed. I propose a way to not use context - if you like we can have a chat.
3. Documentation (/docs) and Changelog has to be updated as well.

[robert-zaremba]

Instead of using a context, can we use it in a straight way? Using a context obfuscates a flow. We should use function parameters wherever possible.
The ReadPersistentCommandFlags has access to flagSet - a set of defined flags. newKeyringFromFlags is called in that function. So we can use flagSet to carry the information (from config or cli flag) about supported algos. We could also add one more structural parameter to the ReadPersistentCommandFlags.

[alessio]

Robert, I agree with you on "using a context obfuscates a flow". Fully. I am a big fan of function parameters. I hate any stateful context and I hate even more using contexts to conveniently pack variables in order to reduce the number of function's arguments because that means that you are keen to sacrifice transparency and ease of debugging for the lazy developer's sake.

And yet, this is what we have now. I like your proposed changes, but I believe they are out of scope here. We should open another issue and try to overhaul the Context in a more holistic way.

[robert-zaremba]

OK, but I would like to avoid adding more XOptions directly into config, and I'm worried that someone else from other team will be motivated to do it (by following an example of what's already implemented)

[orijbot]

Benchmark beginning. Status page: https://github.orijtech.com/benchmark/status?commit=f7a86fac39e0d17149093a401b20eedb00a716bf

[orijbot]

Benchmark finished. See the result: https://github.orijtech.com/benchmark/result?id=bca31cb80c494b359a048ba45a63a543

[orijbot]

Benchmark beginning. Status page: https://github.orijtech.com/benchmark/status?commit=b22f83087ec55d0a1e489f4407ebe96d21598e5d

[robert-zaremba]

I would prefer to put configs into a Context sub structure (not as a direct context attribute). But for short term this is good enough.
I created an issue to tackle the config problem: #8915

[orijbot]

Benchmark beginning. Status page: https://github.orijtech.com/benchmark/status?commit=3090574b36246eecec0ad05efea2216b5ef23b1c

[orijbot]

Benchmark beginning. Status page: https://github.orijtech.com/benchmark/status?commit=deaee531111fb4aad3ccabbfa6771f9a67ae42ce

[orijbot]

Benchmark finished. See the result: https://github.orijtech.com/benchmark/result?id=6ed005157d684478afdd4ef199bcd44b

[orijbot]

Benchmark finished. See the result: https://github.orijtech.com/benchmark/result?id=540cd0b6a7a04c1fa678feb11bad6fee