feat: changed tx args transport format to JSON + proof support #58
Conversation
maht0rz
changed the title
… into feature/json-encoding
… into feature/json-encoding
maht0rz
changed the title
| toJSON: (value: unknown) => any; | ||
| } | ||
|
|
||
| export interface ProofTypes { |
There was a problem hiding this comment.
Can we rename this to something more verbose?
| @@ -150,13 +149,12 @@ export class QueryGraphqlModule< | |||
| ): GraphQLScalarType | ObjectType { | |||
| // This is a temporary workaround until transport-layer has been | |||
There was a problem hiding this comment.
I think I wanted this to be changed somehow when doing the json transport layer - but I am not sure anymore how exactly I wanted this to change. So I guess we can remove this comment 🤔
| const valueFieldLength = MethodParameterDecoder.fieldSize(valueType); | ||
|
|
||
| const dummyValue = valueType.fromFields( | ||
| range(0, valueFieldLength).map(() => Field(0)) | ||
| range(0, valueFieldLength).map(() => Field(0)), |
There was a problem hiding this comment.
Shouldn't we use ProvablePureExtended so we can skip the auxilary data input here? Since the input type is of FlexibleProvablePure
| @@ -45,6 +45,19 @@ export interface ToFieldable { | |||
| toFields: () => Field[]; | |||
| } | |||
|
|
|||
| export interface ToFieldableStatic { | |||
There was a problem hiding this comment.
What does "static" in this context mean? If it is of the sense of being only used on class-types that implement the interface on a static level, we might add new(..args: unknown[]) => unknown to this signature so that it can't be used on objects
| // eslint-disable-next-line @typescript-eslint/consistent-type-assertions | ||
| const paramtypes = Reflect.getMetadata( | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment | ||
| const paramtypes: ProvableExtended<unknown>[] = Reflect.getMetadata( |
There was a problem hiding this comment.
We should reflect the possibility of it being undefined in the type
| methodName | ||
| ); | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment | ||
| const parameterTypes: ProofTypes[] | ToFieldableStatic[] = |
There was a problem hiding this comment.
Either use ArgumentTypes[] here if applicable or (ProofTypes | ToFieldableStatic)[]
| @@ -222,8 +222,6 @@ export class Runtime<Modules extends RuntimeModulesRecord> | |||
| // TODO Remove after changing DFs to type-based approach | |||
| public create(childContainerProvider: ChildContainerProvider) { | |||
| super.create(childContainerProvider); | |||
|
|
|||
| this.registerDependencyFactories([MethodIdFactory]); | |||
There was a problem hiding this comment.
Have you tested that this works? I am worried that when we move this to the AppChain, we get some dependency cycles though Runtime not being configured yet and MethodIdFactory needing an initialized runtime. I had this once during testing that.
Alternative would be register it both on Runtime & Sequencer
| const runtimeModule = this.runtime.resolve(moduleName as any); | ||
|
|
||
| // find types of args for the runtime method thats being called | ||
| const parameterTypes: FlexibleProvable<unknown>[] = Reflect.getMetadata( | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment | ||
| const parameterTypes: |
There was a problem hiding this comment.
Again, integrate the types inside the Array (A | B | C)[]
| @@ -37,11 +37,6 @@ export class TransactionValidator { | |||
| methodPath[1] | |||
| ); | |||
|
|
|||
There was a problem hiding this comment.
I guess validation will be a seperate PR? Would be nice to have for IST
… into feature/json-encoding
…hain/framework into feature/cli
Feature/cli
This PR changes the args transport layer to JSON[] from Field[], fixing encoding/decoding of certain args types such as Struct or Signature.
Proof support will be also possible thanks to this PR -
but we need to add proof argsHash support using public input/output in a separate PR