-
Notifications
You must be signed in to change notification settings - Fork 355
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
Use Include/Exclude Bounds to define range searches #116
Conversation
OK. Even though you already merged it, I'll review the other MR (storage-maps) first, as it will help me gain more context. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it, in that it hides the bound calculation under the impl. It also has a clearer naming IMO.
packages/storage-plus/src/prefix.rs
Outdated
#[derive(Copy, Clone, Debug)] | ||
pub enum Bound<'a> { | ||
Include(&'a [u8]), | ||
Exclude(&'a [u8]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe Inclusive
/ Exclusive
(like in inclusive / exclusive bounds) are better names here.
Migrating cw3-fixed-multisig is a bigger task than I thought, as I need/want to add support for u64 keys natively and not force &id.as_be_bytes() everywhere, but provide a cleaner interface. This is a big boon to the API, but more work than I want to throw in this PR. Will tackle that as a follow up |
I think this is what you were suggesting Mauro - public API doesn't require bit-twiddling.
This is incomplete and definitely missing tests, but please review the idea (and maybe suggest a better path forward).
When we agree on a proper interface and implement it, then I will use the new
Item
andMap
andPrefix/Bound
types to reimplement the storage ofcw3-fixed-multisig
as a real world test case.cw3-fixed-multisig
to new API (Migrate cw3-fixed-multisig #119)Once this is out, I would love to cut a
0.3.0
release, and then we can add secondary indexes in0.3.1
as a non-breaking change