chore: fix unnecessary and inconsistent side-effect counter increments

[nventuro]

The extra increment had no real consequences, but it made the core mode confusing. I adjusted the log to more clearly show what's been done.

I also removed a TODO that was both stale and incorrect.

[benesjan]

• chore: fix unnecessary and inconsistent side-effect counter increments #22245 👈 (View in Graphite)
• merge-train/fairies

This stack of pull requests is managed by Graphite. Learn more about stacking.

[benesjan]

@nventuro I had to do this fix because it broke this test.

Posting screenshot in case the CI run log disappeared:

The issue there was that next_counter function increments the counter but it returns the non-incremented one which then results in the breaking of non-revertible counter being always smaller than revertible one invariant.

Not merging this as I think you should see the fix first.

[benesjan]

My "fix" broke it all and here is AI explanation why:

Why end_setup must bump the side-effect counter twice

The kernel enforces strict inequalities when validating the phase split of a private call. For a given call with min_revertible_side_effect_counter = M:

• Every non-revertible side effect counter (and any expected_non_revertible_side_effect_counter set via in_revertible_phase() queries) must be strictly less than M.
• Every revertible side effect counter (including the start_side_effect_counter of nested private call requests) must be strictly greater than M.
• M itself is added to the sorted side-effect array and must satisfy prev < M < next — i.e. M has to sit in a gap that no actual side effect occupies.

This means end_setup needs to do two things at once:

1. Ensure M is greater than the current counter (which may already equal the counter read by an earlier in_revertible_phase() call, since that function does not bump the counter).
2. Ensure the next side effect after end_setup gets a counter strictly greater than M.

A single increment can only satisfy one of these, not both:

Variant	min_rev	next side effect	issue
min_rev = next_counter() (counter: X → X+1)	X	X+1	expected_non_rev (X) < min_rev (X) fails — PXE unit test end_setup_checking_phases breaks
counter += 1; min_rev = counter (counter: X → X+1)	X+1	X+1	min_rev collides with next side effect counter — e2e validate_increasing_counters fails
counter += 1; min_rev = counter; counter += 1 (X → X+2)	X+1	X+2	✅ both invariants hold

The original pre-PR code (self.side_effect_counter += 1; self.min_revertible_side_effect_counter = self.next_counter();) was doing exactly the two-bump dance — the second increment wasn't an "extra" that could be cleaned up, it was load-bearing.

Fix

Restore the two-bump behavior in end_setup, written explicitly so the intent is obvious:

pub fn end_setup(&mut self) {
    // We bump the counter twice: once so that `min_revertible_side_effect_counter` sits strictly above any
    // non-revertible side effect counter (including queries made via `in_revertible_phase` before this call), and
    // once more so that the next revertible side effect counter is strictly greater than
    // `min_revertible_side_effect_counter`. This ensures `min_revertible_side_effect_counter` occupies a gap that
    // no side effect takes, which the kernel relies on when validating the phase split.
    self.side_effect_counter += 1;
    self.min_revertible_side_effect_counter = self.side_effect_counter;
    self.side_effect_counter += 1;
    ...
}

[AztecBot]

Flakey Tests

🤖 says: This CI run detected 1 tests that failed, but were tolerated due to a .test_patterns.yml entry.

\033FLAKED\033 (8;;http://ci.aztec-labs.com/27a86fa65269ec5a�27a86fa65269ec5a8;;�):  yarn-project/end-to-end/scripts/run_test.sh simple src/e2e_offchain_payment.test.ts (56s) (code: 0)

[AztecBot]

✅ Successfully backported to backport-to-v4-next-staging #22580.