feat: update oval node to support oval factory deployments

[md0x]

Changes proposed in this PR:

• Support for Oval Factories: Implement the new feature that allows permissionless deployments of Oval. Oval factories

• New Configuration: Add a new config ovalConfigsShared

• Introduce WalletManager to allocate wallets for the unlocks. WalletManager

• Add OvalDiscovery service to find permissionless deployments of Oval throughout the factories. OvalDiscovery

• No changes in how searchers interact with Oval. Searchers can use the oval-headers to specify a particular Oval for unlocks, or they can opt to not specify and rely on the Oval mechanism to find the necessary instance.

[linear]

UMA-2703 update oval node to support oval factory deployments

[mrice32]

This looks great! I think this is really well modularized. Only a few small comments

[mrice32]

Does count mean we can use a wallet more than once in a single block? Interesting... I hadn't really considered what the node would do if it ran out of wallets. I think this makes sense.

[md0x]

Correct, this is to handle scenarios where we have more oval-instances than unlockers and spikes in the number of unlocks.

I will also add alerts in those cases.

[mrice32]

+1 on having unit tests for this module

[mrice32]

Why did you choose to have an Array to track wallet usage rather than a mapping?

For instance, I could imagine a mapping from block->OvalInstance->WalletUsed might be a decent alternative: simple to drop blocks once they've passed (just delete from the map) and it's easy to tell if there's already been a wallet assigned (no array traversal/find).

[md0x]

Thanks, that’s a better data structure from the record cleanup. However, initially, I proposed the data structure with this idea in mind:

• We want to use the same unlocker for a given oval instance, regardless of the target block. This approach is also due to a cleanup running every 5 minutes, preventing old assignments from being reused, focusing only on recent assignments.

The reason for this is that multiple unlocks to the same oval instance in target blocks close to each other are likely part of the same backrun. They belong to the same price auction, meaning nonce collisions are not possible if we reuse the unlocker. Do you agree with this?

The previous data structure seemed to work fine with that assumption. However, I do like removing the array and using only mappings that we still need to traverse, for the reason I explained above.

I’ve changed the data structure in the latest commits.

[daywiss]

the naming of this function is misleading. if i see "find", i expect this to be read only to return me something that matches a search, but this is a function that mutates state. perhaps this shoudl be like "updateInstances" or "fetchInstances"

[md0x]

Agreed & updated

[daywiss]

this is surprising, and imo should be moved out to a higher level call to decide how to loop this

[md0x]

This class is a service that finds Oval instances deployed through the Factories.

This recursive function acts as a subscription to the events in the contracts. Instead of a subscription, we manually fetch the events every X minutes because event subscriptions sometimes cause issues with WebSockets closing or failing.

I think having the logic of the event retrieval contained in this class and abstracted away from the rest of the code makes it simple to reason about.

The initialization of the Oval discovery service is run in the index file at a higher level:

oval-node/src/index.ts

Lines 57 to 58 in fee339d

	const ovalDiscovery = OvalDiscovery.getInstance();
	ovalDiscovery.initialize(provider);

Knowing this: Would you extract the logic and handle the event retrieval process differently?

[daywiss]

i would probably at the very least have a function that runs a singular update, and then have a separate call which runs that update in a loop continuously on the class, just to add a bit of composability and testability

[daywiss]

i dont really understand the logic behind how this manages the wallets, maybe need some comments

[md0x]

I've added some comments describing what each of the functions do

[Reinis-FRP]

looking great!
just a nit comment on contract typings

[Reinis-FRP]

Shouldn't these be gitignored and generated as part of build?

Also, for the sake of consistency, maybe we can also add Oval abi to this flow (currently its abi is in ts)

[chrismaree]

yup

[chrismaree]

did you need to use the bluebird promise due to limitations of the number of requests, that would error/fail without pagination?

[md0x]

I reused this existing code, used in other parts of our codebase without modifications to minimize impact. Additionally, the pagination that this has by default, I think, makes our usage of the node URLs efficient without adding much delay. Would you change it to normal Promises?

[chrismaree]

do we have any idea on how much of a performance impact this has (and all the underlying async calls this triggers) on startup times? given this is running in cloud run, is this problematic?

[md0x]

I did some tests, and querying 2 years of blocks in mainnet to find factories takes less than 10 seconds, querying 1 year takes 5.1 seconds. Since this only runs once when we restart the instance or when auto-scaling provisions a new instance, I think it’s fine. Do you agree?