Add support to DecodeWithMemTracking #315
Conversation
| @@ -154,7 +174,7 @@ pub enum ExitError { | |||
|
|
|||
| /// Other normal errors. | |||
| #[cfg_attr(feature = "with-codec", codec(index = 13))] | |||
| Other(Cow<'static, str>), | |||
| Other(String), | |||
There was a problem hiding this comment.
Because DecodeWithMemTracking is not supported for str so this was the only way to fix it I found.
the trait `DecodeWithMemTracking` is not implemented for `str`, which is required by `Cow<'static, str>: DecodeWithMemTracking
Open to suggestions
There was a problem hiding this comment.
Can we add DecodeWithMemTracking to Cow<'static, str> in parity-scale-codec? Seems like it should have been implemented there.
(In this case, scale decoding will always return Cow::Owned, which is String.)
There was a problem hiding this comment.
DecodeWithMemTracking is already implemented for Cow
But the problem is with str since pub trait Decode: Sized and str does not implements Sized
There was a problem hiding this comment.
nevermind, I think I found a walk around just by adding impl DecodeWithMemTracking for Cow<'static, str> {} in parity-scal-codec, I'll propose that change. Thanks
Tried again and I cannot find a way to implement DecodeWithMemTracking for str
There was a problem hiding this comment.
Ok so far I opened an issue on their side, but I would really like to merge this in order to unblock other changes that need it.
Do we know how big is the performance implication in case of using String instead of Cow?
There was a problem hiding this comment.
New update: Basti opened a PR to solve it 🙌
Description
Add support to
DecodeWithMemTrackingthat is required to upgrade another dependencies that depends on this one to polkadotstable2503Also upgrade ethereum to include my latest commit
Similar to what I did with rust-ethereum/ethereum#62