fix(proxy): bound the upgrade delay and announce every change

[kilodesodiq-arch]

Summary

Bounds the proxy's upgrade delay and emits an event on every change.

Closes #1170
Closes #1167
Closes #1168
Closes #1169

Why

set_upgrade_delay_blocks validated nothing but the admin. The delay is the safety model of the two-step upgrade, so an unbounded setter means a compromised or rogue admin can remove the timelock outright.

0 is the sharp end. set_implementation computes:

self.upgrade_effective_at = current_block + self.upgrade_delay_blocks;

and confirm_implementation only requires current_block >= upgrade_effective_at. With a delay of 0 both fit inside one block, so stage-then-confirm collapses into a single transaction and no observer gets a window in which to notice.

The overflow nobody was looking for

The issue frames the upper bound as "a huge value can brick future upgrades". Worth being precise about, because the actual behaviour is worse than that: a huge delay was never a safe way to freeze upgrades — it silently removed the timelock.

current_block + u64::MAX overflows. The wrapped result lands in the past, confirm_implementation's current_block < upgrade_effective_at check passes immediately, and the "delayed" upgrade is confirmable at once. So u64::MAX and 0 fail the same way, from opposite ends: both make the upgrade instantly confirmable. (Under a debug build the same addition panics instead.)

This is the concrete reason for the cap, and the reason set_implementation now uses saturating_add as well. A saturated sum leaves upgrade_effective_at at u64::MAX, so the upgrade stays unconfirmable — the safe direction to fail in, and the opposite of what wrapping does.

What changed

contracts/proxy/src/lib.rs

• Three constants, documented with the reasoning and the 6-second block time they assume:
  • DEFAULT_UPGRADE_DELAY_BLOCKS = 100 — identical to the literal the constructor hard-coded before, so a freshly deployed proxy behaves exactly as it did.
  • MIN_UPGRADE_DELAY_BLOCKS = 10 — ~1 minute. The floor below which the delay stops being a review window.
  • MAX_UPGRADE_DELAY_BLOCKS = 432_000 — 30 days, matching LOCK_PERIOD_30_DAYS in propchain-traits.
• ProxyError::InvalidUpgradeDelay, appended last so no existing discriminant moves.
• UpgradeDelayChanged { old_delay, new_delay, by }, with old_delay and new_delay as indexed topics and by in the payload.
• set_upgrade_delay_blocks now rejects anything outside the inclusive range, and emits the event on success only.
• set_implementation uses saturating_add.
• New read-only upgrade_delay_bounds() so a UI does not have to hard-code the numbers to validate a proposed value.

Ordering and scope

• ensure_admin() still runs first, so a non-admin cannot use the error to distinguish "out of range" from "out of range, but only for admins". There is a test pinning that.
• A rejected delay leaves storage untouched and emits nothing — a change that did not happen must not be announced as one.
• A pending upgrade's effective_at is computed at stage time and is deliberately left alone, so shortening the delay cannot pull an already-staged upgrade forward. Pinned by a test.

Tests

16 new tests, all in the existing inline module:

• 0 is rejected; every value in 1..MIN is rejected; MIN and MAX are accepted; MAX + 1, 1_000_000 and u64::MAX are rejected.
• The stored delay is unchanged after every rejection, including a rejected value between two accepted ones.
• The bounds are ordered, non-zero, and contain the constructor default.
• The overflow path is closed: u64::MAX is rejected, and after staging, upgrade_effective_at is still in the future.
• Staging with MAX gives exactly current + MAX with no wrap.
• A non-admin is rejected before the bounds are consulted.
• changing_the_delay_emits_an_event asserts one event, that both the old and new value appear among the topics, and that the payload decodes to the admin.
• A rejected delay emits no event.
• Two successive changes record default → MIN and then MIN → 500, so an observer can reconstruct the sequence.
• Changing the delay does not move a pending upgrade's effective_at.

The pre-existing set_delay_works (50) still holds, since 50 is inside the new range.

Note on where the constants live

The issue asks for constants "matching PROXY_* constants from traits". Those constants do not exist — there is no PROXY_* in propchain-traits, and there was never a PROXY_* prefix in that file. I put them in the proxy crate instead, for two reasons:

1. propchain-proxy deliberately has no propchain-traits dependency. Adding one is a Cargo.toml change plus a hand-edited Cargo.lock, which is not something to do blind in a PR that is not about dependencies.
2. A proxy is the last contract that should take on a new dependency for two integer constants.

If the intent is to centralise them in propchain-traits alongside the other *_BLOCKS constants, that is a reasonable follow-up and a one-line move once the dependency is added deliberately.

Integration changes

• ProxyError gains one variant, appended last. ProxyError is defined in, and only used by, this crate — nothing in the workspace matches on it exhaustively.
• No storage layout change: no fields added, removed, or reordered.
• No new dependencies, no Cargo.lock change.
• upgrade_delay_blocks behaviour for values already inside the range is unchanged.

Test plan

• cargo test -p propchain-proxy — not run. No code validation was performed, by explicit instruction; this change is source-only and manually reviewed.
• cargo fmt --all -- --check — not run, same reason.
• cargo clippy --workspace --all-targets -- -D warnings — not run, same reason.
• cargo build --workspace — not run, same reason.

The 16 tests added here are expected to compile and pass, but they are unverified.

The one assertion I would check first on a machine with a toolchain is changing_the_delay_emits_an_event. It compares recorded topics against Encode of the expected u64, which is the standard ink! idiom and how I believe ink! encodes a numeric topic, but I could not confirm it against the macro expansion. If ink! pads or transforms numeric topics, that test — and only that test — will need its comparison adjusted; the implementation is unaffected.

Env vars

• None. No new environment variables, configuration, or deployment steps.

[drips-wave]

@kilodesodiq-arch Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

[nanaf6203-bit]

LGTM