Skip to content

Conversation

@SIDDHANTCOOKIE
Copy link
Contributor

@SIDDHANTCOOKIE SIDDHANTCOOKIE commented Jan 24, 2026

Summary

This PR hardens ownership management and improves observability of admin state changes in the Chainvoice smart contract.

Fixes #80

Problem

  • Single-step ownership transfers are error-prone and irreversible
  • Ownership changes are not explicitly acknowledged by the recipient
  • Fee and treasury updates lacked deterministic on-chain signals for indexers and off-chain services

Solution

  • Implemented a two-step ownership transfer mechanism using a pendingOwner state variable
  • Ownership transfer requires explicit acceptance by the pending owner, preventing accidental loss of control
  • Added the ability for the current owner to cancel an in-progress transfer
  • Introduced custom errors for gas-efficient and explicit revert reasons
  • Emitted structured events for ownership lifecycle and admin parameter updates

Technical Details

  • Ownership lifecycle enforced via explicit state checks rather than implicit modifiers
  • Zero address and self-transfers are rejected at initiation
  • Custom errors replace string-based reverts to reduce gas usage
  • Admin setters now emit events with previous and new values for reliable off-chain indexing
  • Pattern aligns with established Ownable2Step semantics while remaining contract-native

Status

  • Fully backward compatible with existing integrations
  • No changes to external function signatures or observable behavior

I just started contributing to StabilityNexus. Requesting review @Zahnentferner 🙏

Summary by CodeRabbit

  • New Features

    • Two-step ownership transfer: initiate, accept, and cancel ownership transfers.
    • Emits events for ownership changes, fee updates, and treasury updates; fee/treasury updates now log previous and new values.
    • Improved validation and error handling for ownership and treasury updates.
  • Tests

    • Added comprehensive tests covering ownership flows, validation scenarios, cancellations, and admin updates.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 24, 2026

📝 Walkthrough

Walkthrough

Added a two-step ownership transfer flow (initiate, accept, cancel), new ownership-related errors and events, and updated admin setters to emit events. Tests covering ownership flows and admin event emissions were added.

Changes

Cohort / File(s) Summary
Ownership Transfer Implementation
contracts/src/Chainvoice.sol
Added pendingOwner (public), initiateOwnershipTransfer(address) (onlyOwner), acceptOwnership(), cancelOwnershipTransfer() (onlyOwner). Introduced InvalidNewOwner and OwnershipNotPending errors and OwnershipTransferInitiated, OwnershipTransferred, OwnershipTransferCancelled events.
Admin Events & Validations
contracts/src/Chainvoice.sol
setFeeAmount(uint256) now emits FeeUpdated(previousFee, newFee) before change. setTreasuryAddress(address) now validates non-zero and emits TreasuryAddressUpdated(previousTreasury, newTreasury) before change. Added corresponding events.
Tests — Ownership & Admin
contracts/test/Chainvoice.t.sol
Added tests covering: initiating transfer, invalid new owner rejection, accepting transfer by pending owner, rejecting non-pending acceptance, canceling transfer, rejecting cancel when none pending, and verifying fee/treasury update events and state.

Sequence Diagram(s)

sequenceDiagram
  participant Owner as Owner (current)
  participant Contract as Chainvoice (contract)
  participant Pending as PendingOwner

  Owner->>Contract: initiateOwnershipTransfer(newOwner)
  activate Contract
  Contract-->>Owner: OwnershipTransferInitiated event
  Contract-->>Pending: pendingOwner set\n(pendingOwner = newOwner)
  deactivate Contract

  Pending->>Contract: acceptOwnership()
  activate Contract
  alt caller == pendingOwner
    Contract-->>Owner: OwnershipTransferred event (previousOwner->newOwner)
    Contract-->>Pending: owner updated, pendingOwner cleared
  else not pending
    Contract-->>Pending: revert OwnershipNotPending()
  end
  deactivate Contract
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 I nudged the keys from paw to paw,

two hops to pass the keeper's law.
Events now sing where changes lie,
Pending waits, then claims the sky. 🥕✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding two-step ownership transfer and admin events, which matches the core modifications in the PR.
Linked Issues check ✅ Passed All requirements from issue #80 are met: two-step ownership transfer (initiateOwnershipTransfer, acceptOwnership, cancelOwnershipTransfer), ownership and admin events with previous/new values, custom errors for gas efficiency, and input validation with access control.
Out of Scope Changes check ✅ Passed All changes are directly related to implementing the two-step ownership transfer pattern and admin events specified in issue #80; no unrelated modifications detected.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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.

Add Two-Step Ownership Transfer & Admin Events

2 participants