node: add `missing_iterations` logs #1154

herr-seppia · 2023-11-23T09:05:08Z

Resolves #1150

goshawk-3 · 2023-11-23T12:15:02Z

node/src/chain/acceptor.rs

+                    prov.create_committee(&sortition::Config {
+                        committee_size: SELECTION_COMMITTEE_SIZE,
+                        round: blk.header().height,
+                        seed: blk.header().seed,


This is wrong. You need mrb.header().seed here.

It also returns:

{"level":"ERROR","message":"Unable to generate voting committee for missed block","target":"node::chain::acceptor"}

To resolve this error you need to update eligiblity_flag:

for iter in 0..blk.header().iteration { let mut prov = provisioners_list.clone(); + prov.update_eligibility_flag(blk.header().height); let committee_keys = prov.create_committee(&sortition::Config { committee_size: SELECTION_COMMITTEE_SIZE, round: blk.header().height, - seed: blk.header().seed, + seed: mrb.header().seed, step: iter * 3, });

This should fix all issues and generate correct result.

Why there is the need of update eligibility flag if the block height is already specified in the sortition config?
Maybe we should change that API (unless there is a use case for calling one of those two methods without the other one)

These are distinct scenarios. In essence, for each step within a single round, it is necessary to invoke the create_committee, whereas the update_eligibility_flag is called only once per round execution.

goshawk-3 · 2023-11-23T12:32:20Z

node/src/chain/acceptor.rs

@@ -270,6 +298,7 @@ impl<DB: database::DB, VM: vm::VMExecution, N: Network> Acceptor<N, DB, VM> {
            fsv_bitset,
            ssv_bitset,
            block_time,
+            generator = blk.header().generator_bls_pubkey.to_base58(),


As this is printed in Info level, why don't we use the truncated bs58 version?

{"level":"INFO","event":"block accepted","height":6,"iter":2,"hash":"ab91718e2b839959...f9e24b5b90e49eac","txs":0,"state_hash":"e72889f7b945edbf...3185aaf382334050","fsv_bitset":15,"ssv_bitset":14,"block_time":13,"generator":"rm6jwcM9xQChnVeHYw6W8HP7kW3E8nejgs4XhmKoYspQnrZL5dDcQjVR5e2UmbyNNF4DJDRdZGdYLgJfZ3qQXuhSCL6UTDkcQ2QEf6RW3aCtfqQMKCFMNaBsfNrZuzrP23Z","dur_ms":"5","target":"node::chain::acceptor"}

For different reason:

Previous node was logging it as full bs58 to properly match the staking address

There is no truncated bs58 for the PublicKeyBytes

Anyway, we can get rid of previous impl, moving it to shortened bs58

There is no value of having a full bs58 here. We may introduce (if not available already) an grpc/graphql API that prints full bs58 of the generator only on demand instead of polluting INFO level logs with extra data.

goshawk-3 · 2023-11-23T12:34:05Z

node/src/chain/acceptor.rs

+            for iter in 0..blk.header().iteration {
+                let mut prov = provisioners_list.clone();
+                let committee_keys =
+                    prov.create_committee(&sortition::Config {
+                        committee_size: SELECTION_COMMITTEE_SIZE,
+                        round: blk.header().height,
+                        seed: blk.header().seed,
+                        step: iter * 3,
+                    });
+                if committee_keys.len() != 1 {
+                    error!(
+                        "Unable to generate voting committee for missed block"
+                    )
+                } else {
+                    let generator = committee_keys
+                        .first()
+                        .expect("committee to have 1 entry")
+                        .to_full_bs58();
+                    warn!(
+                        event = "missed iteration",
+                        height = blk.header().height,
+                        iter,
+                        generator,
+                    );
+                }
+            }
+


nitpick: Could you move this block in a separate function for better readability? I try to keep try_accept_block lean.

goshawk-3 · 2023-11-23T12:39:19Z

node/src/chain/acceptor.rs

+                    });
+                if committee_keys.len() != 1 {
+                    error!(
+                        "Unable to generate voting committee for missed block"


Print here the committee_keys.len().

goshawk-3 · 2023-11-23T14:28:16Z

node/src/chain/acceptor.rs

@@ -228,6 +229,33 @@ impl<DB: database::DB, VM: vm::VMExecution, N: Network> Acceptor<N, DB, VM> {
                Ok(txs)
            })?;

+            for iter in 0..blk.header().iteration {
+                let mut prov = provisioners_list.clone();


Duplicating the provisioners_list for every iteration could be inefficient, especially if a block originates from a higher iteration

Resolves #1150

herr-seppia requested review from HDauven and goshawk-3 November 23, 2023 09:05

herr-seppia force-pushed the missing_iterations branch from a9ca8be to 96fc593 Compare November 23, 2023 12:02

goshawk-3 suggested changes Nov 23, 2023

View reviewed changes

goshawk-3 reviewed Nov 23, 2023

View reviewed changes

herr-seppia added 2 commits November 23, 2023 15:42

node: add to_bs58 to PublicKeyBytes

5b45a42

node: add missing_iterations logs

84ff576

Resolves #1150

herr-seppia force-pushed the missing_iterations branch from 96fc593 to 84ff576 Compare November 23, 2023 14:42

herr-seppia requested a review from goshawk-3 November 23, 2023 14:43

goshawk-3 approved these changes Nov 24, 2023

View reviewed changes

herr-seppia merged commit c5c8718 into master Nov 24, 2023
3 checks passed

herr-seppia deleted the missing_iterations branch November 24, 2023 08:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

node: add `missing_iterations` logs #1154

node: add `missing_iterations` logs #1154

herr-seppia commented Nov 23, 2023

goshawk-3 Nov 23, 2023

goshawk-3 Nov 23, 2023

goshawk-3 Nov 23, 2023

herr-seppia Nov 23, 2023

goshawk-3 Nov 23, 2023

goshawk-3 Nov 23, 2023

herr-seppia Nov 23, 2023

goshawk-3 Nov 23, 2023

goshawk-3 Nov 23, 2023

goshawk-3 Nov 23, 2023

goshawk-3 Nov 23, 2023

node: add missing_iterations logs #1154

node: add missing_iterations logs #1154

Conversation

herr-seppia commented Nov 23, 2023

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

node: add `missing_iterations` logs #1154

node: add `missing_iterations` logs #1154