feat: handle ProposeDisputed event #21

0xyaco · 2024-08-13T02:05:38Z

🤖 Linear

Closes GRT-83

Description

Handles the ResposeDisputed event.

linear · 2024-08-13T02:05:41Z

GRT-83 Implement onResponseDisputed

We want the actors to handle the event ResponseDisputed from prophet Orcale.

This handler should:

Update RequestActivity
Validate if the proposal was correct, Leveraging BlockNumberService
Bond for disputer or proposer based on validation output

Event: https://github.com/defi-wonderland/prophet-core/blob/d01bc1a0fe39d516ff502a669e80b8eb2fe2df86/solidity/interfaces/IOracle.sol#L40

Follow figma sequence diagram: https://www.figma.com/board/BbciqJb5spg35ZglTsRRBb/Offchain?node-id=239-2749&t=y06FKcx0NVro4Jml-4
AC:

Event handler successfully implemented
Unit tests

0xkenj1 · 2024-08-13T17:39:49Z

packages/automated-dispute/src/eboActor.ts

+            await this.protocolProvider.pledgeForDispute(request.prophetData, dispute.prophetData);
+        } catch (err) {
+            if (err instanceof ContractFunctionRevertedError) {
+                this.logger.warn(`Pledging for dispute ${dispute.id} was reverted. Skipping...`);


should we log the error here also ?

Maybe what we want is to have more granular errors thrown from the protocolProvider and handle them here. The actor should be the responsible to decide if errors are recoverable or unrecoverable.

wdyt ?

I will write it down on a linear task

https://linear.app/defi-wonderland/issue/GRT-100/error-handing-and-granularity

That's right, the ideal would be a situation in which we are able to handle each revert explicitly if wanted. I didn't want to go deeper with that right now, as I prefer to have the whole base of the EboActor ready and when everything is set, tackle these details.

I'll add some TODOs in those error handling blocks too, with this piece of info, thanks for the Linear task 🫶

perfect, just wanted to be sure we were on the same page with that :)

0xkenj1 · 2024-08-13T17:40:04Z

packages/automated-dispute/src/eboActor.ts

+            );
+        } catch (err) {
+            if (err instanceof ContractFunctionRevertedError) {
+                this.logger.warn(`Pledging on dispute ${dispute.id} was reverted. Skipping...`);


0xkenj1 · 2024-08-13T17:46:04Z

packages/automated-dispute/src/eboActor.ts

+                this.logger.warn(`Pledging for dispute ${dispute.id} was reverted. Skipping...`);
+            } else {
+                this.logger.error(
+                    `Actor handling request ${this.actorRequest.id} is not able to continue.`,


0xkenj1 · 2024-08-13T17:46:25Z

packages/automated-dispute/src/eboActor.ts

+                this.logger.warn(`Pledging on dispute ${dispute.id} was reverted. Skipping...`);
+            } else {
+                this.logger.error(
+                    `Actor handling request ${this.actorRequest.id} is not able to continue.`,


0xkenj1 · 2024-08-13T17:47:28Z

packages/automated-dispute/src/eboMemoryRegistry.ts

+    /** @inheritdoc */
+    public getResponse(responseId: string): Response | undefined {
+        return this.responses.get(responseId);
+    }
+
+    /** @inheritdoc */
+    public addDispute(disputeId: string, dispute: Dispute): void {
+        this.disputes.set(disputeId, dispute);
+    }


oop encapsulation 🤓

0xkenj1 · 2024-08-13T17:51:54Z

packages/automated-dispute/src/eboActor.ts

+     * @param event `ResponseDisputed` event.
+     */
+    public async onResponseDisputed(event: EboEvent<"ResponseDisputed">): Promise<void> {
+        this.shouldHandleRequest(event.metadata.dispute.requestId);


this is just a double check right ? EboActor is selected by the EboProcessor using the requestId right ?

I was debating myself with this, between having this check or not.

I guess that we could rely on EboProcessor + EboActorsManager to work as expected, but I like the defensiveness of having this check within EboActor. If something goes wrong in the event → request → actor routing between the other two classes, this check will catch the error before even trying to do anything.

excellent, would be different if it was a heavy computation , but in this case works perfectly :)

0xkenj1 · 2024-08-13T17:58:21Z

packages/automated-dispute/tests/eboActor/onResponseDisputed.spec.ts

+function scaffoldEboActor(mockedValues: {
+    requestId: Address;
+    indexedChainId: Caip2ChainId;
+    mockActorResponse: { mockBlockNumber: bigint; mockEpoch: bigint };
+}) {
+    const { requestId, indexedChainId, mockActorResponse } = mockedValues;
+    const { mockBlockNumber, mockEpoch } = mockActorResponse;
+
+    const protocolProvider = new ProtocolProvider(
+        ["http://localhost:8538"],
+        DEFAULT_MOCKED_PROTOCOL_CONTRACTS,
+    );
+
+    const chainRpcUrls = new Map<Caip2ChainId, string[]>();
+    chainRpcUrls.set(indexedChainId, ["http://localhost:8539"]);
+
+    const blockNumberService = new BlockNumberService(chainRpcUrls, logger);
+    vi.spyOn(blockNumberService, "getEpochBlockNumber").mockResolvedValue(mockBlockNumber);
+
+    const registry = new EboMemoryRegistry();
+    const response: Response = {
+        id: "0x01",
+        wasDisputed: false,
+        prophetData: {
+            proposer: "0x01",
+            requestId: requestId,
+            response: {
+                chainId: indexedChainId,
+                block: mockBlockNumber,
+                epoch: mockEpoch,
+            },
+        },
+    };
+
+    vi.spyOn(registry, "getRequest").mockReturnValue(DEFAULT_MOCKED_REQUEST_CREATED_DATA);
+    vi.spyOn(registry, "getResponse").mockReturnValue(response);
+
+    const requestConfig = {
+        id: requestId,
+        epoch: mockEpoch,
+        epochTimestamp: BigInt(Date.UTC(2024, 1, 1, 0, 0, 0, 0)),
+    };
+
+    const actor = new EboActor(
+        requestConfig,
+        protocolProvider,
+        blockNumberService,
+        registry,
+        logger,
+    );
+
+    return {
+        actor,
+        protocolProvider,
+        blockNumberService,
+        registry,
+    };
+}


I think we already have something similar to this. May we move it to helpers or mocks folder ?

Opened a new PR for this! Check it out #22

# 🤖 Linear Part of GRT-83 ## Description - Create the `mocks.buildEboActor(r: Request, l: ILogger)` that enable developers to easily build `EboActor` instances, with access to all its dependencies. - Create the `mocks.buildResponse(r: Request)` that enable developers to easily build `Response` instances based on a request, for data consistency between both entities. - General cleaning of tests using new mock methods.

0xyaco · 2024-08-14T14:30:27Z

@0xnigir1 @0xkenj1 remember that you've already reviewed tests changes on #22, as that PR branched from this one!

0xyaco added 2 commits August 12, 2024 23:01

feat: implement EboActor.onProposeDisputed handler

5314681

Merge remote-tracking branch 'origin/dev' into feat/on-propose-disputed

0f369d6

0xyaco marked this pull request as ready for review August 13, 2024 02:27

0xyaco requested review from 0xkenj1 and 0xnigir1 August 13, 2024 02:48

0xyaco changed the title ~~Feat/on propose disputed~~ feat: handle ProposeDisputed event Aug 13, 2024

docs: add docs to dispute methods on EboActor

66dc205

0xkenj1 reviewed Aug 13, 2024

View reviewed changes

docs: add TODOs to handle errors when using protocol provider

098eb08

0xyaco requested a review from 0xkenj1 August 13, 2024 22:29

0xyaco added 2 commits August 14, 2024 11:04

fix: lint onResponseProposed

9f9a509

0xkenj1 approved these changes Aug 14, 2024

View reviewed changes

0xnigir1 approved these changes Aug 14, 2024

View reviewed changes

0xyaco merged commit e1ba843 into dev Aug 14, 2024
5 checks passed

0xyaco deleted the feat/on-propose-disputed branch August 14, 2024 14:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: handle ProposeDisputed event #21

feat: handle ProposeDisputed event #21

0xyaco commented Aug 13, 2024

linear bot commented Aug 13, 2024

0xkenj1 Aug 13, 2024

0xkenj1 Aug 13, 2024 •

edited

Loading

0xkenj1 Aug 13, 2024

0xyaco Aug 13, 2024

0xkenj1 Aug 13, 2024

0xkenj1 Aug 13, 2024

0xkenj1 Aug 13, 2024

0xkenj1 Aug 13, 2024

0xkenj1 Aug 13, 2024

0xkenj1 Aug 13, 2024

0xyaco Aug 13, 2024

0xkenj1 Aug 13, 2024

0xkenj1 Aug 13, 2024

0xyaco Aug 13, 2024

0xyaco commented Aug 14, 2024

feat: handle ProposeDisputed event #21

feat: handle ProposeDisputed event #21

Conversation

0xyaco commented Aug 13, 2024

🤖 Linear

Description

linear bot commented Aug 13, 2024

Choose a reason for hiding this comment

0xkenj1 Aug 13, 2024 • edited Loading

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

0xyaco commented Aug 14, 2024

0xkenj1 Aug 13, 2024 •

edited

Loading