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

Simplifying client facing API by removing connection parameter #5

Closed
eoin-betdex opened this issue Mar 23, 2022 · 2 comments
Closed

Comments

@eoin-betdex
Copy link
Contributor

Interested in getting thoughts on dropping the connection parameter, or making it optional. I'm not clear on the use case on being able to target multiple or different connections.

Anchor already provides a global straight-forward way to get the currently configure connection that could be used within the generated clients.

getProvider().connection

This would make the interface of e.g., fetch, cleaner IMO, passing just the address of the account to fetch, and save client authors having to pass around the connection.

  static async fetch(address: PublicKey): Promise<State | null> {
    const c = getProvider().connection
    const info = await c.getAccountInfo(address)

    if (info === null) {
      return null
    }
    if (!info.owner.equals(PROGRAM_ID)) {
      throw new Error("account doesn't belong to this program")
    }

    return this.decode(info.data)
  }

...

const res = await State.fetch(state.publicKey)
@kklas
Copy link
Owner

kklas commented Mar 23, 2022

Good question.

I'm certainly against using anchor's getProvider() here because not everyone uses it. Different people will go about this differently. Saber guys use their own thing for example. I also use my own thing in my code to solve this.
Also this would add an additional dependency for the generated client (the anchor package) which I would like to avoid.

On making the connection parameter optional - this would require us to store it as in a global state somewhere (probably something like what anchor's setProvider and getProvider do) which I'm not a fan of. Global mutable state in my experience makes code more difficult to read and often causes unexpected behaviour so I would like to avoid doing this.

The "downside" (which I would argue is not a downside) is that we need to pass the connection manually in each method call.

Let me first explain what my idea is behind the design of this client. The generated should be as simple and straightforward as possible. The generated code should be orthogonal (no overlaps between methods etc.) and composable. This way the generated code can be composed together as lego pieces to create higher lever abstractions which you shape to conform to your specific use case.

So basically in most cases you will want to "wrap" the client into a more sematic higher level abstraction. The Quarry SDK is an example of this. Basically you group related instructions in the same class, have additional helper methods, caching, etc... This cannot be done by codegen because the generator doesn't understand the semantics of your program and how you want to use it.

So coming back to passing the connection on each method call, I think that the way you cache (or not cache) the connection should be left up to the developer. You will in most cases have a wrapper / higher level abstraction around the generated client so just cache it there the way you want and have an API on it that doesn't require the connection.

I'm not clear on the use case on being able to target multiple or different connections.

In most cases the connection probably won't change much during runtime, I agree. But I can imagine someone wanting to be able to switch between devnet and mainnet during development for example. Or changing the commitment for a specific instruction call. Global connection makes this really messy.
Also some people (like me) prefer passing the connection every time. In react for example it can be stored in a context so it's not really a hassle to get it from anywhere in the codebase and having the connection in the method signature indicates that the method will do a network call which makes the code more readable IMO.

TLDR; connection management should be left for the developer as there is not one best way of doing it.

@eoin-betdex
Copy link
Contributor Author

Fair points and good articulation. If nothing more good to get some of that out and written down :)

For some reason I had assumed the anchor package would be pulled in but I see now it's only borsh from serum that is being included included. With that API generally you start with a Program so you already have the connection, the local wallet, and other bits and pieces available so the APIs for fetch etc don't need those provided. Just a different/another layer of abstraction.

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

No branches or pull requests

2 participants