[fastx CLI] Add customisation to genesis #461

patrickkuo · 2022-02-16T00:56:23Z

fixed hardcoded framework sources dir issue
Added genesis customisation
custom deserialiser to support flexible config

Todo: We should rename AuthorityPrivateInfo -> AuthorityServerConfig; AuthorityInfo -> AuthorityConfig etc in config.rs, we can rename them now as we have removed client.rs.

Genesis customization

The genesis process can be customised by providing a genesis config file.

./sui genesis --config genesis.conf

Example genesis.conf

{
  "authorities": [
    {
      "key_pair": "DfwJMkbD877Xehkaa03aPTsP5f4CpSADNCfX3Iukku69ZU81LIldnsFMSR0/K04fmPsHMjODvr6flatiW/8voA==",
      "host": "127.0.0.1",
      "port": 10004,
      "db_path": "./authorities_db/bd654f352c895d9ec14c491d3f2b4e1f98fb07323383bebe9f95ab625bff2fa0",
      "stake": 1
    }
  ],
  "accounts": [
    {
      "address": "bd654f352c895d9ec14c491d3f2b4e1f98fb07323383bebe9f95ab625bff2fa0",
      "gas_objects": [
        {
          "object_id": "5c68ac7ba66ef69fdea0651a21b531a37bf342b7",
          "gas_value": 1000
        }
      ]
    }
  ],
  "move_packages": ["<Paths to custom move packages>"],
  "sui_framework_lib_path": "<Paths to sui framework lib>",
  "move_framework_lib_path": "<Paths to move framework lib>"
}

All attributes in genesis.conf are optional, default value will be use if the attributes are not provided.
For example, the config shown below will create a network of 4 authorities, and pre-populate 2 objects for 4 accounts.

{
  "authorities": [
    {},{},{},{}
  ],
  "accounts": [
    { "gas_objects":[{},{}] },
    { "gas_objects":[{},{}] },
    { "gas_objects":[{},{}] },
    { "gas_objects":[{},{}] }
  ]
}

patrickkuo · 2022-02-16T16:25:34Z

sui/src/utils.rs

@@ -1,25 +1,25 @@
 // Copyright (c) Mysten Labs


Will be moving this to other files when #454 is merged

sblackshear

Nice! Looks excellent overall, only requesting changes for the generalization to a list of paths.

sblackshear · 2022-02-16T20:15:15Z

sui/src/config.rs

+    pub authorities: Vec<AuthorityPrivateInfo>,
+    pub accounts: Vec<AccountConfig>,
+    #[serde(default = "default_framework_path")]
+    pub framework_lib_path: PathBuf,


I think we should generalize this to a list of Move package paths that will be compiled and published in the specified order. This will allow us to include not only the Sui framework, but also custom packages that a particular customer wants (e.g., Geniteam would like a genesis with their demo modules).

sblackshear · 2022-02-16T20:18:17Z

sui/src/sui_commands.rs

@@ -157,42 +201,28 @@ async fn make_server(
    committee: &Committee,
    pre_load_objects: &[Object],
    buffer_size: usize,
-) -> AuthorityServer {
+    module_path: &Path,


Suggested change

module_path: &Path,

package_path: &Path,

sblackshear · 2022-02-16T20:23:44Z

sui_programmability/adapter/src/genesis.rs

-fn create_genesis_module_objects() -> Genesis {
-    let sui_modules = sui_framework::get_sui_framework_modules();
-    let std_modules = sui_framework::get_move_stdlib_modules();
+fn create_genesis_module_objects(lib_dir: &Path) -> SuiResult<Genesis> {


@awelc: I'm just realizing that this will not run the module initializers in the genesis modules, whereas we really want those to get run (since this is the only way for a custom genesis to create object types other than gas objects). I think adding this feature would be a great follow-up to Patrick's PR.

sblackshear · 2022-02-16T20:26:21Z

README.md

+  "accounts": [
+    {
+      "address": "bd654f352c895d9ec14c491d3f2b4e1f98fb07323383bebe9f95ab625bff2fa0",
+      "objects": [


We might want to call this gas_objects if that's the only object type we plan to support (though perhaps we'll want to generalize things a bit later). However, I think the typical approach to creating custom objects should be executing Move module initializers of genesis modules that create the custom objects + transfer them to the appropriate addresses(s). This will be safer + more ergonomic than encoding custom objects in a JSON config.

patrickkuo · 2022-02-17T15:16:31Z

sui/src/sui_commands.rs

+                .collect()
+        };
+
+        if !package.is_framework {


@sblackshear is this the right way to build packages for genesis?

Looks good! We (or more accurately, module init master @awelc :)) will have to replace this code with something that calls into the adapter publishing code in order to run module initializers, but this is fine for now.

It seems like in sui_commands.rs we are doing a lot of the same work which is done when creating genesis in genesis.rs, particularly with respect to creating the framework and standard library modules. Is it possible to merge/reuse these somehow?

Selfishly, this will also likely simplify adding support of module initializers, as it would reduce the number of places where this needs to be done.

Should I create a separate issue for this? Unless there are some reasons why this would not make sense which I don't see after mu cursory look.

oxade · 2022-02-17T15:22:05Z

sui/src/config.rs

+    pub fn default_genesis(path: &Path) -> Result<Self, anyhow::Error> {
+        let working_dir = path.parent().ok_or(anyhow!("Cannot resolve file path."))?;
+        let mut authorities = Vec::new();
+        for _ in 0..4 {


Define constant instead of 0..4

oxade · 2022-02-17T15:22:26Z

sui/src/config.rs

+        for _ in 0..5 {
+            let mut objects = Vec::new();
+            for _ in 0..5 {


Define constant instead of 0..5

oxade · 2022-02-17T15:22:36Z

sui/src/config.rs

+            // Get default authority config from deserialization logic.
+            let mut authority = AuthorityPrivateInfo::deserialize(Value::String(String::new()))?;
+            authority.db_path = working_dir
+                .join("authorities_db")


Define constant instead of "authorities_db"

oxade · 2022-02-17T15:23:22Z

sui/src/config.rs

+            for _ in 0..5 {
+                objects.push(ObjectConfig {
+                    object_id: ObjectID::random(),
+                    gas_value: 1000,


Define constant instead of 1000
Also we should probably use a much higher value like 10k. Gas runs out quickly when I used 1k. Just a thought

oxade · 2022-02-17T17:27:28Z

Thanks for handling this btw. It'll unblock lots of other work.

sblackshear · 2022-02-17T17:30:19Z

One broader thought on genesis packages upon reflection (and apologies if this conflicts with my earlier advice). The system will not work at all without the Sui framework (e.g., the gas type is defined there!) and Move stdlib (since the Sui framework depends on these modules).

So I think there is probably no point in making the paths to these user-configurable--the only thing this lets the user do is misconfigure the system in a way that won't work :).

Instead, I would propose that genesis always include these modules, and only allow the user to supply paths to packages that should also be included. This will also let us simplify types like PackageConfig--can drop is_framework and denylist, since these concepts are only needed for the framework and stdlib.

sblackshear · 2022-02-17T17:24:50Z

sui/src/sui_commands.rs

+                .collect()
+        };
+
+        if !package.is_framework {


Looks good! We (or more accurately, module init master @awelc :)) will have to replace this code with something that calls into the adapter publishing code in order to run module initializers, but this is fine for now.

sblackshear · 2022-02-17T17:24:56Z

sui/src/sui_commands.rs

+        let object =
+            Object::new_package(modules, SuiAddress::default(), TransactionDigest::genesis());
+        info!(
+            "Loaded {}package [{}] from {:?}.",


Suggested change

"Loaded {}package [{}] from {:?}.",

"Loaded {} package [{}] from {:?}.",

patrickkuo · 2022-02-17T17:54:21Z

One broader thought on genesis packages upon reflection (and apologies if this conflicts with my earlier advice). The system will not work at all without the Sui framework (e.g., the gas type is defined there!) and Move stdlib (since the Sui framework depends on these modules).

So I think there is probably no point in making the paths to these user-configurable--the only thing this lets the user do is misconfigure the system in a way that won't work :).

Instead, I would propose that genesis always include these modules, and only allow the user to supply paths to packages that should also be included. This will also let us simplify types like PackageConfig--can drop is_framework and denylist, since these concepts are only needed for the framework and stdlib.

😂 no problem, this makes more sense as Sui and Move libs are mandatory.

patrickkuo · 2022-02-17T23:16:40Z

This PR is ready for more review.

I have seperated framework_lib_path into :

"move_packages": ["<Paths to custom move packages>"],
 "sui_framework_lib_path": "<Paths to sui framework lib>",
 "move_framework_lib_path": "<Paths to move framework lib>"

sui_framework_lib_path and move_framework_lib_path defaulted to ./sui_programmability/framework and ./sui_programmability/framework/deps/move-stdlib, can be overridden in case it's in a different directory @oxade

move_packages are for publishing move packages at genesis, it's now only for non-framework packages

* ability to preload move module

oxade

Thanks for your patience Patrick

Added more metrics for primary node across the core, header_waiter, certificate_waiter & primary to primary endpoint requests.

patrickkuo linked an issue Feb 16, 2022 that may be closed by this pull request

[fastx CLI] Add customisation to genesis #400

Closed

oxade mentioned this pull request Feb 16, 2022

[programmability] Framework Sources Dir Is Compile Time Constant #448

Closed

patrickkuo linked an issue Feb 16, 2022 that may be closed by this pull request

[programmability] Framework Sources Dir Is Compile Time Constant #448

Closed

patrickkuo self-assigned this Feb 16, 2022

patrickkuo commented Feb 16, 2022

View reviewed changes

sui/src/utils.rs

@@ -1,25 +1,25 @@

// Copyright (c) Mysten Labs

Copy link

Contributor Author

patrickkuo Feb 16, 2022

Choose a reason for hiding this comment

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

Will be moving this to other files when #454 is merged

patrickkuo force-pushed the pat/customise-genesis branch from a4c5b61 to 8c4ec46 Compare February 16, 2022 17:52

patrickkuo requested review from lxfind, huitseeker, sblackshear, arun-koshy and oxade February 16, 2022 17:54

patrickkuo marked this pull request as ready for review February 16, 2022 17:54

sblackshear requested changes Feb 16, 2022

View reviewed changes

patrickkuo force-pushed the pat/customise-genesis branch from 8c4ec46 to 5911a2b Compare February 17, 2022 13:16

patrickkuo commented Feb 17, 2022

View reviewed changes

oxade reviewed Feb 17, 2022

View reviewed changes

patrickkuo requested a review from sblackshear February 17, 2022 16:08

sblackshear reviewed Feb 17, 2022

View reviewed changes

patrickkuo requested review from oxade and sblackshear February 17, 2022 21:17

patrickkuo force-pushed the pat/customise-genesis branch 2 times, most recently from fc8732a to d18fcb4 Compare February 18, 2022 16:48

* genesis customisation with config files

4c387b0

* ability to preload move module

fixup after rebase

3c66a43

patrickkuo force-pushed the pat/customise-genesis branch from d18fcb4 to 3c66a43 Compare February 18, 2022 19:01

sblackshear approved these changes Feb 18, 2022

View reviewed changes

oxade approved these changes Feb 18, 2022

View reviewed changes

patrickkuo merged commit bfde55c into main Feb 18, 2022

patrickkuo deleted the pat/customise-genesis branch February 18, 2022 23:08

mwtian pushed a commit that referenced this pull request Sep 12, 2022

[metrics] adding more metrics for primary node (#461)

02e206f

Added more metrics for primary node across the core, header_waiter, certificate_waiter & primary to primary endpoint requests.

mwtian pushed a commit to mwtian/sui that referenced this pull request Sep 29, 2022

[metrics] adding more metrics for primary node (MystenLabs#461)

592ae2b

Added more metrics for primary node across the core, header_waiter, certificate_waiter & primary to primary endpoint requests.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[fastx CLI] Add customisation to genesis #461

[fastx CLI] Add customisation to genesis #461

patrickkuo commented Feb 16, 2022 •

edited

Loading

patrickkuo Feb 16, 2022

sblackshear left a comment

sblackshear Feb 16, 2022

sblackshear Feb 16, 2022

sblackshear Feb 16, 2022

sblackshear Feb 16, 2022

patrickkuo Feb 17, 2022

sblackshear Feb 17, 2022

awelc Feb 17, 2022

awelc Feb 17, 2022

oxade Feb 17, 2022

oxade Feb 17, 2022

oxade Feb 17, 2022

oxade Feb 17, 2022 •

edited

Loading

oxade commented Feb 17, 2022

sblackshear commented Feb 17, 2022

sblackshear Feb 17, 2022

sblackshear Feb 17, 2022

patrickkuo commented Feb 17, 2022

patrickkuo commented Feb 17, 2022

oxade left a comment

	"Loaded {}package [{}] from {:?}.",
	"Loaded {} package [{}] from {:?}.",

[fastx CLI] Add customisation to genesis #461

[fastx CLI] Add customisation to genesis #461

Conversation

patrickkuo commented Feb 16, 2022 • edited Loading

Genesis customization

Choose a reason for hiding this comment

sblackshear left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oxade Feb 17, 2022 • edited Loading

Choose a reason for hiding this comment

oxade commented Feb 17, 2022

sblackshear commented Feb 17, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

patrickkuo commented Feb 17, 2022

patrickkuo commented Feb 17, 2022

oxade left a comment

Choose a reason for hiding this comment

patrickkuo commented Feb 16, 2022 •

edited

Loading

oxade Feb 17, 2022 •

edited

Loading