From f99a0753ee874b81ce1bc73a58a0fdc0285ceaa2 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Thu, 13 Jul 2023 13:57:26 +0200 Subject: [PATCH 1/4] feat: add owner param to ownable initializer --- src/openzeppelin/access/ownable/ownable.cairo | 5 ++--- .../tests/access/test_dual_ownable.cairo | 18 +++++++++++------- .../tests/access/test_ownable.cairo | 2 +- .../tests/mocks/dual_ownable_mocks.cairo | 8 ++++---- 4 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/openzeppelin/access/ownable/ownable.cairo b/src/openzeppelin/access/ownable/ownable.cairo index 1027d4ca7..29e21dde0 100644 --- a/src/openzeppelin/access/ownable/ownable.cairo +++ b/src/openzeppelin/access/ownable/ownable.cairo @@ -71,9 +71,8 @@ mod Ownable { // Internals #[internal] - fn initializer() { - let caller: ContractAddress = get_caller_address(); - _transfer_ownership(caller); + fn initializer(owner: ContractAddress) { + _transfer_ownership(owner); } #[internal] diff --git a/src/openzeppelin/tests/access/test_dual_ownable.cairo b/src/openzeppelin/tests/access/test_dual_ownable.cairo index c093838fc..543e9ca4e 100644 --- a/src/openzeppelin/tests/access/test_dual_ownable.cairo +++ b/src/openzeppelin/tests/access/test_dual_ownable.cairo @@ -1,3 +1,4 @@ +use core::clone::Clone; use array::ArrayTrait; use starknet::ContractAddress; use starknet::contract_address_const; @@ -17,6 +18,7 @@ use openzeppelin::tests::mocks::dual_ownable_mocks::SnakeOwnablePanicMock; use openzeppelin::tests::mocks::dual_ownable_mocks::CamelOwnablePanicMock; use openzeppelin::tests::mocks::non_implementing_mock::NonImplementingMock; use openzeppelin::tests::utils; +use openzeppelin::utils::serde::SerializedAppend; // // Constants @@ -36,14 +38,14 @@ fn NEW_OWNER() -> ContractAddress { fn setup_snake() -> (DualCaseOwnable, IOwnableDispatcher) { let mut calldata = ArrayTrait::new(); - set_caller_address(OWNER()); + calldata.append_serde(OWNER()); let target = utils::deploy(SnakeOwnableMock::TEST_CLASS_HASH, calldata); (DualCaseOwnable { contract_address: target }, IOwnableDispatcher { contract_address: target }) } fn setup_camel() -> (DualCaseOwnable, IOwnableCamelDispatcher) { let mut calldata = ArrayTrait::new(); - set_caller_address(OWNER()); + calldata.append_serde(OWNER()); let target = utils::deploy(CamelOwnableMock::TEST_CLASS_HASH, calldata); ( DualCaseOwnable { @@ -55,16 +57,18 @@ fn setup_camel() -> (DualCaseOwnable, IOwnableCamelDispatcher) { } fn setup_non_ownable() -> DualCaseOwnable { - let calldata = ArrayTrait::new(); - set_caller_address(OWNER()); + let mut calldata = ArrayTrait::new(); + calldata.append_serde(OWNER()); let target = utils::deploy(NonImplementingMock::TEST_CLASS_HASH, calldata); DualCaseOwnable { contract_address: target } } fn setup_ownable_panic() -> (DualCaseOwnable, DualCaseOwnable) { - set_caller_address(OWNER()); - let snake_target = utils::deploy(SnakeOwnablePanicMock::TEST_CLASS_HASH, ArrayTrait::new()); - let camel_target = utils::deploy(CamelOwnablePanicMock::TEST_CLASS_HASH, ArrayTrait::new()); + let mut calldata = ArrayTrait::new(); + calldata.append_serde(OWNER()); + + let snake_target = utils::deploy(SnakeOwnablePanicMock::TEST_CLASS_HASH, calldata.clone()); + let camel_target = utils::deploy(CamelOwnablePanicMock::TEST_CLASS_HASH, calldata); ( DualCaseOwnable { contract_address: snake_target diff --git a/src/openzeppelin/tests/access/test_ownable.cairo b/src/openzeppelin/tests/access/test_ownable.cairo index 7f3980699..a3a4f0af9 100644 --- a/src/openzeppelin/tests/access/test_ownable.cairo +++ b/src/openzeppelin/tests/access/test_ownable.cairo @@ -19,7 +19,7 @@ fn OTHER() -> ContractAddress { fn setup() { testing::set_caller_address(OWNER()); - Ownable::initializer(); + Ownable::initializer(OWNER()); } // diff --git a/src/openzeppelin/tests/mocks/dual_ownable_mocks.cairo b/src/openzeppelin/tests/mocks/dual_ownable_mocks.cairo index f84bc195e..407d63e3b 100644 --- a/src/openzeppelin/tests/mocks/dual_ownable_mocks.cairo +++ b/src/openzeppelin/tests/mocks/dual_ownable_mocks.cairo @@ -4,8 +4,8 @@ mod SnakeOwnableMock { use openzeppelin::access::ownable::Ownable; #[constructor] - fn constructor() { - Ownable::initializer(); + fn constructor(owner: ContractAddress) { + Ownable::initializer(owner); } #[view] @@ -30,8 +30,8 @@ mod CamelOwnableMock { use openzeppelin::access::ownable::Ownable; #[constructor] - fn constructor() { - Ownable::initializer(); + fn constructor(owner: ContractAddress) { + Ownable::initializer(owner); } #[view] From b3655fc2976d24a6b8ca71c47577cd377c416268 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Thu, 13 Jul 2023 17:04:08 +0200 Subject: [PATCH 2/4] feat: apply review updates --- src/openzeppelin/tests/access/test_dual_ownable.cairo | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/openzeppelin/tests/access/test_dual_ownable.cairo b/src/openzeppelin/tests/access/test_dual_ownable.cairo index 543e9ca4e..b2176c974 100644 --- a/src/openzeppelin/tests/access/test_dual_ownable.cairo +++ b/src/openzeppelin/tests/access/test_dual_ownable.cairo @@ -58,17 +58,13 @@ fn setup_camel() -> (DualCaseOwnable, IOwnableCamelDispatcher) { fn setup_non_ownable() -> DualCaseOwnable { let mut calldata = ArrayTrait::new(); - calldata.append_serde(OWNER()); let target = utils::deploy(NonImplementingMock::TEST_CLASS_HASH, calldata); DualCaseOwnable { contract_address: target } } fn setup_ownable_panic() -> (DualCaseOwnable, DualCaseOwnable) { - let mut calldata = ArrayTrait::new(); - calldata.append_serde(OWNER()); - - let snake_target = utils::deploy(SnakeOwnablePanicMock::TEST_CLASS_HASH, calldata.clone()); - let camel_target = utils::deploy(CamelOwnablePanicMock::TEST_CLASS_HASH, calldata); + let snake_target = utils::deploy(SnakeOwnablePanicMock::TEST_CLASS_HASH, ArrayTrait::new()); + let camel_target = utils::deploy(CamelOwnablePanicMock::TEST_CLASS_HASH, ArrayTrait::new()); ( DualCaseOwnable { contract_address: snake_target From 97dd40d46393ace3cd31c236f91fa383580a8eb7 Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Thu, 13 Jul 2023 17:52:42 +0200 Subject: [PATCH 3/4] fix: remove unnecessary mut --- src/openzeppelin/tests/access/test_dual_ownable.cairo | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/openzeppelin/tests/access/test_dual_ownable.cairo b/src/openzeppelin/tests/access/test_dual_ownable.cairo index b2176c974..719cec681 100644 --- a/src/openzeppelin/tests/access/test_dual_ownable.cairo +++ b/src/openzeppelin/tests/access/test_dual_ownable.cairo @@ -57,7 +57,7 @@ fn setup_camel() -> (DualCaseOwnable, IOwnableCamelDispatcher) { } fn setup_non_ownable() -> DualCaseOwnable { - let mut calldata = ArrayTrait::new(); + let calldata = ArrayTrait::new(); let target = utils::deploy(NonImplementingMock::TEST_CLASS_HASH, calldata); DualCaseOwnable { contract_address: target } } From 90fbf4179eb3640efd6b4a3306ef1bb5900c0cbc Mon Sep 17 00:00:00 2001 From: Eric Nordelo Date: Thu, 13 Jul 2023 18:59:02 +0200 Subject: [PATCH 4/4] Update src/openzeppelin/tests/access/test_dual_ownable.cairo Co-authored-by: Andrew Fleming --- src/openzeppelin/tests/access/test_dual_ownable.cairo | 1 - 1 file changed, 1 deletion(-) diff --git a/src/openzeppelin/tests/access/test_dual_ownable.cairo b/src/openzeppelin/tests/access/test_dual_ownable.cairo index 719cec681..87841fe8d 100644 --- a/src/openzeppelin/tests/access/test_dual_ownable.cairo +++ b/src/openzeppelin/tests/access/test_dual_ownable.cairo @@ -1,4 +1,3 @@ -use core::clone::Clone; use array::ArrayTrait; use starknet::ContractAddress; use starknet::contract_address_const;