Skip to content

Commit

Permalink
BCI-1289: Add Ownable test helper (#257)
Browse files Browse the repository at this point in the history
* contracts/.gitignore: remove typechain-types

* Move cairo 0.x code to oldsrc/

* Initial boilerplate for Cairo 1.0

Based on https://github.com/auditless/cairo-template

* create confirmed owner contract

* Remove old helix config

* Add ocr2 aggregator skeleton

* ownable.cairo: update storage

* Scarb.toml: add starknet contract target

* cairo_project.toml: add crate_roots to refer to crate deps

* libraries: add access controller trait, simple write access controller

* simple_write_access_controller.cairo: add constructor, rename to _check_enabled to match previous contract

* simple_read_access_controller.cairo: add skeleton

* aggregator.cairo: Round derives Serde so that it can be returned by dispatch

* aggregator.cairo: change Round.round_id into felt252

* ocr2: add initial aggregator proxy

* ocr2: partial set_config implementation, events

* Scarb.toml: cairo-test needs --starknet flag

* Add span hashing, implement most of set_config, part of transmit

* Billing config, query functions

* Implement payee management, payments and resolve some TODOs

* Clean up recursion in set_payee too

* Add ERC20 calls via abi dispatcher

* Use Ownable in the aggregator

* Re-export ownership functions

* Slight cleanup

* Implement withdraw_funds

* Use access controllers as libraries in aggregator and proxy

* Add a vendor/ directory with cairo and scarb version pinned

* Remove unused files

* ocr2/aggregator: clamp config digest hash, add prefix

* Add withdraw_gas_all annotations around recursion

* aggregator.cairo: pass signatures by reference with verify_signatures

* aggregator.cairo: implement duplicate signer checking

* aggregator.cairo: deref median immediately

* Add the billing controller dispatch in billing access check

* Handle negative values in link_available_for_payment

* Tightly pack Transmission for storage cost savings

* Optimize Oracle struct storage

* Use the same split_felt function

* Resolve more TODOs

* aggregator.cairo: add TODOs to pow

* contracts: add initial tests

* aggregator.cairo: set _oracles_len to index after adding oracles

* aggregator.cairo: read oracle before incrementing index

* Aggregator: add transmitters view function

* multisig.cairo: initial implementation

* aggregator.cairo: add withdraw_gas_all to transmitters_ for cairo-test

* contracts: add initial test_multisig

* BCI-990: Update er protocol to Cairo 1.0 (#228)

* rewrite uptime feed

* remove partial eq

* code review comments

* add external functions from simplewriteaccesscontroller to aggregator + aggregator proxy contracts

* get rid of parens + cairo-format

* remove unnecessary traits

* Fix compilation on alpha.6

* Makefile: build cairo, scarb, and contract tests

* contracts.yml: run cairo tests

* amarna.yml: disable amarna workflow

* contracts/requirements.txt: remove cairo-lang and openzeppelin-cairo-contracts

* Makefile: build and run cairo-format

* lint.yml: checkout cairo for cairo-format

* Makefile: assume the deps are already available in the env

* Install cairo and scarb from github releases

* also download cairo for format check

* Fix the format check

* Resolve formatter issues

* Disable examples check for now since we haven't updated them yet

* contracts/tests/test_multisig.cairo: add tests

* contracts/ocr2/aggregator_proxy.cairo: add external round_data and latest_round_data

* BCI-1188: Rewrite Link Token Cairo 1.0 (#234)

* Makefile: build contracts with release profile

* contracts: import account contracts

* Solidity contracts are still used, moved to solidity/

* Remove oldsrc/

* StarknetValidator.test.ts: yarn format

* Only install venv on CI actions that need it (startNetwork)

* Ignore the rest of the vendor/ folder

* contracts/emergency/sequencer_uptime_feed.cairo: spell transfer_ownership correctly

* port over mock aggregator (#250)

* aggregator: Resolve a couple TODOs

* Extract split_felt to utils

* proxy: We initialize with an initial value so it will never be 0

* aggregator: Reintroduce calculate_reimbursement

* relayer: Stricter assertions on report format

* contracts/ocr2/aggregator_proxy.cairo: require access for latest_round_data and round_data

* contracts/libraries/simple_read_access_controller.cairo: check_access should call SimpleReadAccessController::has_access, readd comment about offchain calls

* contracts/ocr2/mocks/mock_aggregator.cairo: set latest round id

* Update account to use latest implementation

* On rc0 it's no longer necessary to import this

* Integer conversions can now be direct without going through felt252

* Use the new format for should_panic

* cairo format

* Test ERC677 with contract dispatch (#241)

* port aggregator proxy tests to cairo-test

* update to cairo-rc1.0 and 0.2.0-alpha.2

* contracts/ocr2/aggregator.cairo: fix epoch_and_round assertion

* contracts/ocr2/aggregator.cairo: finalize hash with length

* camelcase vars and undo delete file

* fix file name

* BCI-1259: Add upgradeability (#251)

* contracts/ocr2/aggregator.cairo: add billing function

* contracts: run cairo-format

* .github/actions/install-cairo/action.yml: add action

* .github/workflows/contracts.yml: use install-cairo action

* add ownable test helper

* add ownable functions to access controllers

* rename ownership -> ownable

* rename fn to should_implement_ownable

* missed merge conflicts

* fix merge errs

* revert linting

* add IOwnable to Ownable file

* trailing whitespace

---------

Co-authored-by: cfal <[email protected]>
Co-authored-by: Blaž Hrastnik <[email protected]>
Co-authored-by: Augustus <[email protected]>
Co-authored-by: Calvin Wang <[email protected]>
Co-authored-by: Augustus Chang <[email protected]>
Co-authored-by: Calvin Wang <[email protected]>
  • Loading branch information
7 people authored May 12, 2023
1 parent 3d2af60 commit 45f8067
Show file tree
Hide file tree
Showing 7 changed files with 139 additions and 13 deletions.
11 changes: 11 additions & 0 deletions contracts/src/libraries/ownable.cairo
Original file line number Diff line number Diff line change
@@ -1,3 +1,14 @@
use starknet::ContractAddress;

#[abi]
trait IOwnable {
fn owner() -> ContractAddress;
fn proposed_owner() -> ContractAddress;
fn transfer_ownership(new_owner: ContractAddress);
fn accept_ownership();
fn renounce_ownership();
}

#[contract]
mod Ownable {
use starknet::ContractAddress;
Expand Down
30 changes: 29 additions & 1 deletion contracts/src/tests/test_aggregator.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,18 @@ use starknet::testing::set_caller_address;
use starknet::ContractAddress;
use starknet::contract_address_const;
use starknet::class_hash::class_hash_const;
use starknet::class_hash::Felt252TryIntoClassHash;
use starknet::syscalls::deploy_syscall;

use array::ArrayTrait;
use traits::Into;
use traits::TryInto;
use option::OptionTrait;
use core::result::ResultTrait;

use chainlink::ocr2::aggregator::pow;
use chainlink::ocr2::aggregator::Aggregator;

use chainlink::tests::test_ownable::should_implement_ownable;

// TODO: aggregator tests

Expand Down Expand Up @@ -52,6 +60,26 @@ fn setup() -> ContractAddress {
account
}

#[test]
#[available_gas(2000000)]
fn test_ownable() {
let account = setup();
// Deploy aggregator
let mut calldata = ArrayTrait::new();
calldata.append(account.into()); // owner
calldata.append(contract_address_const::<777>().into()); // link token
calldata.append(0); // min_answer
calldata.append(100); // max_answer
calldata.append(contract_address_const::<999>().into()); // billing access controller
calldata.append(8); // decimals
calldata.append(123); // description
let (aggregatorAddr, _) = deploy_syscall(
Aggregator::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false
).unwrap();

should_implement_ownable(aggregatorAddr, account);
}


#[test]
#[available_gas(2000000)]
Expand Down
16 changes: 16 additions & 0 deletions contracts/src/tests/test_aggregator_proxy.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use chainlink::ocr2::mocks::mock_aggregator::MockAggregator;
use chainlink::ocr2::aggregator_proxy::AggregatorProxy;
use chainlink::ocr2::aggregator::Round;
use chainlink::utils::split_felt;
use chainlink::tests::test_ownable::should_implement_ownable;

#[abi]
trait IMockAggregator {
Expand Down Expand Up @@ -54,6 +55,21 @@ fn setup() -> (
(account, mockAggregatorAddr1, mockAggregator1, mockAggregatorAddr2, mockAggregator2)
}

#[test]
#[available_gas(2000000)]
fn test_ownable() {
let (account, mockAggregatorAddr, _, _, _) = setup();
// Deploy aggregator proxy
let mut calldata = ArrayTrait::new();
calldata.append(account.into()); // owner = account
calldata.append(mockAggregatorAddr.into());
let (aggregatorProxyAddr, _) = deploy_syscall(
AggregatorProxy::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false
).unwrap();

should_implement_ownable(aggregatorProxyAddr, account);
}

#[test]
#[available_gas(2000000)]
#[should_panic(expected: ('Ownable: caller is not owner', ))]
Expand Down
14 changes: 8 additions & 6 deletions contracts/src/tests/test_erc677.cairo
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
use chainlink::libraries::token::erc677::ERC677;
use starknet::ContractAddress;
use starknet::contract_address_const;
use starknet::testing::set_caller_address;
use starknet::syscalls::deploy_syscall;
use starknet::class_hash::Felt252TryIntoClassHash;

use array::ArrayTrait;
use traits::Into;
use zeroable::Zeroable;
use chainlink::token::mock::valid_erc667_receiver::ValidReceiver;
use chainlink::token::mock::invalid_erc667_receiver::InvalidReceiver;
use starknet::syscalls::deploy_syscall;
use traits::TryInto;
use zeroable::Zeroable;
use option::OptionTrait;
use starknet::class_hash::Felt252TryIntoClassHash;
use core::result::ResultTrait;

use chainlink::token::mock::valid_erc667_receiver::ValidReceiver;
use chainlink::token::mock::invalid_erc667_receiver::InvalidReceiver;
use chainlink::libraries::token::erc677::ERC677;

#[abi]
trait MockValidReceiver {
fn on_token_transfer(sender: ContractAddress, value: u256, data: Array<felt252>);
Expand Down
30 changes: 27 additions & 3 deletions contracts/src/tests/test_link_token.cairo
Original file line number Diff line number Diff line change
@@ -1,10 +1,19 @@
use chainlink::token::link_token::LinkToken;
use zeroable::Zeroable;
use starknet::ContractAddress;
use starknet::testing::set_caller_address;
use starknet::contract_address_const;
use traits::Into;
use starknet::class_hash::class_hash_const;
use starknet::class_hash::Felt252TryIntoClassHash;
use starknet::syscalls::deploy_syscall;

use array::ArrayTrait;
use traits::Into;
use traits::TryInto;
use zeroable::Zeroable;
use option::OptionTrait;
use core::result::ResultTrait;

use chainlink::token::link_token::LinkToken;
use chainlink::tests::test_ownable::should_implement_ownable;

// only tests link token specific functionality
// erc20 and erc677 functionality is already tested elsewhere
Expand All @@ -16,6 +25,21 @@ fn setup() -> ContractAddress {
account
}

#[test]
#[available_gas(2000000)]
fn test_ownable() {
let account = setup();
// Deploy LINK token
let mut calldata = ArrayTrait::new();
calldata.append(class_hash_const::<123>().into()); // minter
calldata.append(account.into()); // owner
let (linkAddr, _) = deploy_syscall(
LinkToken::TEST_CLASS_HASH.try_into().unwrap(), 0, calldata.span(), false
).unwrap();

should_implement_ownable(linkAddr, account);
}

#[test]
#[available_gas(2000000)]
#[should_panic(expected: ('minter is 0', ))]
Expand Down
6 changes: 4 additions & 2 deletions contracts/src/tests/test_multisig.cairo
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
use array::ArrayTrait;
use starknet::contract_address_const;
use starknet::testing::set_caller_address;
use starknet::class_hash::class_hash_const;

use array::ArrayTrait;

use chainlink::multisig::assert_unique_values;
use chainlink::multisig::Multisig;
use starknet::class_hash::class_hash_const;

// TODO: test execute_confirmation happy path with mocked
// call_contract_syscall
Expand Down
45 changes: 44 additions & 1 deletion contracts/src/tests/test_ownable.cairo
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use chainlink::libraries::ownable::Ownable;
use starknet::contract_address_const;
use starknet::ContractAddress;
use starknet::testing::set_caller_address;
use starknet::testing::set_contract_address;
use zeroable::Zeroable;
use chainlink::libraries::ownable::Ownable;

fn setup() -> (ContractAddress, ContractAddress) {
let owner: ContractAddress = contract_address_const::<1>();
Expand Down Expand Up @@ -121,3 +122,45 @@ fn test_renounce_ownership_panics_if_not_owner() {
set_caller_address(other_user);
Ownable::renounce_ownership();
}
//
// General ownable contract tests
//

#[abi]
trait IOwnable {
fn owner() -> ContractAddress;
fn proposed_owner() -> ContractAddress;
fn transfer_ownership(new_owner: ContractAddress);
fn accept_ownership();
fn renounce_ownership();
}

fn should_implement_ownable(contract_addr: ContractAddress, owner: ContractAddress) {
let contract = IOwnableDispatcher { contract_address: contract_addr };
let acc2: ContractAddress = contract_address_const::<2222>();

// check owner is set correctly
assert(owner == contract.owner(), 'owner does not match');
let caller = starknet::get_caller_address();
assert(!caller.is_zero(), 'test1');

// transfer ownership - check owner unchanged and proposed owner set correctly
set_contract_address(owner); // required to call contract as owner
let caller = starknet::get_caller_address();
assert(!caller.is_zero(), 'test2');
assert(caller == owner, 'test3');
contract.transfer_ownership(acc2);
assert(owner == contract.owner(), 'owner should remain unchanged');
assert(acc2 == contract.proposed_owner(), 'acc2 should be proposed owner');

// accept ownership - check owner changed and proposed owner set to zero
set_contract_address(acc2); // required to call function as acc2
contract.accept_ownership();
assert(contract.owner() == acc2, 'failed to change ownership');
assert(contract.proposed_owner().is_zero(), 'proposed owner should be zero');

// renounce ownership
contract.renounce_ownership();
assert(contract.owner().is_zero(), 'owner not 0 after renounce');
}

0 comments on commit 45f8067

Please sign in to comment.