From 415b38902b5eb4cd84932790764fa5181ad930ac Mon Sep 17 00:00:00 2001
From: Nick Santana <nick@mobilecoin.com>
Date: Sat, 9 Dec 2023 18:06:44 -0800
Subject: [PATCH] Fix reproducible builds due to how derives were populated

Previously `HashSet` was used to prevent duplicate entries in the
computed derive attributes for bindgen generated code. The default
hasher used in `HashSet` is randomly seeded on each process run to
prevent HashDos. This random seeding resulted in changing the ordering
of the resulted derive attributes. This changing order of the derive
attributes prevented reproducible builds as the derived implementations
would move around in the resultant binary. Now `BtreeSet` is used to
prevent duplicate derives during code generation. This provides a
consistent ordering of the derives.
---
 core/build/src/lib.rs | 67 +++++++++++++++++++++++++++++++++----------
 1 file changed, 52 insertions(+), 15 deletions(-)

diff --git a/core/build/src/lib.rs b/core/build/src/lib.rs
index 929443a5..ebdc3697 100644
--- a/core/build/src/lib.rs
+++ b/core/build/src/lib.rs
@@ -7,7 +7,7 @@ use bindgen::{
     callbacks::{IntKind, ParseCallbacks},
     Builder, EnumVariation,
 };
-use std::collections::HashSet;
+use std::collections::BTreeSet;
 use std::{env, path::PathBuf};
 
 static DEFAULT_SGX_SDK_PATH: &str = "/opt/intel/sgxsdk";
@@ -206,43 +206,47 @@ impl SgxParseCallbacks {
             .extend(serialize_types.into_iter().map(ToString::to_string));
         self
     }
-}
-
-impl ParseCallbacks for SgxParseCallbacks {
-    fn item_name(&self, name: &str) -> Option<String> {
-        normalize_item_name(name)
-    }
 
-    fn add_derives(&self, info: &DeriveInfo<'_>) -> Vec<String> {
-        let mut attributes = HashSet::new();
-        let name = info.name;
+    /// Get the derives that should be applied to the provided named type.
+    fn derives(&self, type_name: &str) -> Vec<String> {
+        let mut attributes = BTreeSet::new();
 
-        if self.default_types.iter().any(|n| *n == name) {
+        if self.default_types.iter().any(|n| *n == type_name) {
             attributes.insert("Default");
         }
 
         // The [enum_types] method adds enums to the [copyable_types]
-        if self.copyable_types.iter().any(|n| *n == name) {
+        if self.copyable_types.iter().any(|n| *n == type_name) {
             attributes.insert("Copy");
         }
 
-        if !self.dynamically_sized_types.iter().any(|n| *n == name) {
+        if !self.dynamically_sized_types.iter().any(|n| *n == type_name) {
             // For dynamically sized types we don't even derive Debug, because
             // they are often times packed and packed types can't derive Debug
             // without deriving Copy, however by the dynamic nature one can't
             // derive Copy
             attributes.insert("Debug");
-            if !self.enum_types.iter().any(|n| *n == name) {
+            if !self.enum_types.iter().any(|n| *n == type_name) {
                 attributes.extend(["Eq", "PartialEq", "Hash", "Clone"]);
             }
         };
 
-        if self.serialize_types.iter().any(|n| *n == name) {
+        if self.serialize_types.iter().any(|n| *n == type_name) {
             attributes.extend(["Serialize", "Deserialize"]);
         }
 
         attributes.into_iter().map(String::from).collect::<Vec<_>>()
     }
+}
+
+impl ParseCallbacks for SgxParseCallbacks {
+    fn item_name(&self, name: &str) -> Option<String> {
+        normalize_item_name(name)
+    }
+
+    fn add_derives(&self, info: &DeriveInfo<'_>) -> Vec<String> {
+        self.derives(info.name)
+    }
 
     fn int_macro(&self, name: &str, _value: i64) -> Option<IntKind> {
         const USIZE_SUFFIXES: &[&str] = &["_SIZE", "_BYTES", "_IDX", "_COUNT"];
@@ -328,3 +332,36 @@ pub fn sgx_library_suffix() -> &'static str {
         _ => "",
     }
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn verify_consistency_of_derive_attributes() {
+        // This test prevents a regression where `HashSet` was used to prevent
+        // duplicates in the derives. Using `HashSet` resulted in the dervies
+        // coming back in varying order based on seed of the default hasher.
+        // The seed of the default hasher is randomized per process run.
+        // This inconsistent ordering of the dervies prevented reproducible
+        // builds as the derived implementations would move around in the
+        // resultant binary.
+        let mut callbacks = SgxParseCallbacks::default();
+        callbacks = callbacks.serialize_types(["foo"]);
+        let derives = callbacks.derives("foo");
+
+        let expected = [
+            "Clone",
+            "Debug",
+            "Deserialize",
+            "Eq",
+            "Hash",
+            "PartialEq",
+            "Serialize",
+        ]
+        .into_iter()
+        .map(String::from)
+        .collect::<Vec<_>>();
+        assert_eq!(derives, expected);
+    }
+}