Skip to content

v1.18: fix: ensure vote packets can be retried (backport of #2605)#2611

Closed
mergify[bot] wants to merge 2 commits intov1.18from
mergify/bp/v1.18/pr-2605
Closed

v1.18: fix: ensure vote packets can be retried (backport of #2605)#2611
mergify[bot] wants to merge 2 commits intov1.18from
mergify/bp/v1.18/pr-2605

Conversation

@mergify
Copy link
Copy Markdown

@mergify mergify Bot commented Aug 15, 2024

Problem

Retryable vote packets are not retried so any votes received at the end of a block won't ever get processed by the current leader leading to higher voting latency

This is because when we try to reinsert retryable vote tx packets, the reinserted vote tx's will have timestamps equal to but not greater than the latest stored vote tx's timestamp. So the "taken" vote packet never gets replenished.

Summary of Changes

Allow replacing existing latest votes when the latest vote is "taken" and the slot and timestamps are equivalent to the replaced vote.

Fixes #


This is an automatic backport of pull request #2605 done by [Mergify](https://mergify.com).

(cherry picked from commit ecb44d7)

# Conflicts:
#	core/src/banking_stage/latest_unprocessed_votes.rs
@mergify mergify Bot added the conflicts label Aug 15, 2024
@mergify mergify Bot requested a review from a team as a code owner August 15, 2024 14:52
@mergify
Copy link
Copy Markdown
Author

mergify Bot commented Aug 15, 2024

Cherry-pick of ecb44d7 has failed:

On branch mergify/bp/v1.18/pr-2605
Your branch is up to date with 'origin/v1.18'.

You are currently cherry-picking commit ecb44d7bd7.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   core/src/banking_stage/unprocessed_transaction_storage.rs

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   core/src/banking_stage/latest_unprocessed_votes.rs

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@jstarry jstarry force-pushed the mergify/bp/v1.18/pr-2605 branch from b91584e to 3a725db Compare August 15, 2024 23:13
@jstarry jstarry requested a review from apfitzge August 15, 2024 23:21
@jstarry
Copy link
Copy Markdown

jstarry commented Aug 15, 2024

When fixing conflicts I also pulled in the race condition fix from #1838 because even though v1.18 doesn't have #755, there's still a race condition when inserting the latest entry.

@jstarry jstarry requested a review from AshwinSekar August 15, 2024 23:23
@t-nelson
Copy link
Copy Markdown

to what extent does this improve confirmation times?

the changes seem non-trivial. we should at the very least let them bake on 2.0/testnet for a week or so

@jstarry
Copy link
Copy Markdown

jstarry commented Aug 16, 2024

to what extent does this improve confirmation times?

It's very difficult to know the impact on confirmation times. At the end of any of a leader's slots, any remaining buffered votes effectively get dropped for processing and forwarding and we don't have metrics for that.

From my own speculation I think the impact of this bug is not super big because note that if the next vote from a validator is delivered during the next slot it would be eligible for processing and we wouldn't care that the earlier vote was dropped. But I suspect that sometimes a few consecutive votes from validators are all received towards the end of a leader's slot and all get dropped leading to slower confirmation times and increased vote latency for an individual validator.

the changes seem non-trivial. we should at the very least let them bake on 2.0/testnet for a week or so

I'm cool with that. The 2.0 backport is subject to approval here: #2612

@willhickey
Copy link
Copy Markdown

@apfitzge @jstarry How important do you think this is to backport to v1.18? (we expect v1.18 to be EOL ~ mid October)

@jstarry
Copy link
Copy Markdown

jstarry commented Oct 1, 2024

yeah no need to bp since mainnet will be on v2.0 soon

@jstarry jstarry closed this Oct 1, 2024
@jstarry jstarry deleted the mergify/bp/v1.18/pr-2605 branch October 1, 2024 18:51
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