iroha icon indicating copy to clipboard operation
iroha copied to clipboard

Refactor `data_model`

Open appetrosyan opened this issue 2 years ago • 5 comments

The iroha_data_model crate is used for all data that must be shared between the Iroha peer, and client. Unfortunately, this led to delocalisation of data, and any additional implementation can introduce cyclic dependencies. As such the crate needs to be broken down.

Proposed solution

We re-orient iroha into domain-specific hierarchy, with the data_model used for re-exporting specific crates, client handling the client communication and providing an example implementation.

As such the main workspace should have the following top-level memebers

  • peer
  • client
  • data_model
  • system
  • tools
  1. peer is the closest analogue of the current core crate. However, the smartcontract functionality is to be removed from core. It itself should consist of the following crates:
  • sumeragi
  • block_sync (turned into a separate crate)
  • torii
  • cli
  • config (which aggregates the configurations from domain-specific configs)

The core crate should only contain the consensus logic, and peer-specific implementations.

  1. client remains as is. The iroha_client_cli is moved into tools, as it's not meant to be used outside of the debugging context.

  2. data_model. This crate should contain the encapsulated and domain-first ordered crates for each type of entity that it deals with.

  • world
  • query.
  • instruction
  • expression
    • predicate
    • arithmetic
    • fixed
  • event
    • filter
    • account
    • domain
  • permission
  • version
  • schema
  • wasm
  • domain
    • account
    • asset
    • … Because of the orphan rule, and contrary to the domain-first approach, we will need to encapsulate instructions completely. In other words, instruction should contain both the traits and implementations for usage in sumeragi. As such impl Execute for Mint<Asset> like before, should be implemented in the instruction crate, rather than the asset crate. However, the instruction crate should have a module for each domain-specific entity.
  1. system. This is the most straightforward modification. It should contain the following sub-crates
  • crypto
  • actor
  • logger
  • ffi
  • futures
  • p2p
  • macro
  • config
  • telemetry
  • queue
  • kura
  • primitives, such as ConstString.

This crate is reserved for loosely coupled elements that might be needed either in peer or client or both, but that can be considered a technical detail of the implementation of iroha rather than the main aspect of its design. For example, kura needs to store blocks. As long as this implementation is readable, we don't care how exactly the blocks are stored, just that they're stored. Similarly the usage of the actor system is not necessarily interesting to external users (hence it's outside the data_model), and changes in the internal handling can only affect performance, and not reflect the external API.

How to determine if something belongs in the system crate.

  • The structure is fully internal or partially external (i.e. returned from an endpoint), but cannot affect the blockchain-specific behaviour of Iroha. LoggerConfiguraion can be queried and changed externally, but it doesn't affect consensus.
  • The type is not part of the external API. That is, IntoSchema types should go into the data_model. Example Kura.
  • The trait is used to abstract a concrete implementation from the usage inside the peer. Example sumeragi might need to abstract away the queue, so we can hotswap the implementation.
  • The type/trait is used for profiling/debugging. Example telemetry.
  1. tools. Should contain tools used for debugging. Example, kagami, iroha_client_cli, kura_inspector

appetrosyan avatar Jun 24 '22 11:06 appetrosyan

I think peer and system should be combined. Since the majority of data flow in a running iroha node is between things in both these crates. In my opinion they should be one crate or two where we separate what is essentially our "in house library code", telemetry, actor et cetera.

SamHSmith avatar Jun 24 '22 12:06 SamHSmith

I'd prefer separating the peer-specific API and logic from the implementation details.

appetrosyan avatar Jun 24 '22 12:06 appetrosyan

I like it

Arjentix avatar Jun 24 '22 12:06 Arjentix

In general it looks good, however I have some thoughts about the config module, in the current state Config is a mix of different domains (peer: SumeragiConfiguration, ToriiConfiguration, system: QueueConfig, KuraConfig, LoggerConfig ), so it's not obvious to me why put it inside peer.

Erigara avatar Jun 27 '22 14:06 Erigara

Convert to epic

appetrosyan avatar Aug 11 '22 12:08 appetrosyan