Update ownable check to validate zero address (#398)

* Update ownable check to validate zero address

* Update tests/mocks/Ownable.cairo

Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com>

* Update documentation

* Update docs/Access.md

Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com>

* Update tests/access/test_Ownable.py

Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com>

* update ownable test

* Apply suggestions from code review

Co-authored-by: Andrew Fleming <fleming.andrew@protonmail.com>
Co-authored-by: Martín Triay <martriay@gmail.com>

docs/Access.md

@@ -116,7 +116,7 @@ None.
 
 #### `assert_only_owner`
 
-Reverts if called by any account other than the owner.
+Reverts if called by any account other than the owner. In case of renounced ownership, any call from the default zero address will also be reverted.
 
 Parameters:
 

src/openzeppelin/access/ownable.cairo

@@ -49,6 +49,7 @@ namespace Ownable:
         let (owner) = Ownable.owner()
         let (caller) = get_caller_address()
         with_attr error_message("Ownable: caller is not the owner"):
+            assert_not_zero(caller)
             assert owner = caller
         end
         return ()

tests/access/test_Ownable.py

@@ -5,7 +5,8 @@
     ZERO_ADDRESS,
     assert_event_emitted,
     get_contract_class,
-    cached_contract
+    cached_contract,
+    assert_revert
 )
 
 
@@ -85,6 +86,17 @@ async def test_renounceOwnership(ownable_factory):
     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
+    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 not the owner"
+    )
+
 
 @pytest.mark.asyncio
 async def test_renounceOwnership_emits_event(ownable_factory):

tests/mocks/Ownable.cairo

@@ -5,6 +5,7 @@
 from starkware.cairo.common.cairo_builtins import HashBuiltin
 from starkware.starknet.common.syscalls import get_caller_address
 from openzeppelin.access.ownable import Ownable
+from starkware.cairo.common.bool import TRUE
 
 @constructor
 func constructor{
@@ -45,3 +46,13 @@ func renounceOwnership{
     Ownable.renounce_ownership()
     return ()
 end
+
+@external
+func protected_function{
+        syscall_ptr : felt*,
+        pedersen_ptr : HashBuiltin*,
+        range_check_ptr
+    }() -> (res: felt):
+    Ownable.assert_only_owner()
+    return (TRUE)
+end