diff --git a/contracts/src/account.cairo b/contracts/src/account.cairo index dcda746e8..3318c2c7c 100644 --- a/contracts/src/account.cairo +++ b/contracts/src/account.cairo @@ -1,11 +1,14 @@ -// copied from https://raw.githubusercontent.com/OpenZeppelin/cairo-contracts/861fc416f87addbe23a3b47f9d19ab27c10d5dc8/src/presets/account.cairo (0.9.0) +// copied from +// https://raw.githubusercontent.com/OpenZeppelin/cairo-contracts/861fc416f87addbe23a3b47f9d19ab27c10d5dc8/src/presets/account.cairo +// (0.9.0) // SPDX-License-Identifier: MIT // OpenZeppelin Contracts for Cairo v0.9.0 (presets/account.cairo) /// # Account Preset /// -/// OpenZeppelin's basic account which can change its public key and declare, deploy, or call contracts. +/// OpenZeppelin's basic account which can change its public key and declare, deploy, or call +/// contracts. #[starknet::contract(account)] mod Account { use openzeppelin::account::AccountComponent; diff --git a/contracts/src/emergency/sequencer_uptime_feed.cairo b/contracts/src/emergency/sequencer_uptime_feed.cairo index 68e2a5761..eacde6f95 100644 --- a/contracts/src/emergency/sequencer_uptime_feed.cairo +++ b/contracts/src/emergency/sequencer_uptime_feed.cairo @@ -178,7 +178,8 @@ mod SequencerUptimeFeed { #[l1_handler] fn update_status(ref self: ContractState, from_address: felt252, status: u128, timestamp: u64) { - // Cairo enforces from_address to be a felt252 on the method signature, but we can cast it right after + // Cairo enforces from_address to be a felt252 on the method signature, but we can cast it + // right after let from_address: EthAddress = from_address.try_into().unwrap(); assert(self._l1_sender.read() == from_address, 'EXPECTED_FROM_BRIDGE_ONLY'); diff --git a/contracts/src/libraries/token/erc677.cairo b/contracts/src/libraries/token/erc677.cairo index 20fd6c54a..26ad289b5 100644 --- a/contracts/src/libraries/token/erc677.cairo +++ b/contracts/src/libraries/token/erc677.cairo @@ -12,7 +12,8 @@ trait IERC677Receiver { fn on_token_transfer( ref self: TContractState, sender: ContractAddress, value: u256, data: Array ); - // implements EIP-165, where function selectors are defined by Ethereum ABI using the ethereum function signatures + // implements EIP-165, where function selectors are defined by Ethereum ABI using the ethereum + // function signatures fn supports_interface(ref self: TContractState, interface_id: u32) -> bool; } diff --git a/contracts/src/libraries/upgradeable.cairo b/contracts/src/libraries/upgradeable.cairo index 6b9e6af73..959fe7f3d 100644 --- a/contracts/src/libraries/upgradeable.cairo +++ b/contracts/src/libraries/upgradeable.cairo @@ -25,11 +25,11 @@ mod Upgradeable { // this method assumes replace_class_syscall has a very low possibility of being deprecated // but if it does, we will either have upgraded the contract to be non-upgradeable by then - // because the starknet ecosystem has stabilized or we will be able to upgrade the contract to the proxy pattern - // #[internal] + // because the starknet ecosystem has stabilized or we will be able to upgrade the contract to + // the proxy pattern #[internal] fn upgrade(new_impl: ClassHash) { assert(!new_impl.is_zero(), 'Class hash cannot be zero'); replace_class_syscall(new_impl).unwrap_syscall(); - // TODO: Upgraded(new_impl); + // TODO: Upgraded(new_impl); } } diff --git a/contracts/src/multisig.cairo b/contracts/src/multisig.cairo index 553ea3f46..3a65763cc 100644 --- a/contracts/src/multisig.cairo +++ b/contracts/src/multisig.cairo @@ -343,10 +343,11 @@ mod Multisig { ) .unwrap_syscall(); - // TODO: this shouldn't be necessary. call_contract_syscall returns a Span, which - // is a serialized result, but returning a Span results in an error: + // TODO: this shouldn't be necessary. call_contract_syscall returns a Span, + // which is a serialized result, but returning a Span results in an error: // - // Trait has no implementation in context: core::serde::Serde::> + // Trait has no implementation in context: + // core::serde::Serde::> // // Cairo docs also have an example that returns a Span: // https://github.com/starkware-libs/cairo/blob/fe425d0893ff93a936bb3e8bbbac771033074bdb/docs/reference/src/components/cairo/modules/language_constructs/pages/contracts.adoc#L226 diff --git a/contracts/src/ocr2/aggregator.cairo b/contracts/src/ocr2/aggregator.cairo index 2ff501888..e01c127b5 100644 --- a/contracts/src/ocr2/aggregator.cairo +++ b/contracts/src/ocr2/aggregator.cairo @@ -320,9 +320,7 @@ mod Aggregator { _billing: BillingConfig, // payee management _payees: Map, // - _proposed_payees: Map< - ContractAddress, ContractAddress - > // + _proposed_payees: Map // } #[generate_trait] @@ -616,30 +614,29 @@ mod Aggregator { ) { let mut index = 0; let mut span = oracles.span(); - while let Option::Some(oracle) = span - .pop_front() { - // NOTE: index should start with 1 here because storage is 0-initialized. - // That way signers(pkey) => 0 indicates "not present" - // This is why we increment first, before using the index - index += 1; - - // check for duplicates - let existing_signer = self._signers.read(*oracle.signer); - assert(existing_signer == 0_usize, 'repeated signer'); - - let existing_transmitter = self._transmitters.read(*oracle.transmitter); - assert(existing_transmitter.index == 0_usize, 'repeated transmitter'); - - self._signers.write(*oracle.signer, index); - self._signers_list.write(index, *oracle.signer); - - self - ._transmitters - .write(*oracle.transmitter, Oracle { index, payment_juels: 0_u128 }); - self._transmitters_list.write(index, *oracle.transmitter); - - self._reward_from_aggregator_round_id.write(index, latest_round_id); - }; + while let Option::Some(oracle) = span.pop_front() { + // NOTE: index should start with 1 here because storage is 0-initialized. + // That way signers(pkey) => 0 indicates "not present" + // This is why we increment first, before using the index + index += 1; + + // check for duplicates + let existing_signer = self._signers.read(*oracle.signer); + assert(existing_signer == 0_usize, 'repeated signer'); + + let existing_transmitter = self._transmitters.read(*oracle.transmitter); + assert(existing_transmitter.index == 0_usize, 'repeated transmitter'); + + self._signers.write(*oracle.signer, index); + self._signers_list.write(index, *oracle.signer); + + self + ._transmitters + .write(*oracle.transmitter, Oracle { index, payment_juels: 0_u128 }); + self._transmitters_list.write(index, *oracle.transmitter); + + self._reward_from_aggregator_round_id.write(index, latest_round_id); + }; self._oracles_len.write(index); } } @@ -740,12 +737,12 @@ mod Aggregator { ); // Check all signatures are unique (we only saw each pubkey once) - // NOTE: This relies on protocol-level design constraints (MAX_ORACLES = 31, f = 10) which - // ensures we have enough bits to store a count for each oracle. Whenever the MAX_ORACLES - // is updated, the signed_count parameter should be reconsidered. + // NOTE: This relies on protocol-level design constraints (MAX_ORACLES = 31, f = 10) + // which ensures we have enough bits to store a count for each oracle. Whenever the + // MAX_ORACLES is updated, the signed_count parameter should be reconsidered. // - // Although 31 bits is enough, we use a u128 here for simplicity because BitAnd and BitOr - // operators are defined only for u128 and u256. + // Although 31 bits is enough, we use a u128 here for simplicity because BitAnd and + // BitOr operators are defined only for u128 and u256. assert(MAX_ORACLES == 31_u32, ''); self.verify_signatures(msg, ref signatures, 0_u128); @@ -828,22 +825,21 @@ mod Aggregator { mut signed_count: u128 ) { let mut span = signatures.span(); - while let Option::Some(signature) = span - .pop_front() { - let index = self._signers.read(*signature.public_key); - assert(index != 0_usize, 'invalid signer'); // 0 index == uninitialized - - let indexed_bit = pow(2_u128, index.into() - 1_u128); - let prev_signed_count = signed_count; - signed_count = signed_count | indexed_bit; - assert(prev_signed_count != signed_count, 'duplicate signer'); - - let is_valid = ecdsa::check_ecdsa_signature( - msg, *signature.public_key, *signature.r, *signature.s - ); + while let Option::Some(signature) = span.pop_front() { + let index = self._signers.read(*signature.public_key); + assert(index != 0_usize, 'invalid signer'); // 0 index == uninitialized - assert(is_valid, ''); - }; + let indexed_bit = pow(2_u128, index.into() - 1_u128); + let prev_signed_count = signed_count; + signed_count = signed_count | indexed_bit; + assert(prev_signed_count != signed_count, 'duplicate signer'); + + let is_valid = ecdsa::check_ecdsa_signature( + msg, *signature.public_key, *signature.r, *signature.s + ); + + assert(is_valid, ''); + }; } } @@ -1190,26 +1186,25 @@ mod Aggregator { impl PayeeManagementImpl of super::PayeeManagement { fn set_payees(ref self: ContractState, mut payees: Array) { self.ownable.assert_only_owner(); - while let Option::Some(payee) = payees - .pop_front() { - let current_payee = self._payees.read(payee.transmitter); - let is_unset = current_payee.is_zero(); - let is_same = current_payee == payee.payee; - assert(is_unset | is_same, 'payee already set'); - - self._payees.write(payee.transmitter, payee.payee); - - self - .emit( - Event::PayeeshipTransferred( - PayeeshipTransferred { - transmitter: payee.transmitter, - previous: current_payee, - current: payee.payee - } - ) - ); - } + while let Option::Some(payee) = payees.pop_front() { + let current_payee = self._payees.read(payee.transmitter); + let is_unset = current_payee.is_zero(); + let is_same = current_payee == payee.payee; + assert(is_unset | is_same, 'payee already set'); + + self._payees.write(payee.transmitter, payee.payee); + + self + .emit( + Event::PayeeshipTransferred( + PayeeshipTransferred { + transmitter: payee.transmitter, + previous: current_payee, + current: payee.payee + } + ) + ); + } } fn transfer_payeeship( diff --git a/contracts/src/ocr2/aggregator_proxy.cairo b/contracts/src/ocr2/aggregator_proxy.cairo index 3b781aae0..16cf20264 100644 --- a/contracts/src/ocr2/aggregator_proxy.cairo +++ b/contracts/src/ocr2/aggregator_proxy.cairo @@ -179,7 +179,7 @@ mod AggregatorProxy { self._set_aggregator(address); } - // -- Upgradeable -- + // -- Upgradeable -- #[abi(embed_v0)] impl UpgradeableImpl of IUpgradeable { diff --git a/contracts/src/tests/test_aggregator_proxy.cairo b/contracts/src/tests/test_aggregator_proxy.cairo index 54c8f920d..3a586dc44 100644 --- a/contracts/src/tests/test_aggregator_proxy.cairo +++ b/contracts/src/tests/test_aggregator_proxy.cairo @@ -58,7 +58,8 @@ fn setup() -> ( // Deploy mock aggregator 2 // note: deployment address is deterministic based on deploy_syscall parameters - // so we need to change the decimals parameter to avoid an address conflict with mock aggregator 1 + // so we need to change the decimals parameter to avoid an address conflict with mock aggregator + // 1 let mut calldata2 = ArrayTrait::new(); calldata2.append(10); // decimals = 10 @@ -127,7 +128,7 @@ fn test_query_latest_round_data() { assert(round.started_at == 9, 'started_at should be 9'); assert(round.updated_at == 8, 'updated_at should be 8'); - // latest_answer matches up with latest_round_data + // latest_answer matches up with latest_round_data let latest_answer = AggregatorProxyImpl::latest_answer(@state); assert(latest_answer == 10, '(latest) answer should be 10'); } diff --git a/contracts/src/tests/test_erc677.cairo b/contracts/src/tests/test_erc677.cairo index b96af180e..ecb36b7a5 100644 --- a/contracts/src/tests/test_erc677.cairo +++ b/contracts/src/tests/test_erc677.cairo @@ -30,7 +30,8 @@ use chainlink::token::mock::valid_erc667_receiver::{ MockValidReceiver, MockValidReceiverDispatcher, MockValidReceiverDispatcherTrait }; -// Ignored tests are dependent on upgrading our version of cairo to include this PR https://github.com/starkware-libs/cairo/pull/2912/files +// Ignored tests are dependent on upgrading our version of cairo to include this PR +// https://github.com/starkware-libs/cairo/pull/2912/files fn setup() -> ContractAddress { let account: ContractAddress = contract_address_const::<1>(); @@ -71,7 +72,8 @@ type ComponentState = fn transfer_and_call(receiver: ContractAddress) { let data = ArrayTrait::::new(); - // have to send 0 because ERC20 is not initialized with starting supply when using this library by itself + // have to send 0 because ERC20 is not initialized with starting supply when using this library + // by itself let mut state: ComponentState = ERC677Component::component_state_for_testing(); state.transfer_and_call(receiver, u256 { high: 0, low: 0 }, data); } diff --git a/contracts/src/tests/test_link_token.cairo b/contracts/src/tests/test_link_token.cairo index 0caec4cf3..709bc6413 100644 --- a/contracts/src/tests/test_link_token.cairo +++ b/contracts/src/tests/test_link_token.cairo @@ -23,7 +23,7 @@ use snforge_std::{ }; -// only tests link token specific functionality +// only tests link token specific functionality // erc20 and erc677 functionality is already tested elsewhere fn STATE() -> LinkToken::ContractState { diff --git a/contracts/src/tests/test_multisig.cairo b/contracts/src/tests/test_multisig.cairo index 05444da9c..499eb7fe1 100644 --- a/contracts/src/tests/test_multisig.cairo +++ b/contracts/src/tests/test_multisig.cairo @@ -123,7 +123,7 @@ fn test_submit_transaction() { assert(transaction.calldata_len == sample_calldata().len(), 'should match calldata length'); assert(!transaction.executed, 'should not be executed'); assert(transaction.confirmations == 0, 'should not have confirmations'); -// TODO: compare calldata when loops are supported + // TODO: compare calldata when loops are supported } #[test] @@ -482,9 +482,9 @@ fn test_recursive_set_threshold() { // Checks that the threshold was correctly initialized on deployment assert(multisig.get_threshold() == init_threshold, 'invalid init threshold'); - // Recursive call occurs here - this code proposes a transaction to the - // multisig contract that calls the set_threshold function on the multisig - // contract. + // Recursive call occurs here - this code proposes a transaction to the + // multisig contract that calls the set_threshold function on the multisig + // contract. let mut set_threshold_calldata = ArrayTrait::new(); Serde::serialize(@new_threshold, ref set_threshold_calldata); start_cheat_caller_address_global(s1); @@ -575,9 +575,9 @@ fn test_recursive_set_signers() { assert(*returned_signers.at(1) == s2, 'should match signer 2'); assert(multisig.get_threshold() == 2, 'wrong init threshold'); - // Recursive call occurs here - this code proposes a transaction to the - // multisig contract that calls the set_signers function on the multisig - // contract. + // Recursive call occurs here - this code proposes a transaction to the + // multisig contract that calls the set_signers function on the multisig + // contract. let mut set_signers_calldata = ArrayTrait::new(); Serde::serialize(@new_signers, ref set_signers_calldata); start_cheat_caller_address_global(s1); @@ -676,8 +676,8 @@ fn test_recursive_set_signers_and_threshold() { assert(*returned_signers.at(2) == s3, 'should match signer 3'); assert(multisig.get_threshold() == 3, 'wrong init threshold'); - // Recursive call occurs here - this code proposes a transaction to the - // multisig contract that calls the set_signers_and_threshold function + // Recursive call occurs here - this code proposes a transaction to the + // multisig contract that calls the set_signers_and_threshold function // on the multisig contract. let mut set_signers_and_threshold_calldata = ArrayTrait::new(); Serde::serialize(@new_signers, ref set_signers_and_threshold_calldata); diff --git a/contracts/src/tests/test_sequencer_uptime_feed.cairo b/contracts/src/tests/test_sequencer_uptime_feed.cairo index 89696062a..e99d3b4c4 100644 --- a/contracts/src/tests/test_sequencer_uptime_feed.cairo +++ b/contracts/src/tests/test_sequencer_uptime_feed.cairo @@ -125,7 +125,7 @@ fn test_aggregator_proxy_response() { let latest_round_data = proxy.latest_round_data(); assert(latest_round_data.answer == 0, 'latest_round_data should be 0'); - // latest answer + // latest answer let latest_answer = proxy.latest_answer(); assert(latest_answer == 0, 'latest_answer should be 0'); diff --git a/contracts/src/tests/test_upgradeable.cairo b/contracts/src/tests/test_upgradeable.cairo index 151f7ad17..6f2a56e44 100644 --- a/contracts/src/tests/test_upgradeable.cairo +++ b/contracts/src/tests/test_upgradeable.cairo @@ -43,9 +43,6 @@ fn test_upgrade_and_call() { let mockUpgradeable = IMockUpgradeableDispatcher { contract_address: contractAddr }; assert(mockUpgradeable.foo() == true, 'should call foo'); - // error: Type "snforge_std::cheatcodes::contract_class::DeclareResult" has no member - // "contract_class" - let contract = declare("MockNonUpgradeable").unwrap().contract_class(); mockUpgradeable.upgrade(*(contract.class_hash)); diff --git a/contracts/src/token/link_token.cairo b/contracts/src/token/link_token.cairo index c945c02e9..4ffe888be 100644 --- a/contracts/src/token/link_token.cairo +++ b/contracts/src/token/link_token.cairo @@ -125,12 +125,14 @@ mod LinkToken { } } - // fn increase_allowance(ref self: ContractState, spender: ContractAddress, added_value: u256) -> bool { + // fn increase_allowance(ref self: ContractState, spender: ContractAddress, added_value: u256) + // -> bool { // let mut state = ERC20::unsafe_new_contract_state(); // ERC20::ERC20Impl::increase_allowance(ref state, spender, added_value) // } - // fn decrease_allowance(ref self: ContractState, spender: ContractAddress, subtracted_value: u256) -> bool { + // fn decrease_allowance(ref self: ContractState, spender: ContractAddress, subtracted_value: + // u256) -> bool { // let mut state = ERC20::unsafe_new_contract_state(); // ERC20::ERC20Impl::decrease_allowance(ref state, spender, subtracted_value) // } diff --git a/examples/contracts/aggregator_consumer/Scarb.toml b/examples/contracts/aggregator_consumer/Scarb.toml index 89039f7d0..b014b47e6 100644 --- a/examples/contracts/aggregator_consumer/Scarb.toml +++ b/examples/contracts/aggregator_consumer/Scarb.toml @@ -11,6 +11,9 @@ cairo-version = "2.7.1" [scripts] test = "snforge test" +# [scripts] +# test = "snforge test" + # See more keys and their definitions at https://docs.swmansion.com/scarb/docs/reference/manifest.html [dependencies] diff --git a/examples/contracts/aggregator_consumer/tests/test_consumer.cairo b/examples/contracts/aggregator_consumer/tests/test_consumer.cairo index 40fc7a584..6cd305c8c 100644 --- a/examples/contracts/aggregator_consumer/tests/test_consumer.cairo +++ b/examples/contracts/aggregator_consumer/tests/test_consumer.cairo @@ -119,7 +119,7 @@ fn test_set_and_read_answer() { assert(latest_round.started_at == observation_timestamp, 'bad started_at'); assert(latest_round.updated_at == transmission_timestamp, 'bad updated_at'); - // Now let's test that we can set the answer + // Now let's test that we can set the answer consumer_dispatcher.set_answer(latest_round.answer); assert(answer == consumer_dispatcher.read_answer(), 'Invalid answer'); } diff --git a/examples/contracts/aggregator_consumer/tests/test_price_consumer_with_sequencer.cairo b/examples/contracts/aggregator_consumer/tests/test_price_consumer_with_sequencer.cairo index 4e0fd6381..0b59e0243 100644 --- a/examples/contracts/aggregator_consumer/tests/test_price_consumer_with_sequencer.cairo +++ b/examples/contracts/aggregator_consumer/tests/test_price_consumer_with_sequencer.cairo @@ -75,11 +75,11 @@ fn test_get_latest_price() { IAccessControllerDispatcher { contract_address: uptime_feed_address } .add_access(price_consumer_address); - // The get_latest_price function returns the mock aggregator's latest round answer. At - // this point in the test, there is only one round that is initialized and that is the - // one that the sequencer uptime feed creates when it is deployed. In its constructor, - // a new round is initialized using its initial status as the round's answer, so the - // latest price should be the initial status that was passed into the sequencer uptime + // The get_latest_price function returns the mock aggregator's latest round answer. At + // this point in the test, there is only one round that is initialized and that is the + // one that the sequencer uptime feed creates when it is deployed. In its constructor, + // a new round is initialized using its initial status as the round's answer, so the + // latest price should be the initial status that was passed into the sequencer uptime // feed's constructor. start_cheat_caller_address_global(price_consumer_address); // start_prank(CheatTarget::All, price_consumer_address);