Added tests and corrected code #32
Conversation
DarkLord017
commented
May 20, 2024
•
DarkLord017
commented
- Deleted looping and use teamIndex to directly enter a team
- Added some view function and tests
- Since contract size was large and had to comment out many function to test so i created new file of test just to test the team initialization , join other team and leave a team
contracts/contracts/dapphack.sol
Outdated
| for (uint256 i = 0; i < totalParticipants.length; i++) { | ||
| if(duplication[totalParticipants[i]] == false){ | ||
| duplication[totalParticipants[i]] = true; | ||
| builderToTeam[totalParticipants[i]] = s_teams[s_teams.length - 1]; | ||
| }else{ | ||
| revert("Duplication detected"); | ||
| } | ||
|
|
||
| } | ||
|
|
||
| for(uint256 i = 0 ; i < totalParticipants.length; i++){ | ||
| duplication[totalParticipants[i]] = false; | ||
| } |
There was a problem hiding this comment.
@DarkLord017 I am not able to understand whats the use of duplication mapping here?
There was a problem hiding this comment.
I did that so as if same address are passed in the array the array will store both the address and count both the same addresses as unique member to remove this I did this check . Should I remove it ?
There was a problem hiding this comment.
just create a modifier for that to check the array.
contracts/contracts/dapphack.sol
Outdated
| @@ -134,9 +137,20 @@ contract DappHack is ProjectNFTs { | |||
| } | |||
|
|
|||
| //create mapping for this | |||
| modifier NotInTeam() { | |||
| modifier NotInTeam(address[] memory participants, address MsgSender) { | |||
There was a problem hiding this comment.
Better make modifier take just an array of addresses (remove second param), would become more handy. And send the participant array adding msg.sender to it.
There was a problem hiding this comment.
Sir i did all the changes please check
1. Added few test to check for duplicate participants 2.updated the code to correct NotInTeam modifier and added modifier to check Duplication
contracts/contracts/dapphack.sol
Outdated
| @@ -68,7 +70,8 @@ contract DappHack is ProjectNFTs { | |||
| //winners | |||
| Winner[] public s_winners; | |||
|
|
|||
| mapping(uint256 => Winner) public sponsorToWinner; //track number to winner number | |||
| mapping(uint256 => Winner) public sponsorToWinner; | |||
| mapping(address => bool) private duplication; //track number to winner number | |||
There was a problem hiding this comment.
No need for duplication array
There was a problem hiding this comment.
SIr corrected and committed the changes
| _; | ||
| } | ||
|
|
||
| modifier DuplicateParticipants(address[] memory participant) { |
There was a problem hiding this comment.
Hey this is quite inefficient, you can do something like this
modifier DuplicateParticipants(address[] memory participant) {
mapping(address => bool) seen;
for (uint i = 0; i < participant.length; i++) {
if (seen[participant[i]]) {
revert("Duplicate Participants");
}
seen[participant[i]] = true;
}
// Check msg.sender separately if needed
if (seen[msg.sender]) {
revert("Duplicate Participants including msg.sender");
}
_;
}
