diff --git a/.gitmodules b/.gitmodules index b3caf1fcb..dac7bb0f9 100644 --- a/.gitmodules +++ b/.gitmodules @@ -8,8 +8,11 @@ path = solidity_contracts/lib/v3-core url = https://github.com/Uniswap/v3-core [submodule "solidity_contracts/lib/openzeppelin"] - path = solidity_contracts/lib/openzeppelin + path = solidity_contracts/lib/openzeppelin-uniswapV3 url = https://github.com/OpenZeppelin/openzeppelin-contracts [submodule "solidity_contracts/lib/base64-sol"] path = solidity_contracts/lib/base64-sol url = https://github.com/Brechtpd/base64 +[submodule "solidity_contracts/lib/openzeppelin-contracts"] + path = solidity_contracts/lib/openzeppelin-contracts + url = https://github.com/OpenZeppelin/openzeppelin-contracts diff --git a/remappings.txt b/remappings.txt new file mode 100644 index 000000000..bfef1fca8 --- /dev/null +++ b/remappings.txt @@ -0,0 +1,2 @@ +@openzeppelin-contracts/=solidity_contracts/lib/openzeppelin-contracts/contracts/ +openzeppelin/=solidity_contracts/lib/openzeppelin-uniswapV3/contracts/ diff --git a/solidity_contracts/lib/openzeppelin-contracts b/solidity_contracts/lib/openzeppelin-contracts new file mode 160000 index 000000000..dbb6104ce --- /dev/null +++ b/solidity_contracts/lib/openzeppelin-contracts @@ -0,0 +1 @@ +Subproject commit dbb6104ce834628e473d2173bbc9d47f81a9eec3 diff --git a/solidity_contracts/lib/openzeppelin b/solidity_contracts/lib/openzeppelin-uniswapV3 similarity index 100% rename from solidity_contracts/lib/openzeppelin rename to solidity_contracts/lib/openzeppelin-uniswapV3 diff --git a/solidity_contracts/src/Kakarot/Coinbase.sol b/solidity_contracts/src/Kakarot/Coinbase.sol index 1f1be892f..e26f186a9 100644 --- a/solidity_contracts/src/Kakarot/Coinbase.sol +++ b/solidity_contracts/src/Kakarot/Coinbase.sol @@ -1,28 +1,18 @@ // SPDX-License-Identifier: MIT -pragma solidity >=0.8.0 <0.9.0; +pragma solidity 0.8.27; import {DualVmToken} from "../CairoPrecompiles/DualVmToken.sol"; +import {Ownable2Step, Ownable} from "@openzeppelin-contracts/access/Ownable2Step.sol"; -contract Coinbase { +contract Coinbase is Ownable2Step { /// @dev The EVM address of the DualVmToken for Kakarot ETH. DualVmToken public immutable kakarotEth; - /// @dev State variable to store the owner of the contract - address public owner; - /// Constructor sets the owner of the contract - constructor(address _kakarotEth) { - owner = msg.sender; + constructor(address _kakarotEth) Ownable(msg.sender) { kakarotEth = DualVmToken(_kakarotEth); } - /// Modifier to restrict access to owner only - /// @dev Assert that msd.sender is the owner - modifier onlyOwner() { - require(msg.sender == owner, "Not the contract owner"); - _; - } - /// @notice Withdraws ETH from the contract to a Starknet address /// @dev DualVmToken.balanceOf(this) is the same as address(this).balance /// @param toStarknetAddress The Starknet address to withdraw to @@ -30,11 +20,4 @@ contract Coinbase { uint256 balance = address(this).balance; kakarotEth.transfer(toStarknetAddress, balance); } - - /// @notice Transfers ownership of the contract to a new address - /// @param newOwner The address to transfer ownership to - function transferOwnership(address newOwner) external onlyOwner { - require(newOwner != address(0), "New owner cannot be the zero address"); - owner = newOwner; - } } diff --git a/solidity_contracts/src/UniswapV2Router/libraries/UniswapV2Library.sol b/solidity_contracts/src/UniswapV2Router/libraries/UniswapV2Library.sol index c23d6e00f..3e22328a0 100644 --- a/solidity_contracts/src/UniswapV2Router/libraries/UniswapV2Library.sol +++ b/solidity_contracts/src/UniswapV2Router/libraries/UniswapV2Library.sol @@ -33,7 +33,7 @@ library UniswapV2Library { hex"ff", factory, keccak256(abi.encodePacked(token0, token1)), - hex"f1cfdd2c3c5d1c1326d5ea2b5624ae1ad7c2445c6b96eabc5af1dd53cdf64243" // init code hash + hex"de99574e4d2f5d518d4322aaa25cf5c57d167d84b40c189e60afca95b87d6cdd" // init code hash ) ) ) diff --git a/tests/end_to_end/Kakarot/test_coinbase.py b/tests/end_to_end/Kakarot/test_coinbase.py index 3936a3582..5310eba32 100644 --- a/tests/end_to_end/Kakarot/test_coinbase.py +++ b/tests/end_to_end/Kakarot/test_coinbase.py @@ -1,5 +1,8 @@ import pytest import pytest_asyncio +from eth_abi import encode +from eth_utils import keccak +from eth_utils.address import to_checksum_address from kakarot_scripts.utils.kakarot import deploy, eth_balance_of, fund_address from kakarot_scripts.utils.starknet import invoke @@ -39,23 +42,35 @@ async def test_should_withdraw_all_eth(self, coinbase, owner): assert balance_owner - balance_owner_prev + tx["gas_used"] == 0.001e18 async def test_should_revert_when_not_owner(self, coinbase, other): - with evm_error("Not the contract owner"): + error = keccak(b"OwnableUnauthorizedAccount(address)")[:4] + encode( + ["address"], [other.address] + ) + with evm_error(error): await coinbase.withdraw(0xDEAD, caller_eoa=other.starknet_contract) class TestTransferOwnership: + async def test_should_transfer_ownership(self, coinbase, owner, other): + # initiate transfer process await coinbase.transferOwnership(other.address) + assert await coinbase.pendingOwner() == other.address + assert await coinbase.owner() == owner.address + # accept the transfer + await coinbase.acceptOwnership(caller_eoa=other.starknet_contract) + ZERO_ADDRESS = to_checksum_address(f"0x{0:040x}") + assert await coinbase.pendingOwner() == ZERO_ADDRESS assert await coinbase.owner() == other.address + # teardown await coinbase.transferOwnership( owner.address, caller_eoa=other.starknet_contract ) - - async def test_should_revert_when_new_owner_is_zero_address(self, coinbase): - with evm_error("New owner cannot be the zero address"): - await coinbase.transferOwnership(f"0x{0:040x}") + await coinbase.acceptOwnership(caller_eoa=owner.starknet_contract) async def test_should_revert_when_not_owner(self, coinbase, other): - with evm_error("Not the contract owner"): + error = keccak(b"OwnableUnauthorizedAccount(address)")[:4] + encode( + ["address"], [other.address] + ) + with evm_error(error): await coinbase.transferOwnership( other.address, caller_eoa=other.starknet_contract )