search

Layr-Labs / eigensdk-go

Go SDK for building AVSs on Eigenlayer
https://www.eigenlayer.xyz/
Other
62 stars 42 forks source link

Refactor types and utils #42

Open shrimalmadhur opened 11 months ago

shrimalmadhur commented 11 months ago

Is your feature request related to a problem? Please describe. We need to refactor types and utils so that it's in some standard way and not create circular dependencies.

Describe the solution you'd like

Describe alternatives you've considered

Additional context

RohanShrothrium commented 10 months ago

Can I pick this up?

samlaf commented 10 months ago

Can I pick this up?

By all means! Do you want to first describe the solution you're thinking of? Like break it down into how you would refactor the files etc? Otherwise, feel free to create a PR, but we might ask you to move things around.

RohanShrothrium commented 10 months ago

Can I pick this up?

By all means! Do you want to first describe the solution you're thinking of? Like break it down into how you would refactor the files etc? Otherwise, feel free to create a PR, but we might ask you to move things around.

@samlaf I haven't really given it a thought, but will update the issue with what I think can be done!

RohanShrothrium commented 10 months ago

@samlaf I went through the code. So here are a bunch of things I think are important to refactor:

  1. The util functions used for validating types should be a common file for the types module as I believe this is not going to be used anywhere apart from validating a "type".
  2. There are a few unused functions in the utils itself, not sure if this has been written for future scope. Do we want to remove this? Internal functions used for utils should not start with caps and should remain internal to the module. Eg ReadFile function. But this again calls for answering the previous question, do we want to have unused functions for future scope? I believe these should be implemented when required.
  3. Certain internal functions used in validation by different types should be moved to a common.go under the types module itself.

Let me know what you think about this!

samlaf commented 10 months ago

@RohanShrothrium

  1. agree
  2. agree. ReadFile is completely useless... it's literally wrapping over os.ReadFile with the same interface. Please remove it as well as any you see fit. Only one thing to be careful, it's possible some functions are already used in https://github.com/Layr-Labs/incredible-squaring-avs. I would suggest creating a vscode workspace with both repos and making sure the functions you remove/make private are not being used there.
  3. sgtm! We aren't following any type of pkg directory structure like https://github.com/golang-standards/project-layout so feel free to recommend anything you think would be useful!
RohanShrothrium commented 10 months ago

Hi, @samlaf! I'm just a bit preoccupied with some work and Devconnect will take this up once I am back!

0xpanicError commented 9 months ago

Hey @RohanShrothrium Are you still working on this? If not then I'd like to pick it up.

RohanShrothrium commented 9 months ago

@0xpanicError I totally forgot about this. Will raise a PR today ser!

RohanShrothrium commented 9 months ago

@samlaf I refactored the code with respect to what I mentioned. I will commit this but want to add the following additionally. There are types defined in the crypto module. I don't see any problem here per se, but there are a few types that are imported again in the types module or in other modules. I will add this to the types module. Let me know if this sounds good.