cairo-contracts icon indicating copy to clipboard operation
cairo-contracts copied to clipboard
verified publisher icon OpenZeppelin

Ownable: split assertion checks in two statements

Open achab opened this issue 3 years ago • 2 comments

Fixes #421

This PR splits two checks about address in Ownable contracts in two separate with_attr statements. Currently, the error message only corresponds to the second check.

PR Checklist

  • [x] Tests
  • [ ] Documentation
  • [ ] Changelog entry

achab avatar Aug 09 '22 11:08 achab

Thanks for this improvement, it is now clear that we're not properly testing for the non owner case, just the zero address special case. We do have a test on ERC721SafeMintableMock, not on the library tests.

Could you add the missing tests?

martriay avatar Aug 10 '22 06:08 martriay

Hi there, thanks for the review! I've just added a small test to check that it reverts if the function protected_function is called by an account different from the owner.

achab avatar Aug 12 '22 00:08 achab