feat: build onRequestCreated #18

0xyaco · 2024-08-05T23:24:12Z

🤖 Linear

Closes GRT-81 GRT-77

Description

Standardizes timestamps used through our providers to BigInt type, to be consistent with viem Block.timestamp type
Set up EboActor.onRequestCreated handler

linear · 2024-08-05T23:24:14Z

GRT-81 Implement onRequestCreated

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

This handler should:

Update RequestActivity
Validate if there was a proposal already created ? or maybe wait for a revert (TBD)
Leverage BlockNumberService to obtain block numbers from supported chains based on the epoch timestamp.
Propose a response
Handle the case in which a proposal was already created

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

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

AC:

Event handler successfully implemented
Unit tests

0xnigir1 · 2024-08-07T13:18:56Z

packages/automated-dispute/src/eboActor.ts


    constructor(
        private readonly protocolProvider: ProtocolProvider,
        private readonly blockNumberService: BlockNumberService,
        private readonly requestId: string,
    ) {
-        this.requestActivity = [];
+        this.registry = new EboRegistry();


shouldn't the registry be injected as a Dependency here?
also, maybe use an interface IEboRegistry, so if we wanted to later change for a real DB instead of in-memory maps we can easily switch to that. (i understand that the registry is sort of Database/Cache)

You're absolutely right!

0xnigir1 · 2024-08-07T13:24:48Z

packages/automated-dispute/src/eboRegistry.ts

probably not used now but getRequest should exist right?

We'll probably need that, yep.

Figured we'll add methods in the registry as needed, it'd be easier to know what are we going to use while implementing the EboActor handlers (probably all the get.../add...); there are some edge cases that I want to think about while implemeting the EboActor.

0xkenj1 · 2024-08-07T13:53:08Z

packages/automated-dispute/src/eboActor.ts

+            currentEpoch,
+            chainId,
+            epochBlockNumber,
+        );


we should handle the case in which proposeResponse fails, if it is an rpc failure or if a propose was already posted for the requestId

0xkenj1 · 2024-08-07T13:55:59Z

packages/automated-dispute/src/eboActor.ts

-    public async onRequestCreated(_event: EboEvent<"RequestCreated">): Promise<void> {
-        // TODO: implement
-        return;
+    private alreadyProposed(epoch: bigint, chainId: Caip2ChainId, blockNumber: bigint) {


would call this activePropose. We can have a propose on the registry that was already discarded

also , we should add an isActive field to response object

Does it make sense to have a new propose with the same block? I'd think that if there has already been a proposal with the block N, rejected or not, the agent wouldn't want to propose the same block N again. I might be missing something though.

Left this as alreadyProposed, this will be the last thing to check on the agent side; it'd save it of trying to propose a response that has already been created. The activePropose check is done at the beginning of this method.

0xkenj1 · 2024-08-07T14:17:35Z

packages/automated-dispute/src/eboMemoryRegistry.ts

+    private requests: Map<string, Request>;
+    private responses: Map<string, Response>;
+    private dispute: Map<string, Dispute>;


Suggested change

private requests: Map<string, Request>;

private responses: Map<string, Response>;

private dispute: Map<string, Dispute>;

private requests: Map<string, Request>= new Map();

private responses: Map<string, Response>= new Map();

private dispute: Map<string, Dispute>= new Map();

0xkenj1 · 2024-08-07T14:17:48Z

packages/automated-dispute/src/eboMemoryRegistry.ts

+        this.requests = new Map();
+        this.responses = new Map();
+        this.dispute = new Map();


Suggested change

this.requests = new Map();

this.responses = new Map();

this.dispute = new Map();

0xkenj1 · 2024-08-07T14:18:46Z

packages/automated-dispute/src/types/events.ts

-      : E extends "ResponseCreated"
-        ? ResponseCreated
+      : E extends "ResponseProposed"
+        ? ResponseProposed


0xkenj1 · 2024-08-07T14:19:28Z

packages/automated-dispute/src/types/prophet.ts

+
+    // To be byte-encode when sending it to Prophet
+    response: {
+        chainId: Caip2ChainId; // Pending on-chain definition on CAIP-2 usage


Suggested change

chainId: Caip2ChainId; // Pending on-chain definition on CAIP-2 usage

chainId: Caip2ChainId; //FIXME/TODO: Pending on-chain definition on CAIP-2 usage

0xkenj1 · 2024-08-07T14:21:45Z

packages/automated-dispute/src/interfaces/eboRegistry.ts

@@ -0,0 +1,18 @@
+import { Request, Response } from "../types/prophet.js";
+
+export interface EboRegistry {


lets add a high level description here

0xkenj1 · 2024-08-07T14:22:31Z

packages/automated-dispute/src/eboMemoryRegistry.ts

+        this.dispute = new Map();
+    }
+
+    public addRequest(requestId: string, request: Request) {


Suggested change

public addRequest(requestId: string, request: Request) {

/** @inheritdoc */

public addRequest(requestId: string, request: Request) {

ooaa nice, didn't know that. however, vscode is so magical that doesn't need this tag xd

0xkenj1 · 2024-08-07T14:22:43Z

packages/automated-dispute/src/eboMemoryRegistry.ts

+    public addRequest(requestId: string, request: Request) {
+        this.requests.set(requestId, request);
+    }
+


Suggested change

/** @inheritdoc */

0xnigir1 · 2024-08-07T14:25:35Z

i don't have the business context, if onRequestCreated is called twice on a row, is it possible that the event has different metadata? if yes, is it correct that the request entry on EboMemoryRegistry is overridden (since the requestId is always the same for an EboAgent instance)?

0xkenj1 · 2024-08-07T14:31:43Z

Each EboActor is responsible to handle a single and unique requestId, and also each of the EboAgents have its own EboRegistry.

Not sure if i got it, but maybe this info answers your question @0xnigir1

0xyaco · 2024-08-07T14:43:21Z

That's right, by design each EboActor will be called with onRequestCreated once. If for some reason this is called multiple times, Prophet's contracts are not working as expected and everything is probably in flames on the contracts side.

I like the idea of adding a check for already having a request though, a nice case for using logger.critical 🚒

0xyaco · 2024-08-07T19:30:47Z

packages/automated-dispute/src/eboActor.ts

+        if (this.anyActiveProposal()) {
+            // Skipping new proposal until the actor receives a ResponseDisputed event;
+            // at that moment, it will be possible to re-propose again.
+            this.logger.info(
+                `There is an active proposal for request ${this.requestId}. Skipping...`,
+            );
+
+            return;
+        }


@0xkenj1 included this check prior starting to build the Response (any response will be ignored if there's an active proposal)

0xyaco · 2024-08-07T19:32:17Z

packages/automated-dispute/src/eboActor.ts

-    public async onRequestCreated(_event: EboEvent<"RequestCreated">): Promise<void> {
-        // TODO: implement
-        return;
+    private alreadyProposed(epoch: bigint, chainId: Caip2ChainId, blockNumber: bigint) {


Left this as alreadyProposed, this will be the last thing to check on the agent side; it'd save it of trying to propose a response that has already been created. The activePropose check is done at the beginning of this method.

0xnigir1 · 2024-08-08T13:51:42Z

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

+
+            expect(proposeResponseMock).not.toHaveBeenCalled();
+        });
+


Suggested change

it("throws if request was already handled by the actor", () => {

const actor = new EboActor(

protocolProvider,

blockNumberService,

registry,

requestId,

logger,

);

vi.spyOn(registry, "getRequest").mockReturnValue(BASE_REQUEST);

expect(actor.onRequestCreated(requestCreatedEvent)).rejects.toBeInstanceOf(

InvalidActorState,

);

});

0xnigir1 · 2024-08-08T13:53:57Z

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

+
+import { EboActor } from "../src/eboActor.js";
+import { EboMemoryRegistry } from "../src/eboMemoryRegistry.js";
+import { RequestMismatch } from "../src/exceptions/requestMismatch.js";


Suggested change

import { RequestMismatch } from "../src/exceptions/requestMismatch.js";

import { RequestMismatch } from "../src/exceptions/requestMismatch.js";

import { InvalidActorState } from "../src/exceptions/invalidActorState.exception.js";

this is related to the other comment

linear · 2024-08-28T15:04:42Z

GRT-77 Add constants and types that might be shared across different packages

0xyaco force-pushed the feat/on-request-created branch from 1fe0161 to 4178514 Compare August 6, 2024 22:16

0xyaco added 3 commits August 6, 2024 19:32

fix: bigint for timestamp values

6cf15f3

feat: implement EboActor.onRequestCreated handler

6bb11e6

feat: add registry to actor

40f0e03

0xyaco force-pushed the feat/on-request-created branch from 4178514 to 40f0e03 Compare August 6, 2024 22:33

0xyaco added 3 commits August 6, 2024 19:36

chore: include shared in automated-dispute module

6c7e02e

fix: lint protocolProvider

c8d6702

refactor: use Timestamp alias as bigint

03e75c7

0xyaco changed the title ~~feat: [WIP] build onRequestCreated~~ feat: build onRequestCreated Aug 6, 2024

0xyaco marked this pull request as ready for review August 6, 2024 22:56

0xyaco requested review from 0xkenj1 and 0xnigir1 August 6, 2024 22:56

docs: natspec for eboactor

bd3f0db

0xnigir1 reviewed Aug 7, 2024

View reviewed changes

0xyaco added 2 commits August 7, 2024 10:28

feat: handle already proposed responses

33271d7

feat: use EboRegistry interface

6647610

0xkenj1 reviewed Aug 7, 2024

View reviewed changes

0xyaco added 4 commits August 7, 2024 11:46

docs: add doc on EboRegistry related methods

4a16f80

refactor: simplify eboMemoryRegistry constructor

be694f1

feat: handle ProtocolProvider contract revert

c9bca60

feat: handle invalid state for duplicate request

2aba0ac

0xyaco force-pushed the feat/on-request-created branch from d3dba9d to 2aba0ac Compare August 7, 2024 19:24

0xyaco commented Aug 7, 2024

View reviewed changes

0xyaco requested review from 0xkenj1 and 0xnigir1 August 7, 2024 22:42

0xkenj1 previously approved these changes Aug 8, 2024

View reviewed changes

0xnigir1 reviewed Aug 8, 2024

View reviewed changes

test: test actor when already handled request

a91e4ef

0xyaco dismissed 0xkenj1’s stale review via a91e4ef August 8, 2024 16:31

0xnigir1 approved these changes Aug 8, 2024

View reviewed changes

0xkenj1 approved these changes Aug 8, 2024

View reviewed changes

0xyaco merged commit 3933c43 into dev Aug 8, 2024
5 checks passed

0xyaco deleted the feat/on-request-created branch August 8, 2024 18:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: build onRequestCreated #18

feat: build onRequestCreated #18

0xyaco commented Aug 5, 2024 •

edited

Loading

linear bot commented Aug 5, 2024

0xnigir1 Aug 7, 2024

0xyaco Aug 7, 2024

0xnigir1 Aug 7, 2024

0xyaco Aug 7, 2024

0xkenj1 Aug 7, 2024

0xkenj1 Aug 7, 2024

0xkenj1 Aug 7, 2024

0xyaco Aug 7, 2024

0xyaco Aug 7, 2024

0xkenj1 Aug 7, 2024

0xkenj1 Aug 7, 2024

0xkenj1 Aug 7, 2024

0xkenj1 Aug 7, 2024

0xkenj1 Aug 7, 2024

0xkenj1 Aug 7, 2024

0xnigir1 Aug 7, 2024

0xkenj1 Aug 7, 2024

0xnigir1 commented Aug 7, 2024

0xkenj1 commented Aug 7, 2024

0xyaco commented Aug 7, 2024

0xyaco Aug 7, 2024

0xyaco Aug 7, 2024

0xnigir1 Aug 8, 2024

0xnigir1 Aug 8, 2024

linear bot commented Aug 28, 2024

	this.requests = new Map();
	this.responses = new Map();
	this.dispute = new Map();

	chainId: Caip2ChainId; // Pending on-chain definition on CAIP-2 usage
	chainId: Caip2ChainId; //FIXME/TODO: Pending on-chain definition on CAIP-2 usage

		@@ -0,0 +1,18 @@
		import { Request, Response } from "../types/prophet.js";

		export interface EboRegistry {

	public addRequest(requestId: string, request: Request) {
	/** @inheritdoc */
	public addRequest(requestId: string, request: Request) {

+        it("throws if request was already handled by the actor", () => {
+            const actor = new EboActor(
+                protocolProvider,
+                blockNumberService,
+                registry,
+                requestId,
+                logger,
+            );
+            vi.spyOn(registry, "getRequest").mockReturnValue(BASE_REQUEST);
+            expect(actor.onRequestCreated(requestCreatedEvent)).rejects.toBeInstanceOf(
+                InvalidActorState,
+            );
+        });

	import { RequestMismatch } from "../src/exceptions/requestMismatch.js";
	import { RequestMismatch } from "../src/exceptions/requestMismatch.js";
	import { InvalidActorState } from "../src/exceptions/invalidActorState.exception.js";

feat: build onRequestCreated #18

feat: build onRequestCreated #18

Conversation

0xyaco commented Aug 5, 2024 • edited Loading

🤖 Linear

Description

linear bot commented Aug 5, 2024

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

0xnigir1 commented Aug 7, 2024

0xkenj1 commented Aug 7, 2024

0xyaco commented Aug 7, 2024

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

linear bot commented Aug 28, 2024

0xyaco commented Aug 5, 2024 •

edited

Loading