Submit a vote timestamp every vote #10630
Conversation
mvines
force-pushed
the
ts
branch
5 times, most recently
from
d958165
to
62b4f10
Compare
|
@mvines To fix docs ci failure, you needs this: https://github.com/solana-labs/solana/pull/10640/files Sorry for inconvenience.. |
thanks! |
mergify
bot
dismissed
CriesofCarrots’s stale review
Pull request has been modified.
mvines
force-pushed
the
ts
branch
2 times, most recently
from
43136dd
to
584e6b6
Compare
|
Good news! I've managed to construct a real dependency on #9902 (@ryoqun 👋) with this PR. The because when the node restarts, it drops its tower and re-sends votes for slots it has already voted on. The introduction of a timestamp in each vote causes these duplicate votes to be rejected with Rather than adapting the vote program to handle this case, I think we should just fix #9902 :) |
| @@ -347,6 +345,22 @@ impl Tower { | |||
| last_vote | |||
| } | |||
|
|
|||
| fn maybe_timestamp(&mut self, current_slot: Slot) -> Option<UnixTimestamp> { | |||
There was a problem hiding this comment.
I considered removing the maybe_ (even did it locally once) but the function does still return an Option because the function will not add a timestamp to a slot that's older than the latest slot that has been timestamped.
(although this behaviour might be unnecessary once tower save/restore is working...)
mvines
changed the title
mvines
changed the title
|
@mvines , how do you feel about this one now? I removed the backport labels to do that manually after the vote-program changes are released? (Couldn't figure out how to remove the broken backport rules, though :( ) |
core/src/rpc.rs
Outdated
| @@ -625,7 +625,7 @@ impl JsonRpcRequestProcessor { | |||
| self.check_slot_cleaned_up(&result, slot)?; | |||
| Ok(result.ok()) | |||
| } else { | |||
| Ok(None) | |||
| Err(RpcCustomError::BlockNotAvailable { slot }.into()) | |||
There was a problem hiding this comment.
We actually could break out these rpc error-handling changes and get them landed sooner. I guess I could have done that in June in retrospect!
There was a problem hiding this comment.
Ah, good call. I'll pull that out to land & backport first.
looks good, just gotta deal with the rollout |
looks good, just gotta deal with the rollout
|
|
@Mergifyio refresh ? |
|
Command |
CriesofCarrots
changed the title
* Submit a timestamp for every vote * Submit at most one vote timestamp per second * Submit a timestamp for every new vote Co-authored-by: Tyera Eulberg <[email protected]>
* Submit a vote timestamp every vote (#10630) * Submit a timestamp for every vote * Submit at most one vote timestamp per second * Submit a timestamp for every new vote Co-authored-by: Tyera Eulberg <[email protected]> * Timestamp first vote (#11856) * Cache block time in Blockstore (#11955) * Add blockstore column to cache block times * Add method to cache block time * Add service to cache block time * Update rpc getBlockTime to use new method, and refactor blockstore slightly * Return block_time with confirmed block, if available * Add measure and warning to cache-block-time Co-authored-by: Michael Vines <[email protected]>
getBlockTime has been unreliable in a heavily forking cluster, likely due to slots with timestamped votes not getting confirmed. There doesn't seem to be a strong argument for not including a timestamp more frequently. Doing so should mean that block time is always available and simplifies the implementation