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

changePrizePool and changePrizeArray implimented #29

Conversation

yomanthunder
Copy link
Contributor

@yomanthunder yomanthunder commented Feb 9, 2024

Resolving #15

  • Two view functions for both poolPrize change and prizeArray change
  • Two function to changePrizePool and ChangePrizeArray that can only be access by sponsor , takes in the prizePool or prizeArray (only one argument functions)
  • Basic tests written using mocha and chai

oolChangePayment changePrizePool calculatePrizeArrayChangePayment changePrizeArray and basic tests
@yomanthunder
Copy link
Contributor Author

@lordshashank i think having sponsor number as a parameter make you more prone to attacks , our modifier only check whether you are a sponsor or not meaning others sponsor can tamper with my prizepool or prizearray (which can potentially be stopped with some basic code but makes things more complicated tho)
imo this does create a new feature where other sponsors can sponsor you based on your sponsor Number (but in general in hackathons i have never seen a sponsor sponsoring other sponsor)

@lordshashank
Copy link
Collaborator

@yomanthunder , this makes sense, your implementation would be good for now. Just do following two improvements.

  • sponsorToId array just get initialized for msg.sender in sponsor signup, initialize it for all addresses in _sponsor with same id as this would enable any of them to do prize updates.
  • check the balances of sponsor before and after running the functions in tests so as to make sure the amount gets transferred successfully.
    This is a great solution. Thank you for your contribution.

@yomanthunder
Copy link
Contributor Author

yomanthunder commented Mar 7, 2024

@lordshashank I have made the required changes , you can merge the pull request

@lordshashank
Copy link
Collaborator

LFG🚀

@lordshashank lordshashank merged commit ba85c24 into BlocSoc-iitr:master Mar 9, 2024
@lordshashank lordshashank self-requested a review March 9, 2024 06:45
Copy link
Collaborator

@lordshashank lordshashank left a comment

Choose a reason for hiding this comment

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

GOOD TO GO

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.

2 participants