PCB 作弊快速测试支持 #459
PCB 作弊快速测试支持 #459
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe changes introduce enhancements to the PeerBanHelper application, focusing on peer banning mechanisms, database upgrades, and configuration management. Key modifications include the addition of silent and disconnect ban actions, new parameters for fast PCB testing, and updates to database handling logic. Several classes have been modified to accommodate these changes, including adjustments to logging, concurrency management, and localization for user messages. The configuration files have also been updated to reflect new features and versioning. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PeerBanHelper
participant Database
User->>PeerBanHelper: Initiate Peer Action
PeerBanHelper->>Database: Check Peer Status
alt Silent Ban
PeerBanHelper->>User: Log Silent Ban
else Disconnect Ban
PeerBanHelper->>User: Log Disconnect Ban
else Regular Ban
PeerBanHelper->>Database: Update Peer Status to Banned
PeerBanHelper->>User: Notify Ban Action
end
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range and nitpick comments (1)
src/main/resources/profile.yml (1)
184-188: LGTM, but monitor the impact and adjust if needed.The new
fast-pcb-test-block-durationconfiguration option is well-explained, and the trade-offs between detecting malicious leechers and affecting normal downloaders are clearly outlined.The default value of 30 seconds seems like a reasonable starting point. However, I recommend closely monitoring the real-world performance and impact of this setting on both malicious leechers and normal downloaders. Be prepared to adjust the value as needed to strike the right balance between detection effectiveness and user experience.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (12)
- src/main/java/com/ghostchu/peerbanhelper/PeerBanHelperServer.java (5 hunks)
- src/main/java/com/ghostchu/peerbanhelper/config/ProfileUpdateScript.java (1 hunks)
- src/main/java/com/ghostchu/peerbanhelper/database/DatabaseHelper.java (2 hunks)
- src/main/java/com/ghostchu/peerbanhelper/database/dao/impl/ProgressCheatBlockerPersistDao.java (2 hunks)
- src/main/java/com/ghostchu/peerbanhelper/database/table/ProgressCheatBlockerPersistEntity.java (1 hunks)
- src/main/java/com/ghostchu/peerbanhelper/module/PeerAction.java (1 hunks)
- src/main/java/com/ghostchu/peerbanhelper/module/impl/rule/ProgressCheatBlocker.java (6 hunks)
- src/main/java/com/ghostchu/peerbanhelper/text/Lang.java (1 hunks)
- src/main/resources/lang/en_us/messages.yml (1 hunks)
- src/main/resources/lang/messages_fallback.yml (1 hunks)
- src/main/resources/lang/zh_cn/messages.yml (1 hunks)
- src/main/resources/profile.yml (2 hunks)
Additional context used
yamllint
src/main/resources/lang/messages_fallback.yml
[error] 372-372: no new line character at the end of file
(new-line-at-end-of-file)
src/main/resources/lang/en_us/messages.yml
[error] 372-372: no new line character at the end of file
(new-line-at-end-of-file)
Additional comments not posted (26)
src/main/java/com/ghostchu/peerbanhelper/database/table/ProgressCheatBlockerPersistEntity.java (1)
40-41: New field added. Verify its purpose and usage.The addition of the
fastPcbTestExecuteAtfield is approved.However, please provide more context on the purpose and usage of this field. Consider adding comments to clarify its role within the
ProgressCheatBlockerPersistEntityclass and how it interacts with the fast PCB test functionality.src/main/java/com/ghostchu/peerbanhelper/database/DatabaseHelper.java (2)
47-47: LGTM!The change to the version string is approved.
69-73: LGTM!The changes to handle the database upgrade from version 5 to 6 are approved. The logic to drop and recreate the
ProgressCheatBlockerPersistEntitytable ensures a clean state after the upgrade.src/main/java/com/ghostchu/peerbanhelper/database/dao/impl/ProgressCheatBlockerPersistDao.java (2)
49-50: LGTM!The code changes are approved.
The
FastPcbTestExecuteAtvalue is correctly added as an additional argument when creating a new instance ofProgressCheatBlocker.ClientTask. This change enhances the data being fetched from the database by incorporating the new attribute.
74-75: LGTM!The code changes are approved.
The
FastPcbTestExecuteAtvalue is correctly added as an argument when creating a new entity fromProgressCheatBlocker.ClientTaskRecord. This change ensures that the new attribute is persisted alongside other task details during the flushing operation.src/main/java/com/ghostchu/peerbanhelper/text/Lang.java (2)
325-345: LGTM!The code changes that add new constants to the
Langenum for GUI menu options are approved. The additions follow the existing naming convention, are properly formatted, and expand the functionality without introducing any issues.
345-346: LGTM!The code changes that add new constants
PCB_RULE_PEER_PROGRESS_CHEAT_TESTINGandPCB_DESCRIPTION_PEER_PROGRESS_CHEAT_TESTINGto theLangenum are approved. The additions follow the existing naming convention, are properly formatted, and expand the functionality without introducing any issues.src/main/java/com/ghostchu/peerbanhelper/config/ProfileUpdateScript.java (2)
28-32: LGTM! The newfastPcbTestingfunction looks good.The changes introduce a new
fastPcbTestingfunction annotated with@UpdateScript(version = 19), which sets two configuration parameters:
module.progress-cheat-blocker.fast-pcb-test-percentageis set to0.1d.module.progress-cheat-blocker.fast-pcb-test-block-durationis set to30000.Based on the naming, these settings seem to be related to a new "fast PCB testing" feature that allows temporarily banning peers who connect with 0% progress after downloading a certain percentage of the seed's file data. The
fast-pcb-test-percentagelikely represents this download percentage threshold, whilefast-pcb-test-block-durationspecifies the duration of the temporary ban in milliseconds.This aligns with the PR summary that mentions introducing this mechanism to quickly detect leechers that reset their progress upon disconnection.
Line range hint
35-38: Skipped reviewing thebanDelayWaitfunction since it's unchanged in this diff.src/main/java/com/ghostchu/peerbanhelper/module/impl/rule/ProgressCheatBlocker.java (5)
78-78: LGTM!The code changes are approved.
79-79: LGTM!The code changes are approved.
184-185: LGTM!The code changes are approved.
373-373: LGTM!The code changes are approved.
239-254: LGTM, but verify the changes are thoroughly tested.The code changes are approved.
However, ensure that this change is thoroughly tested to confirm that it behaves as expected and captures the intended fast PCB test scenarios.
Run the following script to verify the changes:
Verification successful
Changes are present and initialized correctly, but ensure thorough testing.
The new logic for the fast PCB test is correctly implemented and the variables are initialized from the configuration. However, please ensure that this change is thoroughly tested to confirm that it behaves as expected and captures the intended fast PCB test scenarios.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the fast PCB test mechanism is working as expected. # Test: Search for the fast PCB test logic. Expect: The new code block is present. rg --type java -A 10 $'if \(fastPcbTestPercentage > 0\) \{' # Test: Search for the usage of the new variables. Expect: They are initialized from the config and used in the new code block. rg --type java -A 5 $'fastPcbTestPercentage|fastPcbTestBlockingDuration'Length of output: 5333
src/main/resources/profile.yml (2)
1-1: LGTM!The
config-versionupdate is approved as it reflects the changes made to the configuration schema.
175-183: LGTM!The new
fast-pcb-test-percentageconfiguration option is well-documented and provides a useful feature to quickly detect progress reset cheating by temporarily banning peers after they download a certain percentage of data.The default value of 0.1 (10%) seems reasonable, and the ability to disable it by setting it to -1 provides flexibility.
src/main/resources/lang/zh_cn/messages.yml (3)
369-369: LGTM!The code changes are approved.
370-370: LGTM, but please provide more context.The localization message is correctly formatted.
However, it's unclear from the provided code what the "Peer 作弊快速测试" (Peer progress cheat quick test) rule does and how it fits into the overall functionality. Could you please provide more information about the purpose and behavior of this rule?
371-371: LGTM!The code changes are approved. The message clearly communicates the temporary nature of the ban and the expected follow-up action to unban the peer.
src/main/resources/lang/messages_fallback.yml (1)
370-370: LGTM!The new localized message string for the downloader statistics request failure scenario looks good.
src/main/resources/lang/en_us/messages.yml (3)
370-370: LGTM!The code changes are approved.
371-371: LGTM!The code changes are approved.
372-372: LGTM!The code changes are approved.
Tools
yamllint
[error] 372-372: no new line character at the end of file
(new-line-at-end-of-file)
src/main/java/com/ghostchu/peerbanhelper/PeerBanHelperServer.java (3)
90-90: LGTM!The addition of the
banWaveLockis a good practice for managing concurrency during ban operations. It ensures that only one thread can execute the critical section at a time, preventing race conditions and inconsistent states.
Line range hint
470-483: LGTM!The changes correctly handle the
BANandSILENT_BANactions by:
- Determining the actual ban duration.
- Creating a
BanMetadataobject with the ban details.- Adding the
BanMetadatatobannedPeerslist.- Adding the torrent to
relaunchlist.- Calling
banPeermethod to ban the peer.- Logging a warning message only for non-silent bans.
The code is well-structured and follows the expected behavior.
766-768: LGTM!The changes ensure that the
resultvariable is updated with the currentCheckResultwhen aBANorSILENT_BANaction is encountered. This guarantees that the finalresultreflects the ban action if any module decides to ban the peer.
src/main/java/com/ghostchu/peerbanhelper/module/PeerAction.java
Outdated
Show resolved
Hide resolved
src/main/java/com/ghostchu/peerbanhelper/module/impl/rule/ProgressCheatBlocker.java
Show resolved
Hide resolved
简述
目前的 ProgressCheatBlocker 存在一个问题，也就是只有 Peer 完整的下载完一次文件，才能在下次重新连接时触发进度回退检测。
尽管这种手段非常有效，但是这最起码会对做种者产生 1.0 分享率的损失，并且发现速度较慢。
解决方案
目前常见的大部分吸血端在连接时都会以 0% 的进度连接，一旦连接断开，它们的进度就会重置。
本 PR 通过设置一个百分比，当对端吸血一定百分比的种子大小的文件数据时，PBH 就会短暂的封禁它们（默认为 15 秒）以便让下载器断开与他们的连接（并从内存 LRU 缓存中逐出（如果有））。
这样在封禁其结束后，吸血端重新以 0% 进度连接时就会立刻被 PCB 捕获。每次捕获在野恶意客户端可以节约 90% 流量的损失。当一个 PCB 特征组通过测试后，在 persist-duration 时间内不会在此对此特征组进行再次检查。
如果吸血端想要绕过此检查，则必须在内存或者 DB 中持久保存其记录的 bitfield 信息（因为胡编乱造一个会导致进度匹配不上，从而触发 PCB 的其它检查）。这无形之中对吸血者提出了更高硬件要求，增加了其吸血成本。
即使吸血者可绕过此次新增的检查，但依旧无法绕过此前已添加的 PCB 的其它检查。因此本 PR 的功能主要是帮助 PCB 的其它模块快速预热、加速恶意客户端的发现。
新增配置文件项
注意事项
与 qBittorrentEE 的 shadowBan 功能不兼容，在开启了 shadowBan 后，由于封禁不再切断连接，因此无法进行快速测试
Summary by CodeRabbit
Summary by CodeRabbit
New Features
SILENT_BANandBAN_FOR_DISCONNECTaction types.Bug Fixes
BAN_FOR_DISCONNECT.Documentation