diff --git a/contracts/src/Chainvoice.sol b/contracts/src/Chainvoice.sol index 042397b4..ba375536 100644 --- a/contracts/src/Chainvoice.sol +++ b/contracts/src/Chainvoice.sol @@ -61,6 +61,7 @@ contract Chainvoice { mapping(address => uint256[]) public receivedInvoices; address public owner; + address public pendingOwner; address public treasuryAddress; uint256 public fee; // native fee per invoice uint256 public accumulatedFees; // native fees accrued (for withdraw) @@ -72,6 +73,10 @@ contract Chainvoice { event InvoiceCancelled(uint256 indexed id, address indexed from, address indexed to, address tokenAddress); event InvoiceBatchCreated(address indexed creator, address indexed token, uint256 count, uint256[] ids); event InvoiceBatchPaid(address indexed payer, address indexed token, uint256 count, uint256 totalAmount, uint256[] ids); + event OwnershipTransferStarted(address indexed previousOwner, address indexed newOwner); + event OwnershipTransferred(address indexed previousOwner, address indexed newOwner); + event FeeUpdated(uint256 oldFee, uint256 newFee); + event TreasuryUpdated(address indexed oldTreasury, address indexed newTreasury); event OwnershipTransferred(address indexed previousOwner, address indexed newOwner); event OwnershipTransferInitiated(address indexed currentOwner, address indexed pendingOwner); @@ -434,13 +439,16 @@ contract Chainvoice { function setFeeAmount(uint256 _fee) external onlyOwner { emit FeeUpdated(fee, _fee); fee = _fee; + emit FeeUpdated(oldFee, _fee); } function setTreasuryAddress(address newTreasury) external onlyOwner { - if (newTreasury == address(0)) revert ZeroAddress(); + require(newTreasury != address(0), "Zero address"); + address oldTreasury = treasuryAddress; emit TreasuryAddressUpdated(treasuryAddress, newTreasury); treasuryAddress = newTreasury; + emit TreasuryUpdated(oldTreasury, newTreasury); } function withdrawFees() external { @@ -453,4 +461,37 @@ contract Chainvoice { (bool success, ) = payable(treasuryAddress).call{value: amount}(""); if (!success) revert WithdrawFailed(); } + + // ========== Governance (Two-Step Ownership) ========== + + /** + * @dev Starts the ownership transfer of the contract to a new account. Replaces the pending transfer if there is one. + * Can only be called by the current owner. + */ + function transferOwnership(address newOwner) external onlyOwner { + pendingOwner = newOwner; + emit OwnershipTransferStarted(owner, newOwner); + } + + /** + * @dev The new owner accepts the ownership transfer. + */ + function acceptOwnership() external { + require(msg.sender == pendingOwner, "Caller is not the pending owner"); + address oldOwner = owner; + owner = pendingOwner; + pendingOwner = address(0); + emit OwnershipTransferred(oldOwner, owner); + } + + /** + * @dev Leaves the contract without owner. It will not be possible to call `onlyOwner` functions. + * Can only be called by the current owner. + */ + function renounceOwnership() external onlyOwner { + address oldOwner = owner; + owner = address(0); + pendingOwner = address(0); + emit OwnershipTransferred(oldOwner, address(0)); + } } diff --git a/contracts/test/Chainvoice.t.sol b/contracts/test/Chainvoice.t.sol index 2abb1ce9..b2fa44c4 100644 --- a/contracts/test/Chainvoice.t.sol +++ b/contracts/test/Chainvoice.t.sol @@ -11,6 +11,8 @@ contract ChainvoiceTest is Test { address alice = address(0xA11CE); address bob = address(0xB0B); + event FeeUpdated(uint256 oldFee, uint256 newFee); + function setUp() public { chainvoice = new Chainvoice(); vm.deal(alice, 10 ether); @@ -115,6 +117,46 @@ contract ChainvoiceTest is Test { } /* ------------------------------------------------------------ */ + /* ADMIN & GOVERNANCE TESTS */ + /* ------------------------------------------------------------ */ + + function testSetFeeAmount_EmitsEvent() public { + uint256 newFee = 0.001 ether; + + // Tell Foundry to expect the FeeUpdated event + // (bool checkTopic1, bool checkTopic2, bool checkTopic3, bool checkData) + vm.expectEmit(false, false, false, true); + emit FeeUpdated(chainvoice.fee(), newFee); + + chainvoice.setFeeAmount(newFee); + + assertEq(chainvoice.fee(), newFee); + } + + function testTwoStepOwnershipTransfer() public { + // Step 1: Current owner (this test contract) starts transfer + chainvoice.transferOwnership(alice); + + // Owner should still be the original owner, pending is Alice + assertEq(chainvoice.owner(), address(this)); + assertEq(chainvoice.pendingOwner(), alice); + + // Step 2: Alice accepts the ownership + vm.prank(alice); + chainvoice.acceptOwnership(); + + // Ownership should now be fully transferred + assertEq(chainvoice.owner(), alice); + assertEq(chainvoice.pendingOwner(), address(0)); + } + + function testAcceptOwnership_RevertsIfNotPending() public { + chainvoice.transferOwnership(alice); + + // Bob tries to accept, but he isn't the pending owner + vm.prank(bob); + vm.expectRevert("Caller is not the pending owner"); + chainvoice.acceptOwnership(); /* OWNERSHIP MANAGEMENT */ /* ------------------------------------------------------------ */