Additions of tests and spelling corrects

.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"
   }
 }

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

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);

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();

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();

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.
      */

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
+        );
+    }
+}

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();
+    }
 }