Skip to content

Add upgrade e2e test and fix PackageUpgradedEvent parsing#456

Open
Bridgerz wants to merge 1 commit intomainfrom
upgrade-test-infrastructure
Open

Add upgrade e2e test and fix PackageUpgradedEvent parsing#456
Bridgerz wants to merge 1 commit intomainfrom
upgrade-test-infrastructure

Conversation

@Bridgerz
Copy link
Copy Markdown
Contributor

Summary

  • Adds an e2e test that exercises the full governance-gated upgrade lifecycle and verifies cascading effects
  • Fixes a bug: PackageUpgradedEvent was missing from HashiEvent::try_parse(), so validators never learned about package upgrades via the event stream — only on full rescrape

Bug fix

PackageUpgradedEvent had a struct definition, MoveType impl, and was handled in the watcher's event loop (watcher.rs:228), but was never added to the try_parse() match arms in move_types/mod.rs. The event was silently dropped at the catch-all _ => return Ok(None). After an upgrade, validators would not update their package version map until a subscription reconnect triggered a full rescrape.

Test details

The test programmatically patches the deployed package at test time (bumps PACKAGE_VERSION, injects a canary module), then verifies:

  1. Watcher picks up new package — all 4 nodes detect the new version via PackageUpgradedEvent
  2. Package ID routingOnchainState.package_id() returns the new package on all nodes
  3. Validator deposit confirmation — full deposit flow (BTC send → mine → request → validator auto-confirm) works against the upgraded package
  4. New code reachable — v2-only canary module is callable
  5. Version gating — disabling v1 rejects old entry points with EVersionDisabled

Files changed

  • crates/hashi-types/src/move_types/mod.rs — add PackageUpgradedEvent to try_parse()
  • crates/e2e-tests/src/upgrade_flow.rs — upgrade helpers (prepare, build, propose/vote/execute, disable)
  • crates/e2e-tests/src/upgrade_tests.rs — the e2e test
  • crates/e2e-tests/src/lib.rs — module declarations + dir() accessor

Test plan

  • cargo test -p e2e-tests -- test_upgrade_v1_to_v2 --ignored — passes in ~44s
  • cargo check -p hashi — compiles clean

@Bridgerz Bridgerz requested a review from bmwill as a code owner April 15, 2026 19:25
@Bridgerz Bridgerz force-pushed the upgrade-test-infrastructure branch 3 times, most recently from 2dc5423 to 0403601 Compare April 15, 2026 20:41
/// 2. Validators confirm deposits post-upgrade — leader routes calls correctly
/// 3. Package ID routing — OnchainState.package_id() returns the new package
#[tokio::test]
#[ignore = "requires localnet (run with --ignored)"]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is missing from being able to run this in CI?

Comment thread crates/e2e-tests/src/upgrade_tests.rs Outdated
Comment on lines +70 to +74
async fn wait_for_deposit_confirmation(
sui_client: &mut sui_rpc::Client,
request_id: Address,
timeout: Duration,
) -> Result<()> {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These functions should already exist elsewhere in the e2e-tests, maybe they just need to be factored out into a common place to that they can be reused?

Comment on lines +11 to +12
#[cfg(test)]
mod tests {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is probably better placed in crates/e2e-tests/tests/upgrade_tests.rs then you shouldn't even need the cfg(test) dance.

Comment on lines +59 to +62
let patched = config_src.replace(
"const PACKAGE_VERSION: u64 = 1;",
"const PACKAGE_VERSION: u64 = 2;",
);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

//!
//! Exercises real cascading effects of upgrading the hashi package:
//! - Rust watcher picks up the new package version via PackageUpgradedEvent
//! - Validators auto-confirm deposits against the upgraded package
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm actually semi-surprised that our system is already smart enough to do this as a part of our txn construction.

Comment on lines +29 to +35
/// Prepare an upgrade package by copying the deployed source and patching it.
///
/// 1. Copies `<test_dir>/packages/hashi` to `<test_dir>/packages/hashi-upgrade`
/// 2. Bumps `PACKAGE_VERSION` from 1 to 2 in `config.move`
/// 3. Sets `published-at` in `Move.toml` to the original package ID
///
/// Returns the path to the patched package directory.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wonder if it would make sense to factor some, not all, of this logic into a reusable functions that we can then leverage in the cli to do upgrades in the future (create, vote, execute proposals). Specifically to do some explicit pre-flight checks (check that VERSION is exactly what we expect it to be +1 of the latest published version, etc.

Thoughts?

Adds an end-to-end test that exercises the full governance-gated upgrade
lifecycle and verifies cascading effects across the stack.

The test found a real bug: PackageUpgradedEvent was missing from
HashiEvent::try_parse(), so validators never learned about package
upgrades via the event stream. Fixed by adding the missing match arm.

Test flow:
- Deposit before upgrade, verify balance survives (state continuity)
- Upgrade via propose/vote/execute+publish+finalize
- Verify all node watchers pick up the new package version
- Deposit after upgrade through full validator confirmation path
- Call v2-only canary module (new code reachable)
- Disable v1, verify entry points rejected
@Bridgerz Bridgerz force-pushed the upgrade-test-infrastructure branch from 0403601 to 42de78f Compare April 15, 2026 21:11
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.

2 participants