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

chore: reorganize data_model crates #5199

Merged

Conversation

mversic
Copy link
Contributor

@mversic mversic commented Oct 29, 2024

  • create new data_model directory (these are not wasm crates, they just have to be usable in no_std)
  • move crates that are shared between iroha tests and wasm there
  • follow up on perf: Directly provide payload to WASM entrypoints #5113 for triggers and smart contracts
  • rework prelude imports for iroha_smart_contracts, iroha_trigger, iroha_executor

@mversic mversic force-pushed the custom_data_model_separation branch 4 times, most recently from 3a0b20c to f22cbf2 Compare October 31, 2024 07:15
@mversic mversic requested a review from outoftardis as a code owner November 2, 2024 17:11
@mversic mversic force-pushed the custom_data_model_separation branch 7 times, most recently from db671b2 to cefbd5e Compare November 3, 2024 11:39
@nxsaken
Copy link
Contributor

nxsaken commented Nov 4, 2024

Why not samples as the root directory with libs and wasm inside? Also, it seems strange that iroha_multisig_data_model is in libs, while the rest of *_data_model are in samples (aren't all of them libs?)

@mversic
Copy link
Contributor Author

mversic commented Nov 4, 2024

Why not samples as the root directory with libs and wasm inside? Also, it seems strange that iroha_multisig_data_model is in libs, while the rest of *_data_model are in samples (aren't all of them libs?)

we need to recognize there are 3 categories of crates:

  1. regular crates (residing in /crates)
  2. wasm crates (built as cdylib)
  3. glue crates (bridge between regular and wasm crates)

Additionally, there is a distinction between crates that we recommend for production (libs) and the ones that we use in our integration tests (samples).

I'm open to suggestions on how to organize

@mversic mversic force-pushed the custom_data_model_separation branch from cefbd5e to 358d50f Compare November 4, 2024 10:09
@nxsaken
Copy link
Contributor

nxsaken commented Nov 4, 2024

Having only one directory for the sample libraries indicates that there is nothing magical about iroha_multisig_data_model compared to the rest of the "glue" crates, that it's a plain old library crate that just happens to be a best-practice example.

How about just a README in the samples directory explaining that wasm contains sample executors, triggers and smart contracts (the fact that they are being tested can be mentioned, but I wouldn't frame it as their purpose), while libs contains satellite library code used by wasm artifacts. The README could mention that iroha_multisig_data_model is the recommended way to implement multisig in production, and that default_executor is the executor used by default.

I would categorize the crates like this:

  1. regular crates (/crates)
  2. samples that build on top of the regular crates (/samples)
    • library code (/samples/libs) for custom data models etc.
    • executable code (/samples/wasm) for the actual artifacts, potentially using the library code

@mversic
Copy link
Contributor Author

mversic commented Nov 5, 2024

Having only one directory for the sample libraries indicates that there is nothing magical about iroha_multisig_data_model compared to the rest of the "glue" crates, that it's a plain old library crate that just happens to be a best-practice example.

How about just a README in the samples directory explaining that wasm contains sample executors, triggers and smart contracts (the fact that they are being tested can be mentioned, but I wouldn't frame it as their purpose), while libs contains satellite library code used by wasm artifacts. The README could mention that iroha_multisig_data_model is the recommended way to implement multisig in production, and that default_executor is the executor used by default.

I would categorize the crates like this:

1. regular crates (`/crates`)

2. samples that build on top of the regular crates (`/samples`)
   
   * library code (`/samples/libs`) for custom data models etc.
   * executable code (`/samples/wasm`) for the actual artifacts, potentially using the library code

ok, I don't disagree, but I'd leave it for another PR. This will conflict with multisig PR

@s8sato s8sato self-assigned this Nov 5, 2024
Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
… contracts

Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
Signed-off-by: Marin Veršić <marin.versic101@gmail.com>
@mversic mversic force-pushed the custom_data_model_separation branch from 358d50f to c389046 Compare November 6, 2024 08:56
@mversic mversic requested a review from DCNick3 as a code owner November 6, 2024 08:56
@mversic mversic merged commit 5677cf1 into hyperledger-iroha:main Nov 6, 2024
16 checks passed
@mversic mversic deleted the custom_data_model_separation branch November 6, 2024 09:25
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.

3 participants