fix: ensure that weighted mappings don't exceed 100

[aricart]

No description provided.

[synadia-claude-reviewer]

Claude finished @aricart's task in 1m 11s —— View job

---

PR Review

• Gather context and read changed files
• Analyze the diff for correctness, security, and performance
• Post inline comments for specific issues
• Summarize overall review

---

Summary

This PR fixes a real integer overflow bug in Mapping.Validate. The accumulator variables (total, perCluster values) were typed as uint8, which wraps at 256. A case like three weights of 90 (sum = 270) would overflow to 14, silently passing validation. Widening the accumulators to uint32 fixes that correctly.

The additional per-entry guard (e.GetWeight() > 100) is a useful complement — it catches inputs where a single weight exceeds the valid range before they even reach the accumulation logic.

Observations

Minor: Double errors for individual weights > 100

When a single Weight > 100 is set, the code emits two errors: one from the new per-entry check at line 159–161, and a second from the total/cluster accumulation (since the entry is still added and the sum will exceed 100). This is not wrong, but the output is slightly redundant. It's worth considering adding a continue after the per-entry error to skip accumulation, or simply accepting the double-error behavior as intentional.

// v2/account_claims.go:159
if e.GetWeight() > 100 {
    vr.AddError(...)
    // consider: continue
}

Weight field is still uint8: This means values 101–255 are storable via direct struct initialization, which is the exact case this fix addresses. The GetWeight() return type is also uint8, so the comparison e.GetWeight() > 100 works correctly (200 > 100 as uint8 is true). No API breakage.

Tests are thorough: The three new tests cover:

1. A single weight exceeding 100 (Weight: 200)
2. Non-clustered accumulation overflow (3×90 = 270, previously wrapped to 14 in uint8)
3. Per-cluster accumulation overflow (same scenario but with Cluster set)

Verdict

The fix is correct and well-tested. The double-error scenario for individual weights > 100 is the only thing worth a second look, but it's not a bug.

[neilalexander]

Is it expected to continue or break here?

[aricart]

continue - we want to get all the errors we can

[neilalexander]

Looks like continue is missing then, we just fall through.

[aricart]

no, one is validating the size of the mapping, the other is an agregagate for cluster (multiple errors can be generated)

[neilalexander]

LGTM