diff --git a/.solhint.json b/.solhint.json index 7c28fa1b..8b70836f 100644 --- a/.solhint.json +++ b/.solhint.json @@ -3,12 +3,14 @@ "plugins": ["prettier"], "rules": { "no-unused-import": "error", + "no-global-import": "error", + "func-visibility": ["error",{"ignoreConstructors":true}], "compiler-version": "off", "max-states-count": "off", "no-empty-blocks": "off", - "func-visibility": ["warn",{"ignoreConstructors":true}], "custom-errors": "off", "reason-string": "off", + "not-rely-on-time": "off", "one-contract-per-file": "off" } } \ No newline at end of file diff --git a/contracts/ForkableBridge.sol b/contracts/ForkableBridge.sol index 6516bc8f..e7cb8015 100644 --- a/contracts/ForkableBridge.sol +++ b/contracts/ForkableBridge.sol @@ -181,10 +181,11 @@ contract ForkableBridge is /** * @dev Allows aynone to take out their forkonomic tokens * and send them to the children-bridge contracts - * Notice that forkonomic tokens are special, as they their main contract - * is on L1, but they are still forkable tokens, as their contract is forked as well. - * We allow to send tokens only to one child, just in case the one child contract + * Notice that forkonomic tokens are special, as their main contract + * is on L1, but they are still forkable tokens, as their contract is forkable as well. + * We allow to send tokens to one child, just in case the other child contract * was updated shortly after the fork and contains an error (e.g. reverts on sending) + * But usually a users would want to sent the tokens to both children. * @param amount Amount of tokens to be sent to the children-bridge contracts * @param useFirstChild boolean indicating for which child the operation should be run * @param useChildTokenAllowance boolean indicating if the child token allowance (previously burned tokens) should be used diff --git a/contracts/ForkonomicToken.sol b/contracts/ForkonomicToken.sol index 65242169..3c94bfbd 100644 --- a/contracts/ForkonomicToken.sol +++ b/contracts/ForkonomicToken.sol @@ -17,9 +17,9 @@ contract ForkonomicToken is bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE"); /// @dev Mapping that stores burned amounts - /// address The address of the token owner - /// bool indicating whether the first or second child was burnt - /// uint256 The amount of burned tokens + /// 1st parameter: address The address of the token owner + /// 2nd parameter: bool indicating whether the first or second child was burnt + /// 3rd parameter: uint256 The amount of burned tokens mapping(address => mapping(bool => uint256)) public childTokenAllowances; /// @inheritdoc IForkonomicToken @@ -53,6 +53,7 @@ contract ForkonomicToken is return _createChildren(); } + /// @inheritdoc IForkonomicToken function splitTokenAndMintOneChild( uint256 amount, bool firstChild, @@ -82,8 +83,7 @@ contract ForkonomicToken is childTokenAllowances[msg.sender][true] += amount; } - /// @dev Allows anyone to split the tokens from the parent contract into the tokens of the children - /// @param amount The amount of tokens to split + /// @inheritdoc IForkonomicToken function splitTokensIntoChildTokens(uint256 amount) external { splitTokenAndMintOneChild(amount, true, false); splitTokenAndMintOneChild(amount, false, true); diff --git a/contracts/lib/BridgeAssetOperations.sol b/contracts/lib/BridgeAssetOperations.sol index 8c7e1d2f..3990685a 100644 --- a/contracts/lib/BridgeAssetOperations.sol +++ b/contracts/lib/BridgeAssetOperations.sol @@ -8,7 +8,7 @@ import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import {IERC20Metadata} from "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol"; library BridgeAssetOperations { - // @dev Error thrown when non-forkable token is intended to be used, but it is not forkable + // @dev Error thrown when forkable token is intended to be used, but it is not forkable error TokenNotForkable(); // @dev Error thrown when token is not issued before error TokenNotIssuedBefore(); diff --git a/contracts/lib/CreateChildren.sol b/contracts/lib/CreateChildren.sol index 1ad06a66..3d918900 100644 --- a/contracts/lib/CreateChildren.sol +++ b/contracts/lib/CreateChildren.sol @@ -35,7 +35,10 @@ library CreateChildren { return StorageSlot.getAddressSlot(_ADMIN_SLOT).value; } - /// @dev Internal function to create the children contracts. + /** + * @dev function to create the children contracts. + * returns the addresses of the two children + */ function createChildren() public returns (address child1, address child2) { address implementation = getImplementation(); address admin = getAdmin(); diff --git a/contracts/mixin/ForkableStructure.sol b/contracts/mixin/ForkableStructure.sol index 6b79a283..7c063e19 100644 --- a/contracts/mixin/ForkableStructure.sol +++ b/contracts/mixin/ForkableStructure.sol @@ -9,12 +9,14 @@ contract ForkableStructure is IForkableStructure, Initializable { // The forkmanager is the only one who can clone the instances and create children address public forkmanager; - // The parent contract is the contract that was holding tokens or logic before the most recent fork + // The parent contract is the contract that was creating this contract during the most recent fork address public parentContract; // The children are the two instances that are created during the fork // Actually an array like this: address[] public children = new address[](2) would be the natural fit, // but this would make the initialization more complex due to proxy construction. + // children[0] stores the first child + // children[1] stores the second child mapping(uint256 => address) public children; modifier onlyBeforeForking() { @@ -28,9 +30,11 @@ contract ForkableStructure is IForkableStructure, Initializable { if (children[0] == address(0x0)) { revert OnlyAfterForking(); } - if (children[1] == address(0x0)) { - revert OnlyAfterForking(); - } + // The following line is not needed, as both children are created + // simultaniously + // if (children[1] == address(0x0)) { + // revert OnlyAfterForking(); + // } _; } @@ -49,7 +53,7 @@ contract ForkableStructure is IForkableStructure, Initializable { } /** - * @dev Initializes the contract. + * @dev Initializes the contract * @param _forkmanager The address of the forkmanager contract. * @param _parentContract The address of the parent contract. */ diff --git a/test/CreateChildren.t.sol b/test/CreateChildren.t.sol new file mode 100644 index 00000000..c9feb0dd --- /dev/null +++ b/test/CreateChildren.t.sol @@ -0,0 +1,72 @@ +// SPDX-License-Identifier: GPL-3.0-only +pragma solidity ^0.8.20; + +import {Test} from "forge-std/Test.sol"; +import {ChainIdManager} from "../contracts/ChainIdManager.sol"; +import {Util} from "./utils/Util.sol"; +import {TransparentUpgradeableProxy} from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol"; +import {CreateChildren} from "../contracts/lib/CreateChildren.sol"; +import {CreateChildrenWrapper} from "./testcontract/CreateChildrenWrapper.sol"; + +// Create a test suite contract +contract CreateChildrenTest is Test { + CreateChildrenWrapper public testContract; + address public admin = address(0x987654321); + address public implementation = address(new CreateChildrenWrapper()); + // Deploy the test contract before running tests + constructor() { + testContract = CreateChildrenWrapper(address(new TransparentUpgradeableProxy(implementation, admin, ""))); + } + + // Test case to check if the implementation address is correctly set + function testImplementationAddress() external { + address receivedImplementation = testContract.getImplementation(); + assertEq( + receivedImplementation, + implementation, + "Implementation address is incorrect" + ); + } + + // Test case to check if the admin address is correctly set + function testAdminAddress() external { + address receivedAdmin = testContract.getAdmin(); + assertEq(receivedAdmin, admin, "Admin address is incorrect"); + } + + // Test case to create children contracts and check their addresses + function testCreateChildren() external { + (address child1, address child2) = testContract.createChildren(); + // Test implementation slot with two different ways + assertEq( + Util.bytesToAddress( + vm.load(address(child2), Util._IMPLEMENTATION_SLOT) + ), + implementation + ); + assertEq( + CreateChildrenWrapper(child1).getImplementation(), + implementation + ); + // Test admin slot with two different ways + assertEq( + Util.bytesToAddress(vm.load(address(child1), Util._ADMIN_SLOT)), + admin + ); + assertEq( + CreateChildrenWrapper(child1).getAdmin(), + admin + ); + + assertEq( + Util.bytesToAddress( + vm.load(address(child2), Util._IMPLEMENTATION_SLOT) + ), + implementation + ); + assertEq( + Util.bytesToAddress(vm.load(address(child2), Util._ADMIN_SLOT)), + admin + ); + } +} diff --git a/test/ForkableStructure.t.sol b/test/ForkableStructure.t.sol index 009345ad..2bbd6b50 100644 --- a/test/ForkableStructure.t.sol +++ b/test/ForkableStructure.t.sol @@ -4,6 +4,7 @@ import {Test} from "forge-std/Test.sol"; import {ForkableStructureWrapper} from "./testcontract/ForkableStructureWrapper.sol"; import {TransparentUpgradeableProxy} from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol"; import {Util} from "./utils/Util.sol"; +import {IForkableStructure} from "../contracts/interfaces/IForkableStructure.sol"; contract ForkStructureTest is Test { bytes32 internal constant _IMPLEMENTATION_SLOT = @@ -81,4 +82,42 @@ contract ForkStructureTest is Test { admin ); } + + function testModifiers() public{ + forkableStructureImplementation = address( + new ForkableStructureWrapper() + ); + forkStructure = ForkableStructureWrapper( + address( + new TransparentUpgradeableProxy( + forkableStructureImplementation, + admin, + "" + ) + ) + ); + forkStructure.initialize(forkmanager, parentContract); + + forkStructure.onlyBeforeForkingTesting(); + vm.expectRevert(IForkableStructure.OnlyAfterForking.selector); + forkStructure.onlyAfterForkingTesting(); + + forkStructure.createChildren(); + + vm.expectRevert(IForkableStructure.NoChangesAfterForking.selector); + forkStructure.onlyBeforeForkingTesting(); + forkStructure.onlyAfterForkingTesting(); + + vm.expectRevert(IForkableStructure.OnlyParentIsAllowed.selector); + forkStructure.onlyParentContractTesting(); + + vm.expectRevert(IForkableStructure.OnlyForkManagerIsAllowed.selector); + forkStructure.onlyForkManagerTesting(); + + vm.prank(forkStructure.forkmanager()); + forkStructure.onlyForkManagerTesting(); + + vm.prank(forkStructure.parentContract()); + forkStructure.onlyParentContractTesting(); + } } diff --git a/test/ForkingManager.t.sol b/test/ForkingManager.t.sol index b88ffc2f..70f589a6 100644 --- a/test/ForkingManager.t.sol +++ b/test/ForkingManager.t.sol @@ -19,6 +19,7 @@ import {IPolygonZkEVM} from "@RealityETH/zkevm-contracts/contracts/interfaces/IP import {TransparentUpgradeableProxy} from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol"; import {ChainIdManager} from "../contracts/ChainIdManager.sol"; import {ForkableZkEVM} from "../contracts/ForkableZkEVM.sol"; +import {Util} from "./utils/Util.sol"; contract ForkingManagerTest is Test { ForkableBridge public bridge; @@ -86,10 +87,6 @@ contract ForkingManagerTest is Test { event Transfer(address indexed from, address indexed to, uint256 tokenId); - function bytesToAddress(bytes32 b) public pure returns (address) { - return address(uint160(uint256(b))); - } - function setUp() public { bridgeImplementation = address(new ForkableBridge()); bridge = ForkableBridge( @@ -282,13 +279,13 @@ contract ForkingManagerTest is Test { // Assert that the fork managers implementation match the ones we provided assertEq( - bytesToAddress( + Util.bytesToAddress( vm.load(address(childForkmanager1), _IMPLEMENTATION_SLOT) ), forkmanagerImplementation ); assertEq( - bytesToAddress( + Util.bytesToAddress( vm.load(address(childForkmanager2), _IMPLEMENTATION_SLOT) ), forkmanagerImplementation @@ -300,13 +297,13 @@ contract ForkingManagerTest is Test { // Assert that the bridges match the ones we provided assertEq( - bytesToAddress( + Util.bytesToAddress( vm.load(address(childBridge1), _IMPLEMENTATION_SLOT) ), bridgeImplementation ); assertEq( - bytesToAddress( + Util.bytesToAddress( vm.load(address(childBridge2), _IMPLEMENTATION_SLOT) ), bridgeImplementation @@ -318,13 +315,13 @@ contract ForkingManagerTest is Test { // Assert that the ZkEVM contracts match the ones we provided assertEq( - bytesToAddress( + Util.bytesToAddress( vm.load(address(childZkevm1), _IMPLEMENTATION_SLOT) ), zkevmImplementation ); assertEq( - bytesToAddress( + Util.bytesToAddress( vm.load(address(childZkevm2), _IMPLEMENTATION_SLOT) ), zkevmImplementation @@ -358,7 +355,7 @@ contract ForkingManagerTest is Test { // Assert that the forkonomic tokens match the ones we provided assertEq( - bytesToAddress( + Util.bytesToAddress( vm.load( address(childForkonomicToken1), _IMPLEMENTATION_SLOT @@ -367,7 +364,7 @@ contract ForkingManagerTest is Test { forkonomicTokenImplementation ); assertEq( - bytesToAddress( + Util.bytesToAddress( vm.load( address(childForkonomicToken2), _IMPLEMENTATION_SLOT @@ -385,13 +382,13 @@ contract ForkingManagerTest is Test { // Assert that the forkonomic tokens match the ones we provided assertEq( - bytesToAddress( + Util.bytesToAddress( vm.load(address(childGlobalExitRoot1), _IMPLEMENTATION_SLOT) ), globalExitRootImplementation ); assertEq( - bytesToAddress( + Util.bytesToAddress( vm.load(address(childGlobalExitRoot2), _IMPLEMENTATION_SLOT) ), globalExitRootImplementation @@ -433,13 +430,13 @@ contract ForkingManagerTest is Test { // Assert that the fork managers implementation match the ones we provided assertEq( - bytesToAddress( + Util.bytesToAddress( vm.load(address(childForkmanager1), _IMPLEMENTATION_SLOT) ), forkmanagerImplementation ); assertEq( - bytesToAddress( + Util.bytesToAddress( vm.load(address(childForkmanager2), _IMPLEMENTATION_SLOT) ), forkmanagerImplementation @@ -451,13 +448,13 @@ contract ForkingManagerTest is Test { // Assert that the bridges match the ones we provided assertEq( - bytesToAddress( + Util.bytesToAddress( vm.load(address(childBridge1), _IMPLEMENTATION_SLOT) ), bridgeImplementation ); assertEq( - bytesToAddress( + Util.bytesToAddress( vm.load(address(childBridge2), _IMPLEMENTATION_SLOT) ), bridgeImplementation @@ -469,13 +466,13 @@ contract ForkingManagerTest is Test { // Assert that the ZkEVM contracts match the ones we provided assertEq( - bytesToAddress( + Util.bytesToAddress( vm.load(address(childZkevm1), _IMPLEMENTATION_SLOT) ), zkevmImplementation ); assertEq( - bytesToAddress( + Util.bytesToAddress( vm.load(address(childZkevm2), _IMPLEMENTATION_SLOT) ), zkevmImplementation @@ -509,7 +506,7 @@ contract ForkingManagerTest is Test { // Assert that the forkonomic tokens match the ones we provided assertEq( - bytesToAddress( + Util.bytesToAddress( vm.load( address(childForkonomicToken1), _IMPLEMENTATION_SLOT @@ -518,7 +515,7 @@ contract ForkingManagerTest is Test { forkonomicTokenImplementation ); assertEq( - bytesToAddress( + Util.bytesToAddress( vm.load( address(childForkonomicToken2), _IMPLEMENTATION_SLOT @@ -536,13 +533,13 @@ contract ForkingManagerTest is Test { // Assert that the forkonomic tokens match the ones we provided assertEq( - bytesToAddress( + Util.bytesToAddress( vm.load(address(childGlobalExitRoot1), _IMPLEMENTATION_SLOT) ), globalExitRootImplementation ); assertEq( - bytesToAddress( + Util.bytesToAddress( vm.load(address(childGlobalExitRoot2), _IMPLEMENTATION_SLOT) ), globalExitRootImplementation diff --git a/test/ForkonomicToken.t.sol b/test/ForkonomicToken.t.sol index c789216d..446ff831 100644 --- a/test/ForkonomicToken.t.sol +++ b/test/ForkonomicToken.t.sol @@ -9,9 +9,6 @@ import {TransparentUpgradeableProxy} from "@openzeppelin/contracts/proxy/transpa contract ForkonomicTokenTest is Test { ForkonomicToken public forkonomicToken; - bytes32 internal constant _IMPLEMENTATION_SLOT = - 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc; - address public forkmanager = address(0x123); address public parentContract = address(0x456); address public minter = address(0x789); diff --git a/test/L1GlobalChainInfoPublisher.t.sol b/test/L1GlobalChainInfoPublisher.t.sol index ee8abecf..2baa4aea 100644 --- a/test/L1GlobalChainInfoPublisher.t.sol +++ b/test/L1GlobalChainInfoPublisher.t.sol @@ -1,9 +1,7 @@ pragma solidity ^0.8.20; -/* solhint-disable not-rely-on-time */ /* solhint-disable reentrancy */ /* solhint-disable quotes */ -/* solhint-disable not-rely-on-time */ import {Test} from "forge-std/Test.sol"; import {VerifierRollupHelperMock} from "@RealityETH/zkevm-contracts/contracts/mocks/VerifierRollupHelperMock.sol"; @@ -97,10 +95,6 @@ contract L1GlobalChainInfoPublisherTest is Test { event Transfer(address indexed from, address indexed to, uint256 tokenId); - function bytesToAddress(bytes32 b) public pure returns (address) { - return address(uint160(uint256(b))); - } - function setUp() public { bridgeImplementation = address(new ForkableBridge()); bridge = ForkableBridge( diff --git a/test/testcontract/CreateChildrenWrapper.sol b/test/testcontract/CreateChildrenWrapper.sol new file mode 100644 index 00000000..1290e7c9 --- /dev/null +++ b/test/testcontract/CreateChildrenWrapper.sol @@ -0,0 +1,29 @@ +// SPDX-License-Identifier: GPL-3.0-only +pragma solidity ^0.8.20; + +// Import the library you want to test +import {CreateChildren} from "../../contracts/lib/CreateChildren.sol"; + +contract CreateChildrenWrapper { + // Constructor to set initial values in storage slots + constructor( + ) {} + + // Function to get the current implementation address for testing purposes + function getImplementation() external view returns (address) { + return CreateChildren.getImplementation(); + } + + // Function to get the current admin address for testing purposes + function getAdmin() external view returns (address) { + return CreateChildren.getAdmin(); + } + + // Function to create children contracts using the library + function createChildren() + external + returns (address child1, address child2) + { + return CreateChildren.createChildren(); + } +} \ No newline at end of file diff --git a/test/testcontract/ForkableStructureWrapper.sol b/test/testcontract/ForkableStructureWrapper.sol index dee2cf7d..39938089 100644 --- a/test/testcontract/ForkableStructureWrapper.sol +++ b/test/testcontract/ForkableStructureWrapper.sol @@ -17,4 +17,12 @@ contract ForkableStructureWrapper is ForkableStructure { function setChild(uint256 index, address child) public { children[index] = child; } + + function onlyBeforeForkingTesting() public view onlyBeforeForking {} + + function onlyAfterForkingTesting() public view onlyAfterForking {} + + function onlyParentContractTesting() public view onlyParent {} + + function onlyForkManagerTesting() public view onlyForkManger {} } diff --git a/test/utils/Util.sol b/test/utils/Util.sol index 21c91e09..79fd8745 100644 --- a/test/utils/Util.sol +++ b/test/utils/Util.sol @@ -1,6 +1,11 @@ pragma solidity ^0.8.20; library Util { + bytes32 internal constant _IMPLEMENTATION_SLOT = + 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc; + bytes32 internal constant _ADMIN_SLOT = + 0xb53127684a568b3173ae13b9f8a6016e243e63b6e8ee1178d6a717850b5d6103; + function bytesToAddress(bytes32 b) public pure returns (address) { return address(uint160(uint256(b))); }