Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: Deterministic Serialization #965

Open
ParkMyCar opened this issue Dec 21, 2023 · 2 comments
Open

feature: Deterministic Serialization #965

ParkMyCar opened this issue Dec 21, 2023 · 2 comments

Comments

@ParkMyCar
Copy link

As far as I understand the proto3 specification does not require the serialized data to be stable or deterministic (docs, notes). I'm wondering if prost would be willing to guarantee the following properties do not change, within a semver compatible version, and if they do change to note it in the changelog:

  • Order in which fields are encoded (e.g. encode them in order of tag number)
  • Order in which entries of a map are encoded, if using the BTreeMap option
  • Order in which repeated entries are encoded (e.g. order in which they exist in a Vec)
  • Encoded format of variable length integers
  • Fields will be encoded a deterministic amount of times (i.e. once) in a message
  • Encoding of packed repeated fields

I totally realize other libraries and other languages may not serialize to the exact same wire format at prost, and that's fine! The goal here is to make sure for a given proto message A, prost always serializes it to the same wire format. As far as I can tell prost already does this, but having the library formally guarantee this, within a semver compatible version, would be useful!

@DavidBM
Copy link

DavidBM commented Jan 4, 2024

We are looking at the same in out internal project. We found that, for our use case, this change solves our issue.

diff --git a/prost-build/src/lib.rs b/prost-build/src/lib.rs
index 908a76a..f9f88be 100644
--- a/prost-build/src/lib.rs
+++ b/prost-build/src/lib.rs
@@ -150,6 +150,7 @@ use crate::extern_paths::ExternPaths;
 use crate::ident::to_snake;
 use crate::message_graph::MessageGraph;
 use crate::path::PathMap;
+use itertools::Itertools;
 
 /// A service generator takes a service descriptor and generates Rust code.
 ///
@@ -925,7 +926,7 @@ impl Config {
             trace!("Writing include file: {:?}", target.join(include_file));
             let mut file = fs::File::create(target.join(include_file))?;
             self.write_includes(
-                modules.keys().collect(),
+                modules.keys().sorted().collect(),
                 &mut file,
                 0,
                 if target_is_env { None } else { Some(&target) },

More changes might be needed for other use cases.

@LucioFranco
Copy link
Member

I think this would be an interesting idea, but before we claim this we should write a test suite that verifies this and keeps track of this. If you're willing to contribute this I see no reason why not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants