-
Notifications
You must be signed in to change notification settings - Fork 45
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(ai): introduce AIServiceRegistry contract #642
Conversation
@yondonfu there are multiple ways to do this. I now created a seperate I've deployed it using my own developer wallet, along with a brand new controller contract. However, it might be more effective if someone else on the team handles the deployment so that it can be directly linked to the main controller contract. One potential issue with this approach is the absence of a |
@yondonfu I also noticed although the proxy contract works automatic verification seems to fail for this contract (see https://arbiscan.io/address/0x73f1755f34c2e769209e0f91a0c385722ed81b9c#code). Are you familiar with this problem? |
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.
there are multiple ways to do this. I now created a seperate AIServiceRegistry contract that inherits all properties from the regular ServiceRegistry contract but we can also keep one ServiceRegistry contract and deploy the AIServiceRegistry contract based on that.
IMO the latter option would be simpler - if we just deploy another instance of the ServiceRegistry code and refer to it as the AIServiceRegistry that would remove the need for additional tests or custom code that specifically references AIServiceRegistry by name.
If a contract is a proxy, you would have to use the arbiscan UI to confirm it is a proxy, make sure it is pointing at the right target implementation address. Though, as mentioned in my previous comment, if a non-proxy version of the ServiceRegistry is deployed as the AIServiceRegistry then won't have to deal with proxy verification on arbiscan. |
Yea looks like the target is verified automatically causing the automatic Proxy verification on Arbitrum explorer to fail 🤔. @iameli stated that he also had problems with that some time ago and was able to get it verified manually. However let's for now just simply things by deploying a non-proxy version 👍🏻. |
This commit introduces the `AIServiceRegistry` contract. This contract serves as a registry for AI subnet orchestrators, enabling them to register and make their services discoverable within the AI subnet. As discussed in #642 we opted to deploy a non-upgradable contract for now connected to LivePeer's [mainnet controller](https://docs.livepeer.org/references/contract-addresses).
ca7a959
to
023d7f3
Compare
This commit adds the AIServiceRegistry contract, facilitating the registration and discoverability of AI subnet orchestrators' services. As discussed in #642, we've chosen to deploy a non-upgradable contract for now, linked to LivePeer's mainnet controller (https://docs.livepeer.org/references/contract-addresses).
023d7f3
to
8a690c6
Compare
@yondonfu, I've re-deployed the contract as an immutable standalone contract. The potential drawback is that if we decide to register this contract with the controller for future upgrades, similar to the standard ServiceRegistry contract, we would need to deploy a new contract. This change would require us to either migrate the existing service URIs to the new contract (though it’s unclear if this is feasible) or compel orchestrators to resubmit their URIs. It's manageable, but worth considering. Additionally, I've set up #643, which, from my understanding, would allow us to register both the Proxy and target with the controller 🤔. I'm fine with using any of the two versions 👍🏻. |
@rickstaa IMO using an immutable standalone contract at this stage would be preferable because deploying an upgradeable proxy involves additional complexity:
In both cases, the operational setup for the contract becomes more complex and I think the tradeoff of potentially having to migrate service URIs to another contract in the future is reasonable to avoid that extra complexity especially since the path to "merging" the AI subnet and the transcoding mainnet at this point is still unclear. |
@yondonfu, thank you for your response; it is helpful. Ultimately, I decided to proceed with the immutable standalone contract (i.e. livepeer/go-livepeer#3004). |
@rickstaa If this is deployed and being used already suggest moving this out of draft so it can be properly reviewed + merged. |
Great idea! I wasn't sure if I should keep it as a draft until we merge with Mainnet but happy to move it out of draft 👍🏻. |
What does this pull request do? Explain your changes. (required)
This pull request introduces the
AIServiceRegistry
contract. This contract serves as a registry for AI subnet orchestrators, enabling them to register and make their services discoverable within the AI subnet.Specific updates (required)
ServiceRegistry
contract under theAIServiceRegistry
name was added.deployments directory
.How did you test each of these updates (required)
I ensured that all tests passed. Deployed the contract and interacted with the contract using foundry cast.
Test Deployments
Controller: https://arbiscan.io/tx/0xf5daeb4fee628130c0d6a12a53439baad67c5bc788d811735a89db66cdfcbb22AIServiceRegistryTarget: https://arbiscan.io/tx/0x1c7282a6a36b88b669201a74caa2f24243024d853e4499ed5d9e8372e803489dAIServiceRegistry: https://arbiscan.io/tx/0x4f6d608a8aefa913503ff610a5754451a73acc3b5511f005fc3258b7a66a739aEDIT: We decide to just deploy a target contract connected to the mainnet controller right now.
Does this pull request close any open issues?
https://linear.app/livepeer-ai-spe/issue/LIV-90/design-and-implement-solution-for-advertising-separate-service-uri-for
Checklist:
yarn test
pass