fix: Consider all fields in is_empty and count_bytes implementation for BatchPayload

rs/bitcoin/replica_types/src/lib.rs

@@ -414,6 +414,7 @@ impl From<v1::SendTransactionResponse> for SendTransactionResponse {
 impl SendTransactionResponse {
     /// Returns the size of this `SendTransactionResponse` in bytes.
     pub fn count_bytes(&self) -> usize {
+        let SendTransactionResponse {} = &self;
         0
     }
 }
@@ -429,7 +430,11 @@ pub struct BitcoinReject {
 impl BitcoinReject {
     /// Returns the size of this `RejectResponse` in bytes.
     pub fn count_bytes(&self) -> usize {
-        size_of_val(&self.reject_code) + self.message.len()
+        let BitcoinReject {
+            reject_code,
+            message,
+        } = &self;
+        size_of_val(reject_code) + message.len()
     }
 }
 
@@ -598,7 +603,11 @@ impl TryFrom<v1::BitcoinAdapterResponse> for BitcoinAdapterResponse {
 impl BitcoinAdapterResponse {
     /// Returns the size of this `BitcoinAdapterResponse` in bytes.
     pub fn count_bytes(&self) -> usize {
-        self.response.count_bytes() + std::mem::size_of::<u64>()
+        let BitcoinAdapterResponse {
+            response,
+            callback_id,
+        } = &self;
+        response.count_bytes() + size_of_val(callback_id)
     }
 }
 

rs/types/types/src/batch.rs

@@ -165,10 +165,19 @@ impl BatchPayload {
     }
 
     pub fn is_empty(&self) -> bool {
-        self.ingress.is_empty()
-            && self.xnet.stream_slices.is_empty()
-            && self.self_validating.is_empty()
-            && self.canister_http.is_empty()
+        let BatchPayload {
+            ingress,
+            xnet,
+            self_validating,
+            canister_http,
+            query_stats,
+        } = &self;
+
+        ingress.is_empty()
+            && xnet.is_empty()
+            && self_validating.is_empty()
+            && canister_http.is_empty()
+            && query_stats.is_empty()
     }
 }
 
@@ -241,12 +250,44 @@ mod tests {
     use super::*;
     use crate::CountBytes;
 
+    /// This is a quick test to check the invariant, that the [`Default`] implementation
+    /// of a payload section actually produces the empty payload,
+    #[test]
+    fn default_batch_payload_is_zero_bytes() {
+        let BatchPayload {
+            ingress,
+            xnet: _, // No implementation of `CountBytes` for xnet.
+            self_validating,
+            canister_http,
+            query_stats,
+        } = BatchPayload::default();
+
+        assert_eq!(ingress.count_bytes(), 0);
+        assert_eq!(self_validating.count_bytes(), 0);
+        assert_eq!(canister_http.len(), 0);
+        assert_eq!(query_stats.len(), 0);
+    }
+
     /// This is a quick test to check the invariant, that the [`Default`] implementation
     /// of a payload section actually produces the empty payload,
     #[test]
     fn default_batch_payload_is_empty() {
-        assert_eq!(IngressPayload::default().count_bytes(), 0);
-        assert_eq!(SelfValidatingPayload::default().count_bytes(), 0);
+        let payload = BatchPayload::default();
+        assert!(payload.is_empty());
+
+        let BatchPayload {
+            ingress,
+            xnet,
+            self_validating,
+            canister_http,
+            query_stats,
+        } = &payload;
+
+        assert!(ingress.is_empty());
+        assert!(xnet.is_empty());
+        assert!(self_validating.is_empty());
+        assert!(canister_http.is_empty());
+        assert!(query_stats.is_empty());
     }
 
     #[test]

rs/types/types/src/batch/ingress.rs

@@ -86,7 +86,10 @@ impl IngressPayload {
 
     /// Return true if the payload is empty.
     pub fn is_empty(&self) -> bool {
-        self.serialized_ingress_messages.is_empty()
+        let IngressPayload {
+            serialized_ingress_messages,
+        } = &self;
+        serialized_ingress_messages.is_empty()
     }
 
     /// Return the [`SignedIngress`] referenced by the [`IngressMessageId`].
@@ -121,7 +124,10 @@ impl IngressPayload {
 
 impl CountBytes for IngressPayload {
     fn count_bytes(&self) -> usize {
-        self.serialized_ingress_messages
+        let IngressPayload {
+            serialized_ingress_messages,
+        } = &self;
+        serialized_ingress_messages
             .values()
             .map(|message| EXPECTED_MESSAGE_ID_LENGTH + message.len())
             .sum()

rs/types/types/src/batch/self_validating.rs

@@ -35,7 +35,8 @@ impl SelfValidatingPayload {
 
     /// Returns true if the payload is empty
     pub fn is_empty(&self) -> bool {
-        self.0.is_empty()
+        let SelfValidatingPayload(responses) = &self;
+        responses.is_empty()
     }
 }
 
@@ -61,6 +62,7 @@ impl TryFrom<pb::SelfValidatingPayload> for SelfValidatingPayload {
 
 impl CountBytes for SelfValidatingPayload {
     fn count_bytes(&self) -> usize {
-        self.0.iter().map(|x| x.count_bytes()).sum()
+        let SelfValidatingPayload(responses) = &self;
+        responses.iter().map(|x| x.count_bytes()).sum()
     }
 }

rs/types/types/src/batch/xnet.rs

@@ -72,4 +72,10 @@ impl XNetPayload {
             })
             .sum()
     }
+
+    /// Returns true if the payload is empty
+    pub fn is_empty(&self) -> bool {
+        let XNetPayload { stream_slices } = &self;
+        stream_slices.is_empty()
+    }
 }

rs/types/types/src/canister_http.rs

@@ -53,7 +53,7 @@ use ic_error_types::{ErrorCode, RejectCode, UserError};
 #[cfg(test)]
 use ic_exhaustive_derive::ExhaustiveSet;
 use ic_management_canister_types::{
-    CanisterHttpRequestArgs, HttpHeader, HttpMethod, TransformContext,
+    CanisterHttpRequestArgs, DataSize, HttpHeader, HttpMethod, TransformContext,
 };
 use ic_protobuf::{
     proxy::{try_from_option_field, ProxyDecodeError},
@@ -471,7 +471,16 @@ pub struct CanisterHttpResponse {
 
 impl CountBytes for CanisterHttpResponse {
     fn count_bytes(&self) -> usize {
-        size_of::<CallbackId>() + size_of::<Time>() + self.content.count_bytes()
+        let CanisterHttpResponse {
+            id,
+            timeout,
+            canister_id,
+            content,
+        } = &self;
+        size_of_val(id)
+            + size_of_val(timeout)
+            + canister_id.get_ref().data_size()
+            + content.count_bytes()
     }
 }
 
@@ -514,7 +523,11 @@ impl From<&CanisterHttpReject> for RejectContext {
 
 impl CountBytes for CanisterHttpReject {
     fn count_bytes(&self) -> usize {
-        size_of::<RejectCode>() + self.message.len()
+        let CanisterHttpReject {
+            reject_code,
+            message,
+        } = &self;
+        size_of_val(reject_code) + message.len()
     }
 }
 
@@ -569,7 +582,8 @@ pub struct CanisterHttpResponseWithConsensus {
 
 impl CountBytes for CanisterHttpResponseWithConsensus {
     fn count_bytes(&self) -> usize {
-        self.proof.count_bytes() + self.content.count_bytes()
+        let CanisterHttpResponseWithConsensus { content, proof } = &self;
+        proof.count_bytes() + content.count_bytes()
     }
 }
 
@@ -585,7 +599,8 @@ pub struct CanisterHttpResponseDivergence {
 
 impl CountBytes for CanisterHttpResponseDivergence {
     fn count_bytes(&self) -> usize {
-        self.shares.iter().map(|share| share.count_bytes()).sum()
+        let CanisterHttpResponseDivergence { shares } = &self;
+        shares.iter().map(|share| share.count_bytes()).sum()
     }
 }
 

rs/types/types/src/consensus/dkg.rs

@@ -588,6 +588,15 @@ impl DkgDataPayload {
             messages,
         }
     }
+
+    /// Returns true if the payload is empty
+    pub fn is_empty(&self) -> bool {
+        let DkgDataPayload {
+            start_height: _,
+            messages,
+        } = self;
+        messages.is_empty()
+    }
 }
 
 impl NiDkgTag {

rs/types/types/src/consensus/payload.rs

@@ -67,10 +67,10 @@ impl BlockPayload {
     /// Return true if it is a normal block and empty
     pub fn is_empty(&self) -> bool {
         match self {
-            BlockPayload::Data(data) => {
-                data.batch.is_empty() && data.dkg.messages.is_empty() && data.idkg.is_none()
+            BlockPayload::Data(DataPayload { batch, dkg, idkg }) => {
+                batch.is_empty() && dkg.is_empty() && idkg.is_none()
             }
-            _ => false,
+            BlockPayload::Summary(_) => false,
         }
     }