From 111a871d19042b7e547d66f7a3e8f486b9b5b910 Mon Sep 17 00:00:00 2001
From: Leo Eichhorn <leo.eichhorn@dfinity.org>
Date: Thu, 16 Jan 2025 11:04:53 +0000
Subject: [PATCH 1/6] fix

---
 rs/types/types/src/batch.rs                 | 53 ++++++++++++++++++---
 rs/types/types/src/batch/ingress.rs         |  5 +-
 rs/types/types/src/batch/self_validating.rs |  3 +-
 rs/types/types/src/batch/xnet.rs            |  6 +++
 4 files changed, 59 insertions(+), 8 deletions(-)

diff --git a/rs/types/types/src/batch.rs b/rs/types/types/src/batch.rs
index 7676e5d084e..0ae2c96cd8e 100644
--- a/rs/types/types/src/batch.rs
+++ b/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]
diff --git a/rs/types/types/src/batch/ingress.rs b/rs/types/types/src/batch/ingress.rs
index e1379a92870..ba4e2bf3a55 100644
--- a/rs/types/types/src/batch/ingress.rs
+++ b/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`].
diff --git a/rs/types/types/src/batch/self_validating.rs b/rs/types/types/src/batch/self_validating.rs
index ca97598690f..1e3a8992314 100644
--- a/rs/types/types/src/batch/self_validating.rs
+++ b/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()
     }
 }
 
diff --git a/rs/types/types/src/batch/xnet.rs b/rs/types/types/src/batch/xnet.rs
index 317de7f135a..f435a7c64d0 100644
--- a/rs/types/types/src/batch/xnet.rs
+++ b/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()
+    }
 }

From bd76daede4f744402ec130e422519b07e37b5b21 Mon Sep 17 00:00:00 2001
From: Leo Eichhorn <leo.eichhorn@dfinity.org>
Date: Thu, 16 Jan 2025 11:24:23 +0000
Subject: [PATCH 2/6] count_bytes

---
 rs/bitcoin/replica_types/src/lib.rs         | 26 ++++++++++++---------
 rs/types/types/src/batch/ingress.rs         |  5 +++-
 rs/types/types/src/batch/self_validating.rs |  3 ++-
 rs/types/types/src/canister_http.rs         | 25 ++++++++++++++++----
 4 files changed, 41 insertions(+), 18 deletions(-)

diff --git a/rs/bitcoin/replica_types/src/lib.rs b/rs/bitcoin/replica_types/src/lib.rs
index 5213797bafe..ba18ae2626e 100644
--- a/rs/bitcoin/replica_types/src/lib.rs
+++ b/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() + std::mem::size_of::<u64>()
     }
 }
 
@@ -718,15 +727,10 @@ pub struct GetSuccessorsResponseComplete {
 impl GetSuccessorsResponseComplete {
     /// Returns the size of this `SendTransactionResponse` in bytes.
     pub fn count_bytes(&self) -> usize {
-        self.count_blocks_bytes() + self.count_next_bytes()
-    }
-
-    pub fn count_blocks_bytes(&self) -> usize {
-        self.blocks.iter().map(|b| b.len()).sum::<usize>()
-    }
-
-    pub fn count_next_bytes(&self) -> usize {
-        self.next.iter().map(|n| n.len()).sum::<usize>()
+        let GetSuccessorsResponseComplete { blocks, next } = &self;
+        let blocks_bytes = blocks.iter().map(|b| b.len()).sum::<usize>();
+        let next_bytes = next.iter().map(|n| n.len()).sum::<usize>();
+        blocks_bytes + next_bytes
     }
 }
 
diff --git a/rs/types/types/src/batch/ingress.rs b/rs/types/types/src/batch/ingress.rs
index ba4e2bf3a55..154a110ac77 100644
--- a/rs/types/types/src/batch/ingress.rs
+++ b/rs/types/types/src/batch/ingress.rs
@@ -124,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()
diff --git a/rs/types/types/src/batch/self_validating.rs b/rs/types/types/src/batch/self_validating.rs
index 1e3a8992314..30f25975a84 100644
--- a/rs/types/types/src/batch/self_validating.rs
+++ b/rs/types/types/src/batch/self_validating.rs
@@ -62,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()
     }
 }
diff --git a/rs/types/types/src/canister_http.rs b/rs/types/types/src/canister_http.rs
index ac019d51355..144b3ee865c 100644
--- a/rs/types/types/src/canister_http.rs
+++ b/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::<CallbackId>()
+            + size_of::<Time>()
+            + 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::<RejectCode>() + 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()
     }
 }
 

From 9969cae02e502ca8ac9025e595cca3dc4c4c4dbf Mon Sep 17 00:00:00 2001
From: Leo Eichhorn <leo.eichhorn@dfinity.org>
Date: Thu, 16 Jan 2025 11:31:01 +0000
Subject: [PATCH 3/6] fix

---
 rs/bitcoin/replica_types/src/lib.rs | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/rs/bitcoin/replica_types/src/lib.rs b/rs/bitcoin/replica_types/src/lib.rs
index ba18ae2626e..2212bd41caa 100644
--- a/rs/bitcoin/replica_types/src/lib.rs
+++ b/rs/bitcoin/replica_types/src/lib.rs
@@ -727,10 +727,15 @@ pub struct GetSuccessorsResponseComplete {
 impl GetSuccessorsResponseComplete {
     /// Returns the size of this `SendTransactionResponse` in bytes.
     pub fn count_bytes(&self) -> usize {
-        let GetSuccessorsResponseComplete { blocks, next } = &self;
-        let blocks_bytes = blocks.iter().map(|b| b.len()).sum::<usize>();
-        let next_bytes = next.iter().map(|n| n.len()).sum::<usize>();
-        blocks_bytes + next_bytes
+        self.count_blocks_bytes() + self.count_next_bytes()
+    }
+
+    pub fn count_blocks_bytes(&self) -> usize {
+        self.blocks.iter().map(|b| b.len()).sum::<usize>()
+    }
+
+    pub fn count_next_bytes(&self) -> usize {
+        self.next.iter().map(|n| n.len()).sum::<usize>()
     }
 }
 

From fe864f272cd70afd4d4070e365f5925baaf09000 Mon Sep 17 00:00:00 2001
From: Leo Eichhorn <leo.eichhorn@dfinity.org>
Date: Thu, 16 Jan 2025 11:53:50 +0000
Subject: [PATCH 4/6] consensus

---
 rs/types/types/src/consensus/dkg.rs     | 9 +++++++++
 rs/types/types/src/consensus/payload.rs | 3 ++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/rs/types/types/src/consensus/dkg.rs b/rs/types/types/src/consensus/dkg.rs
index 905a1cc10a0..11615ed5719 100644
--- a/rs/types/types/src/consensus/dkg.rs
+++ b/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 {
diff --git a/rs/types/types/src/consensus/payload.rs b/rs/types/types/src/consensus/payload.rs
index 6caf56d52cc..0b5473b7a6c 100644
--- a/rs/types/types/src/consensus/payload.rs
+++ b/rs/types/types/src/consensus/payload.rs
@@ -68,7 +68,8 @@ impl BlockPayload {
     pub fn is_empty(&self) -> bool {
         match self {
             BlockPayload::Data(data) => {
-                data.batch.is_empty() && data.dkg.messages.is_empty() && data.idkg.is_none()
+                let DataPayload { batch, dkg, idkg } = data;
+                batch.is_empty() && dkg.is_empty() && idkg.is_none()
             }
             _ => false,
         }

From 63128a57cfa0aafb79693ba8434049c0a1af70a4 Mon Sep 17 00:00:00 2001
From: Leo Eichhorn <leo.eichhorn@dfinity.org>
Date: Thu, 16 Jan 2025 12:36:24 +0000
Subject: [PATCH 5/6] review

---
 rs/types/types/src/consensus/payload.rs | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/rs/types/types/src/consensus/payload.rs b/rs/types/types/src/consensus/payload.rs
index 0b5473b7a6c..7b506d4746c 100644
--- a/rs/types/types/src/consensus/payload.rs
+++ b/rs/types/types/src/consensus/payload.rs
@@ -67,11 +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) => {
-                let DataPayload { batch, dkg, idkg } = data;
+            BlockPayload::Data(DataPayload { batch, dkg, idkg }) => {
                 batch.is_empty() && dkg.is_empty() && idkg.is_none()
             }
-            _ => false,
+            BlockPayload::Summary(_) => false,
         }
     }
 

From 0c2318ad1d3203a0a2c0af2bfdb24ad727e2b563 Mon Sep 17 00:00:00 2001
From: Leo Eichhorn <leo.eichhorn@dfinity.org>
Date: Thu, 16 Jan 2025 12:49:42 +0000
Subject: [PATCH 6/6] use size_of_val

---
 rs/bitcoin/replica_types/src/lib.rs |  4 ++--
 rs/types/types/src/canister_http.rs | 12 ++++++------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/rs/bitcoin/replica_types/src/lib.rs b/rs/bitcoin/replica_types/src/lib.rs
index 2212bd41caa..269d01b2eee 100644
--- a/rs/bitcoin/replica_types/src/lib.rs
+++ b/rs/bitcoin/replica_types/src/lib.rs
@@ -605,9 +605,9 @@ impl BitcoinAdapterResponse {
     pub fn count_bytes(&self) -> usize {
         let BitcoinAdapterResponse {
             response,
-            callback_id: _,
+            callback_id,
         } = &self;
-        response.count_bytes() + std::mem::size_of::<u64>()
+        response.count_bytes() + size_of_val(callback_id)
     }
 }
 
diff --git a/rs/types/types/src/canister_http.rs b/rs/types/types/src/canister_http.rs
index 144b3ee865c..22b4ecb29ff 100644
--- a/rs/types/types/src/canister_http.rs
+++ b/rs/types/types/src/canister_http.rs
@@ -472,13 +472,13 @@ pub struct CanisterHttpResponse {
 impl CountBytes for CanisterHttpResponse {
     fn count_bytes(&self) -> usize {
         let CanisterHttpResponse {
-            id: _,
-            timeout: _,
+            id,
+            timeout,
             canister_id,
             content,
         } = &self;
-        size_of::<CallbackId>()
-            + size_of::<Time>()
+        size_of_val(id)
+            + size_of_val(timeout)
             + canister_id.get_ref().data_size()
             + content.count_bytes()
     }
@@ -524,10 +524,10 @@ impl From<&CanisterHttpReject> for RejectContext {
 impl CountBytes for CanisterHttpReject {
     fn count_bytes(&self) -> usize {
         let CanisterHttpReject {
-            reject_code: _,
+            reject_code,
             message,
         } = &self;
-        size_of::<RejectCode>() + message.len()
+        size_of_val(reject_code) + message.len()
     }
 }