Return pruned entry count from prune_live_chain()

[pablodeymo]

Summary

• Make prune_live_chain() return usize (number of entries pruned) to match prune_gossip_signatures() and prune_aggregated_payloads()
• Update the caller in update_checkpoints() to log all three pruned counts together

Test plan

• cargo check passes

[github-actions]

🤖 Kimi Code Review

Review Summary

The changes in this PR are correct and improve observability of the pruning process. No security vulnerabilities or correctness issues found.

Positive Changes

1. Return value from prune_live_chain (line 422): Adding -> usize return type provides visibility into how many live chain entries were pruned, which is valuable for debugging and monitoring.

2. Enhanced logging (lines 374-378): The updated log message now includes pruned_chain count and changes "Pruned finalized signatures" to "Pruned finalized data" which is more accurate since it covers chain entries, signatures, and payloads.

Minor Suggestions

Line 375: Consider using consistent naming for the return variable:

let pruned_chain_count = self.prune_live_chain(finalized.slot);

This would make the logging line clearer: pruned_chain = pruned_chain_count.

Verification

• The logic correctly handles the case where no pruning occurs (returns 0 instead of unit type)
• The atomicity of the database operations is preserved
• No race conditions introduced since the view is dropped before the write batch begins

The PR is ready to merge.

---

Automated review by Kimi (Moonshot AI) · custom prompt

[github-actions]

🤖 Codex Code Review

Findings

1. Correctness: lost early return value handling in caller?
   In update_checkpoints (around crates/storage/src/store.rs in the hunk starting at line ~367), you now assign pruned_chain from prune_live_chain. That’s fine, but please confirm prune_live_chain is never called elsewhere expecting the old () return type. This is a public method; any external call sites will now fail to compile or need updates. If this is an internal-only API, consider marking it pub(crate) to constrain future breakage.

2. Potential logging semantics change
   The log message now says "Pruned finalized data" and includes pruned_chain. This is accurate, but the old message was "Pruned finalized signatures". Any log parsers/metrics keyed to the old message will break. If you rely on log-based alerts, consider keeping the old message or adding a structured field to preserve compatibility.

Looks good

• Returning the pruned count from prune_live_chain is a clean improvement and you properly avoid a write batch when no keys exist. The count logic is correct and avoids extra reads.

No issues found for consensus-layer logic
This diff only touches storage pruning and logging. No changes to fork choice, attestation processing, justification/finalization, state transition functions, XMSS, or SSZ logic were present in this PR diff.

Suggested next steps

1. If prune_live_chain is used outside this crate, update call sites or document the API change.
2. If logs are used for monitoring, consider keeping backward-compatible message text or adding a stable structured field name.

---

Automated review by OpenAI Codex · custom prompt

[github-actions]

🤖 Claude Code Review

The PR is small, targeted, and clean. Here is the review:

---

Review: Return pruned entry count from prune_live_chain()

Summary: A small consistency fix — prune_live_chain() is brought in line with prune_gossip_signatures() and prune_aggregated_payloads() by returning the pruned entry count, and the log site is updated to report all three together.

---

What looks good

• The count is captured before keys_to_delete is moved into delete_batch, so there's no lifetime or use-after-move issue.
• Replacing keys_to_delete.is_empty() with count == 0 is a minor style improvement — avoids re-calling .len() indirectly.
• Updating the log guard from pruned_sigs > 0 || pruned_payloads > 0 to also include pruned_chain > 0 closes a real observability gap: previously a chain prune with zero signatures/payloads would be silently dropped from the log.
• "Pruned finalized data" is a better log message than "Pruned finalized signatures" given all three dimensions are now reported.
• The doc comment addition is accurate and consistent with the other two prune functions.

---

Pre-existing inconsistency (not introduced here, but worth noting)

The three prune functions have different boundary semantics:

Function	Condition	Prunes finalized slot itself?
prune_live_chain	slot < finalized_slot	No
prune_gossip_signatures	stored.slot <= finalized_slot	Yes
prune_aggregated_payloads	p.slot > finalized_slot (keep)	Yes

prune_live_chain retains the entry at exactly finalized_slot, while the other two remove it. The intent in the doc comment — "only the LiveChain index is pruned" — is consistent with conservative pruning, so this may be deliberate (the finalized block itself stays reachable in the live index). But the asymmetry could cause confusion if a future caller expects uniform behaviour. Worth a clarifying comment at the boundary condition (store.rs:435) if this is intentional.

---

No blocking issues

The change is correct. No new bugs are introduced. The function signature change is purely additive and there is only one call site.

---

Automated review by Claude (Anthropic) · custom prompt

[greptile-apps]

Greptile Summary

This PR refactors prune_live_chain() to return the number of pruned entries, matching the behavior of prune_gossip_signatures() and prune_aggregated_payloads(). The changes improve consistency across pruning methods and provide better observability by logging all three pruned counts together in update_checkpoints().

• Standardized return type across all pruning methods
• Updated logging to include chain pruning count alongside signatures and payloads
• Improved log message from "Pruned finalized signatures" to "Pruned finalized data" for accuracy

Confidence Score: 5/5

• This PR is safe to merge with minimal risk
• Simple refactoring that only changes return type and logging; logic remains identical and follows established patterns from other pruning methods
• No files require special attention

Important Files Changed

Filename	Overview
crates/storage/src/store.rs	Changed prune_live_chain() return type from void to usize for consistency with other pruning methods; updated caller to log all three pruned counts together

_{Last reviewed commit: 4a98524}