Capx Community Quest and Community Quest Forger With Reputation Score Changes #6

sanjaybaskaran01 · 2023-12-25T14:05:30Z

No description provided.

…eputationScore.

shreyas3336 · 2023-12-26T05:39:58Z

contracts/CapxCommunityQuest.sol

@@ -73,15 +96,10 @@ contract CapxCommunityQuest is ReentrancyGuard, PausableUpgradeable, OwnableUpgr
        return string(buffer);
    }

-    function recoverSigner(bytes32 messagehash, bytes memory signature) public pure returns (address) {


Please remove unnecessary imports from the file associated to the deleted functions.

import "@openzeppelin/contracts-upgradeable/utils/cryptography/ECDSAUpgradeable.sol";

shreyas3336 · 2023-12-26T05:41:52Z

contracts/CapxCommunityQuest.sol

-        if(IERC20(currCapxQuest.rewardToken).balanceOf(address(this)) < _rewardAmountInWei) revert NoRewardsToClaim();
+        require(
+            _timestamp < block.timestamp,
+            "CapxCommunityQuest: Cannot Claim Future Rewards"


Convert this error message into an error category.

shreyas3336 · 2023-12-26T05:42:00Z

contracts/CapxCommunityQuest.sol

+            _timestamp < block.timestamp,
+            "CapxCommunityQuest: Cannot Claim Future Rewards"
+        );
+        require(_questId != 0, "CapxCommunityQuest: Invalid Community QuestId");


Convert this error message into an error category.

shreyas3336 · 2023-12-26T05:43:24Z

contracts/CapxCommunityQuest.sol

-            _receiver, 
-            _timestamp, 
-            currCapxQuest.rewardToken, 
+            address(this),


What is the need to have address(this) as a parameter in the even emission ?

shreyas3336 · 2023-12-26T05:46:56Z

contracts/CapxCommunityQuestForger.sol

@@ -69,6 +87,13 @@ contract CapxCommunityQuestForger is Initializable, UUPSUpgradeable, OwnableUpgr
        capxCommunityQuest = _capxCommunityQuest;
    }

+    function updateCapxReputationScoreAddress(address _capxReputationScore)


Why is capxReputationScore contract not part of the initialize function ?

shreyas3336 · 2023-12-26T05:53:00Z

contracts/CapxCommunityQuestForger.sol

-        string memory _questId = string(abi.encodePacked(quest.communityId,"_",uintToStr(quest.questNumber)));
+        require(
+            community[quest.communityId] != address(0),
+            "CapxCommunityQuestForger: Invalid Community Id"


Convert this error message into an error category.

shreyas3336 · 2023-12-26T05:53:10Z

contracts/CapxCommunityQuestForger.sol

-            _rewardAmount
+        require(
+            address(capxQuest) != address(0),
+            "CapxCommunityQuestForger: Invalid Capx Quest ID"


Convert this error message into an error category.

shreyas3336 · 2023-12-26T05:54:46Z

contracts/CapxCommunityQuestForger.sol

+            );
+        }else{
+            revert InvalidRewardType();
+        }
    }

    function emitClaim(


Can't we remove this function as emission of the ReputationScore is in the claim function but the IOU Token reward claim has a seperate function. This would reduce the number of internal contract calls.

The rewardToken is stored in memory in the community contract and not in the forger contract , should I keep a map of communityIds to their respective token contracts in the forger contract? While emitting the event it requires the token contract address as well.

shreyas3336 · 2023-12-26T05:55:08Z

contracts/CapxCommunityQuestForger.sol

+
+    function recoverSigner(bytes32 messagehash, bytes memory signature)


Why is this a public function and not an internal function ?

shreyas3336 · 2023-12-26T05:57:26Z

contracts/interfaces/ICapxCommunityQuest.sol

@@ -22,11 +21,9 @@ interface ICapxCommunityQuest {
    error ClaimedRewardsExceedTotalRewards();


Remove un-used error categories from the interface.

shreyas3336 · 2024-01-02T06:13:36Z

contracts/CapxCommunityQuest.sol

+        QuestDTO memory quest
+    ) external nonReentrant whenNotPaused onlyForger {
+        // Transfer tokens.
+        ITokenPoweredByCapx(quest.rewardToken).addToWhitelist(address(this));


@sanjaybaskaran01 Why are we whitelisting the rewardToken in this method ?

shreyas3336 · 2024-01-02T06:14:43Z

contracts/CapxCommunityQuest.sol

-        string memory _communityQuestId = string(abi.encodePacked(communityId,"_",uintToStr(_questId)));
-        if(currCapxQuest.rewardToken != address(0)) revert QuestIdUsed();
-
+    function setQuestDetails(


What is the reason for naming this function setQuestDetails and not createQuest can we modify the quest details apart from the reference variables once we have created it ?

What is the reason for naming this function setQuestDetails and not createQuest can we modify the quest details apart from the reference variables once we have created it ?

Yes, we utilise the same function for updating the quest details.

shreyas3336 · 2024-01-02T06:15:16Z

contracts/CapxCommunityQuest.sol

-        if(currCapxQuest.rewardToken != address(0)) revert QuestIdUsed();
-
+    function setQuestDetails(
+        QuestDTO memory quest


What is the naming convention behind QuestDTO ?

What is the naming convention behind QuestDTO ?

We are creating transferrable objects for setting the quest details in both communityQuest contract and as well as reputationScore contract.

CommunityForger -> CapxQuestDetails (struct )

Community -> QuestDTO ( struct , part of CapxQuestDetails of CommunityForger )

CapxReputationScore -> QuestDTO ( struct, part of CapxQuestDetails of CommunityForger )

Using the term DTO to signify that they are merely just a transfer objects for respective contracts which will be set as CapxQuestDetail for their respective contracts.

shreyas3336 · 2024-01-02T06:18:50Z

contracts/CapxCommunityQuest.sol

+    ) external nonReentrant whenNotPaused onlyForger {
+        // Transfer tokens.
+        ITokenPoweredByCapx(quest.rewardToken).addToWhitelist(address(this));
+        IERC20(quest.rewardToken).safeTransferFrom(


Why is the transfer of tokens being done from the community quest contract and not from the CommunityQuestForger ?

I wanted all the community specific actions to exist inside the communityQuest contract.

This is the differentiation I was going for:

CapxCommunityQuest Contract:

This contract will handle the details of individual community quests

Its main job is to manage everything about a quest, like tracking rewards, who is participating, and the quest's overall status

It deals with ERC20 token transactions directly. This way, all the important actions and changes for a quest happen in one place, which makes the system more organized and easier to handle

CapxCommunityQuestForger Contract:

This contract acts like creator and manager for multiple CapxCommunityQuest instances.

Its main role is to set up new community quests, establish their initial settings, and link quests to their respective communities.

It should not get involved in the specifics of token transactions for quests. It focuses on higher-level tasks like setting up and managing quests, which keeps things simpler and more focused.

shreyas3336 · 2024-01-02T06:21:44Z

contracts/CapxCommunityQuest.sol

@@ -1,17 +1,22 @@
 //SPDX-License-Identifier: MIT
-pragma solidity ^0.8.16;
+pragma solidity ^0.8.18 .0;


What is this solidity version naming convention ?

Had it modified due to this linting rule:
https://protofire.github.io/solhint/docs/rules/security/compiler-version.html

This is the stack overflow link:
https://stackoverflow.com/questions/72180057/error-compiler-version-0-8-0-does-not-satisfy-the-r-semver-requirement

But reverted the versioning to ^0.8.18 and removed the linting rule for semver.

shreyas3336 · 2024-01-02T06:26:16Z

contracts/CapxCommunityQuestForger.sol

+
+        // Create Quest.
+        if (quest.rewardType == 1) {
+            ICapxCommunityQuest(communityAddress).setQuestDetails(


Before this method is called, the QuestForger should transfer the necessary amount tokens into the community Forger.

shreyas3336 · 2024-01-02T06:30:10Z

contracts/CapxCommunityQuestForger.sol

+
+    function updateRewardType(
+        RewardTypeDTO calldata _newRewards,
+        bytes calldata _IOURewardsData,


What is the need to encode the IOURewardsData when you are only using the corresponding QuestDTO as the only decoded input value ?

shreyas3336 · 2024-01-02T06:34:47Z

contracts/CapxCommunityQuestForger.sol

+        ICapxCommunityQuest(communityAddress).withdrawTokens(tokens);
+    }
+}


Function missing to withdrawETH & WithdrawTokens from CapxCommunityQuestForger.

shreyas3336 · 2024-01-02T06:35:17Z

contracts/CapxReputationScore.sol

+
+    // Scores for different categories
+    // 1. Social 2. Defi 3. Game
+    struct ReputationScoreTypes {


Please define all the structs , events in the Interface.

shreyas3336 · 2024-01-02T06:35:59Z

contracts/interfaces/ICapxCommunityQuest.sol


 interface ICapxCommunityQuest {
-
    error AlreadyClaimed();


Remove unncessary error types from the contract definition.

…ardType fn.

…t to owner.

…ewards.

sanjaybaskaran01 added 2 commits December 24, 2023 12:42

v3.0.3 - CapxReputationContract: CapxReputationScore contract draft.

887aadf

v3.0.4 - CapxCommunityQuestContracts: added claim mechanism for CaxpR…

e19d3db

…eputationScore.

sanjaybaskaran01 added the enhancement New feature or request label Dec 25, 2023

sanjaybaskaran01 requested a review from shreyas3336 December 25, 2023 14:05

sanjaybaskaran01 self-assigned this Dec 25, 2023

sanjaybaskaran01 changed the title ~~Feature/capx reputation score~~ Capx Community Quest and Community Quest Forger With Reputation Score Changes Dec 25, 2023

shreyas3336 requested changes Dec 26, 2023

View reviewed changes

v3.0.4 - CapxCommunityQuestContracts: Major refactor.

51f6088

shreyas3336 requested changes Jan 2, 2024

View reviewed changes

sanjaybaskaran01 and others added 7 commits January 3, 2024 19:18

v3.0.4 - CapxCommunityQuestContracts: Minor refactor.

b20995a

v3.0.4 - CapxCommunityQuestContracts: Forger - Uses DTOs in updateRew…

8e119bf

…ardType fn.

v3.0.4 - CapxCommunityQuestContracts: AuthorizedCaller rewards defaul…

5a1eb47

…t to owner.

v3.0.4 - CapxCommunityQuestContracts: Bug fixes.

f3d08fc

v3.0.4 - CapxCommunityQuestContracts: Bug fixes.

3782ffe

v3.0.5 - Capx Token V2 Updates.

c5680b1

v3.0.4 - CapxCommunityQuestContracts: fn added for withdrawing questR…

4287c4d

…ewards.

sanjaybaskaran01 force-pushed the feature/capxReputationScore branch from 201aac0 to 4287c4d Compare January 10, 2024 08:57

v3.0.4 - CapxCommunityQuestContracts: Modifications made for Capx Tasks.

393c924

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Capx Community Quest and Community Quest Forger With Reputation Score Changes #6

Capx Community Quest and Community Quest Forger With Reputation Score Changes #6

sanjaybaskaran01 commented Dec 25, 2023

shreyas3336 Dec 26, 2023

shreyas3336 Dec 26, 2023

shreyas3336 Dec 26, 2023

shreyas3336 Dec 26, 2023

shreyas3336 Dec 26, 2023

shreyas3336 Dec 26, 2023

shreyas3336 Dec 26, 2023

shreyas3336 Dec 26, 2023

sanjaybaskaran01 Dec 26, 2023

shreyas3336 Dec 26, 2023

shreyas3336 Dec 26, 2023

shreyas3336 Jan 2, 2024

shreyas3336 Jan 2, 2024

sanjaybaskaran01 Jan 3, 2024

shreyas3336 Jan 2, 2024

sanjaybaskaran01 Jan 3, 2024

shreyas3336 Jan 2, 2024

sanjaybaskaran01 Jan 3, 2024

shreyas3336 Jan 2, 2024

sanjaybaskaran01 Jan 3, 2024

shreyas3336 Jan 2, 2024

shreyas3336 Jan 2, 2024

shreyas3336 Jan 2, 2024

shreyas3336 Jan 2, 2024

shreyas3336 Jan 2, 2024


		function recoverSigner(bytes32 messagehash, bytes memory signature)

		@@ -22,11 +21,9 @@ interface ICapxCommunityQuest {
		error ClaimedRewardsExceedTotalRewards();

Capx Community Quest and Community Quest Forger With Reputation Score Changes #6

Are you sure you want to change the base?

Capx Community Quest and Community Quest Forger With Reputation Score Changes #6

Conversation

sanjaybaskaran01 commented Dec 25, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment