[VIP] V6 saitama #908
[VIP] V6 saitama #908
Conversation
✅ Deploy Preview for starknetjs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| transactionVersion: | ||
| | ETransactionVersion.V2 | ||
| | ETransactionVersion.V3 = DEFAULT_TRANSACTION_VERSION, | ||
| instance?: boolean |
There was a problem hiding this comment.
This instance parameter and the new AccountInstance class feels like it could be confusing to users. I suggest evaluating if a different term might be a better fit, maybe something like inheritable or inheritableProvider.
Adding @ivpavici since he's had success with naming things.
There was a problem hiding this comment.
The idea is that the user create Acc from the instance and not class.
Is there a better name to fit I'm fine with it, I am not supper happy with this name also, but it was only one logical to me.
src/channel/rpc_0_6.ts
Outdated
| } | ||
| this.retries = retries || defaultOptions.retries; | ||
| this.headers = { ...defaultOptions.headers, ...headers }; | ||
| this.blockIdentifier = blockIdentifier || defaultOptions.blockIdentifier; | ||
| this.chainId = chainId; | ||
| this.waitMode = waitMode || false; | ||
| this.requestId = 0; | ||
| this.speckVersion = speckVersion; |
There was a problem hiding this comment.
speckVersion should be renamed to specVersion.
src/provider/interface.ts
Outdated
| @@ -34,7 +34,7 @@ import type { | |||
| } from '../types'; | |||
|
|
|||
| export abstract class ProviderInterface { | |||
| public abstract channel: RpcChannel; | |||
| public abstract channel: RpcChannel | undefined; | |||
There was a problem hiding this comment.
This change doesn't seem necessary because of the @ts-expect-error in src/provider/rpc.ts on line 44.
There was a problem hiding this comment.
I wasn't sure will ti effect external implementations
src/provider/rpc.ts
Outdated
| // stop ts complains | ||
| // @ts-expect-error | ||
| this.channel = null; |
There was a problem hiding this comment.
Could also be done with a cast.
| // stop ts complains | |
| // @ts-expect-error | |
| this.channel = null; | |
| this.channel = null as any; |
src/types/lib/index.ts
Outdated
| @@ -155,52 +161,54 @@ export enum TransactionType { | |||
| INVOKE = 'INVOKE_FUNCTION', | |||
| } | |||
|
|
|||
| // TODO: All this should be removed | |||
There was a problem hiding this comment.
Remove the commented blocks?
src/types/provider/response.ts
Outdated
| @@ -70,7 +71,7 @@ export type Storage = RPC.Felt; | |||
|
|
|||
| export type Nonce = string; | |||
|
|
|||
| export type SimulationFlags = RPC.SimulationFlags; | |||
| // export type SimulationFlags = RPC.SimulationFlags; | |||
Co-authored-by: Petar Penović <[email protected]>
|
TODO: Will close this one once extracted important parts. |
Motivation and Resolution
We have this structure in the project:
Starknet | <-> 1. Channel -> 2. Provider -> 3.Account -> 4.Contract
We have this type of structure:
RPC (namespace: types.RPC and RPC) (none in top namespace)
Provider (all in top namespace) (some inherited from RPC)
Account (all in top namespace)
Contract (all in top namespace)
Every level has a specific diff from prev one. inheriting prev. structure + diff
The channel will change with every RPC Release
Provider needs to stay in the same structure until the next major release.
Account and Contract are based on Provider so it will also stay + extend in the same structure until next major.
So it is clear that stability needs to be lined on Provider as so RPC types res and response used by provider need to be re-exported on the top level as "Provider types"
and so to the top.
This is a vertical structure, on the other hand, we have a horizontal structure like in Account - Signer -Hashing
An account provides loose types so that UX can be flexible on the other hand as you go deeper into signer it transforms it into strict union discriminatory types and at the bottom Hashing works only with the strictly defined types.
As I see it, I will need to export RPC Enums to the top level as Provider Enums, and req/res types as provider types. This would be the first step.
The second step will be to check horizontal types and polish them as we can.
The second issue is that we have floating definitions of classes and some types in lib and some other files that are floating around at the moment, they were historically union between sequencer and RPC in some unstructured mess. This would need to be checked and aligned with now only SSOT that is RPC Spec in such a way that it can be extended from 2 RPC Specs as we could potentially have in the future when changes are not in addition like v5 v6 but also restructured.
RPC version (if applicable)
...
Usage related changes
Development related changes
Checklist: