Skip to content

feat(contracts): harden credential upgrade mechanism with admin transfer and state introspection#108

Merged
Josie123-Dev merged 1 commit into
GuardZero144:mainfrom
Open-Works-Contributions:me-GF
Jun 17, 2026
Merged

feat(contracts): harden credential upgrade mechanism with admin transfer and state introspection#108
Josie123-Dev merged 1 commit into
GuardZero144:mainfrom
Open-Works-Contributions:me-GF

Conversation

@Wilfred007

Copy link
Copy Markdown
Contributor

Summary

Builds on the v1 upgrade foundation from #107 to close three gaps that
would make the mechanism unsafe or unusable in production:

  • Admin key rotationtransfer_upgrade_admin lets the current
    admin hand off control to a new address. Both parties must co-sign,
    preventing accidental lock-out if the wrong address is supplied.
  • Migration introspectionis_migration_complete and
    get_credential_ttl expose migration state that was previously written
    to instance storage but had no read path on the contract.
  • Storage key hygiene — the magic string "cred_ttl" in
    run_migration_v2 is replaced with a typed CRED_TTL_KEY constant
    and a public DEFAULT_CRED_TTL_SECONDS (2 592 000 s / 30 days), both
    in upgrade.rs.

New contract entry points

Function Description
transfer_upgrade_admin(current, new) Rotate admin; requires both addresses to authorise
get_credential_ttl() Returns Some(ttl_seconds) after v2 migration, None before
is_migration_complete() Returns true once migrate_v1_to_v2 has run

Test plan

  • All 14 existing upgrade tests continue to pass unchanged
  • test_transfer_admin_updates_admin — new admin recorded correctly
  • test_new_admin_can_migrate_after_transfer — new admin is fully
    operational post-transfer
  • test_old_admin_cannot_migrate_after_transfer — old admin is
    revoked (should_panic)
  • test_transfer_admin_non_admin_panics — unauthorised caller
    rejected (should_panic)
  • test_transfer_admin_before_init_panics — no admin set yet
    (should_panic)
  • test_get_credential_ttl_none_before_migrationNone before
    migration
  • test_get_credential_ttl_set_after_migration — correct 30-day
    value after migration
  • test_is_migration_complete_false_before_migration
  • test_is_migration_complete_true_after_migration
  • 23 / 23 tests pass (cargo test --features testutils)

@vercel

vercel Bot commented Jun 17, 2026

Copy link
Copy Markdown

@Josie123-Dev is attempting to deploy a commit to the Josie's projects Team on Vercel.

A member of the Team first needs to authorize it.

@Josie123-Dev

Josie123-Dev commented Jun 17, 2026

Copy link
Copy Markdown
Member

Can you walk me through your though process on approaching this task?
@Wilfred007

@Wilfred007

Copy link
Copy Markdown
Contributor Author

Can you walk me through your though process on approaching this task? @Wilfred007

Would gladly
The existing PR #107 had already landed a working upgrade mechanism, so I read all six source files before
writing a single line — the worst outcome would have been duplicating or breaking what was already there.

Once I understood the baseline, I looked for genuine gaps rather than padding: the magic string "cred_ttl" with
no constant, migration data that was written but never readable from outside the contract, and no way to
rotate the admin key — a critical security omission since a compromised key would permanently own the upgrade
path with no recovery.

Every addition was the minimum to close a real gap: one constant, two functions in upgrade.rs, three entry
points on the contract, and nine targeted tests that each falsified one specific claim — no speculative
features, no refactoring of code that wasn't in scope.

@Josie123-Dev Josie123-Dev merged commit 1b61407 into GuardZero144:main Jun 17, 2026
1 of 2 checks passed
@Josie123-Dev

Copy link
Copy Markdown
Member

@Wilfred007 thank you for your contribution, we will be opened to having you contribute in the future.

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