[feature] #3094: Generate network with `n` peers #3096

Erigara · 2023-01-30T15:03:23Z

Signed-off-by: Shanin Roman shanin1000@yandex.ru

Description of the Change

Extend ./scripts/test_env.sh to work with arbitrary number of nodes.
Add option to kagami to generate peers in compact format.

Issue

Closes #3094

Benefits

Easier to test iroha with different number of peers.

Possible Drawbacks

None.

Usage Examples or Tests [optional]

# run 10 peers without docker
./scripts/test_env.sh setup bare 10
# run 10 peers with docker
./scripts/test_env.sh setup docker 10

codecov · 2023-01-30T15:08:37Z

Codecov Report

Merging #3096 (814d2db) into iroha2-dev (a4d5c9f) will increase coverage by 2.80%.
The diff coverage is 71.44%.

@@              Coverage Diff               @@
##           iroha2-dev    #3096      +/-   ##
==============================================
+ Coverage       62.33%   65.13%   +2.80%     
==============================================
  Files             169      171       +2     
  Lines           31218    33570    +2352     
==============================================
+ Hits            19459    21866    +2407     
+ Misses          11759    11704      -55

Impacted Files	Coverage Δ
cli/src/main.rs	`1.09% <0.00%> (ø)`
cli/src/torii/mod.rs	`27.65% <ø> (ø)`
cli/src/torii/utils.rs	`72.00% <0.00%> (-12.85%)`	⬇️
client_cli/src/main.rs	`0.24% <0.00%> (-0.01%)`	⬇️
config/base/src/lib.rs	`91.57% <ø> (+55.21%)`	⬆️
config/src/lib.rs	`33.33% <ø> (ø)`
config/src/logger.rs	`70.21% <ø> (ø)`
config/src/path.rs	`0.00% <0.00%> (ø)`
core/src/lib.rs	`100.00% <ø> (ø)`
core/src/smartcontracts/isi/block.rs	`85.71% <ø> (-3.18%)`	⬇️
... and 171 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

tools/kagami/src/main.rs

scripts/test_env.sh

appetrosyan · 2023-02-01T14:15:17Z

scripts/test_env.sh

@@ -7,62 +7,66 @@ TEST=${TEST:-"./test"}
 HOST=${HOST:-"127.0.0.1"}


This code is no longer what I'd call "reasonably small to be best done in bash".

We should consider looking at other alternatives, e.g. python:
https://unix.stackexchange.com/questions/24802/on-which-unix-distributions-is-python-installed-as-part-of-the-default-install

@astrokov7 had the idea of rewriting tests using python's subprocess infrastructure, it might be worth it to do the same here.

appetrosyan · 2023-02-01T14:19:05Z

scripts/test_env.sh

+    mapfile -t -n 3 buffer < <($TEST/kagami crypto -c)
+    public_keys[$1]="${buffer[0]}"
+    private_keys[$1]=$(printf '{"digest_function": "%s", "payload": "%s"}' "${buffer[2]}" "${buffer[1]}")


Seems like we might be doing something wrong in kagami if we need this. Should we adjust the syntax so we can just pipe the output? kagami crypto -c --json?

appetrosyan · 2023-02-01T14:21:55Z

scripts/test_env.sh

+function generate_genesis_key_pair {
+    mapfile -t -n 3 buffer < <($TEST/kagami crypto -c)
+    IROHA_GENESIS_ACCOUNT_PUBLIC_KEY="${buffer[0]}"
+    IROHA_GENESIS_ACCOUNT_PRIVATE_KEY=$(printf '{"digest_function": "%s", "payload": "%s"}' "${buffer[2]}" "${buffer[1]}")
+}


I'd wager this is not what we want to do. If the genesis key pair is not specified, sure, generate a fresh pair. But it might not work in all contexts. If the environment variable is defined and set properly, we should just leave it alone and use it directly.

That was actually the whole point of using environment variables for this purpose.

appetrosyan · 2023-02-01T14:23:50Z

scripts/test_env.sh

    do
-        trusted_peer_entry "iroha$iter"
+        trusted_peer_entry "$iter"
        echo -n ","
    done


printf? It should make sense stylistically.

appetrosyan · 2023-02-01T14:25:22Z

scripts/test_env.sh

+    generate_genesis_key_pair
+    for peer in $(seq 0 $(($1-1)))
+    do
+       generate_p2p_port $peer
+       generate_api_port $peer
+       generate_telemetry_port $peer
+       generate_peer_key_pair $peer
+       hosts[$peer]="$HOST"
+    done
+    SUMERAGI_TRUSTED_PEERS="$(generate_trusted_peers $1)"
+    for peer in $(seq 1 $(($1-1)))
+    do
+        run_peer $peer
+    done
+    run_peer 0 --submit-genesis


This isn't quite what I'd like to happen. Overwriting the same environment variable can have unforeseen consequences. Again one more reason to consider writing this logic in Python.

scripts/test_env.sh

appetrosyan · 2023-02-01T14:31:16Z

scripts/test_env.sh

+            if [ "$N_PEERS" -le 0 ]
+            then
+                echo "Expected number of peers as non-zero positive number (> 0). Recieved: $3"
+                exit 1
+            fi


Similarly, this is not what we do for other variables.

scripts/test_env.sh

appetrosyan · 2023-02-01T14:33:56Z

tools/kagami/src/main.rs

        json: bool,
+        /// Output the key-pair without additional text
+        #[clap(long, short, group = "format")]
+        compact: bool,


I'm all for this format, but IMO the fact that we need to do some bash scripting to use this information as part of JSON means that the format probably needs adjustment. Most people would assume that you can just assign the output of kagami to an environment variable and be done with it.

Signed-off-by: Shanin Roman <shanin1000@yandex.ru>

Erigara added the iroha2-dev The re-implementation of a BFT hyperledger in RUST label Jan 30, 2023

Erigara self-assigned this Jan 30, 2023

Erigara requested review from s8sato, appetrosyan, mversic, Arjentix, SamHSmith, QuentinI, outoftardis, ilchu and pesterev as code owners January 30, 2023 15:03

mversic previously approved these changes Jan 31, 2023

View reviewed changes

tools/kagami/src/main.rs Show resolved Hide resolved

scripts/test_env.sh Outdated Show resolved Hide resolved

mversic self-assigned this Jan 31, 2023

Erigara dismissed mversic’s stale review via 5e30273 January 31, 2023 06:44

Erigara force-pushed the gen_n_peers branch from 4508991 to 5e30273 Compare January 31, 2023 06:44

mversic previously approved these changes Jan 31, 2023

View reviewed changes

Erigara force-pushed the gen_n_peers branch from 5e30273 to e179747 Compare January 31, 2023 07:02

SamHSmith dismissed mversic’s stale review via e179747 February 1, 2023 08:52

appetrosyan force-pushed the gen_n_peers branch from e179747 to d66adab Compare February 1, 2023 14:07

appetrosyan reviewed Feb 1, 2023

View reviewed changes

[feature] hyperledger-iroha#3094: Generate network with n peers

814d2db

Signed-off-by: Shanin Roman <shanin1000@yandex.ru>

Erigara force-pushed the gen_n_peers branch from d66adab to 814d2db Compare February 1, 2023 15:15

appetrosyan approved these changes Feb 1, 2023

View reviewed changes

appetrosyan self-assigned this Feb 6, 2023

pesterev approved these changes Feb 6, 2023

View reviewed changes

appetrosyan merged commit 7316336 into hyperledger-iroha:iroha2-dev Feb 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[feature] #3094: Generate network with `n` peers #3096

[feature] #3094: Generate network with `n` peers #3096

Erigara commented Jan 30, 2023

codecov bot commented Jan 30, 2023 •

edited

Loading

appetrosyan Feb 1, 2023

appetrosyan Feb 1, 2023

appetrosyan Feb 1, 2023

appetrosyan Feb 1, 2023

appetrosyan Feb 1, 2023

appetrosyan Feb 1, 2023

appetrosyan Feb 1, 2023

		@@ -7,62 +7,66 @@ TEST=${TEST:-"./test"}
		HOST=${HOST:-"127.0.0.1"}

[feature] #3094: Generate network with n peers #3096

[feature] #3094: Generate network with n peers #3096

Conversation

Erigara commented Jan 30, 2023

Description of the Change

Issue

Benefits

Possible Drawbacks

Usage Examples or Tests [optional]

codecov bot commented Jan 30, 2023 • edited Loading

Codecov Report

appetrosyan Feb 1, 2023

Choose a reason for hiding this comment

appetrosyan Feb 1, 2023

Choose a reason for hiding this comment

appetrosyan Feb 1, 2023

Choose a reason for hiding this comment

appetrosyan Feb 1, 2023

Choose a reason for hiding this comment

appetrosyan Feb 1, 2023

Choose a reason for hiding this comment

appetrosyan Feb 1, 2023

Choose a reason for hiding this comment

appetrosyan Feb 1, 2023

Choose a reason for hiding this comment

[feature] #3094: Generate network with `n` peers #3096

[feature] #3094: Generate network with `n` peers #3096

codecov bot commented Jan 30, 2023 •

edited

Loading