-
Notifications
You must be signed in to change notification settings - Fork 0
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: persisting strategy registry into db #40
base: dev
Are you sure you want to change the base?
Changes from all commits
b3095ad
a851079
6b7e3ab
3978835
6adb666
aa3ee27
fe62481
9223013
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,22 +1,38 @@ | ||
import { Address, Hex } from "viem"; | ||
|
||
import { Strategy } from "@grants-stack-indexer/repository"; | ||
import { ChainId } from "@grants-stack-indexer/shared"; | ||
|
||
/** | ||
* The strategy registry saves the mapping between the strategy address and the strategy id. Serves as a Cache | ||
* to avoid having to read from the chain to get the strategy id every time. | ||
*/ | ||
//TODO: implement a mechanism to record Strategy that we still don't have a corresponding handler | ||
// we need to store and mark that this strategy is not handled yet, so when it's supported we can process all of the pending events for it | ||
export interface IStrategyRegistry { | ||
/** | ||
* Get the strategy id by the strategy address | ||
* Get the strategy id by the strategy address and chain id | ||
* | ||
* @param chainId - The chain id | ||
* @param strategyAddress - The strategy address | ||
* @returns The strategy id or undefined if the strategy address is not registered | ||
* @returns The strategy or undefined if the strategy address is not registered | ||
*/ | ||
getStrategyId(strategyAddress: Address): Promise<Hex | undefined>; | ||
getStrategyId(chainId: ChainId, strategyAddress: Address): Promise<Strategy | undefined>; | ||
/** | ||
* Save the strategy id by the strategy address | ||
* Save the strategy id by the strategy address and chain id | ||
* @param chainId - The chain id | ||
* @param strategyAddress - The strategy address | ||
* @param strategyId - The strategy id | ||
* @param handled - Whether the strategy is handled | ||
*/ | ||
saveStrategyId( | ||
chainId: ChainId, | ||
strategyAddress: Address, | ||
strategyId: Hex, | ||
handled: boolean, | ||
): Promise<void>; | ||
|
||
/** | ||
* Get all the strategies | ||
* @returns The strategies | ||
*/ | ||
saveStrategyId(strategyAddress: Address, strategyId: Hex): Promise<void>; | ||
getStrategies(filters?: { handled?: boolean; chainId?: ChainId }): Promise<Strategy[]>; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,7 +5,7 @@ export * from "./abis/index.js"; | |
export * from "./utils/index.js"; | ||
export * from "./data-loader/index.js"; | ||
export * from "./eventsFetcher.js"; | ||
export * from "./strategyRegistry.js"; | ||
export * from "./registries/index.js"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will we add a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah that was what i had in mind but will refactor it now There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
export * from "./eventsRegistry.js"; | ||
export * from "./eventsProcessor.js"; | ||
export * from "./orchestrator.js"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -113,15 +113,17 @@ export class Orchestrator { | |
await this.eventsRegistry.saveLastProcessedEvent(this.chainId, event); | ||
|
||
event = await this.enhanceStrategyId(event); | ||
if (event.contractName === "Strategy" && "strategyId" in event) { | ||
if (this.isPoolCreated(event)) { | ||
const handleable = existsHandler(event.strategyId); | ||
await this.strategyRegistry.saveStrategyId( | ||
this.chainId, | ||
event.srcAddress, | ||
event.strategyId, | ||
handleable, | ||
); | ||
} else if (event.contractName === "Strategy" && "strategyId" in event) { | ||
if (!existsHandler(event.strategyId)) { | ||
//TODO: save to registry as unsupported strategy, so when the strategy is handled it will be backwards compatible and process all of the events | ||
//TODO: decide if we want to log this | ||
// this.logger.info( | ||
// `No handler found for strategyId: ${event.strategyId}. Event: ${stringify( | ||
// event, | ||
// )}`, | ||
// ); | ||
// we skip the event if the strategy id is not handled yet | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shouldn't we save it with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we don't need to, we register on every PoolCreated the strategyAddress to strategyId entry with |
||
continue; | ||
} | ||
} | ||
|
@@ -216,9 +218,12 @@ export class Orchestrator { | |
* @returns The strategy id | ||
*/ | ||
private async getOrFetchStrategyId(strategyAddress: Address): Promise<Hex> { | ||
const existingId = await this.strategyRegistry.getStrategyId(strategyAddress); | ||
if (existingId) { | ||
return existingId; | ||
const cachedStrategy = await this.strategyRegistry.getStrategyId( | ||
this.chainId, | ||
strategyAddress, | ||
); | ||
if (cachedStrategy) { | ||
return cachedStrategy.id; | ||
} | ||
|
||
const strategyId = await this.dependencies.evmProvider.readContract( | ||
|
@@ -227,11 +232,20 @@ export class Orchestrator { | |
"getStrategyId", | ||
); | ||
|
||
await this.strategyRegistry.saveStrategyId(strategyAddress, strategyId); | ||
|
||
return strategyId; | ||
} | ||
|
||
/** | ||
* Check if the event is a PoolCreated event from Allo contract | ||
* @param event - The event | ||
* @returns True if the event is a PoolCreated event from Allo contract, false otherwise | ||
*/ | ||
private isPoolCreated( | ||
event: ProcessorEvent<ContractName, AnyEvent>, | ||
): event is ProcessorEvent<"Allo", "PoolCreated"> { | ||
return isAlloEvent(event) && event.eventName === "PoolCreated"; | ||
} | ||
|
||
/** | ||
* Check if the event requires a strategy id | ||
* @param event - The event | ||
|
@@ -240,6 +254,6 @@ export class Orchestrator { | |
private requiresStrategyId( | ||
event: ProcessorEvent<ContractName, AnyEvent>, | ||
): event is ProcessorEvent<"Allo", "PoolCreated"> | ProcessorEvent<"Strategy", StrategyEvent> { | ||
return (isAlloEvent(event) && event.eventName === "PoolCreated") || isStrategyEvent(event); | ||
return this.isPoolCreated(event) || isStrategyEvent(event); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export * from "./strategy/index.js"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am a little concerned about using multiple logger instances. I think it adds tons of overhead, think about a logger that sends logs to a server or prints all the logs into a file, for each instance you will have a connection (1st case) or a dedicated file (2nd case) correct me if iam wrong pls.
i would love to hear @0xyaco 's opinion about having multiple logger instances across the application
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mmm i see what you're pointing, we can better discuss it on the Error handling and Logging milestone but a solution could be a Context object on each call so we know which file or chain emitted a log?