chore(consensus): add config pointer of sequencer address to validator id

[ArniStarkware]

No description provided.

[reviewable-StarkWare]

This change is 

[ArniStarkware]

• chore(starknet_sequencer_node): use default value for validator id #2602
• chore(consensus): add config pointer of sequencer address to validator id #2494 👈 (View in Graphite)
• chore(starknet_batcher): in block builder use the consensus suplied sequncer address #2409 : 1 other dependent PR (#2425 )
• test(starknet_integration_tests): match sequencer address with default validator id #2486
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[alonh5]

Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ArniStarkware and @matan-starkware)

---

crates/starknet_integration_tests/src/utils.rs line 66 at r1 (raw file):

            eth_fee_token_address: fee_token_addresses.eth_fee_token_address,
            strk_fee_token_address: fee_token_addresses.strk_fee_token_address,
            sequencer_address: ContractAddress::from(100_u128),

throughout the PR.

Suggestion:

validator_id

[ArniStarkware]

crates/starknet_sequencer_node/src/config/node_config.rs line 79 at r1 (raw file):

            ]),
        ),
        // TODO(tsabary): set as a regular required parameter.

@Itay-Tsabary-Starkware, In #2409, I deleted this param. In this PR, I added it back just to make it a required param. Can we follow through with the to-do instead of adding it here?

Code quote:

// TODO(tsabary): set as a regular required parameter.

[Itay-Tsabary-Starkware]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @ArniStarkware and @matan-starkware)

---

crates/starknet_sequencer_node/src/config/node_config.rs line 79 at r1 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

@Itay-Tsabary-Starkware, In #2409, I deleted this param. In this PR, I added it back just to make it a required param. Can we follow through with the to-do instead of adding it here?

I am confused. Let's talk in person.

[codecov]

Codecov Report

Attention: Patch coverage is 90.90909% with 1 line in your changes missing coverage. Please review.

Project coverage is 38.18%. Comparing base (e3165c4) to head (5649fa4).
Report is 809 commits behind head on main.

Files with missing lines	Patch %	Lines
...tes/starknet_integration_tests/src/config_utils.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2494      +/-   ##
==========================================
- Coverage   40.10%   38.18%   -1.93%     
==========================================
  Files          26      278     +252     
  Lines        1895    32141   +30246     
  Branches     1895    32141   +30246     
==========================================
+ Hits          760    12273   +11513     
- Misses       1100    18840   +17740     
- Partials       35     1028     +993     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ArniStarkware]

crates/starknet_integration_tests/src/utils.rs line 66 at r1 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

throughout the PR.

Done.

[alonh5]

Reviewed 5 of 5 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @ArniStarkware, @Itay-Tsabary-Starkware, and @matan-starkware)

---

crates/starknet_sequencer_node/src/config/node_config.rs line 79 at r1 (raw file):

Previously, Itay-Tsabary-Starkware wrote…

I am confused. Let's talk in person.

Is there a way to make a param required without making it a pointer? If so, let's do it in this PR; if not, leave it to TODO.

---

crates/starknet_sequencer_node/src/config/node_config.rs line 84 at r2 (raw file):

                "validator_id",
                SerializationType::String,
                "The ID the validator. Also the address of this validator as a starknet contract.",

Suggestion:

The ID of the validator

[ArniStarkware]

Reviewable status: 4 of 5 files reviewed, 2 unresolved discussions (waiting on @alonh5, @Itay-Tsabary-Starkware, and @matan-starkware)

---

crates/starknet_sequencer_node/src/config/node_config.rs line 84 at r2 (raw file):

                "validator_id",
                SerializationType::String,
                "The ID the validator. Also the address of this validator as a starknet contract.",

Done.

[alonh5]

Reviewed 1 of 1 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @ArniStarkware, @Itay-Tsabary-Starkware, and @matan-starkware)

---

crates/starknet_sequencer_node/src/config/node_config.rs line 79 at r1 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

Is there a way to make a param required without making it a pointer? If so, let's do it in this PR; if not, leave it to TODO.

@ArniStarkware, please check with Itay about the above question before merging.

---

crates/starknet_sequencer_node/src/config/node_config.rs line 84 at r4 (raw file):

                "validator_id",
                SerializationType::String,
                "The ID of the validator.\

Suggestion:

"The ID of the validator. \

[ArniStarkware]

crates/starknet_sequencer_node/src/config/node_config.rs line 79 at r1 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

@ArniStarkware, please check with Itay about the above question before merging.

Itay explained the subject of config pointers to me on huddle.
We can set required params without setting them as config pointers. There are two reasons to keep this as a pointer.

1. If we expect the value to be duplicated in one or more locations (we can consult Matan / Shachak). We suspect this is not the case, so this point might not matter.
2. If we want to keep the validator ID as a pointer for ease of use. Otherwise, when the system is set up - when the user needs to put in the config manually - he will have to provide the whole path: consensus_manager_config.consensus_config.validator_id instead of just the name of the pointer: validator_id.

[ArniStarkware]

Reviewable status: 2 of 5 files reviewed, 1 unresolved discussion (waiting on @alonh5, @Itay-Tsabary-Starkware, and @matan-starkware)

---

crates/starknet_sequencer_node/src/config/node_config.rs line 79 at r1 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

Itay explained the subject of config pointers to me on huddle.
We can set required params without setting them as config pointers. There are two reasons to keep this as a pointer.

1. If we expect the value to be duplicated in one or more locations (we can consult Matan / Shachak). We suspect this is not the case, so this point might not matter.
2. If we want to keep the validator ID as a pointer for ease of use. Otherwise, when the system is set up - when the user needs to put in the config manually - he will have to provide the whole path: consensus_manager_config.consensus_config.validator_id instead of just the name of the pointer: validator_id.

Deleted the TODO.
IIUC, it will be easier to use this way.

---

crates/starknet_sequencer_node/src/config/node_config.rs line 84 at r4 (raw file):

                "validator_id",
                SerializationType::String,
                "The ID of the validator.\

Done.

[alonh5]

Reviewed 3 of 3 files at r5, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ArniStarkware, @Itay-Tsabary-Starkware, and @matan-starkware)

---

crates/starknet_sequencer_node/src/config/node_config.rs line 84 at r4 (raw file):

Previously, ArniStarkware (Arnon Hod) wrote…

Done.

Don't see it.

[ArniStarkware]

Reviewable status: 4 of 5 files reviewed, 1 unresolved discussion (waiting on @alonh5, @Itay-Tsabary-Starkware, and @matan-starkware)

---

crates/starknet_sequencer_node/src/config/node_config.rs line 84 at r4 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

Don't see it.

Now done.

[alonh5]

Reviewed 1 of 1 files at r6, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ArniStarkware, @Itay-Tsabary-Starkware, and @matan-starkware)

---

config/sequencer/default_config.json line 953 at r6 (raw file):

  },
  "validator_id": {
    "description": "A required param! The ID of the validator.Also the address of this validator as a starknet contract.",

Here it should update also right? That's how I noticed the space was missing.

Code quote:

validator.Also

[ArniStarkware]

Reviewable status: 4 of 5 files reviewed, 1 unresolved discussion (waiting on @alonh5, @Itay-Tsabary-Starkware, and @matan-starkware)

---

config/sequencer/default_config.json line 953 at r6 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

Here it should update also right? That's how I noticed the space was missing.

Done.

[alonh5]

Reviewed 1 of 1 files at r7, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Itay-Tsabary-Starkware and @matan-starkware)

[alonh5]

Reviewed 2 of 2 files at r8, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Itay-Tsabary-Starkware and @matan-starkware)

[alonh5]

Reviewed 3 of 3 files at r9, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Itay-Tsabary-Starkware and @matan-starkware)