Skip to content
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

fest: support start/stop blocks for querying operator info(pubkey and socket) #334

Merged
merged 4 commits into from
Sep 5, 2024

Conversation

renlulu
Copy link
Contributor

@renlulu renlulu commented Sep 4, 2024

Fixes #259 and #312.

What Changed?

Reviewer Checklist

  • Code is well-documented
  • Code adheres to Go naming conventions
  • Code deprecates any old functionality before removing it

Copy link
Collaborator

@samlaf samlaf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks fine to me. @shrimalmadhur do you want to start enforcing opts arguments so that we don't have breaking changes like this all the time?

@shrimalmadhur
Copy link
Collaborator

This looks fine to me. @shrimalmadhur do you want to start enforcing opts arguments so that we don't have breaking changes like this all the time?

yea this is getting very ugly. these seems like optional params to me - if we don't provide them, it takes default values. We should add extensible options without breaking the method arguments.

@samlaf
Copy link
Collaborator

samlaf commented Sep 4, 2024

Ok @renlulu synced with @shrimalmadhur , we're trying to enforce opts params going forward, such that we don't break client code everytime we add new params to a function.

Would you mind refactoring the function signatures everywhere to use a params struct instead that would for now contain the startBlock and endBlock?

type Opts struct {
   StartBlock *big.Int
   StopBlock *big.Int
}

Not sure about the exact name, Opts or Params would work, prob with a prefix related to name of function in case there's a few of them in a given module.

@renlulu
Copy link
Contributor Author

renlulu commented Sep 5, 2024

Ok @renlulu synced with @shrimalmadhur , we're trying to enforce opts params going forward, such that we don't break client code everytime we add new params to a function.

Would you mind refactoring the function signatures everywhere to use a params struct instead that would for now contain the startBlock and endBlock?

type Opts struct {
   StartBlock *big.Int
   StopBlock *big.Int
}

Not sure about the exact name, Opts or Params would work, prob with a prefix related to name of function in case there's a few of them in a given module.

Ok, make sense, let me refactor it.

Comment on lines +303 to +304
opts.StartBlock,
opts.StopBlock,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will panic if nil is passed as opts. Can we change opts to be Opts instead of *Opts?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in every function

Copy link
Collaborator

@samlaf samlaf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@samlaf samlaf merged commit a1011f6 into Layr-Labs:dev Sep 5, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add an option for NewOperatorsInfoServiceInMemory to start scanning from a given block
3 participants