fix(starknet_integration_tests): fix config when creating multiple nodes

[yair-starkware]

No description provided.

[reviewable-StarkWare]

This change is 

[yair-starkware]

• test(starknet_integration_tests): test mempool syncing in e2e flow test #2628
• test(starknet_integration_tests): test proposer-validator setup in flow test #2348
• fix(starknet_integration_tests): temporary fixes to make the e2e flow test pass #2591
• fix(starknet_integration_tests): fix config when creating multiple nodes #2426 👈 (View in Graphite)
• refactor(starknet_integration_tests): separate sequencer setup from test setup #2299
• test(starknet_integration_tests): test commit blocks by running multiple heights #2242
• feat(tests_integration): one to many network config #2253
• main

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

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 34.10%. Comparing base (e3165c4) to head (a4dc64d).
Report is 797 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2426      +/-   ##
==========================================
- Coverage   40.10%   34.10%   -6.01%     
==========================================
  Files          26      276     +250     
  Lines        1895    32153   +30258     
  Branches     1895    32153   +30258     
==========================================
+ Hits          760    10965   +10205     
- Misses       1100    20200   +19100     
- Partials       35      988     +953     

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

[alonh5]

Reviewed 3 of 4 files at r1, all commit messages.
Reviewable status: 3 of 4 files reviewed, 5 unresolved discussions (waiting on @Itay-Tsabary-Starkware and @yair-starkware)

---

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

// Currently the orchestrator expects the sequencer addresses in the form of [0..n_sequencers).
// TODO(yair): Change this offset to be a non-zero value once the orchestrator is updated.
pub const SEQUENCER_ADDRESS_OFFSET: usize = 0;

Shouldn't this be 100?

Suggestion:

pub const SEQUENCER_ADDRESS_OFFSET: usize = 100;

---

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

    let mempool_p2p_config = create_mempool_p2p_config(sequencer_id, chain_info.chain_id.clone());
    let monitoring_endpoint_config = create_monitoring_endpoint_config(sequencer_id);
    let sequencer_address = create_sequencer_address(sequencer_id);

Use this to set validator id in config.

Code quote:

let sequencer_address = create_sequencer_address(sequencer_id);

---

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

    config.port += u16::try_from(sequencer_id).unwrap();
    config
}

@ShahakShama can you ptal at this?

Code quote:

fn create_mempool_p2p_config(sequencer_id: usize, chain_id: ChainId) -> MempoolP2pConfig {
    let mut config = MempoolP2pConfig::default();
    // When running multiple sequencers on the same machine, we need to make sure their ports are
    // different. Use the sequencer_id to differentiate between them.
    config.network_config.tcp_port += u16::try_from(sequencer_id).unwrap();
    config.network_config.quic_port += u16::try_from(sequencer_id).unwrap();
    config.network_config.chain_id = chain_id;
    config
}

fn create_monitoring_endpoint_config(sequencer_id: usize) -> MonitoringEndpointConfig {
    let mut config = MonitoringEndpointConfig::default();
    config.port += u16::try_from(sequencer_id).unwrap();
    config
}

---

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

                "The sequencer address.",
            ),
            // TODO(Matan): make validator id of consensus point to the sequencer address.

We are removing the sequencer address from the block builder config. You can remove the TODO.

---

crates/starknet_integration_tests/src/integration_test_setup.rs line 61 at r1 (raw file):

        // Derive the configuration for the sequencer node.
        let (config, required_params) = create_config(
            SEQUENCER_ID,

It's confusing that this is an index while validator id is a contract address. Can you change everywhere to differentiate?
use index or number instead of id

Code quote:

SEQUENCER_ID

[alonh5]

Reviewed 1 of 4 files at r1.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @Itay-Tsabary-Starkware and @yair-starkware)

[yair-starkware]

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

---

crates/starknet_integration_tests/src/integration_test_setup.rs line 61 at r1 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

It's confusing that this is an index while validator id is a contract address. Can you change everywhere to differentiate?
use index or number instead of id

Done.

---

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

Previously, alonh5 (Alon Haramati) wrote…

Shouldn't this be 100?

Code moved

---

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

Previously, alonh5 (Alon Haramati) wrote…

Use this to set validator id in config.

No need to create sequencer address anymore

---

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

Previously, alonh5 (Alon Haramati) wrote…

We are removing the sequencer address from the block builder config. You can remove the TODO.

Done.

[alonh5]

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

---

crates/starknet_integration_tests/src/utils.rs line 256 at r3 (raw file):

    // When running multiple sequencers on the same machine, we need to make sure their ports are
    // different. Use the sequencer_index to differentiate between them.
    config.network_config.tcp_port += u16::try_from(sequencer_index).unwrap();

Can we ensure or assert that the ports aren't colliding?

[yair-starkware]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @alonh5 and @Itay-Tsabary-Starkware)

---

crates/starknet_integration_tests/src/utils.rs line 256 at r3 (raw file):

Previously, alonh5 (Alon Haramati) wrote…

Can we ensure or assert that the ports aren't colliding?

It will be checked at the node's startup.

[alonh5]

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

---

crates/starknet_integration_tests/src/utils.rs line 261 at r4 (raw file):

    // different. Use the sequencer_index to differentiate between them.
    config.network_config.tcp_port += u16::try_from(sequencer_index).unwrap();
    config.network_config.quic_port += u16::try_from(sequencer_index).unwrap();

The quic port is configured to be 1 over the tcp port, so the first node's quic port collides with the second's tcp port.
Shahak told me the quic port isn't currently used, which is why this doesn't fail, and they will remove it. Jusy an FYI

[alonh5]

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