Skip to content

Conversation

@nosid91
Copy link

@nosid91 nosid91 commented Jan 26, 2026

Description

This PR implements a secure mechanism to transfer and manage the OWNER role in Guardian smart contracts to address security audit findings (#4055).

  • Add two-step ownership transfer pattern in Access.sol (proposeOwner, claimOwner, removeOwner)
  • Add Guardian CLI commands for owner management (propose-owner, claim-owner, remove-owner)
  • Add comprehensive test suite for ownership transfer scenarios
  • Document owner management workflow in contracts README

Related issue(s)

Fixes #4055

Notes for Reviewer

Why Two-Step Ownership Transfer?

The implementation uses a two-step ownership transfer pattern (propose + claim) instead of direct transfer for the following security reasons:

  1. Prevents accidental transfers: Direct ownership transfer to an incorrect address (typos, copy-paste errors) would result in permanent loss of contract control. The two-step pattern requires the new owner to actively claim ownership, confirming they have access to the destination address.

  2. Validates recipient capability: The claim step verifies that the new owner can actually execute transactions, ensuring they have the private key and proper account setup.

  3. Provides recovery window: If an owner accidentally proposes the wrong address, they can simply propose a different address to overwrite the pending owner before the claim occurs.

  4. Industry standard: This pattern is widely adopted in production contracts (OpenZeppelin's Ownable2Step, major DeFi protocols) as a security best practice.

Security Constraints

  • Owners cannot remove themselves (prevents accidental lockout)
  • Contract address cannot be removed as owner (maintains internal call capability)
  • pendingOwner is cleared after successful claim (prevents replay)

Backwards Compatibility

  • Existing deployed contracts are not affected
  • No changes to existing public interfaces

Checklist

  • Documented (Code comments, README, CLI help)
  • Tested (integration tests with Hedera local node)

@nosid91 nosid91 requested review from a team as code owners January 26, 2026 09:31
…nds.

Signed-off-by: dima <nosid91@gmail.com>
@nosid91 nosid91 force-pushed the dimaredchyts/feat-4055-Lack-of-Method-to-Change-OWNER-Role branch from e064636 to a37f098 Compare January 26, 2026 09:31
@Pyatakov Pyatakov self-assigned this Jan 27, 2026
@Pyatakov Pyatakov changed the title feat: Implement Owner Management for Smart Contracts feat: 4055 Implement Owner Management for Smart Contracts Jan 27, 2026
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