Skip to content

fix(tests): fail loudly if cost model already updated#3545

Open
OlufemiAdeOlusile wants to merge 2 commits into
masterfrom
fail-loud-cost-model-not-updated
Open

fix(tests): fail loudly if cost model already updated#3545
OlufemiAdeOlusile wants to merge 2 commits into
masterfrom
fail-loud-cost-model-not-updated

Conversation

@OlufemiAdeOlusile

@OlufemiAdeOlusile OlufemiAdeOlusile commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Summary

The upgrade test is meant to prove three states: old protocol
version, new protocol version with cost model still short, new
protocol version with cost model fully updated. The middle state can
currently be skipped without anyone noticing, because
test_update_cost_models always proposed the given cost model
values, even when the live cost model was already at or above the
proposed length. That silently loses the "not yet updated" scenario
while still reporting a pass.

Fix

Added is_cost_model_update_needed() in conway_common.py, a
standalone check separate from update_cost_model(). It compares
live cost model length against a proposal file for a caller-given
set of model names, and returns whether an update is still needed.
update_cost_model() itself stays update-only, no gating logic
inside it.

test_update_cost_models (step2 of the upgrade test) now asserts an
update is needed before proposing one, and fails loudly with a clear
message if not, instead of silently re-proposing values that don't
change anything.

test_update_in_pv9 and the PlutusV3 builtins test already had
their own pre-checks before calling update_cost_model(), so they
don't need to change.

Fixture fix

cost_models_list_185_297_v2_v3.json targeted PlutusV3=297, stale
since cardonnay PR #148 (merged) bumped local_slow's Conway genesis
straight to the current mainnet value (350). Updated the fixture to
match.

Verification

  • ./ai_run.sh make lint clean
  • Ran the real end to end upgrade test (all 3 steps) with these
    changes. All green, zero failures. test_update_cost_models
    correctly found the cost model still genuinely short at that
    point, ran the real governance update, and passed.

@OlufemiAdeOlusile
OlufemiAdeOlusile force-pushed the fail-loud-cost-model-not-updated branch from 7c2d5a8 to 03047c2 Compare July 23, 2026 04:48
Add is_cost_model_update_needed() as a standalone check, separate
from update_cost_model(). It compares live cost model length against
a proposal file for a caller-given set of model names and returns
whether an update is still needed.

update_cost_model() stays update-only, no gating logic inside it.

test_update_cost_models (step2 of the upgrade test) is the only
caller that had no pre-check at all. It now asserts an update is
needed before proposing one, so it fails loudly instead of silently
re-proposing values that don't change anything, which would hide the
"cost model not yet updated" test scenario.

test_update_in_pv9 and the PlutusV3 builtins test already had their
own pre-checks before calling update_cost_model, so they're
unaffected.
@OlufemiAdeOlusile
OlufemiAdeOlusile force-pushed the fail-loud-cost-model-not-updated branch from 03047c2 to a92e42b Compare July 24, 2026 06:38
@OlufemiAdeOlusile
OlufemiAdeOlusile marked this pull request as ready for review July 24, 2026 06:38
cost_models_list_185_297_v2_v3.json targeted PlutusV3=297, stale
since cardonnay bumped local_slow's Conway genesis to the current
mainnet value (350) directly. Update the fixture to match.

test_update_cost_models (step2) is the step where this update is
expected to happen. If the cost model is already at the proposed
length going in, that means the "cost model not yet updated" test
scenario was missed, so fail loudly with a clear message instead of
silently letting it slide.

Verified against a real end to end upgrade test run: the cost model
was still genuinely short at this point, so the real governance
update ran and passed.
@OlufemiAdeOlusile
OlufemiAdeOlusile force-pushed the fail-loud-cost-model-not-updated branch from d87f414 to c4bdc7c Compare July 24, 2026 11:00
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