Follow Up Enhancements

[shiv810]

Follow up should be on pull request so that they don't reply under issue and get paid for it.
If there isn't a pull request open in time for the warning, automatically disqualify them (they should open a draft pull as soon as they self assign)
Must be configurable since some tasks are not coding related.
If pull isn't required then follow up under issue as we do now.
"activity which should be considered is only the one from the assignees themselves. I believe currently any activity from anyone is considered in the events."

Orginally Posted in Link

[shiv810]

@ubosshivaditya I want to work on this issue. How do I being? Could you provide a good starting point for this ?

[test-app-ubo]

Certainly! I'd be happy to help you get started on this issue. Here's a good starting point for implementing the requested changes:

1. First, create a draft pull request for this issue. This is important as it allows you to start working on the task and shows others that you're actively addressing it.

2. Review the current implementation of the daemon-disqualifier plugin. You'll need to understand how it currently tracks activity and issues warnings.

3. Modify the plugin to focus on pull requests instead of issue comments for follow-ups. This will involve:

   • Checking for the existence of a pull request when an issue is self-assigned.
   • If no pull request exists, start the warning countdown.
   • If a pull request is opened, reset or cancel the warning.

4. Implement a configurable option to determine whether a pull request is required for a given task. This could be done through:

   • Adding a new field in the issue template or labels to indicate if a PR is required.
   • Modifying the plugin to check this field before enforcing the PR requirement.

5. Update the activity tracking to only consider actions from the assigned user. This will likely involve:

   • Filtering the events considered by the plugin to only include those from the assignee.
   • Modifying how the plugin calculates "activity" to exclude comments or actions from other users.

6. Implement the automatic disqualification if a pull request isn't opened in time. This will require:

   • Setting a configurable time limit for opening a pull request.
   • Adding logic to automatically disqualify the assignee if this time limit is exceeded.

7. Add appropriate error handling and logging throughout these new features.

8. Update any relevant documentation or README files to reflect these changes.

9. Write tests for the new functionality to ensure it works as expected.

Here's a basic pseudocode outline to get you started:

class DaemonDisqualifier {
  // ... existing code ...

  async onIssueAssigned(issue: Issue) {
    if (this.isPullRequestRequired(issue)) {
      await this.startPullRequestCountdown(issue);
    }
  }

  async startPullRequestCountdown(issue: Issue) {
    // Set a timer for X hours
    // If PR not opened within X hours, disqualify assignee
  }

  isPullRequestRequired(issue: Issue): boolean {
    // Check issue labels or body for PR requirement
  }

  async checkActivity(issue: Issue) {
    // Only consider activity from the assignee
    const assigneeActivity = await this.getAssigneeActivity(issue);
    if (!this.isActivitySufficient(assigneeActivity)) {
      await this.issueWarning(issue);
    }
  }

  async getAssigneeActivity(issue: Issue) {
    // Fetch and filter activity to only include assignee's actions
  }

  // ... implement other necessary methods ...
}

Remember, this is just a starting point. You'll need to flesh out these methods and integrate them with the existing codebase. As you work, make sure to commit your changes regularly and update your pull request.

If you have any questions or need clarification on any part of the implementation, feel free to ask. Good luck with your work on this issue!

[shiv810]

@ubosshivaditya Can you check the code and verify if everything is correct https://www.github.com/ubiquity/ubiquity-dollar/blob/0a230f977ad91bbfb01dd28bc1d2ea77faed5237/packages/contracts/src/dollar/libraries/LibUbiquityPool.sol

[test-app-ubo]

Hi @sshivaditya2019,

I've reviewed the LibUbiquityPool.sol file from the provided repository. Here are my observations and suggestions:

1. Security Considerations:

   • Reentrancy Guards: Ensure that functions modifying state are protected against reentrancy attacks. Utilizing the Checks-Effects-Interactions pattern can help mitigate such risks.
   • Input Validation: Validate all external inputs thoroughly to prevent unexpected behaviors or vulnerabilities. For example, ensure that token addresses are valid and not zero addresses.
   • Overflow/Underflow Protection: Although Solidity versions ^0.8.0 have built-in overflow checks, if you're using an older version, consider using libraries like SafeMath.

2. Gas Optimization:

   • Efficient Data Structures: Review the usage of storage variables and mappings to ensure optimal gas usage. For instance, packing multiple uint256 variables into a single uint256 slot can save gas.
   • Minimize External Calls: Reduce the number of external contract calls within loops to save on gas costs and prevent potential bottlenecks.

3. Code Clarity and Maintainability:

   • Comments and Documentation: Adding more descriptive comments and NatSpec documentation will enhance readability and make maintenance easier for future developers.
   • Function Modularity: Break down complex functions into smaller, more manageable pieces. This not only improves readability but also facilitates easier testing and debugging.

4. Functionality Enhancements:

   • Event Emissions: Ensure that significant state changes emit appropriate events. This is crucial for transparency and for front-end applications to track contract activities.
   • Access Control: Implement robust access control mechanisms using modifiers to restrict sensitive functions to authorized roles only. Libraries like OpenZeppelin's AccessControl can be beneficial.

5. Testing and Auditing:

   • Comprehensive Unit Tests: Ensure that there are extensive unit tests covering all possible scenarios, including edge cases. This will help in identifying and mitigating potential issues early in the development cycle.
   • Formal Audits: Before deploying to production, consider having the contract audited by reputable security firms to identify and address vulnerabilities.

6. Compliance and Standards:

   • Adherence to ERC Standards: If the pool interacts with ERC-20 tokens, ensure full compliance with the ERC-20 standard to maintain interoperability.
   • Upgradeability Considerations: If you plan to upgrade the contract in the future, consider using proxy patterns to allow for seamless upgrades without disrupting the state.

Example Enhancement:

// Adding a reentrancy guard
import "@openzeppelin/contracts/security/ReentrancyGuard.sol";

contract LibUbiquityPool is ReentrancyGuard {
    // Existing code...

    function addLiquidity(uint256 amount) external nonReentrant {
        // Function logic...
    }

    // Existing code...
}

Implementing the ReentrancyGuard from OpenZeppelin ensures that the addLiquidity function is protected against reentrancy attacks.

Overall, the contract structure looks solid, but addressing the points above will enhance its security, efficiency, and maintainability. Let me know if you need further assistance or specific clarifications on any part of the code!