feat: improve config debug-ability

[0x009922]

Description

This PR contains a major improvement of the work done in #4239. This is a big step towards better debug-ability of Iroha configuration system due to better errors. And yet, there are still many things to improve further!

Key points:

• New ConfigReader API, which handles configuration sources in a dynamic manner rather than relying on serde completely, thus giving high flexibility in how to handle different cases. It is also built targeting Iroha domain specifically and doesn't have any extra stuff. Much less boilerplate.
• Heavy use of error_stack crate, which gives higher control over errors reporting than eyre does.
• --trace-config works! Uses log & stderrlog crates for it. Seems to be not a heavy solution for this. It is completely separate from tracing stack.

Further enhancements

There are still many things that might be improved.

• Enhance use of attachments API provided by error_stack:
  • Use hooks to colour them conditionally
  • Attach hints and suggestions
  • Unify attachments further as specific structs, not just format!
• Use spans to config files, and build reports showing the actual file contents
• Fine-tune API of ConfigReader, use better naming and abstractions, perhaps.
• Optimise internals of ConfigReader. There is lots of naive code and extra allocations.
• Find a way to reduce extra context changes in error_stack. Sometimes it would be really cool to just "forward" underlying context without changing it, but currently it's how error_stack is designed. So, we have sometimes stacks like "Failed to load config -> Failed to load config -> failed to read file system -> ..."
• Possibly, introduce a slim compile feature for Iroha, which will remove --trace-config support alongside with log & stderrlog dependencies, and more
• Use a single multihash string for private keys, thus removing necessity to set private keys in env as two separate variables (e.g. PRIVATE_KEY_ALGORITHM + PRIVATE_KEY_PAYLOAD, and removing cratchy env_custom API from ConfigReader
  • [suggestion] Adopt multihash format for private keys #4412
• Migrate to Axum, finally! Errors handling in Torii might be improved a lot.
  • [refactor] Migrate from warp to axum/actix #3776

Linked isues

Closes #3470, #4299, #4300

Gallery

Note: I've changed some bits after making these screenshots.

Some terminal screenshots

TODO

• Document internals of iroha_config_base
• Fix iroha_client_cli compilation failure
• Enabled failing test back: fails_with_no_trusted... in cli/src/lib.rs
• Self-review

[github-actions]

@BAStos525

[mversic]

can you explain yourself here? This shouldn't be public

[0x009922]

This warning appears when I run cargo check -p iroha_swarm (this is about TransactionPayload, but the cause is the same):

warning: type `transaction::model::TransactionPayload` is more private than the item `transaction::SignedTransaction::signatures`
   --> data_model/src/transaction.rs:297:5
    |
297 |     pub fn signatures(&self) -> &SignaturesOf<TransactionPayload> {
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ method `transaction::SignedTransaction::signatures` is reachable at visibility `pub`
    |
note: but type `transaction::model::TransactionPayload` is only usable at visibility `pub(crate)`
   --> data_model/src/transaction.rs:101:5
    |
101 |     pub(crate) struct TransactionPayload {
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    = note: `#[warn(private_interfaces)]` on by default

However, it doesn't appear if I check iroha, or iroha_core, or iroha_client (targetly). I guess that is because they all by the end of the day use iroha_data_model with transparent_api enabled, while iroha_swarm uses it without any features.

I guess the temporary solution would be, first of all, to create an issue like "Fix all transparent_api usage across workspace" (you also mentioned _transparent_api here: #4382 (comment)), but in scope of this PR either:

1. Return pub(crate) and silent the warning, temporarily
2. Enable transparent_api for iroha_swarm, temporarily

What do you think?

[mversic]

check what @s8sato did here

if we don't want to leak private types here:
pub fn signatures(&self) -> &SignaturesOf<TransactionPayload> {
we can consider changing the function to:
pub fn signatures(&self) -> &SignaturesOf<SignedTransaction> {

I guess the temporary solution would be, first of all, to create an issue like "Fix all transparent_api usage across workspace" (you also mentioned _transparent_api here: #4382 (comment)), but in scope of this PR either:

there isn't much to fix. We just need to remove transparent_api from test_network because it's leaking into our integration tests and then client appears to have access to internals when in fact it doesn't. But that's too big of an effort so we have to live with it for now

[0x009922]

I tried to change it to SignedTransaction (and SignedBlock), but I need to transmute the type then. What is the proper way to do so?

[DCNick3]

if we don't want to leak private types here:
pub fn signatures(&self) -> &SignaturesOf {
we can consider changing the function to:
pub fn signatures(&self) -> &SignaturesOf {

I don't think it's a good idea, as SignedTransaction is not scale-compatible with TransactionPayload (SignedTransactionV1 = TransactionPayload + SignaturesOf<TransactionPayload>). I guess we can make another versioned structure for transaction signatures (as a newtype around SignaturesOf<TransactionPayload>)? Or we can expose the TransactionPayload itself..

[mversic]

the same as here

[0x009922]

@DCNick3, I would appreciate if you can take a look at the procmacro I wrote here, in config/base/derive.

[0x009922]

Debugging CI: new reports are really helpful!

[DCNick3]

A bit concerned with the inclusion of source code location in the error trace (the at some/source/location.rs lines). I don't think it's useful for users and questionably useful for iroha developers. Maybe remove it/gate being some verbosity flag?

[0x009922]

A bit concerned with the inclusion of source code location in the error trace (the at some/source/location.rs lines). I don't think it's useful for users and questionably useful for iroha developers. Maybe remove it/gate being some verbosity flag?

I am concerned too. There are also complete backtraces attached, which are displayed in dev environment. Anyway, we can disable Location's printing using the hooks. I used it in tests to capture snapshots. Will think about it.