Implement DecodeWithMemTracking for BoundedBTreeSet

[dimartiro]

Description

Implement DecodeWithMemTracking for BoundedBTreeMap

[serban300]

From what I understand BoundedBTreeSet doesn't call on_before_alloc_mem when decoding and we need this in order to support DecodeWithMemTracking. We should use the same approach as in #906

[dimartiro]

From what I understand BoundedBTreeSet doesn't call on_before_alloc_mem when decoding and we need this in order to support DecodeWithMemTracking. We should use the same approach as in #906

I'm not sure if I undertand it. Could you please point me to where on_before_alloc_mem is called in that PR?

[serban300]

Could you please point me to where on_before_alloc_mem is called in that PR?

It's not specifically called in that PR. It's called in BTreeMap::decode() which is used in the PR: https://github.com/paritytech/parity-scale-codec/blob/5c51d86fc3d105e4846f3c0ddb22d26d6d11eea4/src/codec.rs#L1276

And for BtreeSet it's the same

Not correct

[dimartiro]

It's not specifically called in that PR. It's called in BTreeMap::decode() which is used in the PR: https://github.com/paritytech/parity-scale-codec/blob/5c51d86fc3d105e4846f3c0ddb22d26d6d11eea4/src/codec.rs#L1276

And for BtreeSet it's the same

Ok I think I got it, will try to address it today. Thanks!

[dimartiro]

From what I understand BoundedBTreeSet doesn't call on_before_alloc_mem when decoding and we need this in order to support DecodeWithMemTracking. We should use the same approach as in #906

@serban300 I basically did the same as in #906. I'm not sure if I can reuse the same PrependCompactInput from bounded_btree_map, so I redefined it in bounded_btree_set.
I'll wait for your feedback.

[serban300]

@serban300 I basically did the same as in #906. I'm not sure if I can reuse the same PrependCompactInput from bounded_btree_map, so I redefined it in bounded_btree_set.
I'll wait for your feedback.

Ok, thanks ! But please let's try to reuse PrependCompactInput somehow. We should be able to declare it in a shared file.

[bkchr]

Please add some test

[dimartiro]

please let's try to reuse PrependCompactInput somehow. We should be able to declare it in a shared file.

@serban300 Done! sorry for the delay, was harder than I expected with all the macro magic 🪄

Please add some test

@bkchr I even added more tests for jam cases than didn't exist before.

Let me know if there is something missing guys, and thanks for your time!

[serban300]

Sorry, I was a bit busy last week. I will try to take a look these days

[serban300]

LGTM ! Just left a nit.

[dimartiro]

@bkchr I believe the remaining failing tests are not related to this change, right?

[dimartiro]

@bkchr could you please create a new release with this change?

[serban300]

I can do that, but probably tomorrow

[dimartiro]

I can do that, but probably tomorrow

No rush! Thank you 🙏🏻

[serban300]

Done: https://crates.io/crates/bounded-collections/0.3.1