Skip to content
This repository has been archived by the owner on Sep 14, 2023. It is now read-only.

codegen pallet bindings on ChainRune subtype #901

Closed
harrysolovay opened this issue Apr 18, 2023 · 2 comments · Fixed by #919
Closed

codegen pallet bindings on ChainRune subtype #901

harrysolovay opened this issue Apr 18, 2023 · 2 comments · Fixed by #919
Assignees

Comments

@harrysolovay
Copy link
Contributor

harrysolovay commented Apr 18, 2023

We currently codegen pallet bindings, which are accessible from the root mod. Ie.

import { Balances } from "@capi/my-chain"

const call = Balances.transfer({ /* ... */ })

There's an issue with this however: we may want to specify the discovery value / how to connect at runtime.


We can do this if we utilize chain.

import { chain as chain_ } from "@capi/my-chain"
import { SmoldotConnection } from "capi"

const chain = _chain.with(SmoldotConnection.bind({ relayChainSpec }))

const call = chain.extrinsic(Rune.object({
  type: "Balances",
  value: Rune.object({
    type: "transfer",
    // ...
  })
}))

This experience is different from that of utilizing the generated bindings. It's not the "happy path."


Alternatively, we could expose a means of mutating the internally-held connection state.

import { setConnection } from "@capi/my-chain"
import { SmoldotConnection } from "capi"

setConnection(SmoldotConnection.bind({ relayChainSpec }))

However, this too is not ideal.


Per an offline Capi team convo yesterday, we're thinking of changing the look of the codegen, such that the experience would be as follows.

import { chain } from "@capi/my-chain"

const call = chain.Balances.transfer({ /* ... */ })

There's an issue with this: what if a pallet name conflicts with that of a property of chain? Perhaps...

import { chain } from "@capi/my-chain"

const call = chain.pallets.Balances.transfer({ /* ... */ })

I'd personally prefer not to place pallet bindings under yet another prop. That being said, I'm unsure of whether there are better options.


One more thing to note: this approach would be consumable within pattern libraries, which is not true of the current pallet bindings.


@kratico @ryanleecode @tjjfvi –– thoughts?

@kratico
Copy link
Contributor

kratico commented Apr 18, 2023

On the discovery value, I would suggest to evaluate this other alternative

import { CapiClient, Balances } from "@capi/my-chain"

const call = Balances.transfer({ /* ... */ })
const client = new CapiClient(...)

client.run(call)

This is similar to a GQL client executing GQL queries/mutations.

On the naming conflict, I would prefer to keep pallets without the .pallets props.
The other non-pallet props, would probably be better to have their own prop.

@tjjfvi
Copy link
Contributor

tjjfvi commented Apr 18, 2023

There's an issue with this: what if a pallet name conflicts with that of a property of chain?

I think this is fine; the codegen needn't be as robust as the regular API, so no need for a pallets prefix, IMO.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants