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

Get entries in big endian format #7

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

gabrielstoica
Copy link

@gabrielstoica gabrielstoica commented Oct 10, 2024

Hi,

As the current implementation creates a reversed linked list (little endian notation), I thought it'd be a good idea to have a method which formats the result of the getEntriesPaginated method and returns everything in the big endian notation.

Example:
Considering the following linked list: 1 2 3 4 5 6 7 8, by calling the getEntriesPaginatedInOrder with start equal to SENTINEL and pageSize to 4, the result will be 5 6 7 8, compared to the default getEntriesPaginated call which will result in 8 7 6 5.

@gabrielstoica
Copy link
Author

@zeroknots, @kopy-kat would appreciate to get your feedback on this!

@kopy-kat
Copy link
Member

is this needed onchain? couldnt you just call getEntriesPaginated and then reverse this offchain?

@gabrielstoica
Copy link
Author

There may be cases where the order of items is important (e.g. rewarding people on a first-come, first-served basis), therefore manually reverting the list (on-chain) would be an extra step compared to having this natively in the library.

@kopy-kat
Copy link
Member

yeah idm this should be a 1 liner with solady libsort but we can add it into the sentinel list too - in that case I think it would also make sense to use libsort since its much more gas efficient - btw note that consumer contracts like smart accounts or modules anyways need to expose this function so any existing contracts wont get this feature out of the box

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.

2 participants