parity-common icon indicating copy to clipboard operation
parity-common copied to clipboard

implement parity_scale_codec::DecodeWithMemTracking for primitive types

Open xlc opened this issue 8 months ago • 12 comments

I am getting this error

error[E0277]: the trait bound `H256: parity_scale_codec::DecodeWithMemTracking` is not satisfied
    --> /Users/xiliangchen/.cargo/registry/src/index.crates.io-6f17d22bba15001f/sp-runtime-41.1.0/src/traits/mod.rs:1085:16
     |
1085 |     type Output = sp_core::H256;
     |                   ^^^^^^^^^^^^^ the trait `parity_scale_codec::DecodeWithMemTracking` is not implemented for `H256`

so we need DecodeWithMemTracking implemented for H256 and other primitive types

xlc avatar Apr 23 '25 21:04 xlc

CC @serban300

bkchr avatar Apr 23 '25 21:04 bkchr

I am not sure why I only get this error when upgrading to stable2503 for orml and why it wasn't an issue for polkadot-sdk. But I checked DecodeWithMemTracking doesn't appear to be implemented here so it should be an error.

xlc avatar Apr 23 '25 21:04 xlc

ok I might have being reading the old code, it is implemented https://github.com/paritytech/parity-common/blob/a2b580d9fd5a340cea817bc9ed829320d2c9cd73/primitive-types/impls/codec/src/lib.rs#L35, so maybe I wasn't using the right version

xlc avatar Apr 23 '25 21:04 xlc

I am really confused, the DecodeWithMemTracking is included on primitive-types 0.13.2, which is not released?


~~I shouldn't be looking at primitive-types, instead fixed-hash, which didn't implement DecodeWithMemTracking~~

xlc avatar Apr 23 '25 22:04 xlc

ok I need impl-codec 0.7.1 and that resolves the issue. that's a bit confusing. maybe we should update some Cargo.toml file to pin to 0.7.1 instead of 0.7.0?

xlc avatar Apr 23 '25 22:04 xlc

Maybe we need to release a new version of primitive-types (0.13.2) that depends on impl-codec 0.7.1.

How can I reproduce this ? I see that cargo clippy runs successfully for https://github.com/open-web3-stack/open-runtime-module-library/pull/1023

serban300 avatar Apr 24 '25 07:04 serban300

ORML didn't commit Cargo.lock file and I was having an old one using impl-codec 0.7.0 so I got the error locally. The CI generates new Cargo.lock file so it is actually fine.

xlc avatar Apr 24 '25 08:04 xlc

Unable to reproduce this issue also, looks like cargo has resolved to using correct versions.

Image Image

talhadaar avatar Apr 24 '25 08:04 talhadaar

you can do cargo update -p [email protected] to force use the broken combination

xlc avatar Apr 24 '25 08:04 xlc

maybe quick fix is to yank [email protected]?

acatangiu avatar Apr 24 '25 08:04 acatangiu

maybe quick fix is to yank [email protected]?

I don't think we need to do this. [email protected] is a valid release.

you can do cargo update -p [email protected] to force use the broken combination

This worked, managed to reproduce the issue.

Indeed overriding primitive-types with a local version that depends on impl-codec 0.7.1 would prevent cargo update -p [email protected] --precise 0.7.0 from doing anything.

The problem with a new primitive-types release is that it wouldn't be a patch release because the byteorder feature has been removed: https://github.com/paritytech/parity-common/pull/872 . I think this should be a major release since it's not backwards compatible ?

If we release a new major version of primitive-types (14.0) now, this will be used in the next release of polkadot-sdk

LE: I see that primitive-types only uses minor bumps. So this release would be 0.14.0. So maybe we could port this to stable2503 as well.

serban300 avatar Apr 24 '25 09:04 serban300

Opened a PR for mentioning #872 in the changelogs: https://github.com/paritytech/parity-common/pull/913

Not sure if we should publish a new version of primitive-types that depends on impl-codec 0.7.1 though. In the end primitive-types works with impl-codec 0.7.0 as well. The packages that need the impl-codec types that derive DecodeWithMemTracking should depend on impl-codec 0.7.1

serban300 avatar Apr 24 '25 10:04 serban300