fix: add methods to pxe that fetch contract instance and artifact from node instead of db

[porco-rosso-j]

[Edited by Rahul]:
pxe.registerContract requires a contract instance. To add the instance of a new contract is a bit cumbersome.

pxe.getContractInstance(contractAddress) only checks the local DB so ofcourse gives undefined for any new contracts (which is majority of the cases outside of sandbox).

Then the only to get the instance today is getContractInstanceFromDeployParams for which you need to pass deployment salt, constructor args, public keys, deployer wallet address which wallets won't know

The right solution is pxe.getContractInstance() should fetch data from the node/archiver which anyway stores this data in the contract_instance_store.

---

[ Original content]:
Related to this discord post.

Currently, PXE doesn't have an easy way to register ( import ) an existing contract that's been deployed in other PXEs. Contract instance, one of the pxe.registerContract()'s param, has to be manually reconstructed which requires several params such as, salt, constructor args, public keys, deployer wallet address, etc.

*pxe.getContractInstance() only returns instance stored in db if it's already been registered, or undefined.

Since these info is stored in the node, it'd be really helpful for app devs if it can be fetched from node via PXE service. Then, we don't have to hard-code them or run server that holds these information.

Added Methods

• pxe_service.ts:

  public async getContractInstanceFromNode(address: AztecAddress): Promise<ContractInstanceWithAddress | undefined> {
    return await this.node.getContract(address);
  }

  public async getContractArtifactFromNode(address: AztecAddress): Promise<ContractArtifact | undefined> {
    return await this.node.getContractArtifact(address);
  }

• aztec-node/server.ts:

  public getContractArtifact(address: AztecAddress): Promise<ContractArtifact | undefined> {
    return this.contractDataSource.getContractArtifact(address);
  }

[spalladino]

Thanks for the contribution! However, the aztec node should not be exposing the getContractArtifact method, since nodes are not expected to store contract artifacts. This was just a temporary measure to assist with debugging, which we should remove soonish. The node should only be storing contract classes, which are a subset of the artifact.

As for the getContractInstanceFromNode, I'm unsure about it. Given the pxe team has planned on splitting up the pxe and wallet interface (heavily coupled today), I'd expect the pxe to just expose a getContractInstance that returns an instance from its own store (or maybe not even that), and the wallet to expose a getContractInstance that returns data from both its local pxe and connected node transparently. I'll defer to @benesjan and @nventuro on whether to go forward with this change, knowing it'll be changed in the near future.

[porco-rosso-j]

@rahul-kothari @spalladino thanks for the feedback. So I guess we don't want to add getContractArtifactFromNode and getContractArtifact, which is fine.

What Santiago mentions makes me wonder if it's possible for wallets/apps to directly talk to node in sandbox or not -- more concretely, 1) if aztec-node's methods is publicly exposed, and 2) if aztec-node class can be easily instantiated in frontend. Then, I can just modify our wallet instance, adding getContractInstance()
that directly gets instance from node without going through PXE.

[spalladino]

if aztec-node's methods is publicly exposed,

That's the plan! What we want to do is to have wallets expose a single API to dapps, and redirect requests to either the pxe, a node, or something else. So if a dapp requests a tx receipt or an event log, the wallet fetches it from the node, but if the dapp requests a call to a private getter, the wallet asks the pxe to solve it.

[spalladino]

But what I cannot currently comment on (and is what I'm deferring to Nico and Jan) is the timeline for that change.

[benesjan]

I'd expect the pxe to just expose a getContractInstance that returns an instance from its own store (or maybe not even that), and the wallet to expose a getContractInstance that returns data from both its local pxe and connected node transparently.

That makes sense to me. I think we'll get to doing the refactor sometimes after Christmas. Will take a note to include this.

@porco-rosso-j is this a blocker for you?

[porco-rosso-j]

@benesjan no, we'll wait for the changes. The refactor makes sense to me too. All I want as a wallet dev is just an easy way to somehow get contract instance from node in Sandbox. Note, in testnet, we plan to run our own node, then I guess this won't be a problem actually.

[benesjan]

Created an issue to track this. @porco-rosso-j thanks for the feedback!