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

fix: Ensure only fund manager can modify goal in setGoal #156

Merged

Conversation

derianrddev
Copy link
Contributor

@derianrddev derianrddev commented Oct 25, 2024

Pull Request

Changes description

Steps to Resolve the Issue

  1. Create the fund_manager_constants.cairo file in contracts/src/constants/funds/:
  • File name: fund_manager_constants.cairo.
  • Add a constant called FUND_MANAGER_ADDRESS with the fund manager's address.
    pub mod FundManagerConstants {
        pub const FUND_MANAGER_ADDRESS: felt252 =
            0x00a885638f5167da8c38f115077c23ed7411539ea8f019ef09ec025d0c52d0ff;
    }
  1. Import the fund_manager_constants module in contracts/src/constants/fund.cairo:
  • Modify fund.cairo to import the new fund manager constants module:
     pub mod state_constants;
     pub mod fund_constants;
     pub mod fund_manager_constants;
     pub mod starknet_constants;
  1. Import fund manager constants in the Fund contract (contracts/src/fund.cairo):
  • Add the following use statement to access the constants in the Fund contract:
     use gostarkme::constants::{funds::{fund_constants::FundConstants, fund_manager_constants::FundManagerConstants},};
  1. Modify the setGoal function to add fund manager validation:
  • Create a variable fund_manager_address to store the fund manager's address, converted to a ContractAddress.
  • Verify that the caller is the fund manager. If not, throw an error with the message "You are not the fund manager".
     fn setGoal(ref self: ContractState, goal: u256) {
         let caller = get_caller_address();
         let fund_manager_address = contract_address_const::<
             FundManagerConstants::FUND_MANAGER_ADDRESS
         >();
         assert!(fund_manager_address == caller, "You are not the fund manager");
         self.goal.write(goal);
     }
  1. Modify the tests file (contract/tests/test_fund.cairo):
  • Import fund manager constants in the tests:
     use gostarkme::constants::{funds::{fund_manager_constants::FundManagerConstants},};
  1. Create a function to get the fund manager’s address in the tests:
  • Add the FUND_MANAGER() function to return the fund manager’s address:
     fn FUND_MANAGER() -> ContractAddress { 
         contract_address_const::<FundManagerConstants::FUND_MANAGER_ADDRESS>()
      }
  1. Update existing tests (test_set_goal and test_receive_donation_successful):
  • Change the caller address in these tests to be the fund manager's address:
     start_cheat_caller_address_global(FUND_MANAGER());
  1. Create a new test to verify unauthorized access (test_set_goal_unauthorized):
  • Add a new test to check that an error is thrown if someone other than the fund manager attempts to execute the setGoal function.
     #[test]
     #[should_panic(expected: ("You are not the fund manager",))]
     fn test_set_goal_unauthorized() {
         let contract_address = _setup_();
         let dispatcher = IFundDispatcher { contract_address };
         // Change the goal without being the fund manager
         dispatcher.setGoal(22);
     }

Time spent breakdown

  • Development environment setup: 2 hours

    • Setting up the environment, installing dependencies, and necessary tools for the project.
  • Research and issue analysis: 1 hour

    • Reviewing the issue, understanding the requirements, and analyzing how to implement the changes.
  • Solution development: 1 hour

    • Creating the fund_manager_constants.cairo file, defining the constant, and modifying the contract to add validation in the setGoal function.
  • Test modifications: 1 hour

    • Updating existing tests and creating new tests to validate functionality.
  • Debugging and running tests: 30 minutes

    • Running tests, fixing issues, and ensuring everything works as expected.
  • Documentation and PR creation: 30 minutes

    • Writing the PR description, formatting the code, and properly commenting.

Total: 6 hours

Comments

Thank you very much for the opportunity to contribute to this project.

@adrianvrj adrianvrj self-requested a review October 25, 2024 17:20
Copy link
Member

@adrianvrj adrianvrj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks great, could you provide SS with the output of the tests please?

@derianrddev
Copy link
Contributor Author

@adrianvrj
image

@adrianvrj adrianvrj merged commit d79050a into undefinedorgcr:dev Oct 25, 2024
5 checks passed
@adrianvrj
Copy link
Member

@derianrddev great work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[fix] Assert setGoal method is called by fund manager only
2 participants