Merge rlp and rlp2 #13095
Merge rlp and rlp2 #13095
Conversation
racytech
requested review from
somnathb1,
yperbasis,
AskAlexSharov and
taratorio
core/types/access_list_tx.go
Outdated
| @@ -270,11 +250,11 @@ func (tx *AccessListTx) encodePayload(w io.Writer, b []byte, payloadSize, nonceL | |||
| return err | |||
| } | |||
| // encode Data | |||
| if err := rlp.EncodeString(tx.Data, w, b); err != nil { | |||
| if err := rlp.EncodeBytes(tx.Data, w, b); err != nil { | |||
There was a problem hiding this comment.
Why this rename? Isn't it understood that "string" here means a flat string of bytes.
Additionally it creates naming discrepancy with EncodeStringSizePrefix, ParseString and so on.
There was a problem hiding this comment.
Agree. It was a hard decision. We have two functions named EncodeString in a single package one of which had to be changed. I couldn't come up with anything better. EncodeString2 was another candidate, but I didn't like it. Anyways, please drop any ideas.
There was a problem hiding this comment.
Yeah, maybe we can just get rid of the other one. Till then, EncodeStringBytes is a suitable name. I am hoping we can soon truly merge the two sets of implementations for all other methods too.
There was a problem hiding this comment.
I think it's better to use names EncodeString and EncodeString2 to highlight that it's two flavours of the same thing. And eventually we should merge them.
There was a problem hiding this comment.
I wish Go had function overloading features. I was trying merging them into one function by adding additional parameter and by defining Writer interface for a type SomeType []byte, but things wasn't looking good
Since
rlpandrlp2both hadEncodeStringfunction, I decided to change function name inrlppackage toEncodeBytesto avoid name collision.Functions in
parse.gofileList,String,U256,U64,U32were changed toParseList,ParseString,ParseU256etc.Function
EncodeStructSizePrefixwas moved from types package torlppackage which is more semantically suitable