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: implement EboActor.onDisputeStatusChanged handler #24

Merged
merged 6 commits into from
Aug 21, 2024

Conversation

0xyaco
Copy link
Collaborator

@0xyaco 0xyaco commented Aug 16, 2024

🤖 Linear

Closes GRT-85, GRT-101, GRT-102, GRT-103, GRT-104, GRT-105, GRT-106

Description

Handle events notifying dispute status changes.

@0xyaco 0xyaco requested a review from 0xkenj1 August 16, 2024 00:05
Copy link

linear bot commented Aug 16, 2024

GRT-85 Implement onDisputeStatusUpdated

Actors should handle the event DisputeStatusUpdated [1] from Prophet's Oracle.

Given that there are multiple values for the dispute status:

  enum DisputeStatus {
    None, // The dispute has not been started yet
    Active, // The dispute is active and can be escalated or resolved
    Escalated, // The dispute is being resolved by the resolution module
    Won, // The disputer has won the dispute
    Lost, // The disputer has lost the dispute
    NoResolution // The dispute was inconclusive

  }

There will a sub-issue per each status.


[1] https://github.com/defi-wonderland/prophet-core/blob/c25103ea418e47aba0a5958e62e78c3ad1ecd12f/solidity/interfaces/IOracle.sol#L69C3-L69C103

GRT-101 Handle dispute status change to `None`

GRT-102 Handle dispute status change to `Active`

GRT-103 Handle dispute status change to `Escalated`

GRT-104 Handle dispute status change to `Won`

GRT-105 Handle dispute status change to `Lost`

GRT-106 Handle dispute status change to `NoResolution`

@0xyaco 0xyaco requested a review from 0xnigir1 August 16, 2024 00:05
@0xyaco 0xyaco force-pushed the feat/on-dispute-updated branch from c36038b to 7c7a185 Compare August 16, 2024 00:11
Comment on lines +395 to +396
case "Active": // Case handled by ResponseDisputed
case "Lost": // Relevant during periodic request state checks
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we add break here ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The idea is to silently handle those three cases absorbing them and doing nothing; basically we don't need to react immediately to the status being changed; figured that a case fallthrough was pretty simplistic here.

const dispute = this.getDispute(disputeId);

// TODO: define a more verbose error for this
if (dispute === undefined) throw new Error();
Copy link
Collaborator

Choose a reason for hiding this comment

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

define it then 🤣

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Aaaaah damn, sorry, forgot to push the latest commit which has these changes prior opening the PR. Will have to force push 😢

expect(onTerminate).toHaveBeenCalledWith(actorRequest);
});

it.skip("notifies when dispute has been escalated");
Copy link
Collaborator

Choose a reason for hiding this comment

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

niceeeeeee

Comment on lines 5 to 12

import { InvalidActorState } from "./exceptions/invalidActorState.exception.js";
import { InvalidDisputeStatus } from "./exceptions/invalidDisputeStatus.exception.js";
import { RequestMismatch } from "./exceptions/requestMismatch.js";
import { ResponseAlreadyProposed } from "./exceptions/responseAlreadyProposed.js";
import { EboRegistry } from "./interfaces/eboRegistry.js";
import { ProtocolProvider } from "./protocolProvider.js";
import { EboEvent } from "./types/events.js";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
import { InvalidActorState } from "./exceptions/invalidActorState.exception.js";
import { InvalidDisputeStatus } from "./exceptions/invalidDisputeStatus.exception.js";
import { RequestMismatch } from "./exceptions/requestMismatch.js";
import { ResponseAlreadyProposed } from "./exceptions/responseAlreadyProposed.js";
import { EboRegistry } from "./interfaces/eboRegistry.js";
import { ProtocolProvider } from "./protocolProvider.js";
import { EboEvent } from "./types/events.js";
import { InvalidActorState } from "./exceptions";
import { InvalidDisputeStatus } from "./exceptions";
import { RequestMismatch } from "./exceptions";
import { ResponseAlreadyProposed } from "./exceptions";
import { EboRegistry } from "./interfaces";
import { ProtocolProvider } from "./protocolProvider.js";
import { EboEvent } from "./types";

@0xyaco 0xyaco force-pushed the feat/on-dispute-updated branch from 7c7a185 to e5d52fc Compare August 16, 2024 19:34
@0xyaco 0xyaco requested a review from 0xkenj1 August 16, 2024 19:42
0xnigir1
0xnigir1 previously approved these changes Aug 16, 2024
Base automatically changed from feat/on-finalize-request to dev August 16, 2024 21:25
@0xyaco 0xyaco dismissed 0xnigir1’s stale review August 16, 2024 21:25

The base branch was changed.

@0xyaco 0xyaco requested a review from 0xnigir1 August 16, 2024 22:17
0xnigir1
0xnigir1 previously approved these changes Aug 16, 2024
@0xyaco 0xyaco merged commit 770b554 into dev Aug 21, 2024
5 checks passed
@0xyaco 0xyaco deleted the feat/on-dispute-updated branch August 21, 2024 14:11
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.

3 participants