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 EboActorsManager #25

Merged
merged 7 commits into from
Aug 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions packages/automated-dispute/src/eboActor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,15 @@ export class EboActor {
private readonly logger: ILogger,
) {}

/**
* Get the request ID this actor is handling.
*
* @returns request ID
*/
public getRequestId(): string {
return this.actorRequest.id;
}

/**
* Handle `RequestCreated` event.
*
Expand Down
43 changes: 43 additions & 0 deletions packages/automated-dispute/src/eboActorsManager.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import { EboActor } from "./eboActor.js";
import { RequestAlreadyHandled } from "./exceptions/index.js";

export class EboActorsManager {
Copy link
Collaborator

Choose a reason for hiding this comment

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

the manager implementation is always in-memory or is like the Registry that may in the future be a Redis or whatever makes sense? in that case, we should have the proper interface

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's something interesting to think about; in this case I think The Graph should need to scale a lot for this to need something outside the in-memory solution used here. Worst case scenario, at the same there will be 1 item (ie 1 <request, actor> tuple) for every indexed chain, which are not that many.

private readonly requestActorMap: Map<string, EboActor>;

constructor() {
this.requestActorMap = new Map();
}

/**
* Registers the actor and makes it fetchable by the ID of the request the actor is handling.
*
* @param actor an `EboActor` instance that handles the request
*/
public registerActor(actor: EboActor): void {
const requestId = actor.getRequestId();

if (this.requestActorMap.has(requestId)) throw new RequestAlreadyHandled(requestId);

this.requestActorMap.set(requestId, actor);
}

/**
* Get the `EboActor` instance linked with the `requestId`.
*
* @param requestId request ID
* @returns an `EboActor` instance if found by `requestId`, otherwise `undefined`
*/
public getActor(requestId: string): EboActor | undefined {
return this.requestActorMap.get(requestId);
}

/**
* Deletes an actor from the manager, based on its linked request.
*
* @param requestId request ID
* @returns `true` if there was a linked actor for the request ID and it was removed, or `false` if the request was not linked to any actor.
*/
public deleteActor(requestId: string): boolean {
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is returning a boolean ? Should it be void ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

same doubt, i guess on error false is returned instead of throwing

Copy link
Collaborator Author

@0xyaco 0xyaco Aug 16, 2024

Choose a reason for hiding this comment

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

We can do void.

I'm just following the Map.prototype.delete() return type for the sake of consistency.

I guess it doesn't hurt to keep the boolean return type, but now that you mention, I should add some docs on these methods, it would've helped here 😬

Copy link
Collaborator

Choose a reason for hiding this comment

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

yep, would be very helpful for the user of this class, should we add a warn or sth if the requestId doesn't exist?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sounds good. The EboProcessor will most likely warn if the delete returns false when removing actors from the registry; can't think of any situation out of the top of my head where that'd happen, but better to be safe than sorry.

return this.requestActorMap.delete(requestId);
}
}
2 changes: 2 additions & 0 deletions packages/automated-dispute/src/exceptions/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
export * from "./rpcUrlsEmpty.exception.js";
export * from "./invalidActorState.exception.js";
export * from "./requestAlreadyHandled.exception.js";
export * from "./requestMismatch.js";
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export class RequestAlreadyHandled extends Error {
constructor(requestId: string) {
super(`Request ${requestId} is already being handled by another actor.`);

this.name = "RequestAlreadyHandled";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,16 @@ import { beforeEach, describe, expect, it, vi } from "vitest";

import { EboActor } from "../../src/eboActor.js";
import { EboMemoryRegistry } from "../../src/eboMemoryRegistry.js";
import { RequestMismatch } from "../../src/exceptions/requestMismatch.js";
import { RequestMismatch } from "../../src/exceptions/index.js";
import { ProtocolProvider } from "../../src/protocolProvider.js";
import { EboEvent } from "../../src/types/events.js";
import { Response } from "../../src/types/prophet.js";
import { EboEvent, Response } from "../../src/types/index.js";
import mocks from "../mocks/index.js";
import {
DEFAULT_MOCKED_PROTOCOL_CONTRACTS,
DEFAULT_MOCKED_REQUEST_CREATED_DATA,
} from "./fixtures.js";

const logger: ILogger = {
info: vi.fn(),
warn: vi.fn(),
error: vi.fn(),
debug: vi.fn(),
};
const logger: ILogger = mocks.mockLogger();

describe("EboActor", () => {
describe("onRequestCreated", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,10 @@ import { describe, expect, it, vi } from "vitest";

import { InvalidActorState } from "../../src/exceptions/invalidActorState.exception.js";
import { EboEvent } from "../../src/types/events.js";
import mocks from "../mocks/index.js";
import { DEFAULT_MOCKED_REQUEST_CREATED_DATA } from "./fixtures.js";
import mocks from "./mocks/index.js";

const logger: ILogger = {
info: vi.fn(),
warn: vi.fn(),
error: vi.fn(),
debug: vi.fn(),
};
const logger: ILogger = mocks.mockLogger();

describe("EboActor", () => {
describe("onRequestFinalized", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,10 @@ import { describe, expect, it, vi } from "vitest";
import { InvalidActorState } from "../../src/exceptions/invalidActorState.exception";
import { EboEvent } from "../../src/types/events";
import { Response } from "../../src/types/prophet";
import mocks from "../mocks/index.js";
import { DEFAULT_MOCKED_REQUEST_CREATED_DATA } from "./fixtures";
import mocks from "./mocks/index.js";

const logger: ILogger = {
info: vi.fn(),
warn: vi.fn(),
error: vi.fn(),
debug: vi.fn(),
};

const logger: ILogger = mocks.mockLogger();

describe("onResponseDisputed", () => {
const actorRequest = DEFAULT_MOCKED_REQUEST_CREATED_DATA;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,10 @@ import { describe, expect, it, vi } from "vitest";

import { InvalidActorState } from "../../src/exceptions/invalidActorState.exception";
import { EboEvent } from "../../src/types/events";
import mocks from "../mocks/index.ts";
import { DEFAULT_MOCKED_REQUEST_CREATED_DATA } from "./fixtures.ts";
import mocks from "./mocks/index.ts";

const logger: ILogger = {
info: vi.fn(),
warn: vi.fn(),
error: vi.fn(),
debug: vi.fn(),
};

const logger: ILogger = mocks.mockLogger();

describe("onResponseProposed", () => {
const actorRequest = DEFAULT_MOCKED_REQUEST_CREATED_DATA;
Expand Down
80 changes: 80 additions & 0 deletions packages/automated-dispute/tests/eboActorsManager.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import { ILogger } from "@ebo-agent/shared";
import { describe, expect, it, vi } from "vitest";

import { EboActorsManager } from "../src/eboActorsManager.js";
import { RequestAlreadyHandled } from "../src/exceptions/index.js";
import { DEFAULT_MOCKED_REQUEST_CREATED_DATA } from "./eboActor/fixtures.js";
import mocks from "./mocks/index.js";

const logger: ILogger = mocks.mockLogger();

describe("EboActorsManager", () => {
describe("registerActor", () => {
it("registers the actor correctly", () => {
const request = DEFAULT_MOCKED_REQUEST_CREATED_DATA;
const { actor } = mocks.buildEboActor(request, logger);
const actorsManager = new EboActorsManager();
const mockSetRequestActorMap = vi.spyOn(actorsManager["requestActorMap"], "set");

actorsManager.registerActor(actor);

expect(mockSetRequestActorMap).toHaveBeenCalledWith(request.id, actor);
expect(actorsManager.getActor(actor.getRequestId())).toBe(actor);
});

it("throws if the request has already an actor linked to it", () => {
const request = DEFAULT_MOCKED_REQUEST_CREATED_DATA;
const { actor: firstActor } = mocks.buildEboActor(request, logger);
const { actor: secondActor } = mocks.buildEboActor(request, logger);
const actorsManager = new EboActorsManager();

actorsManager.registerActor(firstActor);

expect(() => actorsManager.registerActor(secondActor)).toThrowError(
RequestAlreadyHandled,
);
});
});

describe("getActor", () => {
it("returns undefined if the request is not linked to any actor", () => {
const actorsManager = new EboActorsManager();

expect(actorsManager.getActor("0x9999")).toBeUndefined();
});

it("returns the request's linked actor", () => {
const request = DEFAULT_MOCKED_REQUEST_CREATED_DATA;
const { actor } = mocks.buildEboActor(request, logger);
const actorsManager = new EboActorsManager();

actorsManager.registerActor(actor);

expect(actorsManager.getActor(request.id)).toBe(actor);
});
});

describe("deleteActor", () => {
it("deletes the actor linked to the request", () => {
const request = DEFAULT_MOCKED_REQUEST_CREATED_DATA;
const { actor } = mocks.buildEboActor(request, logger);
const actorsManager = new EboActorsManager();

actorsManager.registerActor(actor);

expect(actorsManager.getActor(request.id)).toBe(actor);

actorsManager.deleteActor(request.id);

expect(actorsManager.getActor(request.id)).toBeUndefined();
});

it("returns false if the request has no actors linked", () => {
const requestId = "0x01";
const actorsManager = new EboActorsManager();

expect(actorsManager.getActor(requestId)).toBeUndefined();
expect(actorsManager.deleteActor(requestId)).toEqual(false);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ import { Caip2ChainId } from "@ebo-agent/blocknumber/dist/types";
import { ILogger } from "@ebo-agent/shared";
import { vi } from "vitest";

import { EboActor } from "../../../src/eboActor";
import { EboMemoryRegistry } from "../../../src/eboMemoryRegistry";
import { ProtocolProvider } from "../../../src/protocolProvider";
import { Request, Response } from "../../../src/types/prophet";
import { DEFAULT_MOCKED_PROTOCOL_CONTRACTS } from "../fixtures";
import { EboActor } from "../../src/eboActor";
import { EboMemoryRegistry } from "../../src/eboMemoryRegistry";
import { ProtocolProvider } from "../../src/protocolProvider";
import { Request, Response } from "../../src/types/prophet";
import { DEFAULT_MOCKED_PROTOCOL_CONTRACTS } from "../eboActor/fixtures";

/**
* Builds a base `EboActor` scaffolded with all its dependencies.
Expand All @@ -16,7 +16,7 @@ import { DEFAULT_MOCKED_PROTOCOL_CONTRACTS } from "../fixtures";
* @param logger logger
* @returns
*/
function buildEboActor(request: Request, logger: ILogger) {
export function buildEboActor(request: Request, logger: ILogger) {
const { id, chainId, epoch, epochTimestamp } = request;

const onTerminate = vi.fn();
Expand Down Expand Up @@ -60,7 +60,7 @@ function buildEboActor(request: Request, logger: ILogger) {
* @param attributes custom attributes to set into the response to build
* @returns a `Response`
*/
function buildResponse(request: Request, attributes: Partial<Response> = {}): Response {
export function buildResponse(request: Request, attributes: Partial<Response> = {}): Response {
const baseResponse: Response = {
id: "0x01",
wasDisputed: false,
Expand All @@ -80,5 +80,3 @@ function buildResponse(request: Request, attributes: Partial<Response> = {}): Re
...attributes,
};
}

export default { buildEboActor, buildResponse };
4 changes: 4 additions & 0 deletions packages/automated-dispute/tests/mocks/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import { buildEboActor, buildResponse } from "./eboActor.js";
import { mockLogger } from "./logger.js";

export default { buildEboActor, buildResponse, mockLogger };
9 changes: 9 additions & 0 deletions packages/automated-dispute/tests/mocks/logger.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { ILogger } from "@ebo-agent/shared";
import { vi } from "vitest";

export const mockLogger: () => ILogger = () => ({
info: vi.fn(),
warn: vi.fn(),
error: vi.fn(),
debug: vi.fn(),
});