Skip to content

[FEATURE] Use direct sum instead of union in Wallet.registerContract #18989

@shiqicao

Description

@shiqicao

Problem Statement

I am overriding registerContract and need to do some pre-processing instanceData is a union, which makes deciding types unsafe and tedious.

registerContract(
    instanceData: AztecAddress | ContractInstanceWithAddress | ContractInstantiationData | ContractInstanceAndArtifact,
    artifact?: ContractArtifact,
    secretKey?: Fr,
  )

in BaseWallet, three embedded functions are created to decide type,

    /** Determines if the provided instance data is already a contract instance with an address. */
    function isInstanceWithAddress(instanceData: any): instanceData is ContractInstanceWithAddress {
      return (instanceData as ContractInstanceWithAddress).address !== undefined;
    }
    /** Determines if the provided instance data is contract instantiation data */
    function isContractInstantiationData(instanceData: any): instanceData is ContractInstantiationData {
      return (instanceData as ContractInstantiationData).salt !== undefined;
    }
    /** Determines if the provided instance data is already a contract */
    function isContractInstanceAndArtifact(instanceData: any): instanceData is ContractInstanceAndArtifact {
      return (
        (instanceData as ContractInstanceAndArtifact).instance !== undefined &&
        (instanceData as ContractInstanceAndArtifact).artifact !== undefined
      );
    }

Issues,

  1. when overriding the function, I need to create and call these functions again, which means copying.
  2. deciding types using these three functions is unsafe, for example, ContractInstantiationData adds address fields, then it will be mistakenly treated as ContractInstanceWithAddress.

Proposed Solution

Using direct sum,

instanceData: 
    {
        type: 'AztecAddress', 
        address: AztecAddress
   } | {
        type: 'ContractInstanceWithAddress', 
        instance: ContractInstanceWithAddress
   } | ...

Also direct sum can also improve the function signature, now you can specify which variant requires artifacts,

instanceData: 
    { 
       type: 'AztecAddress',
       address: AztecAddress,
       artifacts: Artifacts,
    } | {
       type: 'ContractInstantiationData',
       instance: ContractInstantiationData,
       artifacts: Artifacts,
   } | {
      type: 'ContractInstanceWithAddress',
      instance: ContractInstanceWithAddress,
      artifacts?: Artifacts,
   } | {
      type: 'ContractInstanceAndArtifact',
      instance: ContractInstanceAndArtifact,
   }

Isn't this much better? User don't need to inspect body of registerContract to figure out which input requires artifacts. Also ts type checker verifies the inputs instead of checking them at runtime.

Example Use Case

No response

Alternative Solutions

No response

Additional Context

No response

Metadata

Metadata

Assignees

No one assigned

    Labels

    T-feature-requestType: Adding a brand new feature (not to be confused with improving an existing feature).from-communityThis originated from the community :)

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions