Fix : #234 Missing checks for address(0x0) when updating address stat… (

#240) * Fix : #234 Missing checks for address(0x0) when updating address state variables Summary of Changes Made Input Validation in setL1L2CollectionMapping: Added checks to ensure collectionL1 is not address(0) and collectionL2 is not 0 to prevent invalid mappings. Event Emission: Emitted L1L2CollectionMappingUpdated after setting the mapping for transparency. * add setL1L2collectionMapping test in Bridge.t.sol * Update Bridge.sol * Bridge.sol * Update Bridge.t.sol rename test functions * Update Bridge.sol remove require and use if and revert or error * Update Bridge.t.sol * Update Bridge.t.sol vm.expectRevert(InvalidL2AddressError.selector);
ArkProjectNFTs · Sep 30, 2024 · 09ae423 · 09ae423
1 parent fed93cb
commit 09ae423
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 1 deletion.
diff --git a/apps/blockchain/ethereum/src/Bridge.sol b/apps/blockchain/ethereum/src/Bridge.sol
@@ -21,6 +21,8 @@ error CollectionMappingError();
 error NotWhiteListedError();
 error BridgeNotEnabledError();
 error TooManyTokensError();
+error InvalidL1AddressError();
+error InvalidL2AddressError();
 
 uint256 constant MAX_PAYLOAD_LENGTH = 300;
 
@@ -261,7 +263,7 @@ contract Starklane is IStarklaneEvent, UUPSOwnableProxied, StarklaneState, Stark
         address collectionL1 = req.collectionL1;
         for (uint256 i = 0; i < req.tokenIds.length; i++) {
             uint256 id = req.tokenIds[i];
-            _withdrawFromEscrow(ctype, collectionL1, req.ownerL1, id);
+             _withdrawFromEscrow(ctype, collectionL1, req.ownerL1, id);
         }
     }
 
@@ -370,6 +372,12 @@ contract Starklane is IStarklaneEvent, UUPSOwnableProxied, StarklaneState, Stark
         snaddress collectionL2,
         bool force
     ) external onlyOwner {
+          if (collectionL1 == address(0x0)) {
+              revert InvalidL1AddressError();
+            }
+          if (snaddress.unwrap(collectionL2) == 0x0) {
+               revert InvalidL2AddressError();
+            }
         _setL1L2AddressMapping(collectionL1, collectionL2, force);
         emit L1L2CollectionMappingUpdated(collectionL1, snaddress.unwrap(collectionL2));
     }

diff --git a/apps/blockchain/ethereum/test/Bridge.t.sol b/apps/blockchain/ethereum/test/Bridge.t.sol
@@ -29,6 +29,10 @@ contract BridgeTest is Test, IStarklaneEvent {
     address erc1155C1;
     address snCore;
 
+    address collectionL1= address(0x1);
+    snaddress collectionL2 = Cairo.snaddressWrap(0x2);
+
+
     address payable internal alice;
     address payable internal bob;
 
@@ -600,5 +604,31 @@ contract BridgeTest is Test, IStarklaneEvent {
         assert(payload.length == reqContent.length);
         return (nonce, payload);
     }
+
+    // here is test   for set L1 L2 collection 
+
+    function test_SetL1L2CollectionMapping() public {
+
+        // Check for the event emission
+        vm.expectEmit(true, true, true, true);
+        emit L1L2CollectionMappingUpdated(collectionL1, snaddress.unwrap(collectionL2));
+       IStarklane(bridge).setL1L2CollectionMapping(collectionL1, collectionL2, false);
+    }
+
+    function test_SetL1L2CollectionMappingWith_InvalidL1Address() public {
+         vm.expectRevert(InvalidL1AddressError.selector);
+       IStarklane(bridge).setL1L2CollectionMapping(address(0), collectionL2, false);
+    }
+
+    function test_SetL1L2CollectionMappingWith_InvalidL2Address() public {
+        vm.expectRevert(InvalidL2AddressError.selector);
+        IStarklane(bridge).setL1L2CollectionMapping(collectionL1, Cairo.snaddressWrap(0), false);
+    }
+
+    function test_SetL1L2CollectionMappingWith_InvalidUnwrappedL2Address() public {
+        snaddress invalidCollectionL2 = Cairo.snaddressWrap(0x0); 
+        vm.expectRevert(InvalidL2AddressError.selector);
+        IStarklane(bridge).setL1L2CollectionMapping(collectionL1, invalidCollectionL2, false);
+    }
 
 }