Derivable Encode & Decode #509
Conversation
substrate/codec/derive/src/encode.rs
Outdated
| } | ||
| }, | ||
| // NOTE [ToDr] we currently don't use unions at all. | ||
| Data::Union(_) => unimplemented!(), |
There was a problem hiding this comment.
I can't imagine how to implement unions at all. For example we have a type union { a: A, b: B } (where A and B are Encodable). To encode it we need to call either A::encode or B::encode, and there is no way to know which variant a particular union contains. To alleviate that, we could use some sort of tag which gives us essentially a enum : ) So I guess it's to panic with message like unions are not supported rather than not yet implemented.
substrate/codec/derive/src/encode.rs
Outdated
| }, | ||
| }, | ||
| Data::Enum(ref data) => { | ||
| assert!(data.variants.len() < 256, "Currently only enums with less than 255 variants are encodable."); |
There was a problem hiding this comment.
less than 256 / less than or equal 255?
There was a problem hiding this comment.
Changed to at most 256 :)
substrate/network/src/message.rs
Outdated
| /// Remote method call response. | ||
| RemoteCallResponse(RemoteCallResponse), | ||
| /// Chain-specific message | ||
| #[codec(index = "15")] |
There was a problem hiding this comment.
Great catch, don't know where I took 15 from.
pepyakin
left a comment
pepyakin
left a comment
There was a problem hiding this comment.
That's awesome! Merging as soon as CI is green
pepyakin
added
A8-looksgood
and removed
A0-please_review
|
Does this maintain runtime compatibility? |
|
@arkpar yes, it should. All the enums are encoded/decoded the same way as previously. |
|
That's a good question. I assumed that it is and checked that it actually maintains compatibility. (oops, hit submit without refreshing the page) |
* master: README: fixed typo in docker run command (#518) Merge *_at methods. (#515) New flags to listen to all interfaces (#495) If contract reaches max depth, return Err (#503) Some networking cleanups (#504) Derivable Encode & Decode (#509) substrate: return Option in all storage related RPC methods (#510) Build with locked Cargo.lock on CI (#514) Place call data into a newly allocated pages (#502)
|
just saw this: definitely deserving of |
More trace log Finish logger refine Nit .
Farmer mock dsn sync
Closes paritytech/polkadot#146