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

feat: make prc errors ban-able for sync #5884

Merged

Conversation

hansieodendaal
Copy link
Contributor

@hansieodendaal hansieodendaal commented Oct 30, 2023

Description

Made RPC errors ban-able during header-sync and horizon-sync ban-able with a short ban duration.

Closes #5874

Motivation and Context

During sync, we establish a client-server connection and then execute RPC methods from the client to the server, which we know should succeed, so that any RPC errors afterwards are a banable offence. Something else to consider is that sync only acts if better peer metadata is received from a peer, translating to malicious behaviour should a peer not want to play the protocol afterwards.

How Has This Been Tested?

Integration-level unit tests in base_layer\core\tests\tests:

  • block_sync.rs
  • header_sync.rs
  • horizon_sync.rs (still in progress)

What process can a PR reviewer use to test or verify this change?

  • Code walk-through
  • Review unit tests ^^

Breaking Changes

  • None
  • Requires data directory on base node to be deleted
  • Requires hard fork
  • Other - Please specify

Made RPC errors ban-able during header-sync and horizon-sync ban-able with a short
ban duration.
@ghpbot-tari-project ghpbot-tari-project added P-acks_required Process - Requires more ACKs or utACKs P-reviews_required Process - Requires a review from a lead maintainer to be merged labels Oct 30, 2023
@github-actions
Copy link

Test Results (CI)

1 243 tests   1 243 ✔️  13m 31s ⏱️
     39 suites         0 💤
       1 files           0

Results for commit 035bb93.

@github-actions
Copy link

Test Results (Integration tests)

  2 files  11 suites   22m 46s ⏱️
34 tests 32 ✔️ 0 💤 2
36 runs  34 ✔️ 0 💤 2

For more details on these failures, see this check.

Results for commit 035bb93.

@ghpbot-tari-project ghpbot-tari-project removed the P-reviews_required Process - Requires a review from a lead maintainer to be merged label Oct 31, 2023
@SWvheerden SWvheerden merged commit 4ca664e into tari-project:development Oct 31, 2023
@hansieodendaal hansieodendaal deleted the ho_sync_short_ban branch November 21, 2023 12:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-acks_required Process - Requires more ACKs or utACKs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RPC error should result in a short ban
3 participants