fix(pallet-qpow): correct weight calculation for on_finalize

[AdamMomen]

Summary

• This is my first PR so please go easy on me
• This PR fixes incorrect weight calculation in pallet-qpow's on_finalize hook.
• The weight function was named on_finalize_max_history from a deprecated block-time history implementation that was replaced with EMA (Exponential Moving Average), and the storage operation counts were significantly overstated.

Problem

• The on_initialize hook returned a weight claiming: 16 reads / 9 writes
• But the actual on_finalize implementation performs: 7 reads / 5 writes
  This ~2x overestimation wastes block weight capacity.

Root Cause

The weight was inherited from commit 59c0781 which used a BlockTimeHistory ring buffer for difficulty adjustment. When EMA replaced the history system:

• Storage items BlockTimeHistory, HistoryIndex, HistorySize were removed
• BlockTimeEma was added
• But the weight was never updated or renamed

Changes

File	Change
weights.rs	Renamed on_finalize_max_history → on_finalize, corrected reads 16→7, writes 9→5, added missing storage docs
benchmarking.rs	Renamed benchmark, added EMA initialization, improved documentation
lib.rs	Removed TODO, added docs explaining weight purpose

Storage Operations (corrected)

Reads (7):

• Timestamp::Now
• LastBlockTime
• CurrentDifficulty
• System::block_number
• TotalWork
• BlockTimeEma (twice, in update_block_time_ema and adjust_difficulty)

Writes (5):

• TotalWork
• LastBlockDuration (conditional)
• BlockTimeEma
• LastBlockTime
• CurrentDifficulty

Testing

cargo test -p pallet-qpow
23 tests passed ✓

Impact

• Reduces weight overestimation by ~50%
• Improves block space efficiency
• Removes misleading "max_history" naming
• Clarifies weight documentation

---

Note: Proper weight verification would require re-running benchmarks with frame-omni-bencher. This PR corrects the documented storage operations based on code analysis. The execution time estimate (217 µs) is retained until fresh benchmarks can be run.

[AdamMomen]

@czareko I believe you have the most context on this. Would it be possible to take a look?

[czareko]

Thank you @AdamMomen
We will check this soon.

[n13]

AI review:

1. Is this PR needed?

Yes, it addresses a real issue. Your own codebase has a // TODO: update this comment on line 123 of lib.rs, directly above the stale weight call. The current weights claim 16 reads / 9 writes but the actual on_finalize path does far fewer operations. Since this hook runs every single block, the ~2x weight overestimation burns block capacity for nothing. The contributor correctly identified a genuine tech-debt item.

That said, the benefit is modest -- it frees up wasted block weight, not a critical fix. Nice cleanup, not urgent.

---

2. Correctness issues (the PR has bugs)

I traced every storage access in the on_finalize -> adjust_difficulty -> update_block_time_ema call chain. Here's what actually happens:

Actual Reads (7):

#	Storage Item	Location
1	CurrentDifficulty	on_finalize line 130
2	Timestamp::Now	adjust_difficulty line 187
3	LastBlockTime	adjust_difficulty line 188
4	CurrentDifficulty	adjust_difficulty line 189 (read again)
5	System::block_number	adjust_difficulty line 190
6	BlockTimeEma	update_block_time_ema line 143
7	BlockTimeEma	adjust_difficulty line 212

Actual Writes (4, not 5):

#	Storage Item	Location
1	LastBlockDuration	adjust_difficulty line 204
2	BlockTimeEma	update_block_time_ema line 161
3	LastBlockTime	adjust_difficulty line 210
4	CurrentDifficulty	adjust_difficulty line 219

Problems with the PR:

1. TotalWork is phantom. TotalWork is never read or written in the on_finalize code path -- grep confirms it only appears in weights.rs doc comments, nowhere in lib.rs. The PR keeps TotalWork (r:1 w:1) in the doc comments and counts it in the totals. This is wrong.

2. CurrentDifficulty is read twice, not once. The PR documents CurrentDifficulty (r:1 w:1) but it's actually (r:2 w:1) -- once in on_finalize at line 130 and again in adjust_difficulty at line 189. The errors happen to cancel out for reads (phantom TotalWork +1 and missing CurrentDifficulty -1 = net zero), so 7 reads is accidentally correct. But the documentation is wrong.

3. Write count is 5 but should be 4. The phantom TotalWork write inflates the count by one.

4. Execution time not re-benchmarked. The PR keeps Weight::from_parts(217_000_000, 23445) from the old benchmark run. The PR author acknowledges this. The proper fix would re-run frame-omni-bencher.

---

3. Security assessment

Low risk, but worth noting:

• The PR reduces declared weight (from 16r/9w to 7r/5w). Under-declaring weight is the dangerous direction -- it could allow blocks to be heavier than declared, a DoS vector. However, since the actual operations are ~7r/4w, even the PR's proposed 7r/5w is still slightly over-counted (conservative), so no immediate security risk.

• The benchmark change (adding <BlockTimeEma<T>>::put(...)) is correct and important -- without it, the benchmark hits the if current_ema == 0 early-return path in update_block_time_ema, which skips the EMA calculation. This means old benchmarks were measuring a shorter code path than production actually takes.

• No new logic, no new storage items, no extrinsic changes -- attack surface is unchanged.

---

4. Recommendation

The PR is directionally correct but has inaccuracies. I'd suggest:

1. Thank the contributor -- they found a real issue flagged by your own TODO
2. Request corrections: remove TotalWork from docs, fix CurrentDifficulty to (r:2 w:1), change writes from 5 to 4
3. Ideally re-run benchmarks with frame-omni-bencher to get fresh execution time / proof size numbers, or ask the contributor to do so
4. Then it's a clean merge

[AdamMomen]

Fixed:

• Removed phantom TotalWork from storage doc comments
• CurrentDifficulty updated to (r:2 w:1) per code analysis
• Writes corrected to 4
• Code formatting

[illuzen]

Thanks for taking care of that todo! Nice PR @AdamMomen !