Ownable: split assertion checks in two statements (#422)

* split assertion checks in two statements

* fix tests

* added one test for non owner call

src/openzeppelin/access/ownable/library.cairo

@@ -48,8 +48,10 @@ namespace Ownable:
         }():
         let (owner) = Ownable.owner()
         let (caller) = get_caller_address()
-        with_attr error_message("Ownable: caller is not the owner"):
+        with_attr error_message("Ownable: caller is the zero address"):
             assert_not_zero(caller)
+        end
+        with_attr error_message("Ownable: caller is not the owner"):
             assert owner = caller
         end
         return ()

tests/access/test_Ownable.py

@@ -30,29 +30,31 @@ async def ownable_init(contract_classes):
         contract_class=ownable_cls,
         constructor_calldata=[owner.contract_address]
     )
-    return starknet.state, ownable, owner
+    not_owner = await Account.deploy(signer.public_key)
+    return starknet.state, ownable, owner, not_owner
 
 
 @pytest.fixture
 def ownable_factory(contract_classes, ownable_init):
     account_cls, ownable_cls = contract_classes
-    state, ownable, owner = ownable_init
+    state, ownable, owner, not_owner = ownable_init
     _state = state.copy()
     owner = cached_contract(_state, account_cls, owner)
     ownable = cached_contract(_state, ownable_cls, ownable)
-    return ownable, owner
+    not_owner = cached_contract(_state, account_cls, not_owner)
+    return ownable, owner, not_owner
 
 
 @pytest.mark.asyncio
 async def test_constructor(ownable_factory):
-    ownable, owner = ownable_factory
+    ownable, owner, _ = ownable_factory
     expected = await ownable.owner().call()
     assert expected.result.owner == owner.contract_address
 
 
 @pytest.mark.asyncio
 async def test_transferOwnership(ownable_factory):
-    ownable, owner = ownable_factory
+    ownable, owner, _ = ownable_factory
     new_owner = 123
     await signer.send_transaction(owner, ownable.contract_address, 'transferOwnership', [new_owner])
     executed_info = await ownable.owner().call()
@@ -61,7 +63,7 @@ async def test_transferOwnership(ownable_factory):
 
 @pytest.mark.asyncio
 async def test_transferOwnership_emits_event(ownable_factory):
-    ownable, owner = ownable_factory
+    ownable, owner, _ = ownable_factory
     new_owner = 123
     tx_exec_info = await signer.send_transaction(owner, ownable.contract_address, 'transferOwnership', [new_owner])
 
@@ -78,26 +80,36 @@ async def test_transferOwnership_emits_event(ownable_factory):
 
 @pytest.mark.asyncio
 async def test_renounceOwnership(ownable_factory):
-    ownable, owner = ownable_factory
+    ownable, owner, _ = ownable_factory
     await signer.send_transaction(owner, ownable.contract_address, 'renounceOwnership', [])
     executed_info = await ownable.owner().call()
     assert executed_info.result == (ZERO_ADDRESS,)
 
 @pytest.mark.asyncio
 async def test_contract_without_owner(ownable_factory):
-    ownable, owner = ownable_factory
+    ownable, owner, _ = ownable_factory
     await signer.send_transaction(owner, ownable.contract_address, 'renounceOwnership', [])
 
     # Protected function should not be called from zero address
     await assert_revert(
         ownable.protected_function().invoke(),
+        reverted_with="Ownable: caller is the zero address"
+    )
+
+@pytest.mark.asyncio
+async def test_contract_caller_not_owner(ownable_factory):
+    ownable, owner, not_owner = ownable_factory
+
+    # Protected function should only be called from owner
+    await assert_revert(
+        signer.send_transaction(not_owner, ownable.contract_address, 'protected_function', []),
         reverted_with="Ownable: caller is not the owner"
     )
 
 
 @pytest.mark.asyncio
 async def test_renounceOwnership_emits_event(ownable_factory):
-    ownable, owner = ownable_factory
+    ownable, owner, _ = ownable_factory
     tx_exec_info = await signer.send_transaction(owner, ownable.contract_address, 'renounceOwnership', [])
 
     assert_event_emitted(

tests/token/erc721/test_ERC721SafeMintableMock.py

@@ -167,7 +167,7 @@ async def test_safeMint_from_zero_address(erc721_factory):
             TOKEN,
             DATA
         ).invoke(),
-        reverted_with="Ownable: caller is not the owner"
+        reverted_with="Ownable: caller is the zero address"
     )