foundry icon indicating copy to clipboard operation
foundry copied to clipboard

Improve RLP macros

Open junha1 opened this issue 5 years ago • 6 comments

Our RLP heavily depends on Ethereum's implementation(https://github.com/paritytech/rlp). They provide some procedural macros that generate encoding/decoding code for basic objects, but it lacks of many details and functionality. Though it helps on some level, we had to manually implement encoding/decoding routines in many cases. I believe that there will be countless new types of data that should be serialized, and we are going to code repetitive functions again and again if there's no some new powerful metaprogramming support.

(see https://github.com/CodeChain-io/foundry/issues/65)

junha1 avatar Jan 13 '20 06:01 junha1

We need some examples or code suggestions to discuss this more.

Could you give some examples of procedural macros that can be used to create serialize/deserialize RLP encoding?

You don't need to give the definition of the procedural macros. I want to see what can be changed if we use Procedural macros.

majecty avatar Jan 14 '20 01:01 majecty

@majecty We've already used procedural macros, but the point is that it's limited in very simple cases. For example we can't use it on any struct that has a tuple in it.

junha1 avatar Jan 14 '20 01:01 junha1

Before starting change codes a lot, we need to design and schedule it. We can't discuss from the subject "Let's use procedure macro". Someone should analyze the current status and design on how to change code. Then other people can participate in discussing the subject. So @junha1, could you analyze the current code and suggest how to change them? Then we can discuss the effect of change and how we change it, etc.

majecty avatar Jan 14 '20 02:01 majecty

Screenshot from 2020-01-14 12-21-15 These (not all displayed) are either:

  1. Can be directly removed using #[derive]
  2. Can be removed if we solve this issue
  3. Requires very specific optimization or format, so out of this matter.

For example impl Encodable for TrackerInvoices in core/src/blockchain/invoive_db.rs can be removed if we simply add a tuple recognition in RLP macro

That can be done by adding new pattern on this code (see line 122 in https://github.com/CodeChain-io/rlp/blob/master/rlp-derive/src/en.rs)

However it seems to lack priority and quite time consuming, so I'll just close this issue. @majecty @kseo

junha1 avatar Jan 14 '20 03:01 junha1

Using a macro for RLP is a double-edged sword. When you are using macros for RLP you must consider that the order of the items is important in RLP encoding. When you used a macro for some structure instead of implementing it manually, you couldn't change the order of the field.

sgkim126 avatar Jan 14 '20 06:01 sgkim126

We just decided to keep this opened after some discussion with @kseo. It doesn't seem to be important right now but is worth it enough to stay opened and be discussed later.

junha1 avatar Jan 15 '20 01:01 junha1