Repository navigation
feat(decdn_node): expose slash-appeal + scan-floor config knobs (ADR 028) - #14
Conversation
…028) Sync the decdn_node role to upstream operator slash detection + appeal (decdn PR #1089, ADR 028). Add two optional [blockchain] knobs to the rendered node.toml, both emitted only when set (mirroring the existing relay_url optional-emit pattern): - decdn_slash_appeal_address — the SlashAppeal contract, target of `decdn appeal slash`. Consumed only by that CLI; the daemon accepts but ignores the key. Validated (0x+40-hex, non-zero) via a when-guarded assert only when an operator sets it, matching the fail-loud posture of the required contract addresses. - decdn_slash_judge_from_block — scan floor for the daemon's new SlashJudge.Slashed watcher. Bounds the full-chain rescan that otherwise runs on every restart. An always-run assert enforces integer shape so a typo can't be silently coerced to 0 by Jinja's int filter and drop the key without a trace. Contract addresses stay operator-supplied (empty defaults, host_vars) per the no-baked-protocol-facts rule. README + host_vars example document the appeal path. Verified: yamllint, ansible-lint (production profile), template render (unset vs set), live assert execution, galaxy build. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 54 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Free Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe decdn node role adds optional slash appeal address and judge-start-block settings, validates their formats, conditionally writes them to ChangesSlash appeal configuration
Estimated code review effort: 2 (Simple) | ~10 minutes Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces optional configuration variables decdn_slash_appeal_address and decdn_slash_judge_from_block to the decdn_node Ansible role, along with corresponding validation tasks and documentation updates. The feedback suggests using | default('') instead of | length > 0 in the Jinja2 template to safely check if decdn_slash_appeal_address is defined and non-empty, preventing potential compilation errors.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| payment_channel_address = "{{ decdn_payment_channel_address }}" | ||
| capacity_bond_address = "{{ decdn_capacity_bond_address }}" | ||
| slash_judge_address = "{{ decdn_slash_judge_address }}" | ||
| {% if decdn_slash_appeal_address | length > 0 %} |
There was a problem hiding this comment.
Following the general rules, use | default('') instead of | length > 0 to safely check if a variable is defined and non-empty. This prevents template compilation errors if the variable is undefined, None, or null.
{% if decdn_slash_appeal_address | default('') %}
References
- In Jinja2 templates, use
| default('')instead of| length > 0to safely check if a variable is defined and non-empty. Using| length > 0can cause template compilation errors if the variable is undefined,None, ornull(especially when using StrictUndefined).
There was a problem hiding this comment.
Not adopting this one. The StrictUndefined failure mode does not apply here: this repo uses Ansible's default undefined handling (no StrictUndefined in ansible.cfg), and decdn_slash_appeal_address has a string default of "" in defaults/main.yml, so it is never undefined/None in normal use. | length > 0 also matches the established idiom already used for decdn_relay_url two lines up in this same template. Finally, this template gate is paired with a when: decdn_slash_appeal_address | length > 0 assert in tasks/main.yml that must stay identical to it — switching only the template to | default() truthiness would desync the two. If we want repo-wide null-safe hardening it should be a separate consistency pass covering relay_url and the asserts too.
There was a problem hiding this comment.
Pull request overview
Adds operator-facing configuration knobs in the decdn_node Ansible role to support the upstream slashing appeal feature (ADR 028) by optionally emitting new [blockchain] fields into the rendered node.toml, along with validation and documentation updates.
Changes:
- Optionally emit
slash_appeal_addressandslash_judge_from_blockintonode.tomlwhen set. - Add fail-loud validation for the optional SlashAppeal address and for the integer “scan floor” block value.
- Document the new knobs and provide inventory examples for operators.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| ansible/roles/decdn_node/templates/node.toml.j2 | Conditionally emits new [blockchain] keys into node.toml. |
| ansible/roles/decdn_node/tasks/main.yml | Adds asserts to validate the optional appeal address and scan-floor block value. |
| ansible/roles/decdn_node/README.md | Documents the new operator knobs and the appeal command usage. |
| ansible/roles/decdn_node/defaults/main.yml | Introduces defaults for the two new optional variables. |
| ansible/inventory/host_vars/decdn-node-1.yml.example | Shows commented example values for the new optional variables. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - name: Validate the SlashJudge scan-floor block is a non-negative integer | ||
| ansible.builtin.assert: | ||
| that: | ||
| - decdn_slash_judge_from_block | string is match('^[0-9]+$') | ||
| fail_msg: >- | ||
| decdn_slash_judge_from_block must be a non-negative integer (the SlashJudge | ||
| deploy block). Got "{{ decdn_slash_judge_from_block }}". Leave it 0 to scan | ||
| from genesis, or set the deploy block to bound per-restart rescan cost. |
There was a problem hiding this comment.
Addressed via the | int render fix (495bf93) rather than tightening the regex. Once the value is emitted through | int, a leading-zero input normalizes to the mathematically-identical block number (001 -> 1) and always produces valid TOML, so the invalid-config path this flags can no longer occur. Keeping the regex as ^[0-9]+$ deliberately: rejecting 001 would fail loud on a harmless, value-preserving form, whereas normalizing it is friendlier and safe. The regex still catches genuine typos (letters, negatives, floats).
…cal TOML A quoted leading-zero value (e.g. "001") passed the assert and the `| int > 0` gate but rendered the raw string, producing invalid TOML (leading zeros are not TOML integers, so the daemon's config load would fail). Render via `| int` so the emitted value is always canonical. Raised in Copilot PR review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
What
Syncs the
decdn_nodeAnsible role to the upstream operator slash detection + appeal feature (decdnPR #1089, ADR 028 — Slashing appeals). Adds two optional[blockchain]knobs to the renderednode.toml, each emitted only when set (mirroring the existingrelay_urloptional-emit pattern).decdn_slash_appeal_addressSlashAppealcontract — target ofdecdn appeal slash. Consumed only by that CLI; the daemon accepts but ignores the key.decdn_slash_judge_from_blockSlashJudge.Slashedwatcher — bounds the full-chain rescan that otherwise runs on every restart.Why
The role already templates the sibling
slash_judge_addressbut exposed neither of the above, so an operator who gets slashed could not file an appeal from the role-managed (do-not-edit)node.toml, and every restart re-scanned chain history for slashes. This completes the operator surface for the new feature.Notes
host_vars), per AGENTS.md hard-rule feat(ansible): add deployment for deCDN node and internal anvil devnet #1.slash_appeal_addressis validated (0x+40-hex, non-zero) when set;slash_judge_from_blockhas an always-run integer-shape assert so a typo can't be silently coerced to0by Jinja'sintfilter (raised in review).BlockchainConfigindecdn/crates/common/src/config/types.rs.Verification
yamllint (repo config) · ansible-lint (production profile) · template render (unset ⇒ keys absent / set ⇒ keys emitted, valid TOML) · live
ansible-playbookassert matrix (0/int/quoted-int pass; typo/negative fail loud) ·make buildgalaxy collection.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation