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

Remove timestamps from PeerStanding #474

Conversation

jan-ferdinand
Copy link
Member

The struct PeerStanding records the most recent reward and the most recent punishment. Previously, it recorded the time of the respective event. This time was never used. Now, the time is no longer recorded.

This is a breaking change because struct PeerStanding is public, and so are the modified fields.

Note that this PR builds on top of #469.

Don't repeat the logic for testing whether peer standing is bad. Use one
canonical method instead. Also:

- improve documentation of command line argument `peer_tolerance`
- prevent non-sensical peer tolerances from entering the application
- improve code flow by staying on the happy path more
- adhere to max line length more often
- move test code into test module
The struct `PeerStanding` records the most recent reward and the most
recent punishment. Previously, it recorded the time of the respective
event. This time was never used. Now, the time is no longer recorded.
@jan-ferdinand jan-ferdinand force-pushed the jfs/is_standing_bad_deduplicate_pr branch from a94a3d8 to bfa0896 Compare February 27, 2025 12:46
@Sword-Smith
Copy link
Member

Sword-Smith commented Feb 27, 2025

I think the timestamp could be very valuable for debugging purposes -- as it allows you to go into the logs and see what caused the negative sanction. So I'd rather open an issue to make use of this timestamp in e.g. the dashboard.

Also: The CLI command neptune-cli peer-info prints a JSON formatted string of all peer standings. So you can actually see this timestamp if you want to. Type checker didn't catch that because JSON.

@jan-ferdinand jan-ferdinand force-pushed the jfs/is_standing_bad_deduplicate_pr branch 2 times, most recently from 801e21e to d69be6c Compare February 27, 2025 12:57
@jan-ferdinand
Copy link
Member Author

Closing in favor of #475, which suggests that the timestamps should be used.

@jan-ferdinand jan-ferdinand deleted the jfs/remove_timestamp_from_peer_standing_pr branch February 27, 2025 15:33
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.

2 participants