Propose merging nil::marshalling with crypto3::marshalling namespace

[JasonCoombs]

NOTE: merging nil::marshalling into nil::crypto3::marshalling to make a single repo and remove some unnecessary code and file duplication also requires a global Find -> Replace to update all files in nil::marshalling and lines throughout crypto3 headers where nil::marshalling namespace is used currently.

I made the entire change required, and am now building and running test programs successfully after the merge.

Why does nil::marshalling need to be a stand-alone namespace instead of a crypto3 namespace?

[JasonCoombs]

I see here that there is meant to be a nil::marshalling namespace in addition to nil::crypto3::marshalling

https://github.com/NilFoundation/marshalling/blob/master/docs/examples.md

trying to compile for the first time, this was confusing; it was not clear that the nil::marshalling repo was missing from my crypto3 installation.

can the nil::marshalling namespace be moved into nil::crypto3::marshalling ?

nil::marshalling is named "core" here:

https://github.com/NilFoundation/crypto3/tree/master/libs/marshalling

but then we don't get a "core" subdirectory under include/nil/crypto3/

[JasonCoombs]

I was able to compile successfully, the simplest example signing a message with BLS NilFoundation/crypto3-pubkey#26

... but only after merging nil::marshalling with nil::crypto3::marshalling and doing Find -> Replace to update all namespace references in all crypto3 headers.

The only big change that seems to be necessary to make the merged marshalling headers work is here:

NilFoundation/marshalling@cf933fa

and of course in this case multiprecision / integral / basic_type.hpp would then no longer needed, because its class template is merged as in the commit shown above:

https://github.com/NilFoundation/crypto3-multiprecision-marshalling/blob/master/include/nil/crypto3/marshalling/multiprecision/types/detail/integral/basic_type.hpp

[JasonCoombs]

also: NilFoundation/marshalling@df7c34a

[JasonCoombs]

addendum: the crypto3-marshalling repo was archived and is now read-only:

https://github.com/NilFoundation/crypto3-marshalling

unless there is a good reason to keep the nil::marshalling namespace, could the archived repo be deleted and the crypto3-marshalling name used for the marshalling repo, for consistency with the rest of crypto3?

in my merged crypto3-marshalling repo, I also merged type_traits.hpp -- it does not seem necessary for there to be so much duplicated code particularly within type_traits across many repos. what problem does it cause to always provide the multiprecision marshalling type_traits and class template even in a case where they are not used? leave it up to the compiler and linker to optimize out unused code at build time.

see: https://github.com/NilFoundation/crypto3-multiprecision-marshalling/blob/master/include/nil/detail/type_traits.hpp

and:

it seems much more important for the organization of the crypto3 headers to avoid any duplication of code and to minimize duplication of files.

I think it violates some organizational principle for the multiprecision header located in nil/crypto3/marshalling/multiprecision/types/detail/integral/ to contain code in a namespace at nil::crypto3 and for that namespace to be mutually-exclusive with the marshalling namespace at nil::marshalling

declaring and defining the class templates in the same header file, as I am proposing, and locating that header in a directory path nil/crypto3/marshalling that closely resembles the namespace path nil::crypto3::marshalling makes a difference in the minds of new developers and does not cause any less flexibility or compile-time optimization challenge for advanced developers.

[JasonCoombs]

remember that a template that is not instantiated generates no code at compile-time, so there may be zero benefit from spreading the parts of crypto3 across so many subdirectories that the programmer must know about in order to #include optional class templates and types. maybe it would be better to consolidate related class templates, as I demonstrate in the commit above, because a program that uses marshalling but not multiprecision data types will never generate any code for multiprecision since those templates are never instantiated by the low- or fixed-precision mathematic and cryptographic operations that the program is using by including crypto3.

[nemothenoone]

I took a look into this. There is nuance that marshalling library initial intention was not to become a crypto3-exclusive, but to provide an extension suitable, e.g. for network packets marshalling.

These purposes are different because what marshalling library does for crypto3 suite is mostly about serialization/deserialization from/to some byteblob with a very strictly and manually defined internal data structure. Network packets marshalling would require more pre-defined behavior templates (like, everything what goes under some particular network packet field should be hashed (e.g. CRC-ed) and marshalled according to the protocol's format). And such behavior patterns are better to be done automatically.

So the separation was done intentionally to indicate particular modules purpose. We thought that in other way we will end up with a single-repo library full of very different marshalling templates.

So, this is my consideration on behalf of merging them together idea.

Maybe @nkaskov will have some other thoughts on this.