feat(sequencing): add client to cende

[DvirYo-starkware]

No description provided.

[reviewable-StarkWare]

This change is 

[github-actions]

Artifacts upload workflows:

• Started at 2024-12-23T15:22:17Z
• Started at 2024-12-23T15:22:52Z
• Started at 2024-12-23T15:23:54Z
• Started at 2024-12-23T20:59:06Z
• Started at 2024-12-25T09:26:33Z
• Started at 2024-12-25T09:29:51Z
• Started at 2024-12-25T10:01:28Z
• Started at 2024-12-25T11:03:21Z
• Started at 2024-12-25T13:50:30Z
• Started at 2024-12-25T15:54:21Z
• Started at 2024-12-26T08:59:10Z
• Started at 2024-12-26T11:22:22Z

[DvirYo-starkware]

Reviewable status: 0 of 11 files reviewed, 1 unresolved discussion (waiting on @dafnamatsry and @Yael-Starkware)

---

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

            set_pointing_param_paths(&["consensus_manager_config.consensus_config.validator_id"]),
        ),
        (

Mandatory config params are at the top level with pointers to the "real" value.

[Yael-Starkware]

Reviewable status: 0 of 11 files reviewed, 1 unresolved discussion (waiting on @dafnamatsry and @DvirYo-starkware)

---

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

Previously, DvirYo-starkware wrote…

Mandatory config params are at the top level with pointers to the "real" value.

I thought that config pointers are used to share configs between different services.
why does this need to be here?
There are many mandatory config (most are mandatory), that do not appear here.

[Yael-Starkware]

Reviewable status: 0 of 11 files reviewed, 5 unresolved discussions (waiting on @dafnamatsry and @DvirYo-starkware)

---

crates/sequencing/papyrus_consensus_orchestrator/src/cende/mod.rs line 103 at r1 (raw file):

            // Heights that are permitted to be built without writing to Aerospike.
            // Height 1 to make  `end_to_end_flow` test pass.
            const PERMITTED_HEIGHTS: [BlockNumber; 1] = [BlockNumber(1)];

Suggestion:

NO_WRITE_REQUIRED_HEIGHTS

---

crates/sequencing/papyrus_consensus_orchestrator/src/cende/mod.rs line 115 at r1 (raw file):

            }
            let Some(ref blob) = *prev_height_blob.lock().await else {
                debug!("No blob to write to Aerospike.");

when does it happen that there is no blob to write?

Code quote:

debug!("No blob to write to Aerospike.");

---

crates/sequencing/papyrus_consensus_orchestrator/src/cende/mod.rs line 131 at r1 (raw file):

                    }
                }
                Err(err) => {

what is the difference between the 2 errors? one is a communication error and one is an error in the recorder service?
lets write a more specific log message

Code quote:

Err(err) => {

---

crates/sequencing/papyrus_consensus_orchestrator/src/cende/mod.rs line 134 at r1 (raw file):

                    debug!("Failed to write blob to Aerospike. Error: {}", err);
                    // TODO(dvir): change this to `false`. The reason for the current value is to
                    // make the `end_to_end_flow_test` to pass.

lets use mock for tests

Code quote:

                    // TODO(dvir): change this to `false`. The reason for the current value is to
                    // make the `end_to_end_flow_test` to pass.

[DvirYo-starkware]

Reviewable status: 0 of 12 files reviewed, 5 unresolved discussions (waiting on @dafnamatsry, @Itay-Tsabary-Starkware, and @Yael-Starkware)

---

crates/sequencing/papyrus_consensus_orchestrator/src/cende/mod.rs line 115 at r1 (raw file):

Previously, Yael-Starkware (YaelD) wrote…

when does it happen that there is no blob to write?

When starting. You don't have the info from the previous height

---

crates/sequencing/papyrus_consensus_orchestrator/src/cende/mod.rs line 131 at r1 (raw file):

Previously, Yael-Starkware (YaelD) wrote…

what is the difference between the 2 errors? one is a communication error and one is an error in the recorder service?
lets write a more specific log message

Please suggest different messages

---

crates/sequencing/papyrus_consensus_orchestrator/src/cende/mod.rs line 134 at r1 (raw file):

Previously, Yael-Starkware (YaelD) wrote…

lets use mock for tests

In unitests, we use mocks; in flow tests, we should not use mocks.

---

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

Previously, Yael-Starkware (YaelD) wrote…

I thought that config pointers are used to share configs between different services.
why does this need to be here?
There are many mandatory config (most are mandatory), that do not appear here.

I also talked with @Itay-Tsabary-Starkware; at least for now, this is how we treat required parameters.
all the mandatory configs are here.

---

crates/sequencing/papyrus_consensus_orchestrator/src/cende/mod.rs line 103 at r1 (raw file):

            // Heights that are permitted to be built without writing to Aerospike.
            // Height 1 to make  `end_to_end_flow` test pass.
            const PERMITTED_HEIGHTS: [BlockNumber; 1] = [BlockNumber(1)];

Done.

[github-actions]

Benchmark movements:
tree_computation_flow performance regressed!
tree_computation_flow time: [34.430 ms 34.753 ms 35.121 ms]
change: [+1.1221% +2.2387% +3.2708%] (p = 0.00 < 0.05)
Performance has regressed.
Found 21 outliers among 100 measurements (21.00%)
8 (8.00%) high mild
13 (13.00%) high severe

[github-actions]

Benchmark movements:
tree_computation_flow performance improved 😺
tree_computation_flow time: [34.055 ms 34.092 ms 34.131 ms]
change: [-4.7223% -3.1007% -1.5993%] (p = 0.00 < 0.05)
Performance has improved.
Found 2 outliers among 100 measurements (2.00%)
1 (1.00%) high mild
1 (1.00%) high severe

[dafnamatsry]

Reviewable status: 0 of 12 files reviewed, 8 unresolved discussions (waiting on @DvirYo-starkware, @Itay-Tsabary-Starkware, and @Yael-Starkware)

---

crates/sequencing/papyrus_consensus_orchestrator/src/cende/mod.rs line 105 at r3 (raw file):

            // Heights that are permitted to be built without writing to Aerospike.
            // Height 1 to make  `end_to_end_flow` test pass.
            const NO_WRITE_REQUIRED_HEIGHTS: [BlockNumber; 1] = [BlockNumber(1)];

Suggestion:

SKIP_WRITE_HEIGHTS

---

crates/sequencing/papyrus_consensus_orchestrator/src/cende/mod.rs line 109 at r3 (raw file):

            if NO_WRITE_REQUIRED_HEIGHTS.contains(&height) {
                debug!(
                    "height {} is in `PERMITTED_HEIGHTS`, consensus can send proposal without \

Change here as well.

Code quote:

PERMITTED_HEIGHTS

---

crates/sequencing/papyrus_consensus_orchestrator/src/cende/mod.rs line 127 at r3 (raw file):

                    if response.status().is_success() {
                        debug!("Blob written to Aerospike successfully.");
                        sender.send(true).expect("Writing to a one-shot sender should succeed.");

This line repeats itself many times (sometimes with expect, sometimes with unwrap). Please move it to a helper function.

Code quote:

sender.send(true).expect("Writing to a one-shot sender should succeed.");

[DvirYo-starkware]

Reviewable status: 0 of 12 files reviewed, 8 unresolved discussions (waiting on @dafnamatsry, @Itay-Tsabary-Starkware, and @Yael-Starkware)

---

crates/sequencing/papyrus_consensus_orchestrator/src/cende/mod.rs line 109 at r3 (raw file):

Previously, dafnamatsry wrote…

Change here as well.

Done.

---

crates/sequencing/papyrus_consensus_orchestrator/src/cende/mod.rs line 127 at r3 (raw file):

Previously, dafnamatsry wrote…

This line repeats itself many times (sometimes with expect, sometimes with unwrap). Please move it to a helper function.

Done.

---

crates/sequencing/papyrus_consensus_orchestrator/src/cende/mod.rs line 105 at r3 (raw file):

            // Heights that are permitted to be built without writing to Aerospike.
            // Height 1 to make  `end_to_end_flow` test pass.
            const NO_WRITE_REQUIRED_HEIGHTS: [BlockNumber; 1] = [BlockNumber(1)];

Done.

[Yael-Starkware]

Reviewable status: 0 of 12 files reviewed, 6 unresolved discussions (waiting on @dafnamatsry, @DvirYo-starkware, and @Itay-Tsabary-Starkware)

---

crates/sequencing/papyrus_consensus_orchestrator/src/cende/mod.rs line 131 at r1 (raw file):

Previously, DvirYo-starkware wrote…

Please suggest different messages

if the reasons for failure are the ones I wrote above:

1. "The recorder failed to write blob. Error: {}",...
2. "Failed to send a request to the recorder. Error: {}",....

[DvirYo-starkware]

Reviewable status: 0 of 12 files reviewed, 6 unresolved discussions (waiting on @dafnamatsry, @Itay-Tsabary-Starkware, and @Yael-Starkware)

---

crates/sequencing/papyrus_consensus_orchestrator/src/cende/mod.rs line 131 at r1 (raw file):

Previously, Yael-Starkware (YaelD) wrote…

if the reasons for failure are the ones I wrote above:

1. "The recorder failed to write blob. Error: {}",...
2. "Failed to send a request to the recorder. Error: {}",....

Done.

[Yael-Starkware]

Reviewable status: 0 of 12 files reviewed, 4 unresolved discussions (waiting on @dafnamatsry and @DvirYo-starkware)

[Yael-Starkware]

Reviewed 7 of 11 files at r1, 3 of 5 files at r2, 1 of 1 files at r3, 1 of 3 files at r5.
Reviewable status: 10 of 12 files reviewed, 4 unresolved discussions (waiting on @dafnamatsry)

[Yael-Starkware]

Reviewable status: 10 of 12 files reviewed, 4 unresolved discussions (waiting on @dafnamatsry and @DvirYo-starkware)

---

crates/sequencing/papyrus_consensus_orchestrator/src/cende/mod.rs line 115 at r1 (raw file):

Previously, DvirYo-starkware wrote…

When starting. You don't have the info from the previous height

can you add a comment explaining that?

[DvirYo-starkware]

Reviewable status: 9 of 12 files reviewed, 4 unresolved discussions (waiting on @dafnamatsry and @Yael-Starkware)

---

crates/sequencing/papyrus_consensus_orchestrator/src/cende/mod.rs line 115 at r1 (raw file):

Previously, Yael-Starkware (YaelD) wrote…

can you add a comment explaining that?

Done.