EMA update wobble#564
Conversation
Analyzed three files, diff |
|
I found the authoritative sources on Ethereum's difficulty adjustment. The PR's claim of "Ethereum-style" is fundamentally misleading — Ethereum's algorithm differs from this PR's in critical ways that affect security. Authoritative sources on Ethereum's difficulty adjustment
What Ethereum actually does (from
|
| Single block time | adj_factor |
Effective change |
|---|---|---|
| 1–9 s | +1 | +0.0488% |
| 10–19 s | 0 | 0 |
| 20–29 s | −1 | −0.0488% |
| 30–39 s | −2 | −0.0977% |
| 100–109 s | −9 | −0.439% |
| 990 s+ | −99 (capped) | −4.83% |
So Ethereum's -99 cap only activates after a single block takes ~16.5 minutes. Normal slow blocks (e.g. 30 s when target is 13 s) move difficulty down by only ~0.1%.
What PR #564 actually does
raw_ratio = target_time / observed_ema_time
if raw_ratio > 1:
ratio = min(raw_ratio, 1 + 1/2048) // capped UP at +0.0488%
else:
ratio = max(raw_ratio, 1 - 5/100) // capped DOWN at −5%
new_diff = current_diff * ratio- Multiplicative, not additive
- Driven by an EMA (alpha = 1%, ~100-block window) rather than the latest block
- Max DOWN of −5% triggers as soon as the EMA exceeds target by 5.3% (e.g. 12.63 s when target is 12 s) — orders of magnitude easier than Ethereum's 16.5-minute threshold
- Max UP of +0.0488% triggers as soon as the EMA is below target by 0.05% (~6 ms on 12 s target)
Why "Ethereum params" is a misleading framing
| Property | Ethereum (Homestead/Byzantium) | This PR |
|---|---|---|
| Step size | parent/2048 (~0.049%) base step, scaled by signed integer adj_factor |
Asymmetric clamps: +1/2048 up, −5/100 down |
| Per-block max DOWN | −4.83%, but only at block_time ≥ 990 s | −5% at EMA only ~5% above target |
| Per-block max UP | +0.0488% on any sub-target single block | +0.0488% on any sub-target EMA |
| Input signal | Direct single-block time | Smoothed EMA |
| Down-direction trigger threshold | Catastrophic stall (single 16+ min block) | Routine variance |
| Geometry | Additive linear in block time | Multiplicative step function |
Ethereum's asymmetry is "in extreme catastrophe, drop fast; otherwise both directions move at ~0.05% × |delta| steps." This PR's asymmetry is "in routine slow patches, drop fast; in fast patches, always rise at the tiny floor."
The two are not equivalent — and the difference is exactly the source of the exploitable behavior I flagged earlier:
-
min_difficultyfloor trap:1000 × (1 + 1/2048) = 1000.488 → 1000under U512 truncation. Once difficulty hits the floor, the +1/2048 clamp cannot lift it by even 1 unit. (FixedU128 DIV = 10^18confirmed insp-arithmetic-28.0.1/src/fixed_point.rs:2412). Ethereum's additive formula has the sameparent/2048step butMinimumDifficulty = 131_072 = 2^17, well above the escape threshold; the PR's floor of1000is below the2048threshold needed to escape. Theget_min_difficulty()comment ("1000 is safe for clamp values ≥ 0.01%") is mathematically wrong: the relevant constraint ismin × up_clamp ≥ 1, which requiresmin ≥ 2048for a 1/2048 clamp. -
Asymmetric hashrate-response window: To double difficulty in response to a 2× hashrate spike —
ln(2)/ln(1.000488) ≈ 1420 blocks ≈ 4.7 hoursat the 12 s target. Ethereum's equivalent only allows the same +0.0488% per block, but block time is the direct input, so when a 2× hashrate spike arrives blocks come in ~6 s and Ethereum gets +0.0488% per block; the time to catch up is the same (Ethereum has the same hashrate-bandit weakness, which is one reason Ethereum left PoW). What's new here is that the EMA dampens the signal further, slowing response. -
Routine-variance drift: With EMA σ ≈ 7.1% of mean (alpha = 1%, single-block CV = 1), the EMA is above
1.053 × target~23% of the time at equilibrium; each such block snaps difficulty down by 5%, while compensating upward steps are limited to 0.05%. The equilibrium is not at "correct" difficulty — it's biased low.
Closer reference points for what was probably intended
If the goal was Ethereum-style asymmetry, the closer port would be additive with a single divisor, signed integer multiplier, and a −99 lower cap on the multiplier:
# additive form, like EIP-2
step = current_difficulty / 2048
adj_factor = clamp(integer_function_of(observed_block_time, target), -99, +1)
new_difficulty = current_difficulty + step * adj_factorNotably, EIP-2 §Rationale explicitly argues for using the block time directly (not a moving average) so that a single delayed block doesn't keep penalizing future blocks via a slow-decaying EMA. The PR's EMA-then-clamp construction is what produces the "drop 5% every block for ~100 blocks after a single outage" behavior.
Want me to write up a corrected implementation, or stop at the review and let the PR author redesign?
Review of PR #564 — "EMA update wobble"Scope: security and soundness of the new asymmetric difficulty-clamp + EMA retune. TL;DR — exploitable / soundness findings
1. Critical —
|
| Property | Ethereum (EIP-2 / EIP-100) | PR #564 |
|---|---|---|
| Form | Additive: parent + parent/2048 × adj |
Multiplicative: parent × ratio |
| Step size | One unit = parent / 2048 for both directions |
Up step ≤ parent × 1/2048, down step ≤ parent × 5/100 |
| Signal | The actual single-block time | EMA of recent block times |
| Up cap | adj = +1 on any sub-10 s block |
+1/2048 whenever raw_ratio > 1.000488 |
| Down cap | adj = −99 only at block_time ≥ 990 s |
−5% whenever EMA > 1.053 × target |
| Max DOWN trigger | A single 16.5-minute block | EMA drifts 5% above target |
So the 1/2048 in Ethereum is the base unit step that is then multiplied by a signed integer adjustment factor — it is not a percentage cap on the upward direction. And the −99 cap corresponds to a catastrophic single-block stall, not the moderate slow-patch this PR uses as the trigger.
Two illustrative slow-block cases:
| Single block time / EMA, target = 12 s | Ethereum adj | This PR (raw_ratio mode) |
|---|---|---|
| 18 s | 1 - 1 = 0 → 0% change |
EMA ≈ 18 s → ratio = 0.667, clamped to −5% |
| 30 s | 1 - 3 = -2 → −0.097% |
EMA ≈ 30 s → ratio = 0.4, clamped to −5% |
| 5 s | 1 - 0 = +1 → +0.0488% |
EMA ≈ 5 s → ratio = 2.4, clamped to +0.0488% |
The PR's controller treats a routine 30 s block (about 2.5× target) the same as a 5-hour stall; both clamp to −5%. Ethereum treats them very differently (−0.097% vs. −4.83%).
Why this matters
Because the down-clamp triggers on routine deviations and the up-clamp's threshold is essentially zero (only 0.05% below target needed), the EMA spends most of its time in one of the two clamped regimes. In a stochastic equilibrium with σ(EMA)/μ ≈ √(α/(2-α)) ≈ 7.1% (for α = 1% and exponential block-time variance):
- P(EMA > 1.053 × target) ≈ Φ(−0.74) ≈ 23% — clamped −5%
- P(0 < eps < 0.053) ≈ 27% — linear small drop, average ≈ −2.5%
- P(EMA < target) ≈ 50% — almost all clamped at +0.0488%
Expected per-block change at "correct" difficulty:
E[Δ] ≈ 0.23 × (−0.05) + 0.27 × (−0.025) + 0.50 × (+0.0005)
≈ −0.0115 − 0.0068 + 0.0002
≈ −1.8% per block
This is uncompensated downward drift. The controller's stable point is below the correct difficulty, which means block times average below target at equilibrium — the opposite of the desired smoothing. The PR's screenshot shows oscillation amplitude shrinking in a short window with a GPU added/removed; it does not demonstrate equilibrium stability over the >100-block EMA window.
References
- EIP-2 (Homestead)
- EIP-100 (Byzantium)
- go-ethereum
consensus.go—calcDifficultyHomestead(lines 415–462) - ethereum.stackexchange — How does Homestead difficulty work?
- EIP-2 §Rationale explicitly argues against using a moving average for this exact reason — a single anomalous block shouldn't keep penalising the chain through a slow-decaying EMA.
3. High — asymmetric response makes hashrate-bandit attacks ~100× more profitable
To restore difficulty after a hashrate surge of k× requires roughly ln(k) / ln(1 + 0.000488) blocks of binding up-clamp:
| Hashrate spike | Blocks to catch up | Wall-clock at 12 s target |
|---|---|---|
| 2× | ~1 420 | ~4.7 hours |
| 5× | ~3 295 | ~11 hours |
| 10× | ~4 716 | ~15.7 hours |
Compare with the previous symmetric 10% clamp: ln(10)/ln(1.1) ≈ 24 blocks ≈ 4.8 minutes to track a 10× spike. The new tuning is roughly 200× slower at responding to upward hashrate shocks.
This is a classic coin-hopping / hashrate-bandit setup: a multi-pool attacker rotates a large rented hashrate on for a few hours, mines a windfall of cheap blocks, and rotates off before the controller catches up. The 5% down-clamp then snaps difficulty back down quickly — making the attacker's next visit equally profitable.
4. High — slowed EMA amplifies the down-clamp trap
EmaAlpha changed from 100 → 10 (out of 1000), so the latest block now contributes 1% (was 10%). Effective half-life is ~69 blocks (was ~7 blocks).
The combined behavior:
- Some triggering event (validator nap, GPU offline, network split) makes a chunk of blocks slow.
- EMA climbs and stays elevated for ~100 blocks even after blocks return to normal.
- While EMA > 1.053 × target, every block decrements difficulty by 5%.
- Recovery upward is limited to +0.0488% per block.
A back-of-envelope check: a sustained outage producing blocks 5× target time for 1 hour (~600 s actual blocks ⇒ 6 blocks) is mild. But a 3-hour outage producing blocks at 10× target time → ~90 actual blocks. Each saw −5%. Net: 0.95^90 ≈ 0.99%. Starting from 2.7M, that lands at ~26 760 — recoverable but only at +0.0488%/block, which is ~9 600 blocks (~32 h at 12 s target) of recovery. One ~6-hour outage and the floor trap from §1 is reached.
5. Medium — tests don't exercise the production controller
pallets/qpow/src/mock.rs:
pub const TestDifficultyIncreaseClamp: FixedU128 = FixedU128::from_rational(3, 100);
pub const TestDifficultyDecreaseClamp: FixedU128 = FixedU128::from_rational(10, 100);The mock uses a 3% / 10% pair while the runtime uses 1/2048 (~0.05%) / 5%. The mock numbers are ~60× larger on the up side and 2× larger on the down side. Consequences:
test_min_difficulty_can_increasepasses with1000 × 1.03 = 1030(mock), but fails with1000 × 1.000488 = 1000(runtime). The floor-trap bug from §1 is not caught by any test.- The comment in
test_min_difficulty_can_increasestill reads:// Fast blocks → ratio clamped to 1.1 → floor(1000 * 1.1) = 1100, which corresponds to neither the old 10% clamp nor the new 3% mock. It's left over from a prior parameter set. test_difficulty_recovers_after_sleepusesalpha = 500(50%) which is wildly different from the runtime'salpha = 10(1%). It is asserting against numbers that are 50× more responsive than production.
The mock should be aligned with the runtime parameters (or the runtime parameters tested directly under #[cfg(test)] integration tests), and there should be a regression test that asserts:
let lifted = QPow::calculate_difficulty(QPow::get_min_difficulty(), 1, target);
assert!(lifted > QPow::get_min_difficulty());— which would fail today against the runtime config.
6. Medium — docs/comments drift
- New doc comments on the
Configitems give3/100and10/100as illustrative values; runtime uses1/2048and5/100. Pick one and be consistent, or drop the inline percentages. get_min_difficulty's comment is mathematically wrong for the new clamp (see §1).- The PR removed the "low value for development" hint on
InitialDifficultyrather than replacing it with a justification for2_700_000.
7. Low — bias from FixedU128::from_rational rounding
from_rational in sp_arithmetic::FixedU128 rounds down by default. So:
- When
target > observed(up direction),raw_ratiois slightly under-stated, costing fractional bps on every adjustment. - When
target < observed(down direction),raw_ratiois also under-stated, which makes the down move slightly larger.
This is fourth-order compared to §1–§4, but it's directional bias on top of an already-biased controller. Consider FixedU128::from_rational_with_rounding(.., NearestPrefDown) or computing target - observed in the right direction.
8. Suggested concrete revisions
- Block merging until §1 is fixed. This is a clean chain-kill.
- Either:
- Raise
get_min_difficulty()to at least2 / up_clamp(≥ 4096 for1/2048), and derive it fromT::DifficultyIncreaseClampinstead of hard-coding, or - Move to an additive Ethereum-style update (
diff + diff/2048 × adj_factor) where the floor is naturally escapable.
- Raise
- If the goal is genuinely "Ethereum's params":
- Use the additive formula in EIP-2 / EIP-100.
- Drive
adj_factorfrom the single block's time, not the EMA — see EIP-2 §Rationale. If EMA smoothing is desired, do it on the difficulty target itself (i.e. EMA of computed adjustments), not on the input signal. - Match
MinimumDifficulty = 131_072(Ethereum's value) or equivalent — well above the 2048 escape threshold.
- Align
pallets/qpow/src/mock.rswith the runtime params so unit tests actually exercise the production controller; add a regression test for the floor case (calculate_difficulty(min, 1, target) > min). - Tighten the upward clamp threshold asymmetry as well, not just the cap: today the up-clamp binds for any EMA below target by more than
0.05%, throwing away almost all signal. Consider making the up and down clamp triggers symmetric in|delta|even if the caps differ. - Document the choice of
InitialDifficulty = 2_700_000(or revert it — it's an orthogonal change that shouldn't ride a controller-tuning PR).
Appendix — numerical sanity check (Rust pseudo-snippet)
use sp_arithmetic::FixedU128;
use sp_core::U512;
let one_inner: u128 = FixedU128::one().into_inner(); // 1e18
let up_inner: u128 = FixedU128::from_rational(1, 2048).into_inner();
let ratio_inner = one_inner + up_inner; // ~1.000488e18
let current = U512::from(1000u64);
let numerator = current * U512::from(ratio_inner); // 1.000488e21
let adjusted = numerator / U512::from(one_inner); // = 1000
assert_eq!(adjusted, U512::from(1000u64)); // ← floor trap|
omg this review is quite hysterical, yes it's not exactly like ethereum's but it has the same clamps. the supporting links it gives are completely irrelevant and it's analysis is bogus. the min_difficulty thing is correct tho, so will fix that. |
…ants - Warn on raw-result clipping to floor/ceiling so a stuck controller is diagnosable from logs alone. - Add proptest invariants for the additive controller: bounds, monotone-in-block_time, no-change band flatness, step magnitude. - Add runtime-parameter integration test asserting the actual quantus_runtime constants form a coherent EIP-2 configuration — catches the mock/runtime drift that hid the floor trap in #564. - Remove dead TooFarInFuture variant; the 60s drift bound is intentionally enforced via pallet_timestamp::check_inherents.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit d08ec77. Configure here.
|
ok let's just go full ethereum on it |
|
I've done a deep dive on the rewritten PR. Here's a comprehensive review covering all the discussion threads and the new "all-Ethereum" code at commit Overall verdictThe pivot is a huge improvement and the core algorithm is now genuinely EIP-2 Homestead style (additive, single-block driven, with
So the chain-killing stuff is gone. What's left is calibration, cleanup, and one migration concern. Things that still need attention1. Bucket ratio is
|
Re-review of PR #564 at
|
| # | Nit | Status | Where |
|---|---|---|---|
| 1 | Bucket ratio target * 5 / 6 → target * 2 / 3 |
Fixed | pallets/qpow/src/lib.rs:211 |
| 2 | Stale EMA mention in on_finalize doc |
Fixed | pallets/qpow/src/lib.rs:110 |
| 3 | Stale EMA mention in on_finalize benchmark |
Fixed | pallets/qpow/src/benchmarking.rs:18 |
| 4 | Stale "ratio clamped to 1.1/0.9" / "EMA alpha=500" comments in tests | Fixed | pallets/qpow/src/tests.rs:333,367,382 |
| 5 | weights.rs BlockTimeEma storage entries (and read/write counts) |
Fixed | pallets/qpow/src/weights.rs (reads 5→4, writes 4→3) |
| 6 | Grafana block_time_ema panels |
Fixed | both overview.json and quantus-business.json now reference last_block_duration |
Math spot-check on d563a32
/// Calculate new difficulty based on block time.
/// Uses the same formula as Ethereum PoW:
/// diff = parent_diff + (parent_diff / 2048) * max(1 - block_time / divisor, -99)
///
/// The divisor is 10 seconds for a 12s target (scales proportionally).
/// This creates these zones:
/// - < divisor: difficulty increases by 1/2048 (~0.05%)
/// - divisor to 2*divisor: no change
/// - 2*divisor to 3*divisor: difficulty decreases by 1/2048
/// - etc, up to max decrease of 99/2048 (~4.8%)
pub fn calculate_difficulty(
parent_difficulty: U512,
block_time_ms: u64,
target_time_ms: u64,
) -> U512 {
log::debug!(target: "qpow", "📊 Calculating new difficulty ---------------------------------------------");
// Divisor scales with target: 8s divisor for 12s target
// divisor = target * 8 / 12 = target * 2 / 3
let divisor_ms = (target_time_ms * 2 / 3).max(1);
let time_factor = (block_time_ms / divisor_ms) as i64;
let adjustment = core::cmp::max(1i64 - time_factor, -99i64);
- Equilibrium block time =
divisor / ln 2 = 8 / ln 2 ≈ 11.54 sfor the 12 s target — target sits near the middle of the[8s, 16s)no-change band. ✓ test_min_difficulty_can_increase:calculate_difficulty(131_072, 1, 1000)→ increment =131_072/2048 = 64, new =131_136 > min. Floor is escapable. ✓- Sleep-recovery test: 1 h gap → adjustment =
-99→951_688(≈ 95 % of pre-sleep, well above thepre_sleep / 10assertion). ✓
Remaining nits (not blocking, single-line fixes if you care)
- Stale doc-comment on
calculate_difficulty: still says "The divisor is 10 seconds for a 12s target" — should be 8 seconds now. The inline comment two lines below is already correct. One word fix. QPoWInitialDifficultybump2_700_000→4_000_000still has no in-code justification (calibration source / target hashrate). Either drop a one-line comment or split into a separate calibration PR.- Branch is 2 commits behind main (
f997cb0eadd wormhole doc,e661b3cefast-governance feature flip). The unrelateddocs/wormhole-zk.md,runtime/Cargo.toml,.github/workflows/ci.yml, andruntime/src/governance/definitions.rslines in the PR diff will disappear on rebase — worth rebasing so the diff is purely the difficulty rework.
Items intentionally skipped (and that's fine)
- The runtime-parameter integration test (item 7 of my previous review) is now much less important —
get_min_difficulty()is a hardcoded constant and the divisor formula scales proportionally, so mock tests now exercise the same code path as the runtime. The original motivation was that the mock used wildly different clamps from production; that no longer applies.
Verdict
LGTM modulo the three minor nits above. The chain-killing floor trap is gone, the algorithm is genuinely EIP-2 Homestead, and the housekeeping (Grafana, weights, docs, benchmarks) is consistent. Happy to approve once the stale "10 seconds" doc-comment is fixed and the branch is rebased.

The difficulty was wobbling quite violently, this changes the ema parameters to be similar to Ethereum
In tests this smoothed the difficulty tremendously, with less than 20% oscillation even when adding and removing a gpu:
Note
High Risk
High risk because it changes the core PoW difficulty adjustment algorithm and related runtime/storage API, which can materially impact block production stability and network consensus behavior.
Overview
Reworks QPoW difficulty adjustment by removing EMA-based tracking/clamping and switching
pallet_qpow::calculate_difficultyto an Ethereum-style per-block adjustment using last block duration, with updated min difficulty floor and new runtimeInitialDifficulty.Removes the
BlockTimeEmastorage/runtime API surface and related config constants, updates benchmarks/tests/weights accordingly, and updates Prometheus/Grafana to stop emitting/graphingblock_time_emaand instead showlast_block_duration.Reviewed by Cursor Bugbot for commit d563a32. Bugbot is set up for automated code reviews on this repo. Configure here.