Skip to content

Conversation

@nventuro
Copy link
Contributor

This is an initial attempt at trying to make these more readable. The technique can be trivially extended to MultiMap, but first I wanted to see how far I could take it before going all-in - for this reason I only used it in the note data provider.

It'd be very nice to also do something similar for keys, albeit in such a way where we accept anything that can be converted into a string as a key. It'd let us use AztecAddress, Fr, etc. as types, helping make sense of the types and removing a lot of toString calls that make the code hard to read and easy to misuse. However there's K extends Key constraints all over the place, so doing this might not be trivial. I gave up after some brief exploration.

@nventuro nventuro requested a review from Thunkar April 21, 2025 21:14
@Thunkar
Copy link
Contributor

Thunkar commented Apr 22, 2025

Q: Wouldn't it make more sense to force our stores to only accept types that implement to/fromBuffer? Would that be too much work given we use them everywhere?

@nventuro
Copy link
Contributor Author

Our stores don't currently force you to use Buffer, it's almost a coincidence that this is always the underlying type. Yes, we could have a thin wrapper around them with this behavior, or just change everything to use Buffer under the hood. I first wanted to see the impact of this to determine if the change was worth it.

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

Successfully merging this pull request may close these issues.

3 participants