Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

iterator feature is not additive #2193

Closed
chipshort opened this issue Jul 23, 2024 · 2 comments
Closed

iterator feature is not additive #2193

chipshort opened this issue Jul 23, 2024 · 2 comments
Milestone

Comments

@chipshort
Copy link
Collaborator

Enabling the feature adds functions to the Storage trait, which breaks anyone implementing that themselves.
This is probably not a huge problem because few people do that, but I recently ran into this when implementing it based on QueryRaw.

The simplest way to avoid this is a default implementation that either just fails with unimplemented!() or returns an empty iterator.

Ultimately, a better (but breaking) solution would probably have a separate RangedStorage super-trait of Storage, altough I'm not sure how we would propagate that through the codebase without it becoming non-additive again.

@chipshort chipshort changed the title Iterator feature is not additive iterator feature is not additive Jul 23, 2024
@webmaster128 webmaster128 added this to the 2.2.0 milestone Jul 29, 2024
@webmaster128
Copy link
Member

Ultimately, a better (but breaking) solution would probably have a separate RangedStorage super-trait of Storage, altough I'm not sure how we would propagate that through the codebase without it becoming non-additive again.

I would not do that part. It will increase complexity by a lot not only in our codebase but a lot of user codebases. Using CosmWasm without iterator is something that might only be used by Secret Network and they have all sorts of forks. For Ethereum it would be needed too but I don't see CosmWasm on top of Ethereum any time soon.

@chipshort
Copy link
Collaborator Author

Alright, in that case, we can close here, since the other part is already merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants