feat: Implement optimized TreeMap iterator functions

[austinabell]

Closes #426

This PR just makes the implementation more efficient, to avoid unnecessary storage reads

This should be functionally the same externally, but since it will reduce storage reads it's a breaking change in a smart contract context

[matklad]

Probably makes sense to add some smoke tests here. There was a funny bug in std when a combination of nth and rev flat out gave wrong results in some cases, and nobody noticed )

[matklad]

Probably makes sense to add some smoke tests here.

not related to the PR per-se, but it'd be cool to look into setting up property-based/fuzz testing for collections. std collections give us a good, free oracle we can compare against. The two relevant crates in the rust ecosystem are (from most important to least importat)

• https://lib.rs/crates/arbitrary
• https://lib.rs/crates/quickcheck
• https://lib.rs/crates/proptest

Which reminds me that I should dig into https://github.com/graydon/photesthesis at some point...

[austinabell]

Probably makes sense to add some smoke tests here.

not related to the PR per-se, but it'd be cool to look into setting up property-based/fuzz testing for collections. std collections give us a good, free oracle we can compare against. The two relevant crates in the rust ecosystem are (from most important to least importat)

• lib.rs/crates/arbitrary
• lib.rs/crates/quickcheck
• lib.rs/crates/proptest

Which reminds me that I should dig into graydon/photesthesis at some point...

Yeah, I added some basic tests, but yeah fuzzing probably is out of the scope of this. I'd love to tackle this when I have time.

I assume when you mention arbitrary you mean to be used with cargo-fuzz? quickcheck is already used in the SDK for treemap tests, so it is probably the least cost to add those to be run as tests, but definitely would be nice to extensively fuzz all data structures and interactions.

[matklad]

I assume when you mention arbitrary you mean to be used with cargo-fuzz?

Not necessary -- I believe that the arbitrary crate is setup in such a way that is useful even without coverage-guided fuzzers, so it should be possible to use it the same way quickcheck is used, via simple #[test]. I don't think the ecosystem is quite there yet, so it might be that we'll have to do some ground-work here ourselves. arbitrary does seem to be a very well-designed crate that captures the essense of randomized testing, so I'd like to find a way to marry convenient tests with it.

[austinabell]

Not necessary -- I believe that the arbitrary crate is setup in such a way that is useful even without coverage-guided fuzzers, so it should be possible to use it the same way quickcheck is used, via simple #[test]. I don't think the ecosystem is quite there yet, so it might be that we'll have to do some ground-work here ourselves. arbitrary does seem to be a very well-designed crate that captures the essense of randomized testing, so I'd like to find a way to marry convenient tests with it.

And the benefit of doing this over using quickcheck? More control?

Edit: Also just to check, do you think anything in this test should change wrt these points?

[matklad]

And the benefit of doing this over using quickcheck? More control?

The arbitrary crate is a good ecosystem-wide interface. I expect that, over time, a lot of other crates would implement arbitrary::Arbitrary. So, if we make our stuff Adbitrary, then we'll be more inter-operable.

That's I am betting on arbitrary: it's a 1.0 crate which defines just the narrow interface for randomization, without being opinionated about how actual property testing is commenced.

[austinabell]

The arbitrary crate is a good ecosystem-wide interface. I expect that, over time, a lot of other crates would implement arbitrary::Arbitrary. So, if we make our stuff Adbitrary, then we'll be more inter-operable.

That's I am betting on arbitrary: it's a 1.0 crate which defines just the narrow interface for randomization, without being opinionated about how actual property testing is commenced.

You okay with this being done in a separate PR? I can create an issue later.

Rather not increase the scope of this PR if possible

[matklad]

You okay with this being done in a separate PR? I can create an issue later.

Sure, I am always for splitting up PR, and for non-blocking PRs!