feat: pool created handler #11

0xnigir1 · 2024-10-16T18:18:15Z

🤖 Linear

Description

Implements the PoolCreatedHandler on AlloProcessor.
On PoolCreated event, we:

check for possible Metadata on the event
extract some timestamps data from the strategy contract
for MerkleDonation, we calculate an optional matchTokenAmount
we calculate an optional initial pool fund amount
we return a Changeset (will be modified in the future for event emitter) consisting of:
- insert the new Round
- insert each corresponding round roles
- delete the pending round roles

TODO:

move strategy data fetching to the StrategyHandler when we create them
get the correct token given the tokenAddress from event data (we will have a whitelisted map)
use EVM Provider instead of plain viemClient

Note:
Admin/Manager roles for the pool are emitted before the pool is created so a pending round role is inserted in the db. Now that the PoolCreated event is emitted, we can convert pending roles to actual round roles.

Checklist before requesting a review

I have conducted a self-review of my code.
I have conducted a QA.
If it is a core feature, I have included comprehensive tests.

linear · 2024-10-16T18:18:17Z

GIT-57 Base

GIT-58 PoolCreatedHandler

0xkenj1

GJ ser, just some comments

0xkenj1 · 2024-10-16T18:37:33Z

packages/processors/src/allo/allo.processor.ts

-    process(_event: ProtocolEvent<"Allo", AlloEvent>): Promise<Changeset> {
-        throw new Error("Method not implemented.");
+    constructor(
+        private readonly chainId: ChainId,


We can get rid of chainId as attribute, since we don't need processors to be attached to a certain chainId, just the core pipeline. I think we can just use the chainId from the event. Wdyt?

i thought but actually the InsertRound object needs the chainId, we could attach it later but i think that's kindof unclean thing to do, we would be delegating a task to the caller

@0xkenj1 should we pass just the chainId or a Viem Chain type?

My two cents here; probably coupling the proccessor's public interface to viem's types is not the happiest thing to do (ie what if we need to change viem for other lib?)

0xkenj1 · 2024-10-16T18:38:47Z

packages/processors/src/allo/handlers/poolCreated.handler.ts

+    constructor(
+        readonly event: ProtocolEvent<"Allo", "PoolCreated">,
+        private readonly chainId: ChainId,
+        dependencies: {


we can create a type called ProcessorDependencies and use it everywhere, to simplify code

0xkenj1 · 2024-10-16T18:39:56Z

packages/processors/src/allo/handlers/poolCreated.handler.ts

+                round?: unknown;
+                application?: unknown;
+            }>(metadataPointer);
+            const parsedRoundMetadata = RoundMetadataSchema.safeParse(metadata?.round);


should we pass schema validator in getMetadata method?

forgot i implemented that jajaja

i'll come back later to this because i'll need to make adjustments since code flow should go on even if validation fails, so i need to make adjustments to the metadata provider
this makes me think if it's correct to have the validation function as arg 🤔

0xkenj1 · 2024-10-16T18:40:55Z

packages/processors/src/allo/handlers/poolCreated.handler.ts

+                symbol: "USDC", //TODO: get symbol from token
+                name: "USDC", //TODO: get name from token


i would leave these two as unknown wdyt?

actually idk what the TokenType will have, when we copy the map from the old repo we'll see

0xkenj1 · 2024-10-16T18:41:39Z

packages/processors/src/allo/handlers/poolCreated.handler.ts

+            let strategyTimings: {
+                applicationsStartTime: Date | null;
+                applicationsEndTime: Date | null;
+                donationsStartTime: Date | null;
+                donationsEndTime: Date | null;
+            } = {


type StrategyTimings ?

0xkenj1 · 2024-10-16T18:52:58Z

packages/processors/src/helpers/strategy.ts

+    let registrationStartTimeResolved: bigint;
+    let registrationEndTimeResolved: bigint;
+    let allocationStartTimeResolved: bigint;
+    let allocationEndTimeResolved: bigint;
+
+    if (viemClient.chain?.contracts?.multicall3) {
+        const results = await viemClient.multicall({
+            contracts: [
+                {
+                    abi: DonationVotingMerkleDistributionDirectTransferStrategy,
+                    functionName: "registrationStartTime",
+                    address: strategyId,
+                },
+                {
+                    abi: DonationVotingMerkleDistributionDirectTransferStrategy,
+                    functionName: "registrationEndTime",
+                    address: strategyId,
+                },
+                {
+                    abi: DonationVotingMerkleDistributionDirectTransferStrategy,
+                    functionName: "allocationStartTime",
+                    address: strategyId,
+                },
+                {
+                    abi: DonationVotingMerkleDistributionDirectTransferStrategy,
+                    functionName: "allocationEndTime",
+                    address: strategyId,
+                },
+            ],
+            allowFailure: false,
+        });
+        registrationStartTimeResolved = results[0];
+        registrationEndTimeResolved = results[1];
+        allocationStartTimeResolved = results[2];
+        allocationEndTimeResolved = results[3];
+    } else {
+        const results = await Promise.all([
+            viemClient.readContract({
+                abi: DonationVotingMerkleDistributionDirectTransferStrategy,
+                functionName: "registrationStartTime",
+                address: strategyId,
+            }),
+            viemClient.readContract({
+                abi: DonationVotingMerkleDistributionDirectTransferStrategy,
+                functionName: "registrationEndTime",
+                address: strategyId,
+            }),
+            viemClient.readContract({
+                abi: DonationVotingMerkleDistributionDirectTransferStrategy,
+                functionName: "allocationStartTime",
+                address: strategyId,
+            }),
+            viemClient.readContract({
+                abi: DonationVotingMerkleDistributionDirectTransferStrategy,
+                functionName: "allocationEndTime",
+                address: strategyId,
+            }),
+        ]);
+        registrationStartTimeResolved = results[0];
+        registrationEndTimeResolved = results[1];
+        allocationStartTimeResolved = results[2];
+        allocationEndTimeResolved = results[3];
+    }


Suggested change

let registrationStartTimeResolved: bigint;

let registrationEndTimeResolved: bigint;

let allocationStartTimeResolved: bigint;

let allocationEndTimeResolved: bigint;

if (viemClient.chain?.contracts?.multicall3) {

const results = await viemClient.multicall({

contracts: [

{

abi: DonationVotingMerkleDistributionDirectTransferStrategy,

functionName: "registrationStartTime",

address: strategyId,

},

{

abi: DonationVotingMerkleDistributionDirectTransferStrategy,

functionName: "registrationEndTime",

address: strategyId,

},

{

abi: DonationVotingMerkleDistributionDirectTransferStrategy,

functionName: "allocationStartTime",

address: strategyId,

},

{

abi: DonationVotingMerkleDistributionDirectTransferStrategy,

functionName: "allocationEndTime",

address: strategyId,

},

],

allowFailure: false,

});

registrationStartTimeResolved = results[0];

registrationEndTimeResolved = results[1];

allocationStartTimeResolved = results[2];

allocationEndTimeResolved = results[3];

} else {

const results = await Promise.all([

viemClient.readContract({

abi: DonationVotingMerkleDistributionDirectTransferStrategy,

functionName: "registrationStartTime",

address: strategyId,

}),

viemClient.readContract({

abi: DonationVotingMerkleDistributionDirectTransferStrategy,

functionName: "registrationEndTime",

address: strategyId,

}),

viemClient.readContract({

abi: DonationVotingMerkleDistributionDirectTransferStrategy,

functionName: "allocationStartTime",

address: strategyId,

}),

viemClient.readContract({

abi: DonationVotingMerkleDistributionDirectTransferStrategy,

functionName: "allocationEndTime",

address: strategyId,

}),

]);

registrationStartTimeResolved = results[0];

registrationEndTimeResolved = results[1];

allocationStartTimeResolved = results[2];

allocationEndTimeResolved = results[3];

}

const results: {

registrationStartTimeResolved: bigint;

registrationEndTimeResolved: bigint;

allocationStartTimeResolved: bigint;

allocationEndTimeResolved: bigint;

};

const contractCalls = [

{

abi: DonationVotingMerkleDistributionDirectTransferStrategy,

functionName: "registrationStartTime",

address: strategyId,

},

{

abi: DonationVotingMerkleDistributionDirectTransferStrategy,

functionName: "registrationEndTime",

address: strategyId,

},

{

abi: DonationVotingMerkleDistributionDirectTransferStrategy,

functionName: "allocationStartTime",

address: strategyId,

},

{

abi: DonationVotingMerkleDistributionDirectTransferStrategy,

functionName: "allocationEndTime",

address: strategyId,

},

];

if (viemClient.chain?.contracts?.multicall3) {

results = await viemClient.multicall({

contracts: contractCalls,

allowFailure: false,

});

} else {

results = await Promise.all(

contractCalls.map((contractCall) =>

viemClient.readContract(contractCall)

)

);

}

didn't know that trick 🆒

0xkenj1 · 2024-10-16T18:56:31Z

packages/processors/src/helpers/strategy.ts

+    let registrationStartTimeResolved: bigint;
+    let registrationEndTimeResolved: bigint;
+
+    if (viemClient.chain?.contracts?.multicall3) {
+        const results = await viemClient.multicall({
+            contracts: [
+                {
+                    abi: DirectGrantsLiteStrategy,
+                    functionName: "registrationStartTime",
+                    address: strategyAddress,
+                },
+                {
+                    abi: DirectGrantsLiteStrategy,
+                    functionName: "registrationEndTime",
+                    address: strategyAddress,
+                },
+            ],
+            allowFailure: false,
+        });
+        registrationStartTimeResolved = results[0];
+        registrationEndTimeResolved = results[1];
+    } else {
+        const results = await Promise.all([
+            viemClient.readContract({
+                abi: DirectGrantsLiteStrategy,
+                functionName: "registrationStartTime",
+                address: strategyAddress,
+            }),
+            viemClient.readContract({
+                abi: DirectGrantsLiteStrategy,
+                functionName: "registrationEndTime",
+                address: strategyAddress,
+            }),
+        ]);
+        registrationStartTimeResolved = results[0];
+        registrationEndTimeResolved = results[1];
+    }
+


0xkenj1 · 2024-10-16T18:58:16Z

packages/processors/test/helpers/utils.spec.ts

shoulds in tests

0xkenj1 · 2024-10-16T18:59:29Z

packages/shared/src/types/events/allo.ts

+    metadata: {
+        pointer: string;
+        protocol: bigint;
+    };


i think the data for metadata comes as a tuple/array

Suggested change

metadata: {

pointer: string;

protocol: bigint;

};

metadata: [

pointer: string;

protocol: bigint;

];

aaaa thanks for the heads up

0xkenj1 · 2024-10-16T19:00:49Z

packages/processors/test/allo/handlers/poolCreated.handler.spec.ts

+        },
+    };
+
+    return mergeDeep(defaultEvent, overrides) as ProtocolEvent<"Allo", "PoolCreated">;


0xkenj1 · 2024-10-16T19:21:22Z

packages/processors/src/allo/handlers/poolCreated.handler.ts

+        } catch (error: unknown) {
+            console.error(
+                `An error occurred while processing the PoolCreated event. Event: ${this.event} - Error: ${error}`,
+            );
+            return [];
+        }


i think at handlers level we just need to throw the errors, and let the processor to handle them and take actions based on the type of error (like for example retry the processing) and we avoid repeating logic of error handling on the handlers. wdyt ?

like that approach 🫡

0xyaco

Cool stuff! Eager to see more PRs; left some random comments.

0xyaco · 2024-10-17T09:31:33Z

packages/processors/src/allo/allo.processor.ts

-    process(_event: ProtocolEvent<"Allo", AlloEvent>): Promise<Changeset> {
-        throw new Error("Method not implemented.");
+    constructor(
+        private readonly chainId: ChainId,


My two cents here; probably coupling the proccessor's public interface to viem's types is not the happiest thing to do (ie what if we need to change viem for other lib?)

0xyaco · 2024-10-17T09:49:42Z

packages/processors/src/allo/handlers/poolCreated.handler.ts

+
+import type { Changeset, NewRound, PendingRoundRole } from "@grants-stack-indexer/repository";
+import type { ChainId, ProtocolEvent } from "@grants-stack-indexer/shared";
+import { isAlloNativeToken } from "@grants-stack-indexer/shared/";


Does the last / mean something?

wildcat eye to catch that 👁️

0xyaco · 2024-10-17T09:53:10Z

packages/processors/src/helpers/strategy.ts

+export function extractStrategyFromId(_id: Address): Strategy | undefined {
+    const id = _id.toLowerCase();
+    /* eslint-disable no-fallthrough */
+    switch (id) {
+        // SQFSuperfluidv1
+        case "0xf8a14294e80ff012e54157ec9d1b2827421f1e7f6bde38c06730b1c031b3f935":
+            return {
+                id: id as SanitizedStrategyId,
+                name: "allov2.SQFSuperFluidStrategy",
+                groups: ["allov2.SQFSuperFluidStrategy"],
+            };


Asking to get more business context; are strategies going to be always hardcoded or do you plan to use some kind of API/service/repository to get these mappings?

we'll have a mapping in shared, from ID to the handler class actually. This is just the same class from the current indexer

I'll add the TODO to make it clear

0xyaco · 2024-10-17T10:11:19Z

packages/processors/src/helpers/tokenMath.ts

+export const calculateAmountInUsd = (
+    amount: bigint,
+    tokenPriceInUsd: number,
+    tokenDecimals: number,
+    truncateDecimals?: number,
+): number => {


I'd strongly suggest including at this point of the project, given that you are building the bases of your features, some kind of bigdecimal/bignumber lib to handle money values; anywhere you mention USD, price, etc. It'll make the development extremely robust and you won't need to worry about Number precision, "stringification" of numbers, "numberification" of strings, etc.

There are two drawbacks that you might need to take into account:

Operations between these values might imply some special methods (eg a.plus(b)) and not the js operators (a + b).

Performance will be definitely worse, but if you are not doing 100M of operations per second it shouldn't be something to worry about.

As an bonus comment to show the degeneracy of JS numbers implementation, try doing this:

const n = 9332654729891549; console.log(n.toString());

Check the last digit of the output. ☠️

great contribution ser 🫡
I also lean into this but we would have to change the DB data type at some point since now they handle it like this with truncation (don't ask why)

cc @0xkenj1 should we make the change now or leave it as a todo/nth for after the MVP?

as discussed offline, we ended up using bignumber and will change DB types to numeric, so we return string instead of hardcasting to number

jahabeebs

looks good, just some optional suggestions 👍🏻

jahabeebs · 2024-10-17T20:54:21Z

packages/processors/src/helpers/tokenMath.ts

+
+    if (truncateDecimals !== undefined) {
+        if (truncateDecimals < 0 || truncateDecimals > 18) {
+            throw new Error("Truncate decimals must be between 0 and 18");


not sure if we prefer a custom error here?

you're right ser 💯

jahabeebs · 2024-10-17T20:54:50Z

packages/processors/src/allo/allo.processor.ts

+            case "PoolCreated":
+                return new PoolCreatedHandler(event, this.chainId, this.dependencies).handle();
+            default:
+                throw new Error(`Unknown event name: ${event.eventName}`);


not sure if we prefer a custom error here?

yes we do 🤣

actually here i have my implementation for this error:

import { ContractName } from "@grants-stack-indexer/shared"; export class UnsupportedEventException extends Error { constructor( contract: ContractName, public readonly eventName: string, ) { super(`Event ${eventName} unsupported for ${contract} processor`); } }

thanks for this xd

0xkenj1 · 2024-10-17T21:13:02Z

packages/processors/src/allo/allo.processor.ts

+            case "PoolCreated":
+                return new PoolCreatedHandler(event, this.chainId, this.dependencies).handle();
+            default:
+                throw new Error(`Unknown event name: ${event.eventName}`);


yes we do 🤣

actually here i have my implementation for this error:

import { ContractName } from "@grants-stack-indexer/shared"; export class UnsupportedEventException extends Error { constructor( contract: ContractName, public readonly eventName: string, ) { super(`Event ${eventName} unsupported for ${contract} processor`); } }

0xkenj1 · 2024-10-17T21:15:21Z

packages/processors/src/helpers/strategy.ts

+
+/**
+ * Gets the strategy data for the DonationVotingMerkleDistributionDirectTransferStrategy
+ * @param viemClient - The viem client


Suggested change

* @param viemClient - The viem client

* @param evmProvider

0xkenj1 · 2024-10-17T21:15:30Z

packages/processors/src/helpers/strategy.ts

+
+/**
+ * Gets the strategy data for the DirectGrantsStrategy
+ * @param viemClient - The viem client


0xkenj1 · 2024-10-17T21:15:47Z

packages/processors/src/helpers/tokenMath.ts

+
+    if (truncateDecimals !== undefined) {
+        if (truncateDecimals < 0 || truncateDecimals > 18) {
+            throw new Error("Truncate decimals must be between 0 and 18");


jahabeebs

👍🏻

0xnigir1 added 3 commits October 14, 2024 13:39

feat(envio): add tx fields to config.yaml

6adc784

feat: implement PoolCreated event handling

a89704c

test: add pool created handler tests

40efd36

0xnigir1 requested review from jahabeebs, 0xkenj1 and 0xyaco October 16, 2024 18:18

0xkenj1 reviewed Oct 16, 2024

View reviewed changes

0xnigir1 added 2 commits October 16, 2024 16:33

Merge remote-tracking branch 'origin/dev' into feat/pool-created-handler

dd1a3c8

fix: pr comments

afba646

0xnigir1 requested a review from 0xkenj1 October 16, 2024 21:48

0xyaco reviewed Oct 17, 2024

View reviewed changes

0xnigir1 added 2 commits October 17, 2024 09:54

fix: add todo and fix typo

7fc6f04

feat: add bignumberjs for money ops

11c9bbe

0xnigir1 requested a review from 0xyaco October 17, 2024 19:55

jahabeebs previously approved these changes Oct 17, 2024

View reviewed changes

0xkenj1 reviewed Oct 17, 2024

View reviewed changes

fix: add specific exceptions

89d2a63

0xnigir1 dismissed jahabeebs’s stale review via 89d2a63 October 17, 2024 21:39

0xnigir1 requested review from jahabeebs and 0xkenj1 October 17, 2024 21:40

0xkenj1 approved these changes Oct 17, 2024

View reviewed changes

jahabeebs approved these changes Oct 17, 2024

View reviewed changes

0xyaco approved these changes Oct 18, 2024

View reviewed changes

0xnigir1 merged commit f585a86 into dev Oct 18, 2024
6 checks passed

0xnigir1 deleted the feat/pool-created-handler branch October 18, 2024 11:36

		symbol: "USDC", //TODO: get symbol from token
		name: "USDC", //TODO: get name from token

feat: pool created handler #11

feat: pool created handler #11

Conversation

0xnigir1 commented Oct 16, 2024 • edited Loading

🤖 Linear

Description

Checklist before requesting a review

linear bot commented Oct 16, 2024

0xkenj1 left a 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

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 left a 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

jahabeebs left a 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

jahabeebs left a comment

Choose a reason for hiding this comment

0xnigir1 commented Oct 16, 2024 •

edited

Loading