agreement: use proposal-vote verified timing for dynamic lambda calculation #5671
Conversation
Codecov Report
@@ Coverage Diff @@
## feature/dynamic-lambda #5671 +/- ##
==========================================================
+ Coverage 55.16% 55.18% +0.02%
==========================================================
Files 465 465
Lines 65072 65097 +25
==========================================================
+ Hits 35897 35926 +29
+ Misses 26783 26780 -3
+ Partials 2392 2391 -1
... and 14 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
remove commented out code from when we were trying to get receivedAt for proposal-votes
fix comment to say validated instead of received
agreement/events.go
Outdated
| if e.T == payloadVerified { | ||
| e.Input.Proposal.validatedAt = d | ||
| } else if e.T == voteVerified && e.Input.UnauthenticatedVote.R.Step == 0 { | ||
| // if this is a proposal vote (step 0), record the validatedAt time on the vote |
There was a problem hiding this comment.
I think that attaching it to votes of any step might be okay.
The overhead of reading the clock is already there, so just storing it along with the vote doesn't seem too much of an overhead. It also keeps it more consistent (I can now read the time any vote was verified, no matter the step)
There was a problem hiding this comment.
OK ... I could just pull it up into the demux.go code so it isn't here (and could remove the overhead of reading the clock for all voteValidated if we wanted)
cce
force-pushed
the
dynamic-lambda-vote-validatedAt
branch
from
df24501 to
e178503
Compare
After talking to @yossigi we discussed using the proposal-vote (vote for step 0) arrival or verified time — rather than the payload arrival or verified time — when aggregating timings to use for calculating dynamic lambda. This matches the original definition of lambda as the time to send the initial proposal-vote, rather than the full payload.
We did a little digging together and landed on adding a new
readLowestVotequery event to accompany thereadLowestValue/readLowestPayloadevents that were added as part of the feature/pipeline and speculative assembly branches.This also adds a validatedAt time to the vote type similar to the similar receivedAt and validatedAt timings added to proposal payloads from #5041, #4583, and #3703 (and originally from the feature/pipeline branch).
It also updates the
TestPlayerRetainsReceivedValidatedAtandTestPlayerRetainsReceivedValidatedAtPPtests to check these timings are making it through.The PP message contains its own copy of the proposal-vote, but since it is redundant in most cases (since you would have already received the AV message of the proposal-vote) it will be discarded and only one
voteVerifiedevent per unique proposal-vote will be delivered. Since we are stamping the time atvoteVerifiedtime that should be enough to ensure the first timing is what is retained.