chore: Use msgpack to reduce redundancy #612

ludamad · 2023-07-17T20:36:37Z

Description

Going through lists-of-fields now that we have poor-man's reflection.

This currently DOES NOT switch barretenberg's serialization to msgpack. This instead uses msgpack common reflection utils to derive such read and write functions.

For example:
MSGPACK_FIELDS(arg1, arg2);

is now used to define

void read(buf, obj) {
   read(buf, obj.arg1);
   read(buf, obj.arg2);
}

for any object that has a MSGPACK_FIELDS macro. This is also used for write

Also removes a debug utility I introduced during the hackathon that has not really been used, fine to remove

Checklist:

[*] I have reviewed my diff in github, line by line.
[*] Every change is related to the PR description.
[*] The branch has been merged with/rebased against the head of its merge target.
[*] There are no unexpected formatting changes, superfluous debug logs, or commented-out code.
[*] There are no circuit changes, OR a cryptographer has been assigned for review.
[*] New functions, classes, etc. have been documented according to the doxygen comment format. Classes and structs must have @brief describing the intended functionality.
[*] If existing code has been modified, such documentation has been added or updated.
[*] No superfluous include directives have been added.
[*] I have linked to any issue(s) it resolves.
[*] I'm happy for the PR to be merged at the reviewer's next convenience.

ludamad · 2023-07-17T22:38:22Z

cpp/src/barretenberg/serialize/msgpack_impl/func_traits.hpp

-    Args args;                      // Args instance
-    R ret;                          // Holds return type
-    MSGPACK_FIELDS(args, ret);      // Macro from msgpack library to serialize/deserialize fields
+    typedef std::tuple<typename std::decay<Vs>::type...> Args; // Define a tuple type that holds all argument types


Needed to support const Type& args

…ancy-msgpack

ludamad · 2023-07-18T21:26:43Z

cpp/src/barretenberg/dsl/acir_format/ecdsa_secp256r1.test.cpp

@@ -120,24 +120,25 @@ TEST(ECDSASecp256r1, test_hardcoded)

    size_t num_variables =
        generate_r1_constraints(ecdsa_r1_constraint, witness_values, pub_key_x, pub_key_y, hashed_message, signature);
+
    acir_format constraint_system{
        .varnum = static_cast<uint32_t>(num_variables),
        .public_inputs = {},


Reordered to match serialization order (hard to work around doing this)

…ancy-msgpack

spalladino

LGTM, but don't count it as an approval since much of the cpp magic used here still escapes me.

cpp/src/barretenberg/common/serialize.hpp

cpp/src/barretenberg/crypto/schnorr/c_bind.cpp

ludamad · 2023-07-18T21:49:32Z

LGTM, but don't count it as an approval since much of the cpp magic used here still escapes me.

Foiled again! It's almost like I should avoid C++ magic

dbanks12 · 2023-07-19T14:31:30Z

Cannot parse "This currently DOES NOT switch barretenberg's serialization to msgpack. This instead uses msgpack common reflection utils to derive such read and write functions."

Could you explain this like im a 5 year old?

dbanks12

LGTM, left some minor comments.

cpp/src/barretenberg/common/serialize.hpp

cpp/src/barretenberg/dsl/acir_format/acir_format.hpp

cpp/src/barretenberg/dsl/acir_format/acir_format.test.cpp

cpp/src/barretenberg/dsl/acir_format/acir_to_constraint_buf.hpp

dbanks12 · 2023-07-19T14:37:32Z

cpp/src/barretenberg/serialize/msgpack_apply.hpp

+ * @brief Passthrough method for better error reporting.
+ */
+template <msgpack_concepts::HasMsgPack T> void msgpack_apply(const auto& func, auto&... args)
+{
+    std::apply(func, msgpack::drop_keys(std::tie(args...)));


Can you expand on this comment a bit (or the fn below)? What does it mean that they are passthrough methods and how might they be used?

cpp/src/barretenberg/serialize/msgpack_impl/drop_keys.hpp

ludamad · 2023-07-19T15:14:47Z

Cannot parse "This currently DOES NOT switch barretenberg's serialization to msgpack. This instead uses msgpack common reflection utils to derive such read and write functions."

Could you explain this like im a 5 year old?

I hope the edited text now makes more sense

Co-authored-by: ludamad <[email protected]>

ludamad added 5 commits July 17, 2023 16:35

chore: Use msgpack to reduce redundancy

94a8a02

chore: unused file

a114a87

fix: test build

98e15b2

fix: build

b2c0c53

fix: build

aab73e4

ludamad commented Jul 17, 2023

View reviewed changes

ludamad and others added 6 commits July 17, 2023 20:39

fix: build

83ec662

fix: build

a48d7db

fix: build

ac90a26

Merge branch 'master' into chore/reduce-redundancy-msgpack

c4c795b

fix: build

0aa2482

fix: build

3f8113a

ludamad changed the title ~~chore: Use msgpack to reduce redundancy (WIP)~~ chore: Use msgpack to reduce redundancy Jul 18, 2023

ludamad requested review from dbanks12 and codygunton July 18, 2023 18:50

ludamad mentioned this pull request Jul 18, 2023

chore: reduce redundancy with msgpack AztecProtocol/aztec-packages#1108

Merged

Merge remote-tracking branch 'origin/master' into chore/reduce-redund…

077245a

…ancy-msgpack

ludamad force-pushed the chore/reduce-redundancy-msgpack branch from 065f83f to 077245a Compare July 18, 2023 21:05

ludamad requested a review from charlielye July 18, 2023 21:06

chore: remove commented code

09053ea

ludamad commented Jul 18, 2023

View reviewed changes

ludamad added 2 commits July 18, 2023 17:29

Merge remote-tracking branch 'origin/master' into chore/reduce-redund…

a6fccd4

…ancy-msgpack

fix: revert bad ts main.json

c50487f

ludamad requested a review from spalladino July 18, 2023 21:33

spalladino reviewed Jul 18, 2023

View reviewed changes

cpp/src/barretenberg/common/serialize.hpp Show resolved Hide resolved

cpp/src/barretenberg/crypto/schnorr/c_bind.cpp Outdated Show resolved Hide resolved

ludamad and others added 2 commits July 18, 2023 17:50

fix: review feedback

ed1c385

fix: typo

ef31375

ludamad added noir-ci Run the Noir CI and removed noir-ci Run the Noir CI labels Jul 19, 2023

dbanks12 approved these changes Jul 19, 2023

View reviewed changes

fix: review feedback

29005c5

ludamad merged commit 2838de6 into master Jul 19, 2023
2 checks passed

ludamad deleted the chore/reduce-redundancy-msgpack branch July 19, 2023 15:35

ludamad added a commit to AztecProtocol/aztec-packages that referenced this pull request Jul 22, 2023

chore: Use msgpack to reduce redundancy (AztecProtocol/barretenberg#612)

434575c

Co-authored-by: ludamad <[email protected]>

ludamad added a commit to AztecProtocol/aztec-packages that referenced this pull request Jul 24, 2023

chore: Use msgpack to reduce redundancy (AztecProtocol/barretenberg#612)

e5a2196

Co-authored-by: ludamad <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: Use msgpack to reduce redundancy #612

chore: Use msgpack to reduce redundancy #612

ludamad commented Jul 17, 2023 •

edited

Loading

ludamad Jul 17, 2023

ludamad Jul 18, 2023

spalladino left a comment

ludamad commented Jul 18, 2023 •

edited

Loading

dbanks12 commented Jul 19, 2023

dbanks12 left a comment

dbanks12 Jul 19, 2023

ludamad commented Jul 19, 2023

chore: Use msgpack to reduce redundancy #612

chore: Use msgpack to reduce redundancy #612

Conversation

ludamad commented Jul 17, 2023 • edited Loading

Description

Checklist:

ludamad Jul 17, 2023

Choose a reason for hiding this comment

ludamad Jul 18, 2023

Choose a reason for hiding this comment

spalladino left a comment

Choose a reason for hiding this comment

ludamad commented Jul 18, 2023 • edited Loading

dbanks12 commented Jul 19, 2023

dbanks12 left a comment

Choose a reason for hiding this comment

dbanks12 Jul 19, 2023

Choose a reason for hiding this comment

ludamad commented Jul 19, 2023

ludamad commented Jul 17, 2023 •

edited

Loading

ludamad commented Jul 18, 2023 •

edited

Loading