Skip to content
This repository has been archived by the owner on Nov 22, 2021. It is now read-only.

feat(registry): return structs for vault info #56

Closed
wants to merge 2 commits into from

Conversation

knarz
Copy link
Contributor

@knarz knarz commented Oct 23, 2020

As requested in #11, getVaultInfo now returns a struct containing all relevant data

@banteg
Copy link
Member

banteg commented Oct 23, 2020

Very good contribution, thanks!

What should be the approach for maintaining multiple registry versions?

  • Automatically populate the old registry on deploy?
  • Make this just another view on the original registry?
  • Maintain multiple versions?

What should be the naming/versioning scheme? I guess we can't just point registry.ychad.eth to a different implementation since people depend on it, maybe something like v1.registry.ychad.eth is more appropriate.

@knarz
Copy link
Contributor Author

knarz commented Oct 24, 2020

If we want to be really fancy we could do something EIP2535-esque but that might also be overkill—and also not address the problem of depreciating the currently deployed version.

Versioning sounds like the simplest solution right now; maybe have latest.registry.ychad.eth in addition all the versions.

Are there actually any smart contracts currently deployed which depend on the registry?

@lbertenasco
Copy link
Contributor

Hey @knarz ! Awesome addition to the registry, thanks!

Yes, currently there are no smart contracts using the registry.

We'd need to do a mayor version bump to the registry since these are breaking changes. (we need to figure out a good ens registry subdomain naming scheme as well)

I'd also love to abstract current vault/strategy parsing logic to the current v2 registry release to add more information for frontend + future proofing it for V2 Vault design :)

Would you mind changing contract name to YRegistryV2 so we can continue to work on it?

I'll create a few issues tomorrow to detail the improvements :)

@lbertenasco
Copy link
Contributor

Thanks for changing the contract to YRegistryV2 @knarz .
Added issue #58 with a few improvements, we can talk about them there :)

@fubuloubu
Copy link
Member

@knarz first off, wanted to thank you for taking a stab at this! We actually took a lot of this work to inform the next iteration of this idea w/ v2 Vaults.

Please feel free to take a look here: yearn/yearn-vaults#62

@fubuloubu fubuloubu closed this Nov 6, 2020
@knarz knarz deleted the add-getvaultsinfo branch November 7, 2020 00:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants