Skip to content

test(ethexe-runtime-common): repro for stale region hash on full-region clear (auto-tester) — refs #5373#5528

Draft
grishasobol wants to merge 1 commit into
masterfrom
auto-tester/ethexe-runtime-common-a07d9f041168
Draft

test(ethexe-runtime-common): repro for stale region hash on full-region clear (auto-tester) — refs #5373#5528
grishasobol wants to merge 1 commit into
masterfrom
auto-tester/ethexe-runtime-common-a07d9f041168

Conversation

@grishasobol

Copy link
Copy Markdown
Member

MemoryPages::remove_and_store_regions leaves a stale region hash in self[region_idx] when removing the last page in a region. The aggregate MemoryPages hash does not change after the removal, despite the region being observationally empty.

Scenario: invalid_combination
Hash: a07d9f041168
Unit: ethexe-runtime-common
Tracks: closed issue #5373 "Update of memory pages keeps hash of empty region" — the // TODO #5373 marker is still present in the source.

Observed

assertion left != right failed: MemoryPages hash must change when the last page in a region is removed
  left:  Some(HashOf<MemoryPages>(0x4aa82a5799b2fbc2ff783c644209c6c8180f6e61a283cc746fa05ba034efc4b3))
  right: Some(HashOf<MemoryPages>(0x4aa82a5799b2fbc2ff783c644209c6c8180f6e61a283cc746fa05ba034efc4b3))

Reproduces 3/3 deterministically.

Rubric items satisfied

  • (a) Contradicts a documented invariant// TODO #5373 at ethexe/runtime/common/src/state.rs:1058 explicitly marks this code as broken; closed issue #5373 documents the invariant: "Update of memory pages keeps hash of empty region" must NOT happen.
  • (b) Violates a state-mutation identity — store(state) is deterministic in state; removing the only page must change state; therefore hash(store(state_after_remove)) ≠ hash(store(state_before_remove)). Observed: equal hashes.

Root cause

// ethexe/runtime/common/src/state.rs:1057-1062
for (region_idx, region) in updated_regions {
    // TODO #5373
    if let Some(region_hash) = region.store(storage).to_inner() {
        self[region_idx] = region_hash.into();
    }
}

region.store(storage) returns MaybeHashOf::empty() for an empty region (no pages left after removal). .to_inner() then returns None, so the if-let block does not execute and self[region_idx] retains the OLD (non-empty) region hash from before the removal.

Suggested fix

Replace the if-let with an unconditional update that handles both branches:

for (region_idx, region) in updated_regions {
    self[region_idx] = region.store(storage);
}

This requires self to hold MaybeHashOf<Region> per slot (or an analogous "empty" sentinel) so an empty region is representable. Currently self[region_idx] is HashOf<...>, so a one-line replacement is not enough — the bug is correctly tagged with the TODO because a small refactor of the index type is needed.

Note: closed issue #5373 already proposes this work. This PR provides a failing test that fixes a closed-but-not-implemented issue should be marked with.

Practical reachability

Anywhere pallet-gear-style page lifecycle clears regions: gr_exit, terminated programs, lazy-page release on full-region eviction. State-hash mismatch across nodes is consensus-critical for ethexe — nodes that handle this code path differently (e.g. different versions during an upgrade) would diverge at commitment time.

Test source

// auto_tester_a07d9f041168
// scenario: invalid_combination
// param_signature: remove_and_store_regions stale region hash on full region clear

// Reproduces the bug tracked by `// TODO #5373` at ethexe/runtime/common/src/state.rs:1057-1062.
// Closed upstream issue: https://github.com/gear-tech/gear/issues/5373
//
// When `remove_and_store_regions` empties a region (last page removed),
// `region.store(storage)` returns `MaybeHashOf::empty()`. The if-let in the
// update loop then never fires, so `self[region_idx]` keeps its old
// (non-empty) hash — the MemoryPages aggregate hash is stale.
//
// Expected: removing the only page in a region must change the MemoryPages
// hash (the region is no longer represented by its old content hash).
// Observed: hash before == hash after.

use ethexe_runtime_common::state::{MemStorage, MemoryPages, Storage};
use gear_core::{memory::PageBuf, pages::GearPage};
use std::collections::BTreeMap;

#[test]
fn remove_last_page_in_region_must_clear_region_hash() {
    let storage = MemStorage::default();
    let mut pages = MemoryPages::default();

    // Add one page to region 0.
    let page0 = GearPage::from(0u16);
    let buf = PageBuf::new_zeroed();
    let page_hash = storage.write_page_data(buf);

    let mut page_map = BTreeMap::new();
    page_map.insert(page0, page_hash);
    pages.update_and_store_regions(&storage, page_map);

    let hash_with_page = pages.clone().store(&storage).to_inner();

    // Remove the only page — region 0 is now empty.
    pages.remove_and_store_regions(&storage, &vec![page0]);

    let hash_after_remove = pages.clone().store(&storage).to_inner();

    // The aggregate hash MUST change: an empty region is observationally
    // different from a region holding one page.
    assert_ne!(
        hash_with_page, hash_after_remove,
        "MemoryPages hash must change when the last page in a region is removed; \
         see TODO #5373 in state.rs:1057-1062 (the if-let skips empty regions \
         and leaves a stale region hash in self[region_idx])"
    );
}

Generated by /gear-dev:tester (auto-tester). This is a draft PR — requires human review before merge. Closed issue #5373 may need reopening since the TODO is still in source.

…es remove_and_store_regions (auto-tester) a07d9f041168 — refs #5373

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant