Skip to content
New issue

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

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

Already on GitHub? # to your account

Return Pending rather than loop around if no new finalized hash in submit_transaction #1378

Merged
merged 2 commits into from
Jan 18, 2024

Conversation

jsdw
Copy link
Collaborator

@jsdw jsdw commented Jan 18, 2024

I also brazenly decided to remove the debug logging (though kept the timeout), so that it can run a bit faster again. I'm hoping that it won't pose an issue any more, but ifg it does then we can add more logging back again :)

I had a go at reworkign the code a little to simplify it, but it turned out about the same in the end! I thought about a more state-machine like approach, but it would become very verbose or end up being split across multiple functions etc, so I didn't for now.

Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>
// Keep waiting for more finalized blocks until we find it (get rid of any other block refs
// now, since none of them were what we were looking for anyway).
// Not found it! If follow ev is pending, then return pending here and wait for
// a new one to come in, else loop around and see if we get another one immediately.
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: We could probably remove the else blocks to get rid of some extra indentation; up to you 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I did actually think abotu this, but you'd end up with a bit much for my liking in the new else thing eg

let Some((SeenBlockMarker::Finalized, block_ref)) = seen_blocks.remove(hash) else {
    seen_blocks.clear();
    if follow_ev_is_pending {
        return Poll::Pending;
    } else {
        continue;
    }
};

// ..

Unless you had another idea? :)

@jsdw jsdw marked this pull request as ready for review January 18, 2024 17:54
@jsdw jsdw requested a review from a team as a code owner January 18, 2024 17:54
@jsdw jsdw merged commit 0f48d54 into master Jan 18, 2024
11 checks passed
@jsdw jsdw deleted the jsdw-avoid-infiniteish-loops branch January 18, 2024 17:54
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants