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

Creating a proposer client interface #60

Closed
wants to merge 1 commit into from

Conversation

nisdas
Copy link
Member

@nisdas nisdas commented Mar 6, 2018

Addresses #59

  • Create a basic interface so that when geth sharding-proposer is entered a basic proposer rpc - client is initiated

@prestonvanloon
Copy link
Member

Could you add a pull request description? What are the changes you are making here?

@prestonvanloon prestonvanloon self-requested a review March 7, 2018 04:05
@nisdas
Copy link
Member Author

nisdas commented Mar 7, 2018

Added a description above

@rauljordan rauljordan changed the title Creating a Collator client interface Creating a proposer client interface Mar 8, 2018
@rauljordan rauljordan added this to the Ruby milestone Mar 8, 2018
@nisdas nisdas self-assigned this Mar 28, 2018
@terencechain
Copy link
Member

Should we add tests and avoid the duplicate code within clients?

Copy link
Member

@prestonvanloon prestonvanloon left a comment

Choose a reason for hiding this comment

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

I have a lot of concerns about the patterns we are choosing here. The code is already getting wildly wild and it's probably my fault (I started it).

We have agreed that there are two problems here:

  1. There is pretty much no new code, it's the same code as the collator client.
  2. There are no tests in this PR.

I think the latter is a product of the former. So let's start with addressing 1 with abstraction. I'd like to see these common methods abstracted into a new interface that both of these clients use. Think about all the things they have in common:

  • Endpoint
  • Client
  • Keystore
  • CLI context
  • SMC
  • Even their constructor methods are very similar

Can you create a "client" interface for sharding that provides all of the above and that the proposer and collator can use?

Read:
https://golang.org/doc/effective_go.html#interfaces_and_types
https://github.com/golang/go/wiki/CodeReviewComments#interfaces

@rauljordan
Copy link
Contributor

I think we should call it shardingClient so that we're different that the ethclient or other packages in the repo.

@terencechain
Copy link
Member

I really like this initiative to address our client side abstraction. +1 to shardingClient

@rauljordan
Copy link
Contributor

So the common methods can be receivers for shardingClient, while specific ones can be receivers of CollatorClient and ProposerClient.

@nisdas
Copy link
Member Author

nisdas commented Mar 29, 2018

Hey guys, I will be putting up a design proposal soon for refactoring/abstraction of our code so as to provide a solution for the issues that have been raised
Update:
Here is the design proposal
https://docs.google.com/document/d/1__74pTWFpzfLLJ8017XeWQWJfs89YgTGxuefjkFUAaY/edit?usp=sharing

@nisdas nisdas mentioned this pull request Mar 29, 2018
3 tasks
@nisdas nisdas closed this Mar 29, 2018
Redidacove pushed a commit to Redidacove/prysm that referenced this pull request Aug 13, 2024
* Week 0 & 1 update

* Update development-updates.md

* Update development-updates.md

---------

Co-authored-by: Mário Havel <61149543+taxmeifyoucan@users.noreply.github.com>
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.

4 participants