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

Simplify register operator avswriter fn #276

Merged
merged 3 commits into from
Jun 23, 2024

Conversation

samlaf
Copy link
Collaborator

@samlaf samlaf commented Jun 20, 2024

Fixes # .

What Changed?

Simplify this function which was needlessly complicated. If users need the low level granularity than they can just use the bindings directly is what Im thinking. Most users of the high level writer client probably just want this to happen and not ask too many questions. That's also my case when writing test cases that use this function.

Reviewer Checklist

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

@samlaf samlaf requested a review from shrimalmadhur June 20, 2024 03:49
@samlaf samlaf force-pushed the simplify-register-operator-avswriter-fn branch from 9997768 to 0f9e40f Compare June 21, 2024 07:11
var operatorToAvsRegistrationSigSalt [32]byte
_, err = rand.Read(operatorToAvsRegistrationSigSalt[:])
if err != nil {
panic(err)
Copy link
Collaborator

Choose a reason for hiding this comment

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

why Panic? why not return error?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good catch. prob copied from somewhere and forgot to change. 276562a


curBlockNum, err := w.ethClient.BlockNumber(context.Background())
if err != nil {
panic(err)
Copy link
Collaborator

Choose a reason for hiding this comment

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

why panic?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

}
curBlock, err := w.ethClient.BlockByNumber(context.Background(), big.NewInt(int64(curBlockNum)))
if err != nil {
panic(err)
Copy link
Collaborator

Choose a reason for hiding this comment

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

why panic?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@samlaf samlaf force-pushed the simplify-register-operator-avswriter-fn branch from 276562a to 6e4a4f3 Compare June 23, 2024 13:53
@samlaf samlaf merged commit edeaeb1 into dev Jun 23, 2024
3 checks passed
@samlaf samlaf deleted the simplify-register-operator-avswriter-fn branch June 23, 2024 14:37
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.

2 participants