refactor: add NumberedBlock

this allows to get rid of the pairs of fields, and just have three
fields in syncing::Status.

found out that one cannot use deny_unknown_fields within an untagged
tree, but the syncing::Status is only needs deserialize for testing.

crates/pathfinder/src/rpc.rs

@@ -2203,19 +2203,11 @@ mod tests {
 
         #[tokio::test]
         async fn syncing() {
+            use crate::rpc::types::reply::syncing::NumberedBlock;
             let expected = Syncing::Status(syncing::Status {
-                starting_block_hash: StarknetBlockHash(
-                    StarkHash::from_be_slice(b"starting").unwrap(),
-                ),
-                starting_block_num: StarknetBlockNumber(1),
-                current_block_hash: StarknetBlockHash(
-                    StarkHash::from_be_slice(b"current").unwrap(),
-                ),
-                current_block_num: StarknetBlockNumber(2),
-                highest_block_hash: StarknetBlockHash(
-                    StarkHash::from_be_slice(b"highest").unwrap(),
-                ),
-                highest_block_num: StarknetBlockNumber(3),
+                starting: NumberedBlock::from(("abbacd", 1)),
+                current: NumberedBlock::from(("abbace", 2)),
+                highest: NumberedBlock::from(("abbacf", 3)),
             });
 
             let storage = setup_storage();

crates/pathfinder/src/rpc/types.rs

@@ -673,7 +673,6 @@ pub mod reply {
     /// Describes Starknet's syncing status RPC reply.
     #[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
     #[serde(untagged)]
-    #[serde(deny_unknown_fields)]
     pub enum Syncing {
         False(bool),
         Status(syncing::Status),
@@ -700,35 +699,91 @@ pub mod reply {
         use serde_with::serde_as;
 
         /// Represents Starknet node syncing status.
-        #[serde_as]
-        #[derive(Copy, Clone, Debug, Deserialize, Serialize, PartialEq)]
-        #[serde(deny_unknown_fields)]
+        #[derive(Copy, Clone, Debug, PartialEq, Serialize, Deserialize)]
         pub struct Status {
-            pub starting_block_hash: StarknetBlockHash,
-            #[serde_as(as = "StarknetBlockNumberAsHexStr")]
-            pub starting_block_num: StarknetBlockNumber,
-            pub current_block_hash: StarknetBlockHash,
-            #[serde_as(as = "StarknetBlockNumberAsHexStr")]
-            pub current_block_num: StarknetBlockNumber,
-            pub highest_block_hash: StarknetBlockHash,
-            #[serde_as(as = "StarknetBlockNumberAsHexStr")]
-            pub highest_block_num: StarknetBlockNumber,
+            #[serde(flatten, with = "prefix_starting")]
+            pub starting: NumberedBlock,
+            #[serde(flatten, with = "prefix_current")]
+            pub current: NumberedBlock,
+            #[serde(flatten, with = "prefix_highest")]
+            pub highest: NumberedBlock,
         }
 
+        serde_with::with_prefix!(prefix_starting "starting_");
+        serde_with::with_prefix!(prefix_current "current_");
+        serde_with::with_prefix!(prefix_highest "highest_");
+
         impl std::fmt::Display for Status {
             fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
                 write!(
                     f,
-                    "starting: ({}, {}), current: ({}, {}), highest: ({}, {})",
-                    self.starting_block_num.0,
-                    self.starting_block_hash.0,
-                    self.current_block_num.0,
-                    self.current_block_hash.0,
-                    self.highest_block_num.0,
-                    self.highest_block_hash.0,
+                    "starting: {:?}, current: {:?}, highest: {:?}",
+                    self.starting, self.current, self.highest,
                 )
             }
         }
+
+        /// Block hash and a number, for `starknet_syncing` response only.
+        #[serde_as]
+        #[derive(Clone, Copy, Serialize, Deserialize, PartialEq)]
+        pub struct NumberedBlock {
+            #[serde(rename = "block_hash")]
+            pub hash: StarknetBlockHash,
+            #[serde_as(as = "StarknetBlockNumberAsHexStr")]
+            #[serde(rename = "block_num")]
+            pub number: StarknetBlockNumber,
+        }
+
+        impl std::fmt::Debug for NumberedBlock {
+            fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+                write!(fmt, "({}, {})", self.hash.0, self.number.0)
+            }
+        }
+
+        impl From<(StarknetBlockHash, StarknetBlockNumber)> for NumberedBlock {
+            fn from((hash, number): (StarknetBlockHash, StarknetBlockNumber)) -> Self {
+                NumberedBlock { hash, number }
+            }
+        }
+
+        /// Helper to make it a bit less painful to write examples.
+        #[cfg(test)]
+        impl<'a> From<(&'a str, u64)> for NumberedBlock {
+            fn from((h, n): (&'a str, u64)) -> Self {
+                use pedersen::StarkHash;
+                NumberedBlock {
+                    hash: StarknetBlockHash(StarkHash::from_hex_str(h).unwrap()),
+                    number: StarknetBlockNumber(n),
+                }
+            }
+        }
+    }
+
+    #[test]
+    fn roundtrip_syncing() {
+        use syncing::NumberedBlock;
+        let examples = [
+            (line!(), "false", Syncing::False(false)),
+            // this shouldn't exist but it exists now
+            (line!(), "true", Syncing::False(true)),
+            (
+                line!(),
+                r#"{"starting_block_hash":"0xa","starting_block_num":"0x1","current_block_hash":"0xb","current_block_num":"0x2","highest_block_hash":"0xc","highest_block_num":"0x3"}"#,
+                Syncing::Status(syncing::Status {
+                    starting: NumberedBlock::from(("a", 1)),
+                    current: NumberedBlock::from(("b", 2)),
+                    highest: NumberedBlock::from(("c", 3)),
+                }),
+            ),
+        ];
+
+        for (line, input, expected) in examples {
+            let parsed = serde_json::from_str::<Syncing>(&input).unwrap();
+            let output = serde_json::to_string(&parsed).unwrap();
+
+            assert_eq!(parsed, expected, "example from line {}", line);
+            assert_eq!(&output, input, "example from line {}", line);
+        }
     }
 
     /// Describes an emitted event returned by starknet_getEvents

crates/pathfinder/src/state/sync.rs

@@ -15,7 +15,7 @@ use crate::{
         transport::EthereumTransport,
         Chain,
     },
-    rpc::types::reply::{syncing, Syncing as SyncStatus},
+    rpc::types::reply::{syncing, syncing::NumberedBlock, Syncing as SyncStatus},
     sequencer::{self, reply::Block},
     state::{calculate_contract_state_hash, state_tree::GlobalStateTree, update_contract_state},
     storage::{
@@ -205,12 +205,10 @@ where
                     match &mut *state.status.write().await {
                         SyncStatus::False(_) => {}
                         SyncStatus::Status(status) => {
-                            status.current_block_hash = block_hash;
-                            status.current_block_num = StarknetBlockNumber(block_num);
+                            status.current = NumberedBlock::from((block_hash, StarknetBlockNumber(block_num)));
 
-                            if status.highest_block_num.0 <= block_num {
-                                status.highest_block_num = StarknetBlockNumber(block_num);
-                                status.highest_block_hash = block_hash;
+                            if status.highest.number.0 <= block_num {
+                                status.highest = status.current;
                             }
                         }
                     }
@@ -330,24 +328,26 @@ async fn update_sync_status_latest(
     use crate::rpc::types::{BlockNumberOrTag, Tag};
     let poll_interval = head_poll_interval(chain);
 
+    let starting = NumberedBlock::from((starting_block_hash, starting_block_num));
+
     loop {
         match sequencer
             .block_by_number(BlockNumberOrTag::Tag(Tag::Latest))
             .await
         {
             Ok(block) => {
-                let latest_hash = block.block_hash.unwrap();
-                let latest_num = block.block_number.unwrap();
+                let latest = {
+                    let latest_hash = block.block_hash.unwrap();
+                    let latest_num = block.block_number.unwrap();
+                    NumberedBlock::from((latest_hash, latest_num))
+                };
                 // Update the sync status.
                 match &mut *state.status.write().await {
                     sync_status @ SyncStatus::False(_) => {
                         *sync_status = SyncStatus::Status(syncing::Status {
-                            starting_block_hash,
-                            starting_block_num,
-                            current_block_hash: starting_block_hash,
-                            current_block_num: starting_block_num,
-                            highest_block_hash: latest_hash,
-                            highest_block_num: latest_num,
+                            starting,
+                            current: starting,
+                            highest: latest,
                         });
 
                         tracing::debug!(
@@ -356,9 +356,8 @@ async fn update_sync_status_latest(
                         );
                     }
                     SyncStatus::Status(status) => {
-                        if status.highest_block_hash != latest_hash {
-                            status.highest_block_hash = latest_hash;
-                            status.highest_block_num = latest_num;
+                        if status.highest.hash != latest.hash {
+                            status.highest = latest;
 
                             tracing::debug!(
                                 %status,