Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update ownable check to validate zero address #398

Merged
merged 9 commits into from
Jul 19, 2022
2 changes: 1 addition & 1 deletion docs/Access.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,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:

Expand Down
1 change: 1 addition & 0 deletions src/openzeppelin/access/ownable.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -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 ()
Expand Down
14 changes: 13 additions & 1 deletion tests/access/test_Ownable.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
ZERO_ADDRESS,
assert_event_emitted,
get_contract_class,
cached_contract
cached_contract,
assert_revert
)


Expand Down Expand Up @@ -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_post_renounceOwnership(ownable_factory):
martriay marked this conversation as resolved.
Show resolved Hide resolved
ownable, owner = ownable_factory
await signer.send_transaction(owner, ownable.contract_address, 'renounceOwnership', [])

# Protected function cannot be called from zero address
martriay marked this conversation as resolved.
Show resolved Hide resolved
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):
Expand Down
11 changes: 11 additions & 0 deletions tests/mocks/Ownable.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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