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.

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

Track SCP latencies in milliseconds for overlay survey #4334

Merged
merged 1 commit into from
Jun 1, 2024

Conversation

bboston7
Copy link
Contributor

Description

Fixes #4333 by recording SCP latencies in milliseconds rather than nanoseconds, which could overflow when placed in a uint32_t.

I also checked the other survey data fields for potential overflow and didn't find anything else that could overflow. However, in doing so I noticed that I failed to bump
TIME_SLICED_SURVEY_MIN_OVERLAY_PROTOCOL_VERSION to 34 when I bumped the overlay protocol version, so I fixed that as well.

Checklist

  • Reviewed the contributing document
  • Rebased on top of master (no merge commits)
  • Ran clang-format v8.0.0 (via make format or the Visual Studio extension)
  • Compiles
  • Ran all tests
  • If change impacts performance, include supporting evidence per the performance document

src/protocol-curr/xdr Outdated Show resolved Hide resolved
src/protocol-next/xdr Outdated Show resolved Hide resolved
src/overlay/SurveyDataManager.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@marta-lokhova marta-lokhova left a comment

Choose a reason for hiding this comment

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

Thanks for the fix, I think we need to wait for the update on rust side to merge this.

Fixes stellar#4333 by recording SCP latencies in milliseconds rather than
nanoseconds, which could overflow when placed in a `uint32_t`.

I also checked the other survey data fields for potential overflow and
didn't find anything else that could overflow. However, in doing so I
noticed that I failed to bump
`TIME_SLICED_SURVEY_MIN_OVERLAY_PROTOCOL_VERSION` to `34` when I bumped
the overlay protocol version, so I fixed that as well.
@graydon
Copy link
Contributor

graydon commented May 31, 2024

r+ 24f4745

@latobarita latobarita merged commit 0f189aa into stellar:master Jun 1, 2024
15 checks passed
@bboston7 bboston7 deleted the scp-stats-ms branch September 16, 2024 22:44
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.

Survey SCP stats can overflow
4 participants