Implement similarity verification in Monitor #2539
Conversation
kuzdogan
left a comment
kuzdogan
left a comment
There was a problem hiding this comment.
Small comments. Looks good in general. Just, we should check if the monitor is really working on prod now. I checked the logs and there seemed to be a lot of errors and no INFO level logs
services/monitor/src/ChainMonitor.ts
Outdated
| private async triggerSimilarityVerificationFallback( | ||
| creatorTxHash: string, | ||
| address: string, | ||
| ) { | ||
| if (!this.similarityVerificationClient) { | ||
| this.chainLogger.debug("Similarity search disabled, skipping fallback", { | ||
| address, | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| this.chainLogger.info("Triggering similarity search fallback", { |
There was a problem hiding this comment.
This whole section seems unnecessary and can be handled inside the SimilarityVerificationClient. Then here we can simple call this.client.trigger()
| export default class SimilarityVerificationClient { | ||
| private baseUrls: string[]; | ||
| private clientLogger: Logger; | ||
| private enabled: boolean; |
There was a problem hiding this comment.
What's the use case of having enabled? We always start it with true and don't change at all no?
|
One question regarding this. Do I remember correctly that we wanted to have a delay between contract deployment and similarity search submission, such that it is possible for users to verify themselves before monitor does? Is this implemented in any way here? |
Right!!! I totally forgot about it while implementing this PR. Do you think is it as straightforward as as adding a delay before triggering each similarity request? That would be easy to implement. IMO it's not worth to implement any other advanced mechanism involving queues. |
|
yes, simply add a delay, but don't await the call to not block anything. I don't think this needs any fancy mechanism. Also make it configurable through an env variable. |
|
@manuelwedler I pushed this change. Good catch! thanks :) |
See #2506
This PR: