Add special case to handle validator manager update

[mateuszsikora]

No description provided.

[coderabbitai]

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

📝 Walkthrough

Summary by CodeRabbit

• Bug Fixes

  • Resolved an edge-case during upgrades between versions 0.7.0 and 0.7.1+ that could double-count resources and misreport statistics.
  • Ensures the correct validators manager is applied after transitions, preventing inconsistencies in accumulated state.
  • Improves reliability of parallel accumulation paths during version transitions.

• Other

  • No changes to public APIs.

Walkthrough

Adds version-gated corrective logic in accumulate.ts to handle validatorsManager during transitions between GpVersion 0.7.0 and 0.7.1+. When the current manager changes but remains in the service list, the code re-accumulates the new validatorsManager and updates privilegedServices to avoid double-counting. Duplicated handling exists in parallel accumulation paths.

Changes

Cohort / File(s)	Summary of Change
Transition accumulation repair (validatorsManager) packages/jam/transition/accumulate/accumulate.ts	Introduces conditional re-accumulation for validatorsManager when upgrading from 0.7.0 to 0.7.1+, deriving currentManager, checking compatibility, re-running accumulation for the new validatorsManager, and correcting privilegedServices.validatorsManager. Mirrors this fix in both parallel-accumulation branches; adds imports for PrivilegedServices and version utilities.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Caller
  participant Accumulator
  participant InputState
  participant CurrentState
  participant Services
  participant Compat as Compatibility

  Caller->>Accumulator: accumulate(updatedState, services)
  Accumulator->>CurrentState: Read privilegedServices.validatorsManager (currentManager)
  Accumulator->>InputState: Read privilegedServices.validatorsManager (inputManager)
  Accumulator->>Compat: isTransition(0.7.0 -> 0.7.1+ )?
  Compat-->>Accumulator: result

  alt Version-compatible repair needed
    Accumulator->>Services: accumulate(all services)  Note right of Services: Standard pass
    Accumulator->>Accumulator: Detect new validatorsManager present in updated state and services
    Accumulator->>Services: Re-accumulate(new validatorsManager)  Note right of Services: Prevent double-counting
    Accumulator->>CurrentState: Update privilegedServices.validatorsManager to new manager
  else Normal path
    Accumulator->>Services: accumulate(all services)  Note right of Services: No special handling
  end

  Note over Accumulator,Services: The repair logic is duplicated in parallel accumulation branches
  Accumulator-->>Caller: final accumulated state

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• Avoid misusing state update by copying the services state. #548 — Adjusts accumulation around privilegedServices/validatorsData, similar to this PR’s validatorsManager state handling.

Poem

I hop through versions, 0.7’s glade,
Counting carrots once—no double-paid.
A manager new, a manager old,
I nudge the ledger to truth be told.
With parallel paws and careful care,
One final state—precise and fair. 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description Check	❓ Inconclusive	No pull request description was provided, so there is insufficient information in the PR itself to confirm the author's intent or rationale and the check is therefore inconclusive.	Please add a brief description stating why the special-case was added, the expected behavioral change (including affected GpVersion ranges), and any test or migration notes so reviewers can assess impact quickly.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title accurately and concisely summarizes the primary change: adding a special-case to handle validator manager updates during accumulation, which matches the changes implementing version-specific re-accumulation and privilegedServices adjustments.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

packages/jam/transition/accumulate/accumulate.ts (2)

305-308: Name this initialManager for clarity

This is the manager at the start of the parallel pass; it doesn’t reflect any mid-loop updates. Renaming avoids confusion with “current” state that is being mutated below.

-    const currentManager = (inputStateUpdate.privilegedServices ?? this.state.privilegedServices).manager;
+    const initialManager = (inputStateUpdate.privilegedServices ?? this.state.privilegedServices).manager;

And update the reference in the condition below:

-        serviceId === currentManager
+        serviceId === initialManager

---

328-357: Tighten 0.7.0-only patch, fix typos, and avoid unnecessary re-accumulation

• Use Compatibility.is(V0_7_0) for precision/readability.
• Guard against newV === serviceId to skip redundant self re-accumulation.
• Use != null to accept both null/undefined.
• Reword and lower the log level; current message has typos and is alarming.

-      if (
-        Compatibility.isLessThan(GpVersion.V0_7_1) &&
-        Compatibility.isGreaterOrEqual(GpVersion.V0_7_0) &&
-        serviceId === currentManager
-      ) {
+      if (Compatibility.is(GpVersion.V0_7_0) && serviceId === currentManager) {
         const newV = currentState.privilegedServices?.validatorsManager;
-        if (currentState.privilegedServices !== null && newV !== undefined && serviceIds.includes(newV)) {
-          logger.info(
-            "Entering completly incorrect code that problably reverts validatorsManager change. This is valid in 0.7.0 only and incorrect in 0.7.1+",
-          );
+        if (currentState.privilegedServices != null && newV !== undefined && newV !== serviceId && serviceIds.includes(newV)) {
+          logger.debug(
+            "Applying 0.7.0 compatibility patch: re-accumulating validatorsManager; must not run on >=0.7.1",
+          );
           // Since serviceIds already contains newV, this service gets accumulated twice.
           // To avoid double-counting, we skip stats and gas cost tracking here.
           // We need this accumulation to get the correct `validatorsManager`
           const { stateUpdate } = await this.accumulateSingleService(
             newV,
             accumulateData.getOperands(newV),
             accumulateData.getGasCost(newV),
             slot,
             entropy,
             checkpoint,
           );

           const correctV =
             stateUpdate?.privilegedServices?.validatorsManager ?? this.state.privilegedServices.validatorsManager;
           currentState.privilegedServices = PrivilegedServices.create({
             ...currentState.privilegedServices,
             validatorsManager: correctV,
           });
         }
       }

Also, confirm the intent to ignore gas/stats from this corrective re-accumulation (it’s correct, just worth an inline comment clarifying it’s intentional).

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 135f62f and 223c323.

📒 Files selected for processing (1)

• packages/jam/transition/accumulate/accumulate.ts (3 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.ts

⚙️ CodeRabbit configuration file

**/*.ts: as conversions must not be used. Suggest using tryAs conversion methods.

**/*.ts: Classes with static Codec field must have private constructor and static create method.

**/*.ts: Casting a bigint (or U64) using Number(x) must have an explanation comment why
it is safe.

**/*.ts: When making changes to code with comments containing links (in classes, constants, methods, etc.)
to graypaper.fluffylabs.dev, ensure those links point to the current version for this update.

Files:

• packages/jam/transition/accumulate/accumulate.ts

🧬 Code graph analysis (1)

packages/jam/transition/accumulate/accumulate.ts (3)

packages/jam/block/common.ts (2)

• ServiceGas (31-31)
• tryAsServiceGas (32-32)

packages/core/utils/compatibility.ts (1)

• Compatibility (45-104)

packages/jam/state/privileged-services.ts (1)

• PrivilegedServices (27-54)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)

• GitHub Check: run (22.x)
• GitHub Check: state_transition (22.x)
• GitHub Check: benchmarks (22.x)
• GitHub Check: run (22.x)
• GitHub Check: test (22.x)
• GitHub Check: e2e (22.x)

🔇 Additional comments (2)

packages/jam/transition/accumulate/accumulate.ts (2)

28-32: Import PrivilegedServices looks correct

No issues with bringing PrivilegedServices into scope for state updates.

---

34-34: Compatibility/GpVersion import is appropriate

Needed for the 0.7.0-specific guard.

[tomusdrw]

lgtm!

[github-actions]

View all

File	Benchmark	Ops
math/mul_overflow.ts[0]	multiply and bring back to u32	265447053.76 ±6.18%	fastest ✅
math/mul_overflow.ts[1]	multiply and take modulus	260024089.86 ±7.05%	2.04% slower
codec/decoding.ts[0]	manual decode	23489613.73 ±0.6%	86.38% slower
codec/decoding.ts[1]	int32array decode	172418829.29 ±2.66%	fastest ✅
codec/decoding.ts[2]	dataview decode	170013470.02 ±2.25%	1.4% slower
codec/encoding.ts[0]	manual encode	3435980.21 ±0.13%	18.62% slower
codec/encoding.ts[1]	int32array encode	4222255.95 ±0.26%	fastest ✅
codec/encoding.ts[2]	dataview encode	4216358.77 ±0.24%	0.14% slower
math/switch.ts[0]	switch	269420536.99 ±6.06%	fastest ✅
math/switch.ts[1]	if	262746018.99 ±6.07%	2.48% slower
math/count-bits-u64.ts[0]	standard method	1553567.7 ±0.2%	86.36% slower
math/count-bits-u64.ts[1]	magic	11387666.74 ±0.31%	fastest ✅
collections/map-set.ts[0]	2 gets + conditional set	119303.63 ±0.06%	fastest ✅
collections/map-set.ts[1]	1 get 1 set	61788.22 ±0%	48.21% slower
bytes/hex-from.ts[0]	parse hex using Number with NaN checking	142044.1 ±0.1%	84.36% slower
bytes/hex-from.ts[1]	parse hex from char codes	907927.51 ±0.07%	fastest ✅
bytes/hex-from.ts[2]	parse hex from string nibbles	565552.22 ±0.2%	37.71% slower
bytes/hex-to.ts[0]	number toString + padding	332461.51 ±0.23%	fastest ✅
bytes/hex-to.ts[1]	manual	16757.65 ±0.04%	94.96% slower
codec/bigint.compare.ts[0]	compare custom	274734857.74 ±4.53%	fastest ✅
codec/bigint.compare.ts[1]	compare bigint	269582399.44 ±4.72%	1.88% slower
codec/bigint.decode.ts[0]	decode custom	176873694.13 ±3.1%	fastest ✅
codec/bigint.decode.ts[1]	decode bigint	111326763.25 ±1.92%	37.06% slower
hash/index.ts[0]	hash with numeric representation	191.83 ±0.31%	27.07% slower
hash/index.ts[1]	hash with string representation	122.13 ±0.23%	53.57% slower
hash/index.ts[2]	hash with symbol representation	184.64 ±0.07%	29.81% slower
hash/index.ts[3]	hash with uint8 representation	163.48 ±0.25%	37.85% slower
hash/index.ts[4]	hash with packed representation	263.04 ±0.68%	fastest ✅
hash/index.ts[5]	hash with bigint representation	189.94 ±0.85%	27.79% slower
hash/index.ts[6]	hash with uint32 representation	204.53 ±0.33%	22.24% slower
math/add_one_overflow.ts[0]	add and take modulus	265202371.05 ±6.55%	fastest ✅
math/add_one_overflow.ts[1]	condition before calculation	261252820.37 ±6.19%	1.49% slower
math/count-bits-u32.ts[0]	standard method	92186952.51 ±2.08%	63.46% slower
math/count-bits-u32.ts[1]	magic	252320283.55 ±7.01%	fastest ✅
logger/index.ts[0]	console.log with string concat	7652857.32 ±46.87%	fastest ✅
logger/index.ts[1]	console.log with args	115358.58 ±103.06%	98.49% slower
codec/view_vs_collection.ts[0]	Get first element from Decoded	32613.92 ±0.28%	53.75% slower
codec/view_vs_collection.ts[1]	Get first element from View	70521.56 ±0.28%	fastest ✅
codec/view_vs_collection.ts[2]	Get 50th element from Decoded	32643.94 ±0.2%	53.71% slower
codec/view_vs_collection.ts[3]	Get 50th element from View	39283.07 ±0.16%	44.3% slower
codec/view_vs_collection.ts[4]	Get last element from Decoded	32621.57 ±0.24%	53.74% slower
codec/view_vs_collection.ts[5]	Get last element from View	27710.48 ±0.18%	60.71% slower
codec/view_vs_object.ts[0]	Get the first field from Decoded	524933.62 ±0.3%	fastest ✅
codec/view_vs_object.ts[1]	Get the first field from View	106136.06 ±0.15%	79.78% slower
codec/view_vs_object.ts[2]	Get the first field as view from View	106575.74 ±0.08%	79.7% slower
codec/view_vs_object.ts[3]	Get two fields from Decoded	517348.53 ±0.32%	1.44% slower
codec/view_vs_object.ts[4]	Get two fields from View	84953.47 ±0.12%	83.82% slower
codec/view_vs_object.ts[5]	Get two fields from materialized from View	179099.48 ±0.18%	65.88% slower
codec/view_vs_object.ts[6]	Get two fields as views from View	84565.03 ±0.1%	83.89% slower
codec/view_vs_object.ts[7]	Get only third field from Decoded	518451.64 ±0.34%	1.23% slower
codec/view_vs_object.ts[8]	Get only third field from View	105735.23 ±0.08%	79.86% slower
codec/view_vs_object.ts[9]	Get only third field as view from View	105682.06 ±0.1%	79.87% slower
bytes/compare.ts[0]	Comparing Uint32 bytes	20404.14 ±0.11%	3.7% slower
bytes/compare.ts[1]	Comparing raw bytes	21189.04 ±0.16%	fastest ✅
hash/blake2b.ts[0]	hasher with simple allocator	0.0461 ±0.4%	fastest ✅
hash/blake2b.ts[1]	hasher with page allocator	0.046 ±0.3%	0.22% slower
collections/map_vs_sorted.ts[0]	Map	329817.7 ±0.06%	fastest ✅
collections/map_vs_sorted.ts[1]	Map-array	103782.04 ±0.03%	68.53% slower
collections/map_vs_sorted.ts[2]	Array	69742.37 ±0.16%	78.85% slower
collections/map_vs_sorted.ts[3]	SortedArray	198122.37 ±0.26%	39.93% slower
crypto/ed25519.ts[0]	native crypto	5.27 ±19.42%	82.77% slower
crypto/ed25519.ts[1]	wasm lib	10.88 ±0.02%	64.42% slower
crypto/ed25519.ts[2]	wasm lib batch	30.58 ±0.53%	fastest ✅

Benchmarks summary: 63/63 OK ✅