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.

Sign up for GitHub

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

Jump to bottom

Fix: improve UX of findCheckpointHints #750

Merged
merged 8 commits into from
Apr 13, 2023
Merged

Fix: improve UX of findCheckpointHints #750

merged 8 commits into from
Apr 13, 2023

Conversation

folkyatina
Copy link
Member

findCheckpointHints returns NOT_FOUND for unfinalized requests instead of reverting.
Motivation: compatibility with getClaimableEther()

TheDZhon
TheDZhon approved these changes Apr 12, 2023
View reviewed changes
Copy link
Contributor

@TheDZhon TheDZhon left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Please consider adding a few docs about 1-based indexing for the checkpoints (it's not stated explicitly while _start and _end are called indexes).

There are merge conflicts on the way 🌵

@TheDZhon TheDZhon mentioned this pull request Apr 12, 2023
Merged
16 tasks
@TheDZhon
Copy link
Contributor

image

TheDZhon
TheDZhon approved these changes Apr 13, 2023
View reviewed changes
Copy link
Contributor

@TheDZhon TheDZhon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much better docs 👍

@TheDZhon TheDZhon merged commit a8f645e into fix/shapella-upgrade-from-rc1-to-rc2 Apr 13, 2023
@TheDZhon TheDZhon deleted the fix/find-hints-for-zero-checlpoints branch April 13, 2023 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TheDZhon TheDZhon TheDZhon approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@folkyatina @TheDZhon @Jeday