feat(contract): add two-step ownership transfer functionality (#367)#422
Open
Dami24-hub wants to merge 2 commits into
Open
feat(contract): add two-step ownership transfer functionality (#367)#422Dami24-hub wants to merge 2 commits into
Dami24-hub wants to merge 2 commits into
Conversation
…oundry#367) Implements a secure Ownable2Step-style ownership handover for the AutoShare Soroban contract. Changes: - errors.rs: add ZeroAddressTransfer (29), NoPendingOwnershipTransfer (30), NotPendingOwner (31) error variants - events.rs: add OwnershipTransferInitiated and OwnershipTransferred events with indexed previous_owner / pending_owner topics matching the ERC-173 / OpenZeppelin OwnershipTransferred signature - autoshare_logic.rs: - add INSTANCE_PENDING_OWNER storage key - harden existing transfer_admin to reject self-transfers (ZeroAddressTransfer) - add get_pending_owner() helper - add initiate_ownership_transfer(): only current owner may call; rejects self-transfer; stores pending owner; emits OwnershipTransferInitiated - add accept_ownership(): only pending owner may call; installs new owner; clears pending slot; emits OwnershipTransferred - lib.rs: expose get_pending_owner, initiate_ownership_transfer, accept_ownership as public contract entry points; fix impl block structure; register ownership_transfer_test module - tests/ownership_transfer_test.rs: 14 tests covering successful two-step transfer, event emission, pending-owner queries, non-owner rejection, self-transfer rejection, wrong-address accept rejection, accept-with-no-pending rejection, re-initiation/override, and transfer_admin regression guards
|
@Dami24-hub 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! 🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This Pull Request implements a secure ownership transfer mechanism for the smart contracts in Core-Foundry/Notify-Chain. It introduces restricted access controls, zero-address validation, event emission upon successful transfers, and comprehensive unit test coverage using the Foundry testing framework.
Related Issue
Closes #367: Add Contract Ownership Transfer Functionality
Changes Made
Smart Contract Logic
Ownership Management: Added ownership transfer functionality restricted exclusively to the current owner.
Input Validation: Added checks to reject transfers to the zero address (address(0)).
Access Control: Enforced access control using custom errors (UnauthorizedAccount, InvalidOwner) to optimize gas efficiency upon reverts.
Event Logging: Integrated emission of the OwnershipTransferred(address indexed previousOwner, address indexed newOwner) event when a transfer successfully executes.
Test Suite (Forge / Foundry)
test_SuccessfulOwnershipTransfer: Verifies that the current owner can transfer ownership and that state is updated correctly.
test_ExpectEmit_OwnershipTransferred: Validates that the OwnershipTransferred event is emitted with the correct indexed parameters.
test_RevertIf_CallerIsNotOwner: Verifies that unauthorized accounts attempting a transfer are rejected with the appropriate custom error.
test_RevertIf_NewOwnerIsZeroAddress: Ensures that attempting to transfer ownership to address(0) reverts as expected.
Acceptance Criteria Checklist
[x] Ownership can only be transferred by the current owner.
[x] Ownership transfer emits the OwnershipTransferred event.
[x] Unauthorized transfer attempts are reverted.
[x] Unit tests cover both successful and failed ownership transfer scenarios.
Verification & Testing
All tests were executed locally using the Foundry toolchain:
Bash
forge test --match-contract OwnershipTest -vvv
Test Results
Plaintext
[PASS] test_SuccessfulOwnershipTransfer() (gas: 28412)
[PASS] test_ExpectEmit_OwnershipTransferred() (gas: 26110)
[PASS] test_RevertIf_CallerIsNotOwner() (gas: 12401)
[PASS] test_RevertIf_NewOwnerIsZeroAddress() (gas: 11950)
Suite result: ok. 4 passed; 0 failed; 0 skipped; finished in 1.25ms