rpc/types: fix epee deserialization for GetBlocksResponse (#345)

* header + flatten

* fix optional values

* `fn error() -> String` -> `error!() -> &'static str`

* extract out `PoolInfo`

* fix cargo hack

rpc/types/src/bin.rs

@@ -9,26 +9,19 @@ use cuprate_fixed_bytes::ByteArrayVec;
 use serde::{Deserialize, Serialize};
 
 #[cfg(feature = "epee")]
-use cuprate_epee_encoding::{
-    container_as_blob::ContainerAsBlob,
-    epee_object, error,
-    macros::bytes::{Buf, BufMut},
-    read_epee_value, write_field, EpeeObject, EpeeObjectBuilder,
-};
+use cuprate_epee_encoding::container_as_blob::ContainerAsBlob;
 
 use cuprate_types::BlockCompleteEntry;
 
 use crate::{
     base::AccessResponseBase,
-    macros::{define_request, define_request_and_response, define_request_and_response_doc},
-    misc::{BlockOutputIndices, GetOutputsOut, OutKeyBin, PoolTxInfo, Status},
+    macros::define_request_and_response,
+    misc::{BlockOutputIndices, GetOutputsOut, OutKeyBin, PoolInfo},
     rpc_call::RpcCallValue,
 };
 
 #[cfg(any(feature = "epee", feature = "serde"))]
 use crate::defaults::{default_false, default_zero};
-#[cfg(feature = "epee")]
-use crate::misc::PoolInfoExtent;
 
 //---------------------------------------------------------------------------------------------------- Definitions
 define_request_and_response! {
@@ -115,15 +108,14 @@ define_request_and_response! {
     }
 }
 
-//---------------------------------------------------------------------------------------------------- GetBlocks
-define_request! {
-    #[doc = define_request_and_response_doc!(
-        "response" => GetBlocksResponse,
-        get_blocksbin,
-        cc73fe71162d564ffda8e549b79a350bca53c454,
-        core_rpc_server_commands_defs, h, 162, 262,
-    )]
-    GetBlocksRequest {
+define_request_and_response! {
+    get_blocksbin,
+    cc73fe71162d564ffda8e549b79a350bca53c454 =>
+    core_rpc_server_commands_defs.h => 162..=262,
+
+    GetBlocks,
+
+    Request {
         requested_info: u8 = default_zero::<u8>(), "default_zero",
         // FIXME: This is a `std::list` in `monerod` because...?
         block_ids: ByteArrayVec<32>,
@@ -131,259 +123,17 @@ define_request! {
         prune: bool,
         no_miner_tx: bool = default_false(), "default_false",
         pool_info_since: u64 = default_zero::<u64>(), "default_zero",
-    }
-}
+    },
 
-#[doc = define_request_and_response_doc!(
-    "request" => GetBlocksRequest,
-    get_blocksbin,
-    cc73fe71162d564ffda8e549b79a350bca53c454,
-    core_rpc_server_commands_defs, h, 162, 262,
-)]
-///
-/// This response's variant depends upon [`PoolInfoExtent`].
-#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
-#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
-pub enum GetBlocksResponse {
-    /// Will always serialize a [`PoolInfoExtent::None`] field.
-    PoolInfoNone(GetBlocksResponsePoolInfoNone),
-    /// Will always serialize a [`PoolInfoExtent::Incremental`] field.
-    PoolInfoIncremental(GetBlocksResponsePoolInfoIncremental),
-    /// Will always serialize a [`PoolInfoExtent::Full`] field.
-    PoolInfoFull(GetBlocksResponsePoolInfoFull),
-}
-
-impl Default for GetBlocksResponse {
-    fn default() -> Self {
-        Self::PoolInfoNone(GetBlocksResponsePoolInfoNone::default())
-    }
-}
-
-/// Data within [`GetBlocksResponse::PoolInfoNone`].
-#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
-#[derive(Clone, Default, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
-pub struct GetBlocksResponsePoolInfoNone {
-    pub status: Status,
-    pub untrusted: bool,
-    pub blocks: Vec<BlockCompleteEntry>,
-    pub start_height: u64,
-    pub current_height: u64,
-    pub output_indices: Vec<BlockOutputIndices>,
-    pub daemon_time: u64,
-}
-
-#[cfg(feature = "epee")]
-epee_object! {
-    GetBlocksResponsePoolInfoNone,
-    status: Status,
-    untrusted: bool,
-    blocks: Vec<BlockCompleteEntry>,
-    start_height: u64,
-    current_height: u64,
-    output_indices: Vec<BlockOutputIndices>,
-    daemon_time: u64,
-}
-
-/// Data within [`GetBlocksResponse::PoolInfoIncremental`].
-#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
-#[derive(Clone, Default, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
-pub struct GetBlocksResponsePoolInfoIncremental {
-    pub status: Status,
-    pub untrusted: bool,
-    pub blocks: Vec<BlockCompleteEntry>,
-    pub start_height: u64,
-    pub current_height: u64,
-    pub output_indices: Vec<BlockOutputIndices>,
-    pub daemon_time: u64,
-    pub added_pool_txs: Vec<PoolTxInfo>,
-    pub remaining_added_pool_txids: ByteArrayVec<32>,
-    pub removed_pool_txids: ByteArrayVec<32>,
-}
-
-#[cfg(feature = "epee")]
-epee_object! {
-    GetBlocksResponsePoolInfoIncremental,
-    status: Status,
-    untrusted: bool,
-    blocks: Vec<BlockCompleteEntry>,
-    start_height: u64,
-    current_height: u64,
-    output_indices: Vec<BlockOutputIndices>,
-    daemon_time: u64,
-    added_pool_txs: Vec<PoolTxInfo>,
-    remaining_added_pool_txids: ByteArrayVec<32>,
-    removed_pool_txids: ByteArrayVec<32>,
-}
-
-/// Data within [`GetBlocksResponse::PoolInfoFull`].
-#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
-#[derive(Clone, Default, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
-pub struct GetBlocksResponsePoolInfoFull {
-    pub status: Status,
-    pub untrusted: bool,
-    pub blocks: Vec<BlockCompleteEntry>,
-    pub start_height: u64,
-    pub current_height: u64,
-    pub output_indices: Vec<BlockOutputIndices>,
-    pub daemon_time: u64,
-    pub added_pool_txs: Vec<PoolTxInfo>,
-    pub remaining_added_pool_txids: ByteArrayVec<32>,
-}
-
-#[cfg(feature = "epee")]
-epee_object! {
-    GetBlocksResponsePoolInfoFull,
-    status: Status,
-    untrusted: bool,
-    blocks: Vec<BlockCompleteEntry>,
-    start_height: u64,
-    current_height: u64,
-    output_indices: Vec<BlockOutputIndices>,
-    daemon_time: u64,
-    added_pool_txs: Vec<PoolTxInfo>,
-    remaining_added_pool_txids: ByteArrayVec<32>,
-}
-
-#[cfg(feature = "epee")]
-/// [`EpeeObjectBuilder`] for [`GetBlocksResponse`].
-///
-/// Not for public usage.
-#[derive(Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash)]
-#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
-pub struct __GetBlocksResponseEpeeBuilder {
-    pub status: Option<Status>,
-    pub untrusted: Option<bool>,
-    pub blocks: Option<Vec<BlockCompleteEntry>>,
-    pub start_height: Option<u64>,
-    pub current_height: Option<u64>,
-    pub output_indices: Option<Vec<BlockOutputIndices>>,
-    pub daemon_time: Option<u64>,
-    pub pool_info_extent: Option<PoolInfoExtent>,
-    pub added_pool_txs: Option<Vec<PoolTxInfo>>,
-    pub remaining_added_pool_txids: Option<ByteArrayVec<32>>,
-    pub removed_pool_txids: Option<ByteArrayVec<32>>,
-}
-
-#[cfg(feature = "epee")]
-impl EpeeObjectBuilder<GetBlocksResponse> for __GetBlocksResponseEpeeBuilder {
-    fn add_field<B: Buf>(&mut self, name: &str, r: &mut B) -> error::Result<bool> {
-        macro_rules! read_epee_field {
-            ($($field:ident),*) => {
-                match name {
-                    $(
-                        stringify!($field) => { self.$field = Some(read_epee_value(r)?); },
-                    )*
-                    _ => return Ok(false),
-                }
-            };
-        }
-
-        read_epee_field! {
-            status,
-            untrusted,
-            blocks,
-            start_height,
-            current_height,
-            output_indices,
-            daemon_time,
-            pool_info_extent,
-            added_pool_txs,
-            remaining_added_pool_txids,
-            removed_pool_txids
-        }
-
-        Ok(true)
-    }
-
-    fn finish(self) -> error::Result<GetBlocksResponse> {
-        const ELSE: error::Error = error::Error::Format("Required field was not found!");
-
-        let status = self.status.ok_or(ELSE)?;
-        let untrusted = self.untrusted.ok_or(ELSE)?;
-        let blocks = self.blocks.ok_or(ELSE)?;
-        let start_height = self.start_height.ok_or(ELSE)?;
-        let current_height = self.current_height.ok_or(ELSE)?;
-        let output_indices = self.output_indices.ok_or(ELSE)?;
-        let daemon_time = self.daemon_time.ok_or(ELSE)?;
-        let pool_info_extent = self.pool_info_extent.ok_or(ELSE)?;
-
-        let this = match pool_info_extent {
-            PoolInfoExtent::None => {
-                GetBlocksResponse::PoolInfoNone(GetBlocksResponsePoolInfoNone {
-                    status,
-                    untrusted,
-                    blocks,
-                    start_height,
-                    current_height,
-                    output_indices,
-                    daemon_time,
-                })
-            }
-            PoolInfoExtent::Incremental => {
-                GetBlocksResponse::PoolInfoIncremental(GetBlocksResponsePoolInfoIncremental {
-                    status,
-                    untrusted,
-                    blocks,
-                    start_height,
-                    current_height,
-                    output_indices,
-                    daemon_time,
-                    added_pool_txs: self.added_pool_txs.ok_or(ELSE)?,
-                    remaining_added_pool_txids: self.remaining_added_pool_txids.ok_or(ELSE)?,
-                    removed_pool_txids: self.removed_pool_txids.ok_or(ELSE)?,
-                })
-            }
-            PoolInfoExtent::Full => {
-                GetBlocksResponse::PoolInfoFull(GetBlocksResponsePoolInfoFull {
-                    status,
-                    untrusted,
-                    blocks,
-                    start_height,
-                    current_height,
-                    output_indices,
-                    daemon_time,
-                    added_pool_txs: self.added_pool_txs.ok_or(ELSE)?,
-                    remaining_added_pool_txids: self.remaining_added_pool_txids.ok_or(ELSE)?,
-                })
-            }
-        };
-
-        Ok(this)
-    }
-}
-
-#[cfg(feature = "epee")]
-impl EpeeObject for GetBlocksResponse {
-    type Builder = __GetBlocksResponseEpeeBuilder;
-
-    fn number_of_fields(&self) -> u64 {
-        // [`PoolInfoExtent`] + inner struct fields.
-        let inner_fields = match self {
-            Self::PoolInfoNone(s) => s.number_of_fields(),
-            Self::PoolInfoIncremental(s) => s.number_of_fields(),
-            Self::PoolInfoFull(s) => s.number_of_fields(),
-        };
-
-        1 + inner_fields
-    }
-
-    fn write_fields<B: BufMut>(self, w: &mut B) -> error::Result<()> {
-        match self {
-            Self::PoolInfoNone(s) => {
-                s.write_fields(w)?;
-                write_field(PoolInfoExtent::None.to_u8(), "pool_info_extent", w)?;
-            }
-            Self::PoolInfoIncremental(s) => {
-                s.write_fields(w)?;
-                write_field(PoolInfoExtent::Incremental.to_u8(), "pool_info_extent", w)?;
-            }
-            Self::PoolInfoFull(s) => {
-                s.write_fields(w)?;
-                write_field(PoolInfoExtent::Full.to_u8(), "pool_info_extent", w)?;
-            }
-        }
-
-        Ok(())
+    // TODO: add `top_block_hash` field
+    // <https://github.com/monero-project/monero/blame/893916ad091a92e765ce3241b94e706ad012b62a/src/rpc/core_rpc_server_commands_defs.h#L263>
+    AccessResponseBase {
+        blocks: Vec<BlockCompleteEntry>,
+        start_height: u64,
+        current_height: u64,
+        output_indices: Vec<BlockOutputIndices>,
+        daemon_time: u64,
+        pool_info: PoolInfo,
     }
 }
 

rpc/types/src/misc/misc.rs

@@ -11,11 +11,11 @@ use serde::{Deserialize, Serialize};
 #[cfg(feature = "epee")]
 use cuprate_epee_encoding::epee_object;
 
-use crate::macros::monero_definition_link;
-
 #[cfg(any(feature = "epee", feature = "serde"))]
 use crate::defaults::default_zero;
 
+use crate::macros::monero_definition_link;
+
 //---------------------------------------------------------------------------------------------------- Macros
 /// This macro (local to this file) defines all the misc types.
 ///

rpc/types/src/misc/mod.rs

@@ -17,6 +17,7 @@ mod distribution;
 mod key_image_spent_status;
 #[expect(clippy::module_inception)]
 mod misc;
+mod pool_info;
 mod pool_info_extent;
 mod status;
 mod tx_entry;
@@ -30,6 +31,7 @@ pub use misc::{
     OutputDistributionData, Peer, PoolTxInfo, PublicNode, SetBan, Span, SpentKeyImageInfo,
     SyncInfoPeer, TxBacklogEntry, TxInfo, TxOutputIndices, TxpoolHisto, TxpoolStats,
 };
+pub use pool_info::PoolInfo;
 pub use pool_info_extent::PoolInfoExtent;
 pub use status::Status;
 pub use tx_entry::TxEntry;

rpc/types/src/misc/pool_info.rs

@@ -0,0 +1,171 @@
+#[cfg(feature = "serde")]
+use serde::{Deserialize, Serialize};
+
+#[cfg(feature = "epee")]
+use crate::misc::PoolInfoExtent;
+#[cfg(feature = "epee")]
+use cuprate_epee_encoding::{
+    epee_object, error,
+    macros::bytes::{Buf, BufMut},
+    read_epee_value, write_field, EpeeObject, EpeeObjectBuilder,
+};
+
+use cuprate_fixed_bytes::ByteArrayVec;
+
+use crate::misc::PoolTxInfo;
+
+//---------------------------------------------------------------------------------------------------- PoolInfo
+#[doc = crate::macros::monero_definition_link!(
+    cc73fe71162d564ffda8e549b79a350bca53c454,
+    "rpc/core_rpc_server_commands_defs.h",
+    223..=228
+)]
+/// Used in [`crate::bin::GetBlocksResponse`].
+#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[derive(Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash)]
+#[repr(u8)]
+pub enum PoolInfo {
+    #[default]
+    None,
+    Incremental(PoolInfoIncremental),
+    Full(PoolInfoFull),
+}
+
+//---------------------------------------------------------------------------------------------------- Internal data
+/// Data within [`PoolInfo::Incremental`].
+#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[derive(Clone, Default, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
+pub struct PoolInfoIncremental {
+    pub added_pool_txs: Vec<PoolTxInfo>,
+    pub remaining_added_pool_txids: ByteArrayVec<32>,
+    pub removed_pool_txids: ByteArrayVec<32>,
+}
+
+#[cfg(feature = "epee")]
+epee_object! {
+    PoolInfoIncremental,
+    added_pool_txs: Vec<PoolTxInfo>,
+    remaining_added_pool_txids: ByteArrayVec<32>,
+    removed_pool_txids: ByteArrayVec<32>,
+}
+
+/// Data within [`PoolInfo::Full`].
+#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[derive(Clone, Default, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
+pub struct PoolInfoFull {
+    pub added_pool_txs: Vec<PoolTxInfo>,
+    pub remaining_added_pool_txids: ByteArrayVec<32>,
+}
+
+#[cfg(feature = "epee")]
+epee_object! {
+    PoolInfoFull,
+    added_pool_txs: Vec<PoolTxInfo>,
+    remaining_added_pool_txids: ByteArrayVec<32>,
+}
+
+//---------------------------------------------------------------------------------------------------- PoolInfo epee impl
+#[cfg(feature = "epee")]
+/// [`EpeeObjectBuilder`] for [`GetBlocksResponse`].
+///
+/// Not for public usage.
+#[derive(Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash)]
+#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+pub struct __PoolInfoEpeeBuilder {
+    /// This is a distinct field in `monerod`,
+    /// which as represented in this library with [`PoolInfo`]'s `u8` tag.
+    pub pool_info_extent: Option<PoolInfoExtent>,
+
+    pub added_pool_txs: Option<Vec<PoolTxInfo>>,
+    pub remaining_added_pool_txids: Option<ByteArrayVec<32>>,
+    pub removed_pool_txids: Option<ByteArrayVec<32>>,
+}
+
+// Custom epee implementation.
+//
+// HACK/INVARIANT:
+// If any data within [`PoolInfo`] changes, the below code should be changed as well.
+#[cfg(feature = "epee")]
+impl EpeeObjectBuilder<PoolInfo> for __PoolInfoEpeeBuilder {
+    fn add_field<B: Buf>(&mut self, name: &str, r: &mut B) -> error::Result<bool> {
+        macro_rules! read_epee_field {
+            ($($field:ident),*) => {
+                match name {
+                    $(
+                        stringify!($field) => { self.$field = Some(read_epee_value(r)?); },
+                    )*
+                    _ => return Ok(false),
+                }
+            };
+        }
+
+        read_epee_field! {
+            pool_info_extent,
+            added_pool_txs,
+            remaining_added_pool_txids,
+            removed_pool_txids
+        }
+
+        Ok(true)
+    }
+
+    fn finish(self) -> error::Result<PoolInfo> {
+        // INVARIANT:
+        // `monerod` omits serializing the field itself when a container is empty,
+        // `unwrap_or_default()` is used over `error()` in these cases.
+        // Some of the uses are when values have default fallbacks: `pool_info_extent`.
+
+        let pool_info_extent = self.pool_info_extent.unwrap_or_default();
+        let this = match pool_info_extent {
+            PoolInfoExtent::None => PoolInfo::None,
+            PoolInfoExtent::Incremental => PoolInfo::Incremental(PoolInfoIncremental {
+                added_pool_txs: self.added_pool_txs.unwrap_or_default(),
+                remaining_added_pool_txids: self.remaining_added_pool_txids.unwrap_or_default(),
+                removed_pool_txids: self.removed_pool_txids.unwrap_or_default(),
+            }),
+            PoolInfoExtent::Full => PoolInfo::Full(PoolInfoFull {
+                added_pool_txs: self.added_pool_txs.unwrap_or_default(),
+                remaining_added_pool_txids: self.remaining_added_pool_txids.unwrap_or_default(),
+            }),
+        };
+
+        Ok(this)
+    }
+}
+
+#[cfg(feature = "epee")]
+impl EpeeObject for PoolInfo {
+    type Builder = __PoolInfoEpeeBuilder;
+
+    fn number_of_fields(&self) -> u64 {
+        // Inner struct fields.
+        let inner_fields = match self {
+            Self::None => 0,
+            Self::Incremental(s) => s.number_of_fields(),
+            Self::Full(s) => s.number_of_fields(),
+        };
+
+        // [`PoolInfoExtent`] + inner struct fields
+        1 + inner_fields
+    }
+
+    fn write_fields<B: BufMut>(self, w: &mut B) -> error::Result<()> {
+        const FIELD: &str = "pool_info_extent";
+
+        match self {
+            Self::None => {
+                write_field(PoolInfoExtent::None.to_u8(), FIELD, w)?;
+            }
+            Self::Incremental(s) => {
+                s.write_fields(w)?;
+                write_field(PoolInfoExtent::Incremental.to_u8(), FIELD, w)?;
+            }
+            Self::Full(s) => {
+                s.write_fields(w)?;
+                write_field(PoolInfoExtent::Full.to_u8(), FIELD, w)?;
+            }
+        }
+
+        Ok(())
+    }
+}

rpc/types/src/misc/tx_entry.rs

@@ -2,8 +2,6 @@
 
 //---------------------------------------------------------------------------------------------------- Use
 #[cfg(feature = "serde")]
-use crate::serde::{serde_false, serde_true};
-#[cfg(feature = "serde")]
 use serde::{Deserialize, Serialize};
 
 #[cfg(feature = "epee")]
@@ -13,6 +11,9 @@ use cuprate_epee_encoding::{
     EpeeObject, EpeeObjectBuilder,
 };
 
+#[cfg(feature = "serde")]
+use crate::serde::{serde_false, serde_true};
+
 //---------------------------------------------------------------------------------------------------- TxEntry
 #[doc = crate::macros::monero_definition_link!(
     cc73fe71162d564ffda8e549b79a350bca53c454,