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

Useful response improves reputation #4130

Merged
merged 3 commits into from
Jul 20, 2022
Merged

Conversation

pinges
Copy link
Contributor

@pinges pinges commented Jul 19, 2022

Currently there is no way for peers improve their reputation. With this PR peers improve their reputation when they send a useful response.

Signed-off-by: Stefan stefan.pingel@consensys.net

Signed-off-by: Stefan <stefan.pingel@consensys.net>
@pinges pinges requested a review from macfarla July 19, 2022 07:18
@pinges pinges enabled auto-merge (squash) July 20, 2022 01:42
Copy link
Contributor

@macfarla macfarla left a comment

Choose a reason for hiding this comment

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

LGTM. nit on spelling but I can avert my eyes if needed for expediency

@@ -362,6 +362,14 @@ public void compareTo_withDifferentNodeId() {
assertThat(peer2.compareTo(peer1)).isEqualTo(-1);
}

@Test
public void recordUsefullResponse() {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: s/Usefull/Useful

@pinges pinges merged commit b7877be into hyperledger:main Jul 20, 2022
Copy link
Contributor

@fab-10 fab-10 left a comment

Choose a reason for hiding this comment

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

I am seeing in the logs peers with very high reputation, for example:

FullSyncTargetManager","message":"Caught up to best peer: PeerId 0xb3e02a84270c590f562e2c8b8274930c54b0a00ae001d44bd790b55c3bbc578c70738e6d4a2d32df78b6cf27a071fe54a068f3192c42ac29bd81a6663fe4fa0f, reputation PeerReputation 561, validated? true, disconnected? false, client: Geth/v1.10.21-stable-67109427/linux-amd64/go1.18.5, connection 1312721736, enode enode://b3e02a84270c590f562e2c8b8274930c54b0a00ae001d44bd790b55c3bbc578c70738e6d4a2d32df78b6cf27a071fe54a068f3192c42ac29bd81a6663fe4fa0f@34.92.61.128:29888, chain state: ChainState{estimatedHeight=7369229, bestBlock=BestBlock{totalDifficulty=0x0000000000000000000000000000000000000000000000000000000000a45b44, blockHash=0xb7c2ba396d3f4561fe619796e7ef399080823161f4b831c5b3f48445e7bf13a1, number=7369228}}. Current peers: 3","throwable":""}

since there is no cap to how much the reputation can increase.

I think make sense to have an upper bound, a peer can increase its repution until it reaches 100 again, otherwise a peer that is good for a long time, and start to be useless will take a long time before its reputation goes below


@Test
public void shouldIncreaseScore() {
reputation.recordUsefulResposne();
Copy link
Contributor

Choose a reason for hiding this comment

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

typo

eum602 pushed a commit to lacchain/besu that referenced this pull request Nov 3, 2023
* useful response improves reputation

Signed-off-by: Stefan <stefan.pingel@consensys.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants