cluster: fix dead deposit amounts validation in unmarshalers

[pinebit]

Validate the parsed deposit amounts instead of the zero-valued named return in the v1.8, v1.9 and v1.10-11 definition unmarshalers.

The checks called deposit.VerifyDepositAmounts(def.DepositAmounts, def.Compounding) where def is the named return value, still zero at that point — the parsed data lives in defJSON. VerifyDepositAmounts(nil, false) returns nil via its empty-slice early return, so the checks always passed and definitions with invalid deposit amounts (below the 1ETH minimum, above the maximum, or summing to less than 32ETH) unmarshaled without error. The bug dates back to the introduction of partial deposits in v1.8 and was copied into the v1.9 and v1.10-11 unmarshalers.

The v1.8 and v1.9 unmarshalers now pass compounding=false since those versions don't support compounding; the v1.10-11 unmarshaler passes the parsed compounding flag so large amounts are only accepted for compounding validators.

Note this is defense in depth: the DKG path already re-validates amounts after loading (dkg/disk.go), but other consumers of definition/lock unmarshaling relied on the dead check.

category: bug
ticket: none

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.27%. Comparing base (2c0ccd6) to head (38fb91c).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4636      +/-   ##
==========================================
+ Coverage   58.22%   58.27%   +0.04%     
==========================================
  Files         247      247              
  Lines       34082    34082              
==========================================
+ Hits        19843    19860      +17     
+ Misses      11768    11754      -14     
+ Partials     2471     2468       -3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
80.0% Duplication on New Code

See analysis details on SonarQube Cloud

[Copilot]

Pull request overview

Fixes a long-standing bug in cluster definition unmarshaling where deposit amount validation was inadvertently performed against the zero-valued named return (def) instead of the parsed JSON struct (defJSON), causing invalid deposit_amounts to be accepted for v1.8, v1.9, and v1.10–v1.11 definitions.

Changes:

• Validate defJSON.DepositAmounts (and parsed defJSON.Compounding for v1.10–v1.11) instead of def.DepositAmounts/def.Compounding during unmarshaling.
• Force compounding=false for v1.8 and v1.9 definitions (since those versions don’t support compounding).
• Add a regression test covering invalid/valid deposit amount cases across v1.8–v1.11, including the compounding-specific upper bound.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
cluster/definition.go	Fixes dead deposit amount validation by checking parsed JSON fields (and correct compounding behavior by version).
cluster/cluster_test.go	Adds regression tests ensuring invalid deposit amounts fail unmarshaling and compounding rules are enforced.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.