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

feat: ensure relative timestamps #64

Merged
merged 10 commits into from
Oct 11, 2024
Merged

Conversation

xorsal
Copy link
Contributor

@xorsal xorsal commented Oct 7, 2024

🤖 Linear

Closes OPO-650

Esentially this PR changes the definition of some parameters from being a timestamp to be a number of seconds.

This PR will introduce duplicate calls in some modules.
For example the internal function _validateDispute in Validator.sol does the following check:

    if (ORACLE.disputeCreatedAt(_disputeId) == 0) revert Validator_InvalidDispute();

and now some parts of the dispute modules need to query the dispute creation time to verify if a given dispute is within the escalation window. As a result, any function that uses _validateDispute and also queries the dispute's creation time will make the same call twice.
I can will make another PR to fix this problem later.


Another thing to consider is startTime from Escalation struct in IERC20ResolutionModule.

  /**
   * @notice Escalation data for a dispute
   * @param startTime The timestamp at which the dispute was escalated
   * @param totalVotes The total amount of votes cast for the dispute
   */
  struct Escalation {
    uint256 startTime;
    uint256 totalVotes;
  }

This could be redefined as something like "the number of seconds after the creation of the dispute when it was escalated".

Copy link

linear bot commented Oct 7, 2024

Copy link

github-actions bot commented Oct 7, 2024

Copy link

github-actions bot commented Oct 9, 2024

@0xShaito 0xShaito merged commit b88fda6 into dev Oct 11, 2024
4 checks passed
@0xShaito 0xShaito deleted the feat/ensure-relative-timestamps branch October 11, 2024 10:25
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