Conversation
Time Submission Status
|
WalkthroughAdds a new "Enable Peer Blacklisting" section to docs/node-upgrade-guide.md describing peer blacklisting, configuration options (p2p.blacklist), enablement methods (append or manual edit), CLI management commands (add/list/remove), verification steps, and a note about PostgreSQL and upgrade requirements. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Assessment against linked issues
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://waps.l3s.uni-hannover.de/live/im_/https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
docs/node-upgrade-guide.md (3)
109-109: Consistency: use “kwild” (the node binary) instead of “kwil-db”.Elsewhere in the guide we refer to the node as “kwild”. Aligning terminology reduces confusion.
Apply this diff:
-Starting with recent versions, kwil-db includes peer blacklisting functionality to improve network stability and give operators better control over problematic peers. +Starting with recent versions, kwild includes peer blacklisting functionality to improve network stability and give operators better control over problematic peers.
113-116: Fix markdown lint: use asterisks for unordered lists (MD004).Switching to asterisks aligns with the rest of the doc and satisfies markdownlint.
Apply this diff:
-- **Manual peer control** - Block specific peers via CLI commands -- **Persistent storage** - Blacklist survives node restarts -- **Automatic protection** - Auto-blacklist peers that exhaust connection retries (when enabled) -- **Better network health** - Prevent resource waste from problematic peers +* **Manual peer control** - Block specific peers via CLI commands +* **Persistent storage** - Blacklist survives node restarts +* **Automatic protection** - Auto-blacklist peers that exhaust connection retries (when enabled) +* **Better network health** - Prevent resource waste from problematic peers
120-120: Fix “emphasis used as heading” (MD036): make Options real headings.Converting to proper subheadings improves structure and satisfies markdownlint.
Apply these diffs:
-**Option 1: Automated (append to config)** +#### Option 1: Automated (append to config)-**Option 2: Manual Configuration** +#### Option 2: Manual configurationAlso applies to: 150-150
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/node-upgrade-guide.md(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/node-upgrade-guide.md
113-113: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
114-114: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
115-115: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
116-116: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
120-120: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
150-150: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: acceptance-test
🔇 Additional comments (2)
docs/node-upgrade-guide.md (2)
154-163: Verify p2p.blacklist Config Schema
I wasn’t able to find any references top2p.blacklistor the keys below in the codebase—please locate and confirm the actual config struct and defaults before merging:
enableauto_blacklist_on_max_retriesauto_blacklist_duration(ensure this is interpreted in nanoseconds)If the implementation uses different keys, units, or defaults, update
docs/node-upgrade-guide.mdto match.
165-181: Verify blacklist commands in the CLI implementation
The docs introducekwild blacklist add|list|removewith--reasonand--durationflags, but the CLI handlers aren’t part of this repo. Please confirm in the kwil-cli (or kwild) repository that:
blacklist add,blacklist list, andblacklist removesubcommands are implemented.- The
--reasonflag is recognized and properly passed through.- The
--durationflag accepts values like"1h"and parses them into a validtime.Duration.
No code changes needed here—just ensure the docs align with the actual CLI behavior.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
docs/node-upgrade-guide.md (3)
146-149: Update the note to reflect the guarded/idempotent behavior.The text claims the heredoc “safely appends,” but we’re now providing an idempotent guarded script that updates or appends as needed.
-This bash heredoc command (`cat >> file << 'EOF'`) safely appends the configuration block to the bottom of your config file without modifying existing sections. +The script above is idempotent: if a [p2p.blacklist] section already exists, it updates the keys in-place with sed; otherwise it appends a new section. It also creates a timestamped backup before making changes.
122-144: Make the config update idempotent and guard against duplicate TOML tables (fulfills “sed commands” acceptance criterion).The here-doc always appends a new [p2p.blacklist] section and will create duplicate-table parse errors if the section already exists. Replace it with an idempotent, sed-based script that updates in-place when present and appends otherwise. This also meets the issue requirement to supply automated sed commands.
Apply this diff:
-**Option 1: Automated (append to config)** +#### Option 1: Automated (idempotent update or append) @@ -Use this bash command to automatically append the blacklist configuration to the end of your `config.toml` file if the blacklist section doesn't already exist: +Use this script to idempotently add or update the blacklist configuration in your `config.toml`: @@ -```bash -# Stop your node first -sudo systemctl stop kwild - -# Add blacklist configuration to end of config.toml -# Note: Adjust the path if you used a different directory during setup -cat >> ~/truf-node-operator/my-node-config/config.toml << 'EOF' - -# peer blacklisting configuration -[p2p.blacklist] -# enable peer blacklisting functionality -enable = true -# automatically blacklist peers that exhaust connection retries -auto_blacklist_on_max_retries = true -# duration to blacklist peers that exhaust connection retries (0 = permanent) -auto_blacklist_duration = "1h" -EOF - -# Restart node -sudo systemctl start kwild -``` +```bash +#!/usr/bin/env bash +set -euo pipefail + +# Stop your node first +sudo systemctl stop kwild + +# Path to your config.toml (adjust if you used a different directory) +CFG="${CFG:-$HOME/truf-node-operator/my-node-config/config.toml}" + +# Make a timestamped backup +cp -a "$CFG" "${CFG}.bak.$(date +%Y%m%d_%H%M%S)" + +# If the [p2p.blacklist] section exists, update it in-place (idempotent). +if grep -qE '^\s*\[p2p\.blacklist\]\s*$' "$CFG"; then + sed -i -E '/^\s*\[p2p\.blacklist\]\s*$/, /^\s*\[/{ + s/^(enable\s*=\s*).*/\1true/; + s/^(auto_blacklist_on_max_retries\s*=\s*).*/\1true/; + s/^(auto_blacklist_duration\s*=\s*).*/\1"1h"/; + }' "$CFG" +else + # Append a fresh section at the end + cat >> "$CFG" <<'EOF' +# peer blacklisting configuration +[p2p.blacklist] +# enable peer blacklisting functionality +enable = true +# automatically blacklist peers that exhaust connection retries +auto_blacklist_on_max_retries = true +# duration to blacklist peers that exhaust connection retries +# For versions supporting human-readable durations: +auto_blacklist_duration = "1h" +# For older versions, use nanoseconds instead (uncomment the next line): +# auto_blacklist_duration = 3600000000000 +EOF +fi + +# Restart node +sudo systemctl start kwild +```Note: If your current release expects integer nanoseconds (not strings), switch the sed replacement of auto_blacklist_duration to 3600000000000 and/or uncomment the numeric line in the append block.
191-201: Add Troubleshooting and an Operator rollout announcement template (acceptance criteria).The linked issue requires troubleshooting guidance and a communication template. Append both after Verification.
### Verification @@ kwild blacklist list+### Troubleshooting
+
+Common issues and fixes:
+
+* Duplicate table error for [p2p.blacklist]:
- Symptom: kwild fails to start with a TOML parse error about a duplicated table.
- Fix: remove the extra section or re-run the idempotent sed script above. To inspect where the section appears:
- nl -ba "$HOME/truf-node-operator/my-node-config/config.toml" | sed -n '/^\s*[p2p.blacklist]/,/^\s*[/{p}'
+* Blacklist CLI not found:
- Ensure your kwild binary is updated:
kwild version. Upgrade if needed and restart.
+* Auto-blacklist not working:
- Verify config values:
- kwild print-config | sed -n '/^[p2p.blacklist]/,/^[/{p}'
- Check logs for connection retry/exhaust messages.
+### Operator rollout announcement template
+
+Use this template to notify your community/operators:
+
+text +Subject: Enabling peer blacklisting on <Network/Cluster> nodes + +We are enabling peer blacklisting on all <Network/Cluster> kwild nodes to improve stability and protect against misbehaving peers. + +Action for operators: +- Upgrade to the latest kwild binary. +- Enable the blacklist in config.toml (see Node Upgrade Guide, Section "Enable Peer Blacklisting"). +- Optionally enable auto-blacklist of peers that exhaust connection retries. + +Verification: +- Run: kwild print-config | grep -A5 blacklist +- Run: kwild blacklist list + +If you encounter issues, please reply on <Discord/Issue tracker>. +Timeline: <date/time window>. +
+---</blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (8)</summary><blockquote> <details> <summary>docs/node-upgrade-guide.md (8)</summary><blockquote> `107-107`: **Remove “(New Feature)” from the heading or replace with a concrete minimum version.** “(New Feature)” will age quickly. Prefer a stable heading, or explicitly state the minimum kwild version in the body. ```diff -## 6. (New Feature) Enable Peer Blacklisting +## 6. Enable Peer Blacklisting
109-110: Specify the minimum kwild version that supports blacklisting and duration format.“Starting with recent versions” is ambiguous. Clarify the minimum version so operators can decide whether to upgrade before enabling.
-Starting with recent versions, kwil-db includes peer blacklisting functionality to improve network stability and give operators better control over problematic peers. +Starting with kwild vX.Y.Z+, kwil-db includes peer blacklisting functionality to improve network stability and give operators better control over problematic peers. Ensure your node is at least vX.Y.Z (`kwild version`) before enabling.Please replace vX.Y.Z with the actual minimum version that introduced the feature and, if applicable, the release that switched auto_blacklist_duration to a human-readable duration.
113-116: Use consistent unordered list markers per markdownlint (MD004).The rest of the doc prefers asterisks. Switch dashes to asterisks for consistency.
-- **Manual peer control** - Block specific peers via CLI commands -- **Persistent storage** - Blacklist survives node restarts -- **Automatic protection** - Auto-blacklist peers that exhaust connection retries (when enabled) -- **Better network health** - Prevent resource waste from problematic peers +* **Manual peer control** - Block specific peers via CLI commands +* **Persistent storage** - Blacklist survives node restarts +* **Automatic protection** - Auto-blacklist peers that exhaust connection retries (when enabled) +* **Better network health** - Prevent resource waste from problematic peers
120-120: Use a heading instead of emphasis for “Option 1” (MD036).-**Option 1: Automated (append to config)** +#### Option 1: Automated (idempotent update or append)
150-150: Use a heading instead of emphasis for “Option 2” (MD036).-**Option 2: Manual Configuration** +#### Option 2: Manual Configuration
165-172: Document nanosecond alternative in Supported duration formats and verify current support.Add the ns form so operators on older builds aren’t blocked.
**Supported duration formats:** -- "30s" - 30 seconds -- "5m" - 5 minutes -- "1h" - 1 hour (default) -- "2h30m" - 2 hours and 30 minutes -- "1h30m45s" - 1 hour, 30 minutes, and 45 seconds -- "0s" or "0" - Permanent blacklist +* "30s" - 30 seconds +* "5m" - 5 minutes +* "1h" - 1 hour (default) +* "2h30m" - 2 hours and 30 minutes +* "1h30m45s" - 1 hour, 30 minutes, and 45 seconds +* "0s" or "0" - Permanent blacklist +* 3600000000000 - 1 hour in nanoseconds (for older versions that require integer durations)
173-189: CLI examples look good. Consider clarifying the peer ID placeholder format.Minor: add a short note that peer IDs are libp2p-style PeerIDs.
If desired, append a one-liner after the block: “Note: Replace 12D3KooWExample... with the target peer’s libp2p PeerID.”
193-193: Grammar fix in Verification intro.Add “the” for readability.
-Check that blacklist is working: +Check that the blacklist is working:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/node-upgrade-guide.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/node-upgrade-guide.md
[grammar] ~193-~193: There might be a mistake here.
Context: ...Example... ### Verification Check that blacklist is working:bash # Should ...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
docs/node-upgrade-guide.md
113-113: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
114-114: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
115-115: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
116-116: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
120-120: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
150-150: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
166-166: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
167-167: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
168-168: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
169-169: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
170-170: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
171-171: Unordered list style
Expected: asterisk; Actual: dash
(MD004, ul-style)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: acceptance-test
- GitHub Check: lint
🔇 Additional comments (1)
docs/node-upgrade-guide.md (1)
154-163: Add version-compatible duration examples for auto_blacklist_duration — confirm parser behaviorI couldn't find code that parses p2p.blacklist in this repo, so I can't confirm whether the runtime requires numeric nanoseconds or accepts human-readable duration strings (docs already show human-readable formats). Other config readers (e.g., extensions/tn_cache) use time.ParseDuration, which suggests string durations are supported elsewhere, but this isn't definitive for p2p.blacklist.
Files to update:
- docs/node-upgrade-guide.md — occurrences around lines ~132–140 and ~155–162 (auto_blacklist_duration examples and the "Supported duration formats" list).
Suggested doc change (keep both forms and a short note):
# peer blacklisting configuration [p2p.blacklist] # enable peer blacklisting functionality enable = true # automatically blacklist peers that exhaust connection retries auto_blacklist_on_max_retries = true -# duration to blacklist peers that exhaust connection retries (0 = permanent) -auto_blacklist_duration = "1h" +# duration to blacklist peers that exhaust connection retries +# For versions that accept human-readable durations (recommended): +auto_blacklist_duration = "1h" +# For older versions that expect an integer number of nanoseconds, uncomment: +# auto_blacklist_duration = 3600000000000 # 1h in ns +# Use 0 or "0s"/"0" for permanent depending on your releasePlease confirm which format the targeted release(s) accept so we can finalize the examples.
2 participants
Description
Related Problem
resolves: https://github.com/trufnetwork/truf-network/issues/1101
How Has This Been Tested?
Summary by CodeRabbit