From 9b0e83191ac1bb61b9818052ea66722d50074c31 Mon Sep 17 00:00:00 2001 From: Mitchell Tracy Date: Mon, 10 Jun 2024 16:27:40 -0400 Subject: [PATCH 1/9] start outlining idealized flows --- ...6973-refactor-base-contract-interaction.md | 730 ++++++++++++++++++ 1 file changed, 730 insertions(+) create mode 100644 in-progress/6973-refactor-base-contract-interaction.md diff --git a/in-progress/6973-refactor-base-contract-interaction.md b/in-progress/6973-refactor-base-contract-interaction.md new file mode 100644 index 0000000..fbb816f --- /dev/null +++ b/in-progress/6973-refactor-base-contract-interaction.md @@ -0,0 +1,730 @@ +| | | +| -------------------- | --------------------------------- | +| Issue | [title](github.com/link/to/issue) | +| Owners | @you | +| Approvers | @alice @bob | +| Target Approval Date | YYYY-MM-DD | + + + +## Executive Summary + +This is a refactor of the `BaseContractInteraction` class to simplify the API and improve the user (developer) experience. + +The refactored approach mimics Viem's API, with some enhancements and modifications to fit our needs. + +Example: +```ts + +// Create a new schnorr account + +const signingPrivateKey = GrumpkinPrivateKey.random(); +const signingPublicKey = new Schnorr().computePublicKey(alice.signingPrivateKey); + +const alice = { + signingPrivateKey, + signingPublicKey, + secretKey: Fr.random(), +} + +const aliceDeploymentOptions = { + constructorArtifact: 'constructor', + constructorArgs: { + signing_pub_key_x: alice.signingPublicKey.x, + signing_pub_key_y: alice.signingPublicKey.y + }, + salt: 42, + publicKeysHash: deriveKeys(alice.secretKey).publicKeys.hash(), + deployer: AztecAddress.ZERO +} + +const aliceContractInstance = getContractInstanceFromDeployParams( + SchnorrAccountContract.artifact, + aliceDeploymentOptions +); + +const aliceCompleteAddress = CompleteAddress.fromSecretKeyAndInstance(alice.secretKey, aliceContractInstance); + +await pxe.registerAccount( + alice.secretKey, + aliceCompleteAddress.partialAddress +); +await waitForAccountSynch(pxe, aliceCompleteAddress); + +const aliceAuthWitProvider = new SchnorrAuthWitnessProvider(alice.signingPrivateKey) +const nodeInfo = await pxe.getNodeInfo(); +const aliceInterface = new DefaultAccountInterface(aliceAuthWitProvider, aliceCompleteAddress, nodeInfo) +const aliceWallet = new AccountWallet(pxe, aliceInterface); + +const paymentMethod = new PrivateFeePaymentMethod(bananaCoin.address, bananaFPC.address, aliceWallet, refundSecret); + +await aliceContractInstance.send.constructor({ + wallet: aliceWallet, + args: aliceDeploymentOptions.constructorArgs, + paymentMethod +}).wait(); + + +// Deploy BananaCoin as an instance of TokenContract + +const bananaCoinDeploymentOptions = { + constructorArtifact: 'constructor', + constructorArgs: { + admin: aliceWallet.getAddress(), + name: 'BananaCoin', + symbol: 'BC', + decimals: 18 + }, + salt: 43, + publicKeysHash: Fr.ZERO, + deployer: aliceWallet.getAddress() +} + +const bananaCoinInstance = getContractInstanceFromDeployParams( + TokenContract.artifact, + bananaCoinDeploymentOptions +); + + +// puts the artifact and instance in the PXE +await aliceWallet.registerContract({ + artifact: TokenContract.artifact, + instance: bananaCoinInstance +}); + +// create function call to deploy the contract class +// will consume the capsule when called +const tokenContractClass = getContractClassFromArtifact(TokenContract.artifact); +const registerer = getRegistererContract(); +const registerBCClassFnCall = registerer.prepare.register({ + // this is an instance of the contract class + // so the address is stubbed in + args: { + artifact_hash: tokenContractClass.artifactHash, + private_functions_root: tokenContractClass.privateFunctionsRoot, + public_bytecode_commitment: tokenContractClass.publicBytecodeCommitment + }, +}); + +// create function call to deploy the contract instance +const deployerContract = getDeployerContract(); +const deployBCInstanceFnCall = deployerContract.prepare.deploy({ + args: { + salt: bananaCoinInstance.salt, + contract_class_id: bananaCoinInstance.contractClassId, + initialization_hash: bananaCoinInstance.initializationHash, + public_keys_hash: bananaCoinInstance.publicKeysHash, + universal_deploy: bananaCoinInstance.deployer.isZero(), + } +}); + +// create function call to initialize the contract +const initializeBCFnCall = bananaCoinInstance.prepare.constructor({ + args: bananaCoinDeploymentOptions.constructorArgs +}); + +// prepare a capsule for contract class registration +const encodedBytecode = bufferAsFields( + bananaCoinClass.packedBytecode, + MAX_PACKED_PUBLIC_BYTECODE_SIZE_IN_FIELDS +); + +// simulate to get the gas estimation +const { request: deployTokenRequest } = await aliceWallet.multiCall.simulate({ + calls: [registerBCClassFnCall, deployBCInstanceFnCall, initializeBCFnCall], + capsules: [encodedBytecode], + paymentMethod +}) + +await aliceWallet.multiCall.send(deployTokenRequest).wait(); + + +// Now use the contract +const { result: privateBalance } = await bananaCoinInstance.simulate.balance_of_private({ + wallet: aliceWallet, + args: {owner: aliceWallet.getAddress()}, +}); + +const bobAddress = AztecAddress.random(); + +// Gas estimation is done automatically and set on the request object +const { request: transferRequest } = await bananaCoinInstance.simulate.transfer({ + wallet: aliceWallet, + args: { + from: aliceAddress, + to: bobAddress, + value: privateBalance, + nonce: 0n + }, + paymentMethod +}); + +// Proving is done automatically since `request` wouldn't have it set after just simulation. +// Users can prove explicitly ahead of time with +// const { request: requestWithProof } = await bananaCoin.prove.transfer(request); +// await bananaCoin.send.transfer(requestWithProof).wait(); +await bananaCoinInstance.send.transfer(transferRequest).wait(); +``` + +## Introduction + +Developers and users have to think too hard when submitting transactions. + +This is due to a wonky API when interacting with the `BaseContractInterface`, and people need to know what's happening under the hood. + +Further, it is presently difficult to estimate the gas cost of a transaction. + +### The old UML + +```mermaid +classDiagram + +class BaseContractInteraction { + Tx + TxExecutionRequest + Wallet +} + +class BatchCall { + FunctionCall[] +} +BatchCall -->BaseContractInteraction + +class ContractFunctionInteraction { + AztecAddress contractAddress + FunctionAbi + any[] args +} +ContractFunctionInteraction --> BaseContractInteraction + +class DeployMethod { + Fr publicKeysHash + ContractArtifact + PostDeployCtor + any[] args + string|FunctionArtifact constructorNameOrArtifact +} +DeployMethod --> BaseContractInteraction + +class DeployAccountMethod { + AuthWitnessProvider + FunctionArtifact feePaymentArtifact +} +DeployAccountMethod --> DeployMethod + + +``` + + +### What was really bad about the old API? + +The `BaseContractInteraction` was an abstract base class, *with non-private state*, which its subclasses modified. + +In the base class needed to perform outrageous defensive programming, such as: +```ts + // Ensure we don't accidentally use a version of tx request that has estimateGas set to true, leading to an infinite loop. + this.txRequest = undefined; + const txRequest = await this.create({ + fee: { paymentMethod, gasSettings: GasSettings.default() }, + estimateGas: false, + }); + // Ensure we don't accidentally cache a version of tx request that has estimateGas forcefully set to false. + this.txRequest = undefined; +``` +Reasoning about the state became too difficult, especially when it came to performing accurate gas estimations. + +### How are we fixing it? + +Whenever we called `contract.methods.someFunction`, we were getting a `ContractFunctionInteraction` object, which was a subclass of `BaseContractInterface`, +but we didn't know what we were going to do with it yet, e.g. simulate, prove, or send. + +So, we're changing the generated typescript interfaces to contracts to read like `contract.{action}.someFunction` where `action` is one of `prepare`, `simulate`, `prove`, or `send`. + +These functions will always accept `ContractFunctionArgs` which will be converted into a `TxExecutionRequest`. + + + + + + + +By changing the + + +Old way: +```ts +const aliceAddress = AztecAddress.random(); +const privateBalance = await bananaCoin.methods.balance_of_private(aliceAddress).simulate(); + +const bobAddress = AztecAddress.random(); +const paymentMethod = new PrivateFeePaymentMethod(bananaCoin.address, bananaFPC.address, aliceWallet, refundSecret); +const interaction = await bananaCoin.methods + .transfer(aliceAddress, bobAddress, privateBalance, 0n) + .send({ + fee: { + gasSettings, + paymentMethod + }, + }) + .wait(); +``` + +## Interface + +The key operations we want to perform are: +- Producing a valid Tx Request +- Simulating a Tx Request + - This has private and public variants +- Estimating the gas cost of a Tx Request +- Proving a Tx Request +- Submitting a Tx Request + +Currently, the `BaseContractInterface` +```ts +/** + * Represents options for calling a (constrained) function in a contract. + * Allows the user to specify the sender address and nonce for a transaction. + */ +export type SendMethodOptions = { + /** Wether to skip the simulation of the public part of the transaction. */ + skipPublicSimulation?: boolean; + /** The fee options for the transaction. */ + fee?: FeeOptions; + /** Whether to run an initial simulation of the tx with high gas limit to figure out actual gas settings (will default to true later down the road). */ + estimateGas?: boolean; +}; + + +export abstract class BaseContractInteraction { + private tx?: Tx; + private txRequest?: TxExecutionRequest; + + /** + * Create a transaction execution request ready to be simulated. + * @param options - An optional object containing additional configuration for the transaction. + * @returns A transaction execution request. + */ + public abstract create(options?: SendMethodOptions): Promise; + + public async prove(options: SendMethodOptions = {}): Promise { + const txRequest = this.txRequest ?? (await this.create(options)); + this.tx = await this.wallet.proveTx(txRequest, !options.skipPublicSimulation); + return this.tx; + } + + public async estimateGas( + paymentMethod: FeePaymentMethod, + ): Promise> { + // REFACTOR: both `this.txRequest = undefined` below are horrible, we should not be caching stuff that doesn't need to be. + // This also hints at a weird interface for create/request/estimate/send etc. + + + const simulationResult = await this.wallet.simulateTx(txRequest, true); + const { totalGas: gasLimits, teardownGas: teardownGasLimits } = getGasLimits(simulationResult, Gas.empty()); + return { gasLimits, teardownGasLimits }; + } +} +``` + +Then for subclasses we have something like: +```ts +export class ContractFunctionInteraction extends BaseContractInteraction { + + constructor( + wallet: Wallet, + protected contractAddress: AztecAddress, + protected functionDao: FunctionAbi, + protected args: any[], + ) { + super(wallet); + if (args.some(arg => arg === undefined || arg === null)) { + throw new Error('All function interaction arguments must be defined and not null. Received: ' + args); + } + } + + + /** + * Create a transaction execution request that represents this call, encoded and authenticated by the + * user's wallet, ready to be simulated. + * @param opts - An optional object containing additional configuration for the transaction. + * @returns A Promise that resolves to a transaction instance. + */ + public async create(opts?: SendMethodOptions): Promise { + if (this.functionDao.functionType === FunctionType.UNCONSTRAINED) { + throw new Error("Can't call `create` on an unconstrained function."); + } + if (!this.txRequest) { + const calls = [this.request()]; + const fee = opts?.estimateGas ? await this.getFeeOptionsFromEstimatedGas({ calls, fee: opts?.fee }) : opts?.fee; + this.txRequest = await this.wallet.createTxExecutionRequest({ calls, fee }); + } + return this.txRequest; + } + + /** + * Simulate a transaction and get its return values + * Differs from prove in a few important ways: + * 1. It returns the values of the function execution + * 2. It supports `unconstrained`, `private` and `public` functions + * + * @param options - An optional object containing additional configuration for the transaction. + * @returns The result of the transaction as returned by the contract function. + */ + public async simulate(options: SimulateMethodOptions = {}): Promise { + if (this.functionDao.functionType == FunctionType.UNCONSTRAINED) { + return this.wallet.simulateUnconstrained(this.functionDao.name, this.args, this.contractAddress, options?.from); + } + + const txRequest = await this.create(); + const simulatedTx = await this.wallet.simulateTx(txRequest, true, options?.from); + + // As account entrypoints are private, for private functions we retrieve the return values from the first nested call + // since we're interested in the first set of values AFTER the account entrypoint + // For public functions we retrieve the first values directly from the public output. + const rawReturnValues = + this.functionDao.functionType == FunctionType.PRIVATE + ? simulatedTx.privateReturnValues?.nested?.[0].values + : simulatedTx.publicOutput?.publicReturnValues?.[0].values; + + return rawReturnValues ? decodeReturnValues(this.functionDao.returnTypes, rawReturnValues) : []; + } +} + +``` + +```ts +export class DeployMethod extends BaseContractInteraction { + private instance?: ContractInstanceWithAddress = undefined; + + /** Constructor function to call. */ + private constructorArtifact: FunctionArtifact | undefined; + + /** Cached call to request() */ + private functionCalls?: ExecutionRequestInit; + + constructor( + private publicKeysHash: Fr, + wallet: Wallet, + private artifact: ContractArtifact, + private postDeployCtor: (address: AztecAddress, wallet: Wallet) => Promise, + private args: any[] = [], + constructorNameOrArtifact?: string | FunctionArtifact, + ) { + super(wallet); + this.constructorArtifact = getInitializer(artifact, constructorNameOrArtifact); + } + + /** + * Create a contract deployment transaction, given the deployment options. + * This function internally calls `request()` and `sign()` methods to prepare + * the transaction for deployment. The resulting signed transaction can be + * later sent using the `send()` method. + * + * @param options - An object containing optional deployment settings, contractAddressSalt, and from. + * @returns A Promise resolving to an object containing the signed transaction data and other relevant information. + */ + public override async create(options: DeployOptions = {}): Promise { + if (!this.txRequest) { + this.txRequest = await this.wallet.createTxExecutionRequest(await this.request(options)); + } + return this.txRequest; + } + + + // REFACTOR: Having a `request` method with different semantics than the ones in the other + // derived ContractInteractions is confusing. We should unify the flow of all ContractInteractions. + + /** + * Returns an array of function calls that represent this operation. Useful as a building + * block for constructing batch requests. + * @param options - Deployment options. + * @returns An array of function calls. + * @remarks This method does not have the same return type as the `request` in the ContractInteraction object, + * it returns a promise for an array instead of a function call directly. + */ + public async request(options: DeployOptions = {}): Promise { + if (!this.functionCalls) { + // TODO: Should we add the contracts to the DB here, or once the tx has been sent or mined? + // Note that we need to run this registerContract here so it's available when computeFeeOptionsFromEstimatedGas + // runs, since it needs the contract to have been registered in order to estimate gas for its initialization, + // in case the initializer is public. This hints at the need of having "transient" contracts scoped to a + // simulation, so we can run the simulation with a set of contracts, but only "commit" them to the wallet + // once this tx has gone through. + await this.wallet.registerContract({ artifact: this.artifact, instance: this.getInstance(options) }); + + const deployment = await this.getDeploymentFunctionCalls(options); + const bootstrap = await this.getInitializeFunctionCalls(options); + + if (deployment.calls.length + bootstrap.calls.length === 0) { + throw new Error(`No function calls needed to deploy contract ${this.artifact.name}`); + } + + const request = { + calls: [...deployment.calls, ...bootstrap.calls], + authWitnesses: [...(deployment.authWitnesses ?? []), ...(bootstrap.authWitnesses ?? [])], + packedArguments: [...(deployment.packedArguments ?? []), ...(bootstrap.packedArguments ?? [])], + fee: options.fee, + }; + + if (options.estimateGas) { + // Why do we call this seemingly idempotent getter method here, without using its return value? + // This call pushes a capsule required for contract class registration under the hood. And since + // capsules are a stack, when we run the simulation for estimating gas, we consume the capsule + // that was meant for the actual call. So we need to push it again here. Hopefully this design + // will go away soon. + await this.getDeploymentFunctionCalls(options); + request.fee = await this.getFeeOptionsFromEstimatedGas(request); + } + + this.functionCalls = request; + } + + return this.functionCalls; + } + + /** + * Returns calls for registration of the class and deployment of the instance, depending on the provided options. + * @param options - Deployment options. + * @returns A function call array with potentially requests to the class registerer and instance deployer. + */ + protected async getDeploymentFunctionCalls(options: DeployOptions = {}): Promise { + const calls: FunctionCall[] = []; + + // Set contract instance object so it's available for populating the DeploySendTx object + const instance = this.getInstance(options); + + // Obtain contract class from artifact and check it matches the reported one by the instance. + // TODO(@spalladino): We're unnecessarily calculating the contract class multiple times here. + const contractClass = getContractClassFromArtifact(this.artifact); + if (!instance.contractClassId.equals(contractClass.id)) { + throw new Error( + `Contract class mismatch when deploying contract: got ${instance.contractClassId.toString()} from instance and ${contractClass.id.toString()} from artifact`, + ); + } + + // Register the contract class if it hasn't been published already. + if (!options.skipClassRegistration) { + if (await this.wallet.isContractClassPubliclyRegistered(contractClass.id)) { + this.log.debug( + `Skipping registration of already registered contract class ${contractClass.id.toString()} for ${instance.address.toString()}`, + ); + } else { + this.log.info( + `Creating request for registering contract class ${contractClass.id.toString()} as part of deployment for ${instance.address.toString()}`, + ); + calls.push((await registerContractClass(this.wallet, this.artifact)).request()); + } + } + + // Deploy the contract via the instance deployer. + if (!options.skipPublicDeployment) { + calls.push(deployInstance(this.wallet, instance).request()); + } + + return { + calls, + }; + } + + /** + * Returns the calls necessary to initialize the contract. + * @param options - Deployment options. + * @returns - An array of function calls. + */ + protected getInitializeFunctionCalls(options: DeployOptions): Promise { + const { address } = this.getInstance(options); + const calls: FunctionCall[] = []; + if (this.constructorArtifact && !options.skipInitialization) { + const constructorCall = new ContractFunctionInteraction( + this.wallet, + address, + this.constructorArtifact, + this.args, + ); + calls.push(constructorCall.request()); + } + return Promise.resolve({ + calls, + }); + } + + /** + * Send the contract deployment transaction using the provided options. + * This function extends the 'send' method from the ContractFunctionInteraction class, + * allowing us to send a transaction specifically for contract deployment. + * + * @param options - An object containing various deployment options such as contractAddressSalt and from. + * @returns A SentTx object that returns the receipt and the deployed contract instance. + */ + public override send(options: DeployOptions = {}): DeploySentTx { + const txHashPromise = super.send(options).getTxHash(); + const instance = this.getInstance(options); + this.log.debug( + `Sent deployment tx of ${this.artifact.name} contract with deployment address ${instance.address.toString()}`, + ); + return new DeploySentTx(this.wallet, txHashPromise, this.postDeployCtor, instance); + } + + /** + * Builds the contract instance to be deployed and returns it. + * + * @param options - An object containing various deployment options. + * @returns An instance object. + */ + public getInstance(options: DeployOptions = {}): ContractInstanceWithAddress { + if (!this.instance) { + this.instance = getContractInstanceFromDeployParams(this.artifact, { + constructorArgs: this.args, + salt: options.contractAddressSalt, + publicKeysHash: this.publicKeysHash, + constructorArtifact: this.constructorArtifact, + deployer: options.universalDeploy ? AztecAddress.ZERO : this.wallet.getAddress(), + }); + } + return this.instance; + } + + /** + * Prove the request. + * @param options - Deployment options. + * @returns The proven tx. + */ + public override prove(options: DeployOptions): Promise { + return super.prove(options); + } + + /** + * Estimates gas cost for this deployment operation. + * @param options - Options. + */ + public override estimateGas(paymentMethod?: FeePaymentMethod) { + return super.estimateGas(paymentMethod ?? new NativeFeePaymentMethod(this.wallet.getAddress())); + } + + /** Return this deployment address. */ + public get address() { + return this.instance?.address; + } + + /** Returns the partial address for this deployment. */ + public get partialAddress() { + return this.instance && computePartialAddress(this.instance); + } + +} + +``` + + +Then we have further specialization: +```ts +export class DeployAccountMethod extends DeployMethod { + #authWitnessProvider: AuthWitnessProvider; + #feePaymentArtifact: FunctionArtifact | undefined; + + constructor( + authWitnessProvider: AuthWitnessProvider, + publicKeysHash: Fr, + wallet: Wallet, + artifact: ContractArtifact, + args: any[] = [], + constructorNameOrArtifact?: string | FunctionArtifact, + feePaymentNameOrArtifact?: string | FunctionArtifact, + ) { + super( + publicKeysHash, + wallet, + artifact, + (address, wallet) => Contract.at(address, artifact, wallet), + args, + constructorNameOrArtifact, + ); + + this.#authWitnessProvider = authWitnessProvider; + this.#feePaymentArtifact = + typeof feePaymentNameOrArtifact === 'string' + ? getFunctionArtifact(artifact, feePaymentNameOrArtifact) + : feePaymentNameOrArtifact; + } + + protected override async getInitializeFunctionCalls(options: DeployOptions): Promise { + const exec = await super.getInitializeFunctionCalls(options); + + if (options.fee && this.#feePaymentArtifact) { + const { address } = this.getInstance(); + const emptyAppPayload = EntrypointPayload.fromAppExecution([]); + const feePayload = await EntrypointPayload.fromFeeOptions(address, options?.fee); + + exec.calls.push({ + name: this.#feePaymentArtifact.name, + to: address, + args: encodeArguments(this.#feePaymentArtifact, [emptyAppPayload, feePayload]), + selector: FunctionSelector.fromNameAndParameters( + this.#feePaymentArtifact.name, + this.#feePaymentArtifact.parameters, + ), + type: this.#feePaymentArtifact.functionType, + isStatic: this.#feePaymentArtifact.isStatic, + returnTypes: this.#feePaymentArtifact.returnTypes, + }); + + exec.authWitnesses ??= []; + exec.packedArguments ??= []; + + exec.authWitnesses.push(await this.#authWitnessProvider.createAuthWit(emptyAppPayload.hash())); + exec.authWitnesses.push(await this.#authWitnessProvider.createAuthWit(feePayload.hash())); + exec.packedArguments.push(...emptyAppPayload.packedArguments); + exec.packedArguments.push(...feePayload.packedArguments); + } + + return exec; + } +} +``` + + + +## Implementation + +Delve into the specifics of the design. Include diagrams, code snippets, API descriptions, and database schema changes as necessary. Highlight any significant changes to the existing architecture or interfaces. + +Discuss any alternative or rejected solutions. + +## Change Set + +Fill in bullets for each area that will be affected by this change. + +- [ ] L1 Contracts +- [ ] Enshrined L2 Contracts +- [ ] Private Kernel Circuits +- [ ] Public Kernel Circuits +- [ ] Rollup Circuits +- [ ] Aztec.nr +- [ ] Noir +- [ ] AVM +- [ ] Sequencer +- [ ] Fees +- [ ] P2P Network +- [ ] Cryptography +- [ ] DevOps + +## Test Plan + +Outline what unit and e2e tests will be written. Describe the logic they cover and any mock objects used. + +## Documentation Plan + +Identify changes or additions to the user documentation or protocol spec. + + +## Rejection Reason + +If the design is rejected, include a brief explanation of why. + +## Abandonment Reason + +If the design is abandoned mid-implementation, include a brief explanation of why. + +## Implementation Deviations + +If the design is implemented, include a brief explanation of deviations to the original design. From 4b1601235a4f78bdabf60a05b789c87ab8b1238a Mon Sep 17 00:00:00 2001 From: Mitchell Tracy Date: Tue, 11 Jun 2024 15:42:01 -0400 Subject: [PATCH 2/9] continue improving design. begin implementation thoughts --- ...6973-refactor-base-contract-interaction.md | 1004 ++++++++--------- 1 file changed, 479 insertions(+), 525 deletions(-) diff --git a/in-progress/6973-refactor-base-contract-interaction.md b/in-progress/6973-refactor-base-contract-interaction.md index fbb816f..f3be529 100644 --- a/in-progress/6973-refactor-base-contract-interaction.md +++ b/in-progress/6973-refactor-base-contract-interaction.md @@ -13,60 +13,43 @@ This is a refactor of the `BaseContractInteraction` class to simplify the API an The refactored approach mimics Viem's API, with some enhancements and modifications to fit our needs. -Example: -```ts - -// Create a new schnorr account - -const signingPrivateKey = GrumpkinPrivateKey.random(); -const signingPublicKey = new Schnorr().computePublicKey(alice.signingPrivateKey); - -const alice = { - signingPrivateKey, - signingPublicKey, - secretKey: Fr.random(), -} - -const aliceDeploymentOptions = { - constructorArtifact: 'constructor', - constructorArgs: { - signing_pub_key_x: alice.signingPublicKey.x, - signing_pub_key_y: alice.signingPublicKey.y - }, - salt: 42, - publicKeysHash: deriveKeys(alice.secretKey).publicKeys.hash(), - deployer: AztecAddress.ZERO -} - -const aliceContractInstance = getContractInstanceFromDeployParams( - SchnorrAccountContract.artifact, - aliceDeploymentOptions -); +### Account Creation -const aliceCompleteAddress = CompleteAddress.fromSecretKeyAndInstance(alice.secretKey, aliceContractInstance); - -await pxe.registerAccount( - alice.secretKey, - aliceCompleteAddress.partialAddress -); -await waitForAccountSynch(pxe, aliceCompleteAddress); - -const aliceAuthWitProvider = new SchnorrAuthWitnessProvider(alice.signingPrivateKey) -const nodeInfo = await pxe.getNodeInfo(); -const aliceInterface = new DefaultAccountInterface(aliceAuthWitProvider, aliceCompleteAddress, nodeInfo) -const aliceWallet = new AccountWallet(pxe, aliceInterface); +```ts +const pxe = await createCompatibleClient(rpcUrl, debugLogger); +const secretKey = Fr.random(); +const salt = 42; + +// Returns the wallet and deployment options. +// Does not modify PXE state. +const { wallet: aliceWallet, deploymentOptions } = await getSchnorrWallet(pxe, secretKey, salt); + +// Register the account in PXE, and wait for it to sync. +await aliceWallet.registerAccountInPXE({ sync: true }); + +// No wallet or secret passed to FPC +const paymentMethod = new PrivateFeePaymentMethod(someCoin.address, someFPC.address); + +// Changes to the PXE (e.g. notes, nullifiers, auth wits, contract deployments, capsules) are not persisted. +const { request: deployAliceAccountRequest } = await aliceWallet.deploy.simulate({ + artifact: SchnorrAccountContract.artifact, + deploymentOptions, + paymentMethod, + skipClassRegistration: true, + skipPublicDeployment: true, + skipInitialization: false, + // gasSettings: undefined => automatic gas estimation. the returned `request` will have the gasSettings set. +}); -const paymentMethod = new PrivateFeePaymentMethod(bananaCoin.address, bananaFPC.address, aliceWallet, refundSecret); +const { contractInstance } = await aliceWallet.deploy.send(deployAliceAccountRequest).wait(); -await aliceContractInstance.send.constructor({ - wallet: aliceWallet, - args: aliceDeploymentOptions.constructorArgs, - paymentMethod -}).wait(); +debugLogger(`Alice's account deployed at ${contractInstance.address.toString()}`); +``` -// Deploy BananaCoin as an instance of TokenContract +### Deploy and Use Contract +```ts const bananaCoinDeploymentOptions = { constructorArtifact: 'constructor', constructorArgs: { @@ -80,76 +63,27 @@ const bananaCoinDeploymentOptions = { deployer: aliceWallet.getAddress() } -const bananaCoinInstance = getContractInstanceFromDeployParams( - TokenContract.artifact, - bananaCoinDeploymentOptions -); - - -// puts the artifact and instance in the PXE -await aliceWallet.registerContract({ +const { request: deployTokenRequest } = await aliceWallet.deploy.simulate({ artifact: TokenContract.artifact, - instance: bananaCoinInstance -}); - -// create function call to deploy the contract class -// will consume the capsule when called -const tokenContractClass = getContractClassFromArtifact(TokenContract.artifact); -const registerer = getRegistererContract(); -const registerBCClassFnCall = registerer.prepare.register({ - // this is an instance of the contract class - // so the address is stubbed in - args: { - artifact_hash: tokenContractClass.artifactHash, - private_functions_root: tokenContractClass.privateFunctionsRoot, - public_bytecode_commitment: tokenContractClass.publicBytecodeCommitment - }, -}); - -// create function call to deploy the contract instance -const deployerContract = getDeployerContract(); -const deployBCInstanceFnCall = deployerContract.prepare.deploy({ - args: { - salt: bananaCoinInstance.salt, - contract_class_id: bananaCoinInstance.contractClassId, - initialization_hash: bananaCoinInstance.initializationHash, - public_keys_hash: bananaCoinInstance.publicKeysHash, - universal_deploy: bananaCoinInstance.deployer.isZero(), - } -}); - -// create function call to initialize the contract -const initializeBCFnCall = bananaCoinInstance.prepare.constructor({ - args: bananaCoinDeploymentOptions.constructorArgs -}); - -// prepare a capsule for contract class registration -const encodedBytecode = bufferAsFields( - bananaCoinClass.packedBytecode, - MAX_PACKED_PUBLIC_BYTECODE_SIZE_IN_FIELDS -); - -// simulate to get the gas estimation -const { request: deployTokenRequest } = await aliceWallet.multiCall.simulate({ - calls: [registerBCClassFnCall, deployBCInstanceFnCall, initializeBCFnCall], - capsules: [encodedBytecode], + deploymentOptions: bananaCoinDeploymentOptions, + skipClassRegistration: false, + skipPublicDeployment: false, + skipInitialization: false, paymentMethod }) -await aliceWallet.multiCall.send(deployTokenRequest).wait(); +const { contractInstance: bananaCoinInstance } = await aliceWallet.deploy.send(deployTokenRequest).wait(); - -// Now use the contract -const { result: privateBalance } = await bananaCoinInstance.simulate.balance_of_private({ - wallet: aliceWallet, +const { result: privateBalance } = await aliceWallet.simulate({ + contractInstance: bananaCoinInstance, + functionName: 'balance_of_private' args: {owner: aliceWallet.getAddress()}, }); -const bobAddress = AztecAddress.random(); -// Gas estimation is done automatically and set on the request object -const { request: transferRequest } = await bananaCoinInstance.simulate.transfer({ - wallet: aliceWallet, +const { request: transferRequest } = await aliceWallet.simulate({ + contractInstance: bananaCoinInstance, + functionName: 'transfer', args: { from: aliceAddress, to: bobAddress, @@ -159,18 +93,92 @@ const { request: transferRequest } = await bananaCoinInstance.simulate.transfer( paymentMethod }); + // Proving is done automatically since `request` wouldn't have it set after just simulation. // Users can prove explicitly ahead of time with -// const { request: requestWithProof } = await bananaCoin.prove.transfer(request); -// await bananaCoin.send.transfer(requestWithProof).wait(); -await bananaCoinInstance.send.transfer(transferRequest).wait(); +// const { request: requestWithProof } = await aliceWallet.prove(request); +// await aliceWallet.send(requestWithProof).wait(); +await aliceWallet.send(transferRequest).wait(); + +``` + +### Scoping Instances + +A bit of a "nice to have". + +```ts +const aliceBananaCoinInstance = aliceWallet.useContractInstanceAt(TokenContract.artifact, bananaCoinInstance.address); +const {result: privateBalance} = await aliceBananaCoinInstance.simulate.balance_of_private({ + args: {owner: aliceWallet.getAddress()}, +}); ``` + ## Introduction -Developers and users have to think too hard when submitting transactions. +Developers and users have to think too hard when deploying accounts and submitting transactions. + +This is due in part to holding mutable state in the `BaseContractInteraction` class, which is a base class for all contract interactions: +it has multiple subclasses that mutate the state, so it is hard to know what has been set. + +For example, the current attempt to estimate gas has a section that reads: +```ts + // REFACTOR: both `this.txRequest = undefined` below are horrible, we should not be caching stuff that doesn't need to be. + // This also hints at a weird interface for create/request/estimate/send etc. + + // Ensure we don't accidentally use a version of tx request that has estimateGas set to true, leading to an infinite loop. + this.txRequest = undefined; + const txRequest = await this.create({ + fee: { paymentMethod, gasSettings: GasSettings.default() }, + estimateGas: false, + }); + // Ensure we don't accidentally cache a version of tx request that has estimateGas forcefully set to false. + this.txRequest = undefined; +``` + +This push for a refactor was motivated by the realization that we need a more sophisticated gas estimation mechanism where we first simulate without the FPC to get the gas cost of app logic, then plug that into a simulation with the FPC. + +Doing this well also requires that simulations to be idempotent: all changes to the PXE including notes, nullifiers, auth wits, contract deployments, capsules, etc. should not be persisted until the user explicitly sends the transaction. + +This would fix comments seen in the `DeployMethod` class like: +```ts +// TODO: Should we add the contracts to the DB here, or once the tx has been sent or mined? +// Note that we need to run this registerContract here so it's available when computeFeeOptionsFromEstimatedGas +// runs, since it needs the contract to have been registered in order to estimate gas for its initialization, +// in case the initializer is public. This hints at the need of having "transient" contracts scoped to a +// simulation, so we can run the simulation with a set of contracts, but only "commit" them to the wallet +// once this tx has gone through. +await this.wallet.registerContract({ artifact: this.artifact, instance: this.getInstance(options) }); + +// ... + +if (options.estimateGas) { + // Why do we call this seemingly idempotent getter method here, without using its return value? + // This call pushes a capsule required for contract class registration under the hood. And since + // capsules are a stack, when we run the simulation for estimating gas, we consume the capsule + // that was meant for the actual call. So we need to push it again here. Hopefully this design + // will go away soon. + await this.getDeploymentFunctionCalls(options); + request.fee = await this.getFeeOptionsFromEstimatedGas(request); +} +``` + +We also want to unify the mechanism for creating, simulating, proving, and sending transactions, regardless of whether they are for deploying a contract, calling a function, or deploying an account. + +This would clear up concerns like: + +> // REFACTOR: Having a `request` method with different semantics than the ones in the other + // derived ContractInteractions is confusing. We should unify the flow of all ContractInteractions. + + + + + + +Implementing this in the current API would be difficult + +The complexity in the current API is shown elsewhere -This is due to a wonky API when interacting with the `BaseContractInterface`, and people need to know what's happening under the hood. Further, it is presently difficult to estimate the gas cost of a transaction. @@ -216,475 +224,421 @@ DeployAccountMethod --> DeployMethod ``` -### What was really bad about the old API? -The `BaseContractInteraction` was an abstract base class, *with non-private state*, which its subclasses modified. -In the base class needed to perform outrageous defensive programming, such as: -```ts - // Ensure we don't accidentally use a version of tx request that has estimateGas set to true, leading to an infinite loop. - this.txRequest = undefined; - const txRequest = await this.create({ - fee: { paymentMethod, gasSettings: GasSettings.default() }, - estimateGas: false, - }); - // Ensure we don't accidentally cache a version of tx request that has estimateGas forcefully set to false. - this.txRequest = undefined; -``` -Reasoning about the state became too difficult, especially when it came to performing accurate gas estimations. +## Interface + +The key operations we want to perform are: +- Producing a valid Tx Request +- Simulating a Tx Request +- Estimating the gas cost of a Tx Request +- Proving a Tx Request +- Submitting a Tx Request -### How are we fixing it? +These operations should be accessible from a `Wallet`, which will generally be `Schnorr` or `Signerless`. -Whenever we called `contract.methods.someFunction`, we were getting a `ContractFunctionInteraction` object, which was a subclass of `BaseContractInterface`, -but we didn't know what we were going to do with it yet, e.g. simulate, prove, or send. +This is analogous to `viem` having a `walletClient` and a `publicClient`. -So, we're changing the generated typescript interfaces to contracts to read like `contract.{action}.someFunction` where `action` is one of `prepare`, `simulate`, `prove`, or `send`. -These functions will always accept `ContractFunctionArgs` which will be converted into a `TxExecutionRequest`. +### What is a Wallet? +```ts +export type Wallet = AccountInterface & PXE & AccountKeyRotationInterface; +export interface AccountInterface extends AuthWitnessProvider, EntrypointInterface { + /** Returns the complete address for this account. */ + getCompleteAddress(): CompleteAddress; + /** Returns the address for this account. */ + getAddress(): AztecAddress; + /** Returns the chain id for this account */ + getChainId(): Fr; + /** Returns the rollup version for this account */ + getVersion(): Fr; +} +export interface AuthWitnessProvider { + createAuthWit( + messageHashOrIntent: + | Fr + | Buffer + | { + /** The caller to approve */ + caller: AztecAddress; + /** The action to approve */ + action: ContractFunctionInteraction | FunctionCall; + /** The chain id to approve */ + chainId?: Fr; + /** The version to approve */ + version?: Fr; + }, + ): Promise; +} -By changing the +export interface EntrypointInterface { + /** + * Generates an execution request out of set of function calls. + * @param execution - The execution intents to be run. + * @returns The authenticated transaction execution request. + */ + createTxExecutionRequest(execution: ExecutionRequestInit): Promise; +} +export class TxExecutionRequest { + constructor( + /** + * Sender. + */ + public origin: AztecAddress, + /** + * Selector of the function to call. + */ + public functionSelector: FunctionSelector, + /** + * The hash of arguments of first call to be executed (usually account entrypoint). + * @dev This hash is a pointer to `argsOfCalls` unordered array. + */ + public firstCallArgsHash: Fr, + /** + * Transaction context. + */ + public txContext: TxContext, + /** + * An unordered array of packed arguments for each call in the transaction. + * @dev These arguments are accessed in Noir via oracle and constrained against the args hash. The length of + * the array is equal to the number of function calls in the transaction (1 args per 1 call). + */ + public argsOfCalls: PackedValues[], + /** + * Transient authorization witnesses for authorizing the execution of one or more actions during this tx. + * These witnesses are not expected to be stored in the local witnesses database of the PXE. + */ + public authWitnesses: AuthWitness[], + ) {} + // ... +} -Old way: -```ts -const aliceAddress = AztecAddress.random(); -const privateBalance = await bananaCoin.methods.balance_of_private(aliceAddress).simulate(); +export type ExecutionRequestInit = { + /** The function calls to be executed. */ + calls: FunctionCall[]; + /** Any transient auth witnesses needed for this execution */ + authWitnesses?: AuthWitness[]; + /** Any transient packed arguments for this execution */ + packedArguments?: PackedValues[]; + /** Any transient capsules needed for this execution */ + capsules?: Fr[][]; + /** The fee payment method to use */ + paymentMethod: FeePaymentMethod; + /** The gas settings */ + gasSettings: GasSettings; +}; -const bobAddress = AztecAddress.random(); -const paymentMethod = new PrivateFeePaymentMethod(bananaCoin.address, bananaFPC.address, aliceWallet, refundSecret); -const interaction = await bananaCoin.methods - .transfer(aliceAddress, bobAddress, privateBalance, 0n) - .send({ - fee: { - gasSettings, - paymentMethod - }, - }) - .wait(); ``` -## Interface -The key operations we want to perform are: -- Producing a valid Tx Request -- Simulating a Tx Request - - This has private and public variants -- Estimating the gas cost of a Tx Request -- Proving a Tx Request -- Submitting a Tx Request +## Implementation -Currently, the `BaseContractInterface` -```ts -/** - * Represents options for calling a (constrained) function in a contract. - * Allows the user to specify the sender address and nonce for a transaction. - */ -export type SendMethodOptions = { - /** Wether to skip the simulation of the public part of the transaction. */ - skipPublicSimulation?: boolean; - /** The fee options for the transaction. */ - fee?: FeeOptions; - /** Whether to run an initial simulation of the tx with high gas limit to figure out actual gas settings (will default to true later down the road). */ - estimateGas?: boolean; -}; +### Drop `isFeePayer` from `FeeEntrypointPayload` -export abstract class BaseContractInteraction { - private tx?: Tx; - private txRequest?: TxExecutionRequest; +I think removing this makes the code simpler and more flexible: account contracts can just check if fee payer has been set and set it if not. - /** - * Create a transaction execution request ready to be simulated. - * @param options - An optional object containing additional configuration for the transaction. - * @returns A transaction execution request. - */ - public abstract create(options?: SendMethodOptions): Promise; +Concerned users can just inspect the simulation result to see if they are paying the fee. - public async prove(options: SendMethodOptions = {}): Promise { - const txRequest = this.txRequest ?? (await this.create(options)); - this.tx = await this.wallet.proveTx(txRequest, !options.skipPublicSimulation); - return this.tx; - } +Then we just have `EntrypointPayload` as a single, concrete class. - public async estimateGas( - paymentMethod: FeePaymentMethod, - ): Promise> { - // REFACTOR: both `this.txRequest = undefined` below are horrible, we should not be caching stuff that doesn't need to be. - // This also hints at a weird interface for create/request/estimate/send etc. +### Translating user requests to TxExecutionRequests +Consider that we have - const simulationResult = await this.wallet.simulateTx(txRequest, true); - const { totalGas: gasLimits, teardownGas: teardownGasLimits } = getGasLimits(simulationResult, Gas.empty()); - return { gasLimits, teardownGasLimits }; - } -} +```ts +const { request: transferRequest } = await aliceWallet.simulate({ + contractInstance: bananaCoinInstance, + functionName: 'transfer', + args: { + from: aliceAddress, + to: bobAddress, + value: privateBalance, + nonce: 0n + }, + paymentMethod +}); ``` -Then for subclasses we have something like: -```ts -export class ContractFunctionInteraction extends BaseContractInteraction { +We need to ultimately get to a `TxExecutionRequest`. - constructor( - wallet: Wallet, - protected contractAddress: AztecAddress, - protected functionDao: FunctionAbi, - protected args: any[], - ) { - super(wallet); - if (args.some(arg => arg === undefined || arg === null)) { - throw new Error('All function interaction arguments must be defined and not null. Received: ' + args); - } - } +There are a few key steps to get there. +Assume we're dealing with schnorr. - /** - * Create a transaction execution request that represents this call, encoded and authenticated by the - * user's wallet, ready to be simulated. - * @param opts - An optional object containing additional configuration for the transaction. - * @returns A Promise that resolves to a transaction instance. - */ - public async create(opts?: SendMethodOptions): Promise { - if (this.functionDao.functionType === FunctionType.UNCONSTRAINED) { - throw new Error("Can't call `create` on an unconstrained function."); - } - if (!this.txRequest) { - const calls = [this.request()]; - const fee = opts?.estimateGas ? await this.getFeeOptionsFromEstimatedGas({ calls, fee: opts?.fee }) : opts?.fee; - this.txRequest = await this.wallet.createTxExecutionRequest({ calls, fee }); - } - return this.txRequest; - } +#### Translate the `contractInstance`, `functionName` and `args` to a `FunctionCall` - /** - * Simulate a transaction and get its return values - * Differs from prove in a few important ways: - * 1. It returns the values of the function execution - * 2. It supports `unconstrained`, `private` and `public` functions - * - * @param options - An optional object containing additional configuration for the transaction. - * @returns The result of the transaction as returned by the contract function. - */ - public async simulate(options: SimulateMethodOptions = {}): Promise { - if (this.functionDao.functionType == FunctionType.UNCONSTRAINED) { - return this.wallet.simulateUnconstrained(this.functionDao.name, this.args, this.contractAddress, options?.from); - } - - const txRequest = await this.create(); - const simulatedTx = await this.wallet.simulateTx(txRequest, true, options?.from); - - // As account entrypoints are private, for private functions we retrieve the return values from the first nested call - // since we're interested in the first set of values AFTER the account entrypoint - // For public functions we retrieve the first values directly from the public output. - const rawReturnValues = - this.functionDao.functionType == FunctionType.PRIVATE - ? simulatedTx.privateReturnValues?.nested?.[0].values - : simulatedTx.publicOutput?.publicReturnValues?.[0].values; - - return rawReturnValues ? decodeReturnValues(this.functionDao.returnTypes, rawReturnValues) : []; - } +Get the contract artifact out of the PXE using the contractInstance's contractClassId. + +Scan its `FunctionAbi`s for the one with the given name. + +Construct the `FunctionCall` as: +```ts +{ + name: functionAbi.name, + args, + selector: FunctionSelector.fromNameAndParameters(functionAbi.name, functionAbi.parameters), + type: functionAbi.functionType, + to: contractInstance.address, + isStatic: functionAbi.isStatic, + returnTypes: functionAbi.returnTypes, } - ``` -```ts -export class DeployMethod extends BaseContractInteraction { - private instance?: ContractInstanceWithAddress = undefined; - - /** Constructor function to call. */ - private constructorArtifact: FunctionArtifact | undefined; - - /** Cached call to request() */ - private functionCalls?: ExecutionRequestInit; - - constructor( - private publicKeysHash: Fr, - wallet: Wallet, - private artifact: ContractArtifact, - private postDeployCtor: (address: AztecAddress, wallet: Wallet) => Promise, - private args: any[] = [], - constructorNameOrArtifact?: string | FunctionArtifact, - ) { - super(wallet); - this.constructorArtifact = getInitializer(artifact, constructorNameOrArtifact); - } - /** - * Create a contract deployment transaction, given the deployment options. - * This function internally calls `request()` and `sign()` methods to prepare - * the transaction for deployment. The resulting signed transaction can be - * later sent using the `send()` method. - * - * @param options - An object containing optional deployment settings, contractAddressSalt, and from. - * @returns A Promise resolving to an object containing the signed transaction data and other relevant information. - */ - public override async create(options: DeployOptions = {}): Promise { - if (!this.txRequest) { - this.txRequest = await this.wallet.createTxExecutionRequest(await this.request(options)); - } - return this.txRequest; - } - // REFACTOR: Having a `request` method with different semantics than the ones in the other - // derived ContractInteractions is confusing. We should unify the flow of all ContractInteractions. - /** - * Returns an array of function calls that represent this operation. Useful as a building - * block for constructing batch requests. - * @param options - Deployment options. - * @returns An array of function calls. - * @remarks This method does not have the same return type as the `request` in the ContractInteraction object, - * it returns a promise for an array instead of a function call directly. - */ - public async request(options: DeployOptions = {}): Promise { - if (!this.functionCalls) { - // TODO: Should we add the contracts to the DB here, or once the tx has been sent or mined? - // Note that we need to run this registerContract here so it's available when computeFeeOptionsFromEstimatedGas - // runs, since it needs the contract to have been registered in order to estimate gas for its initialization, - // in case the initializer is public. This hints at the need of having "transient" contracts scoped to a - // simulation, so we can run the simulation with a set of contracts, but only "commit" them to the wallet - // once this tx has gone through. - await this.wallet.registerContract({ artifact: this.artifact, instance: this.getInstance(options) }); - - const deployment = await this.getDeploymentFunctionCalls(options); - const bootstrap = await this.getInitializeFunctionCalls(options); - - if (deployment.calls.length + bootstrap.calls.length === 0) { - throw new Error(`No function calls needed to deploy contract ${this.artifact.name}`); - } - - const request = { - calls: [...deployment.calls, ...bootstrap.calls], - authWitnesses: [...(deployment.authWitnesses ?? []), ...(bootstrap.authWitnesses ?? [])], - packedArguments: [...(deployment.packedArguments ?? []), ...(bootstrap.packedArguments ?? [])], - fee: options.fee, - }; - - if (options.estimateGas) { - // Why do we call this seemingly idempotent getter method here, without using its return value? - // This call pushes a capsule required for contract class registration under the hood. And since - // capsules are a stack, when we run the simulation for estimating gas, we consume the capsule - // that was meant for the actual call. So we need to push it again here. Hopefully this design - // will go away soon. - await this.getDeploymentFunctionCalls(options); - request.fee = await this.getFeeOptionsFromEstimatedGas(request); - } - - this.functionCalls = request; - } - - return this.functionCalls; - } +1. Extract the "app" EntrypointPayload - /** - * Returns calls for registration of the class and deployment of the instance, depending on the provided options. - * @param options - Deployment options. - * @returns A function call array with potentially requests to the class registerer and instance deployer. - */ - protected async getDeploymentFunctionCalls(options: DeployOptions = {}): Promise { - const calls: FunctionCall[] = []; - - // Set contract instance object so it's available for populating the DeploySendTx object - const instance = this.getInstance(options); - - // Obtain contract class from artifact and check it matches the reported one by the instance. - // TODO(@spalladino): We're unnecessarily calculating the contract class multiple times here. - const contractClass = getContractClassFromArtifact(this.artifact); - if (!instance.contractClassId.equals(contractClass.id)) { - throw new Error( - `Contract class mismatch when deploying contract: got ${instance.contractClassId.toString()} from instance and ${contractClass.id.toString()} from artifact`, - ); - } - - // Register the contract class if it hasn't been published already. - if (!options.skipClassRegistration) { - if (await this.wallet.isContractClassPubliclyRegistered(contractClass.id)) { - this.log.debug( - `Skipping registration of already registered contract class ${contractClass.id.toString()} for ${instance.address.toString()}`, - ); - } else { - this.log.info( - `Creating request for registering contract class ${contractClass.id.toString()} as part of deployment for ${instance.address.toString()}`, - ); - calls.push((await registerContractClass(this.wallet, this.artifact)).request()); - } - } - - // Deploy the contract via the instance deployer. - if (!options.skipPublicDeployment) { - calls.push(deployInstance(this.wallet, instance).request()); - } - - return { - calls, - }; - } +2. Collect all authwits + 1. For the app payload + 2. For the fee payment + 3. For anything needed by the fee payment method +3. - /** - * Returns the calls necessary to initialize the contract. - * @param options - Deployment options. - * @returns - An array of function calls. - */ - protected getInitializeFunctionCalls(options: DeployOptions): Promise { - const { address } = this.getInstance(options); - const calls: FunctionCall[] = []; - if (this.constructorArtifact && !options.skipInitialization) { - const constructorCall = new ContractFunctionInteraction( - this.wallet, - address, - this.constructorArtifact, - this.args, - ); - calls.push(constructorCall.request()); - } - return Promise.resolve({ - calls, - }); - } - /** - * Send the contract deployment transaction using the provided options. - * This function extends the 'send' method from the ContractFunctionInteraction class, - * allowing us to send a transaction specifically for contract deployment. - * - * @param options - An object containing various deployment options such as contractAddressSalt and from. - * @returns A SentTx object that returns the receipt and the deployed contract instance. - */ - public override send(options: DeployOptions = {}): DeploySentTx { - const txHashPromise = super.send(options).getTxHash(); - const instance = this.getInstance(options); - this.log.debug( - `Sent deployment tx of ${this.artifact.name} contract with deployment address ${instance.address.toString()}`, - ); - return new DeploySentTx(this.wallet, txHashPromise, this.postDeployCtor, instance); - } - /** - * Builds the contract instance to be deployed and returns it. - * - * @param options - An object containing various deployment options. - * @returns An instance object. - */ - public getInstance(options: DeployOptions = {}): ContractInstanceWithAddress { - if (!this.instance) { - this.instance = getContractInstanceFromDeployParams(this.artifact, { - constructorArgs: this.args, - salt: options.contractAddressSalt, - publicKeysHash: this.publicKeysHash, - constructorArtifact: this.constructorArtifact, - deployer: options.universalDeploy ? AztecAddress.ZERO : this.wallet.getAddress(), - }); - } - return this.instance; - } - /** - * Prove the request. - * @param options - Deployment options. - * @returns The proven tx. - */ - public override prove(options: DeployOptions): Promise { - return super.prove(options); - } +### Kitchen Sink - /** - * Estimates gas cost for this deployment operation. - * @param options - Options. - */ - public override estimateGas(paymentMethod?: FeePaymentMethod) { - return super.estimateGas(paymentMethod ?? new NativeFeePaymentMethod(this.wallet.getAddress())); - } +```ts - /** Return this deployment address. */ - public get address() { - return this.instance?.address; - } +// Create a new schnorr account + +const aliceSecretKey = Fr.random(), - /** Returns the partial address for this deployment. */ - public get partialAddress() { - return this.instance && computePartialAddress(this.instance); - } +const signingPrivateKey = deriveSigningKey(aliceSecretKey); +const signingPublicKey = new Schnorr().computePublicKey(signingPrivateKey); + +const aliceDeploymentOptions = { + constructorArtifact: 'constructor', + constructorArgs: { + signing_pub_key_x: signingPublicKey.x, + signing_pub_key_y: signingPublicKey.y + }, + salt: 42, + publicKeysHash: deriveKeys(aliceSecretKey).publicKeys.hash(), + deployer: AztecAddress.ZERO } -``` +const aliceContractInstance = getContractInstanceFromDeployParams( + SchnorrAccountContract.artifact, + aliceDeploymentOptions +); +const aliceCompleteAddress = CompleteAddress.fromSecretKeyAndInstance(aliceSecretKey, aliceContractInstance); -Then we have further specialization: -```ts -export class DeployAccountMethod extends DeployMethod { - #authWitnessProvider: AuthWitnessProvider; - #feePaymentArtifact: FunctionArtifact | undefined; +await pxe.registerAccount( + aliceSecretKey, + aliceCompleteAddress.partialAddress +); +await waitForAccountSynch(pxe, aliceCompleteAddress); - constructor( - authWitnessProvider: AuthWitnessProvider, - publicKeysHash: Fr, - wallet: Wallet, - artifact: ContractArtifact, - args: any[] = [], - constructorNameOrArtifact?: string | FunctionArtifact, - feePaymentNameOrArtifact?: string | FunctionArtifact, - ) { - super( - publicKeysHash, - wallet, - artifact, - (address, wallet) => Contract.at(address, artifact, wallet), - args, - constructorNameOrArtifact, - ); - - this.#authWitnessProvider = authWitnessProvider; - this.#feePaymentArtifact = - typeof feePaymentNameOrArtifact === 'string' - ? getFunctionArtifact(artifact, feePaymentNameOrArtifact) - : feePaymentNameOrArtifact; - } - - protected override async getInitializeFunctionCalls(options: DeployOptions): Promise { - const exec = await super.getInitializeFunctionCalls(options); - - if (options.fee && this.#feePaymentArtifact) { - const { address } = this.getInstance(); - const emptyAppPayload = EntrypointPayload.fromAppExecution([]); - const feePayload = await EntrypointPayload.fromFeeOptions(address, options?.fee); - - exec.calls.push({ - name: this.#feePaymentArtifact.name, - to: address, - args: encodeArguments(this.#feePaymentArtifact, [emptyAppPayload, feePayload]), - selector: FunctionSelector.fromNameAndParameters( - this.#feePaymentArtifact.name, - this.#feePaymentArtifact.parameters, - ), - type: this.#feePaymentArtifact.functionType, - isStatic: this.#feePaymentArtifact.isStatic, - returnTypes: this.#feePaymentArtifact.returnTypes, - }); - - exec.authWitnesses ??= []; - exec.packedArguments ??= []; - - exec.authWitnesses.push(await this.#authWitnessProvider.createAuthWit(emptyAppPayload.hash())); - exec.authWitnesses.push(await this.#authWitnessProvider.createAuthWit(feePayload.hash())); - exec.packedArguments.push(...emptyAppPayload.packedArguments); - exec.packedArguments.push(...feePayload.packedArguments); - } - - return exec; - } +const aliceAuthWitProvider = new SchnorrAuthWitnessProvider(signingPrivateKey) +const nodeInfo = await pxe.getNodeInfo(); +const aliceInterface = new DefaultAccountInterface(aliceAuthWitProvider, aliceCompleteAddress, nodeInfo) +const aliceWallet = new AccountWallet(pxe, aliceInterface); + + +// Everything above could be rolled into a single function call: +// const wallet = await getSchnorrAccount( +// pxe, +// alice.secretKey, +// deriveSigningKey(alice.secretKey), +// aliceDeploymentOptions.salt +// ).getWallet(); + +const paymentMethod = new PrivateFeePaymentMethod(someCoin.address, someFPC.address); + +const { request: deployAliceAccountRequest } = await aliceWallet.deploy.simulate({ + artifact: SchnorrAccountContract.artifact, + deploymentOptions: aliceDeploymentOptions, + paymentMethod, + skipClassRegistration: true, + skipPublicDeployment: true, +}); + +await aliceWallet.deploy.send(deployAliceAccountRequest).wait(); + + +// Deploy BananaCoin as an instance of TokenContract + +const bananaCoinDeploymentOptions = { + constructorArtifact: 'constructor', + constructorArgs: { + admin: aliceWallet.getAddress(), + name: 'BananaCoin', + symbol: 'BC', + decimals: 18 + }, + salt: 43, + publicKeysHash: Fr.ZERO, + deployer: aliceWallet.getAddress() } -``` +const bananaCoinInstance = getContractInstanceFromDeployParams( + TokenContract.artifact, + bananaCoinDeploymentOptions +); +// simulate to get the gas estimation +const { request: deployTokenRequest } = await aliceWallet.deploy.simulate({ + artifact: TokenContract.artifact, + deploymentOptions: bananaCoinDeploymentOptions, + skipClassRegistration: false, // injects the capsule and function call below + skipPublicDeployment: false, // injects the function call below + // skipInitialization: false, assumed false since we specified the initializerFunction + paymentMethod +}) -## Implementation +// wallet.deploy does the following under the hood: + +// Use contract instances to prepare function calls +// const initializeBCFnCall = bananaCoinInstance.prepareCall({ +// functionName: 'constructor', +// args: bananaCoinDeploymentOptions.constructorArgs +// }); + +// await aliceWallet.registerContract({ +// artifact: TokenContract.artifact, +// instance: bananaCoinInstance +// }); +// note: if we are `simulating`, the contract will be transient and not actually registered + + +// prepare a capsule for contract class registration +// const encodedBytecode = bufferAsFields( +// bananaCoinClass.packedBytecode, +// MAX_PACKED_PUBLIC_BYTECODE_SIZE_IN_FIELDS +// ); + +// inject a function call to deploy the contract class, which will consume the capsule +// const tokenContractClass = getContractClassFromArtifact(TokenContract.artifact); +// const registerBCClassFnCall = getRegistererContract().prepareCall({ +// functionName: 'register', +// args: { +// artifact_hash: tokenContractClass.artifactHash, +// private_functions_root: tokenContractClass.privateFunctionsRoot, +// public_bytecode_commitment: tokenContractClass.publicBytecodeCommitment +// } +// }); + +// inject a function call to deploy the contract instance +// const deployBCInstanceFnCall = getDeployerContract().prepareCall({ +// functionName: 'deploy', +// args: { +// salt: bananaCoinInstance.salt, +// contract_class_id: bananaCoinInstance.contractClassId, +// initialization_hash: bananaCoinInstance.initializationHash, +// public_keys_hash: bananaCoinInstance.publicKeysHash, +// universal_deploy: bananaCoinInstance.deployer.isZero(), +// } +// }); + + + +const { contractInstance: bananaCoinInstance } = await aliceWallet.deploy.send(deployTokenRequest).wait(); + + + +// Now use the contract +const { result: privateBalance } = await aliceWallet.simulate({ + contractInstance: bananaCoinInstance, + functionName: 'balance_of_private' + args: {owner: aliceWallet.getAddress()}, +}); + +// Can also scope the instance to Alice's wallet +// But this requires the contract to be registered in the wallet +const aliceBananaCoinInstance = aliceWallet.useContractInstanceAt(BananaCoin, bananaCoinInstance.address); + +const {result: privateBalance} = await aliceBananaCoinInstance.simulate.balance_of_private({ + args: {owner: aliceWallet.getAddress()}, +}); + + +// Let's transfer the balance to Bob + +const bobAddress = AztecAddress.random(); + +// Gas estimation is done automatically and set on the request object +const { request: transferRequest } = await aliceWallet.simulate({ + contractInstance: bananaCoinInstance, + functionName: 'transfer', + args: { + from: aliceAddress, + to: bobAddress, + value: privateBalance, + nonce: 0n + }, + paymentMethod +}); + + +// Proving is done automatically since `request` wouldn't have it set after just simulation. +// Users can prove explicitly ahead of time with +// const { request: requestWithProof } = await aliceWallet.prove(request); +// await aliceWallet.send(requestWithProof).wait(); +await aliceWallet.send(transferRequest).wait(); + + + + +///////////////////////// +/// Over on Bob's PXE /// +///////////////////////// + +// somehow Bob gets the following: +const bananaCoinInstance: ContractInstanceWithAddress = { + version: 1, + contractClassId: getContractClassFromArtifact(TokenContract.artifact).id, + salt, // somehow bob gets this + initializationHash, // somehow bob gets this + publicKeysHash, // somehow bob gets this + address, // somehow bob gets this + deployer, // somehow bob gets this +}; + +pxe.registerContract({ + artifact: TokenContract.artifact, + instance: bananaCoinInstance, +}); + +const { result: bobPrivateBalance } = await bobWallet.simulate({ + contractInstance: bobBananaCoinInstance, + functionName: 'balance_of_private' + args: {owner: bobWallet.getAddress()}, +}); + + +// can also use the wallet to load a particular instance +const bobBananaCoinInstance = bobWallet.useContractInstanceAt(BananaCoin, bananaCoinInstance.address); + +const { result: bobPrivateBalance } = await bobBananaCoinInstance.simulate.balance_of_private({ + args: {owner: bobWallet.getAddress()}, +}); + +const { request: transferRequest } = await bobBananaCoinInstance.simulate.transfer({ + args: { + from: bobWallet.getAddress(), + to: aliceWallet.getAddress(), + value: bobPrivateBalance, + nonce: 0n + }, + paymentMethod +}); + +await bobBananaCoinInstance.send.transfer(transferRequest).wait(); +``` Delve into the specifics of the design. Include diagrams, code snippets, API descriptions, and database schema changes as necessary. Highlight any significant changes to the existing architecture or interfaces. From c75ee1f6cd8b417a9f0183c3125cbc7c8977919b Mon Sep 17 00:00:00 2001 From: Mitchell Tracy Date: Wed, 12 Jun 2024 17:06:37 -0400 Subject: [PATCH 3/9] flesh out implementation for simulate with gas estimation --- ...6973-refactor-base-contract-interaction.md | 289 +++++++++++++----- 1 file changed, 217 insertions(+), 72 deletions(-) diff --git a/in-progress/6973-refactor-base-contract-interaction.md b/in-progress/6973-refactor-base-contract-interaction.md index f3be529..a51ccf7 100644 --- a/in-progress/6973-refactor-base-contract-interaction.md +++ b/in-progress/6973-refactor-base-contract-interaction.md @@ -90,7 +90,7 @@ const { request: transferRequest } = await aliceWallet.simulate({ value: privateBalance, nonce: 0n }, - paymentMethod + paymentMethod, }); @@ -171,17 +171,6 @@ This would clear up concerns like: // derived ContractInteractions is confusing. We should unify the flow of all ContractInteractions. - - - - -Implementing this in the current API would be difficult - -The complexity in the current API is shown elsewhere - - -Further, it is presently difficult to estimate the gas cost of a transaction. - ### The old UML ```mermaid @@ -277,63 +266,60 @@ export interface AuthWitnessProvider { ): Promise; } -export interface EntrypointInterface { - /** - * Generates an execution request out of set of function calls. - * @param execution - The execution intents to be run. - * @returns The authenticated transaction execution request. - */ - createTxExecutionRequest(execution: ExecutionRequestInit): Promise; -} - export class TxExecutionRequest { constructor( - /** - * Sender. - */ + // All these are the same: public origin: AztecAddress, - /** - * Selector of the function to call. - */ public functionSelector: FunctionSelector, - /** - * The hash of arguments of first call to be executed (usually account entrypoint). - * @dev This hash is a pointer to `argsOfCalls` unordered array. - */ public firstCallArgsHash: Fr, - /** - * Transaction context. - */ public txContext: TxContext, - /** - * An unordered array of packed arguments for each call in the transaction. - * @dev These arguments are accessed in Noir via oracle and constrained against the args hash. The length of - * the array is equal to the number of function calls in the transaction (1 args per 1 call). - */ public argsOfCalls: PackedValues[], + public authWitnesses: AuthWitness[], + // Add: /** - * Transient authorization witnesses for authorizing the execution of one or more actions during this tx. - * These witnesses are not expected to be stored in the local witnesses database of the PXE. + * Transient capsules needed for this execution. */ - public authWitnesses: AuthWitness[], + public capsules: Fr[][], ) {} // ... } -export type ExecutionRequestInit = { - /** The function calls to be executed. */ - calls: FunctionCall[]; - /** Any transient auth witnesses needed for this execution */ - authWitnesses?: AuthWitness[]; - /** Any transient packed arguments for this execution */ - packedArguments?: PackedValues[]; - /** Any transient capsules needed for this execution */ - capsules?: Fr[][]; - /** The fee payment method to use */ - paymentMethod: FeePaymentMethod; - /** The gas settings */ - gasSettings: GasSettings; -}; +export type TxExecutionRequestEntrypoint = Pick + +export interface ExecutionRequestInit { + contractInstance: ContractInstanceWithAddress; + functionName: string; + args: any; + paymentMethod?: FeePaymentMethod; + contractArtifact?: ContractArtifact; + functionAbi?: FunctionAbi; + from?: AztecAddress; + simulatePublicFunctions?: boolean; +} + +export interface FeePaymentMethod { + getSetup(gasSettings: GasSettings): Promise<{ + functionCalls: FunctionCall[], + authWitnesses: AuthWitness[], + }>; + + getEquivalentAztBalance(): Promise; +} + + +export interface EntrypointInterface { + createTxExecutionRequestEntrypoint( + functionCalls: { + appFunctionCalls: FunctionCall[]; + setupFunctionCalls: FunctionCall[]; + }, + authWitnessProvider: AuthWitnessProvider + ): TxExecutionRequestEntrypoint; +} ``` @@ -377,32 +363,191 @@ Assume we're dealing with schnorr. Get the contract artifact out of the PXE using the contractInstance's contractClassId. -Scan its `FunctionAbi`s for the one with the given name. +```ts + +function findFunctionAbi(contractArtifact: ContractArtifact, functionName: string): FunctionAbi { + const functionAbi = contractArtifact.abi.find((abi) => abi.name === functionName); + if (!functionAbi) { + throw new Error(`Function ${functionName} not found in contract artifact`); + } + return functionAbi; +} + +function makeFunctionCall( + functionAbi: FunctionAbi, + instanceAddress: AztecAddress, + args: any, +): FunctionCall { + return FunctionCall.from({ + name: functionAbi.name, + args: mapArgsObjectToArray(functionAbi.parameters, args), + selector: FunctionSelector.fromNameAndParameters(functionAbi.name, functionAbi.parameters), + type: functionAbi.functionType, + to: instanceAddress, + isStatic: functionAbi.isStatic, + returnTypes: functionAbi.returnTypes, + }); +} + + +``` + +Note that since `gasSettings` was not provided, we need to determine them. + +In order to do this, we need to simulate the function call without the FPC to get the gas cost of the app logic, then simulate with the FPC to get the gas cost of the entire transaction. + +#### Get the `TxExecutionRequestEntrypoint` + +```ts +createTxExecutionRequestAccountEntrypoint( + functionCalls: { + appFunctionCalls: FunctionCall[]; + setupFunctionCalls: FunctionCall[]; + }, + authWitnessProvider: AuthWitnessProvider + ): TxExecutionRequestEntrypoint { + const appPayload = EntrypointPayload.fromFunctionCalls(functionCalls.appFunctionCalls); + const setupPayload = EntrypointPayload.fromFunctionCalls(functionCalls.setupFunctionCalls); + const abi = this.getEntrypointAbi(); + const entrypointPackedArgs = PackedValues.fromValues(encodeArguments(abi, [appPayload, setupPayload])); + const firstCallArgsHash = entrypointPackedArgs.hash(); + const argsOfCalls = [...appPayload.packedArguments, ...feePayload.packedArguments, entrypointPackedArgs]; + const functionSelector = FunctionSelector.fromNameAndParameters(abi.name, abi.parameters); + + // Does not insert into PXE + const appAuthWit = await authWitnessProvider.createAuthWit(appPayload.hash()); + const setupAuthWit = await authWitnessProvider.createAuthWit(setupPayload.hash()); + + return { + functionSelector, + firstCallArgsHash, + argsOfCalls, + authWitnesses: [appAuthWit, setupAuthWit], + } +} +``` + +#### Fill in the `TxExecutionRequest` -Construct the `FunctionCall` as: ```ts -{ - name: functionAbi.name, - args, - selector: FunctionSelector.fromNameAndParameters(functionAbi.name, functionAbi.parameters), - type: functionAbi.functionType, - to: contractInstance.address, - isStatic: functionAbi.isStatic, - returnTypes: functionAbi.returnTypes, +// within alice wallet. +// This is a low level call that requires us to have resolved the contract artifact and function abi. +// It is intentionally synchronous. +#getTxExecutionRequest(requestInit: ExecutionRequestInit) { + if (!requestInit.functionAbi) { + throw new Error('Function ABI must be provided'); + } + + const builder = new TxExecutionRequestBuilder(); + builder.setOrigin(this.getAddress()); + + builder.setTxContext({ + chainId: this.getChainId(), + version: this.getVersion(), + gasSettings: requestInit.gasSettings, + }); + + const setup = requestInit.paymentMethod.getSetup(requestInit.gasSettings); + + builder.addAuthWitnesses(setup.authWitnesses); + + // Could also allow users to pass the artifact and short-circuit this + const appFunctionCall = makeFunctionCall( + requestInit.functionAbi, + requestInit.contractInstance.address, + requestInit.args + ); + const entrypointInfo = createTxExecutionRequestAccountEntrypoint({ + appFunctionCalls: [appFunctionCall], + setupFunctionCalls: setup.functionCalls, + }, this.account); + + builder.setFunctionSelector(entrypointInfo.functionSelector); + builder.setFirstCallArgsHash(entrypointInfo.firstCallArgsHash); + builder.setArgsOfCalls(entrypointInfo.argsOfCalls); + builder.addAuthWitnesses(entrypointInfo.authWitnesses); + + return builder.build(); + } ``` +#### Define top-level `Simulate` + +```ts +// helpers somewhere -1. Extract the "app" EntrypointPayload +function decodeSimulatedTx(simulatedTx: SimulatedTx, functionAbi: FunctionAbi): DecodedReturn | [] { + const rawReturnValues = + functionAbi.functionType == FunctionType.PRIVATE + ? simulatedTx.privateReturnValues?.nested?.[0].values + : simulatedTx.publicOutput?.publicReturnValues?.[0].values; -2. Collect all authwits - 1. For the app payload - 2. For the fee payment - 3. For anything needed by the fee payment method -3. + return rawReturnValues ? decodeReturnValues(functionAbi.returnTypes, rawReturnValues) : []; +} + +// within alice wallet + +async #simulateInner(requestInit: ExecutionRequestInit): { + tx: SimulatedTx, + result: DecodedReturn | [], + request: ExecutionRequestInit, +} { + const txExecutionRequest = this.getTxExecutionRequest(initRequest); + // Call the PXE + const simulatedTx = await this.simulateTx(txExecutionRequest, builder.simulatePublicFunctions, builder.from); + const decodedReturn = decodeSimulatedTx(simulatedTx, builder.functionAbi); + return { + tx: simulatedTx, + result: decodedReturn, + request: initRequest, + }; +} + +async simulate(requestInit: ExecutionRequestInit): { + const builder = new ExecutionRequestInitBuilder(requestInit); + + if (!builder.functionAbi) { + const contractArtifact = builder.contractArtifact ?? await pxe.getContractArtifact(builder.contractInstance.contractClassId); + builder.setContractArtifact(contractArtifact); + const functionAbi = findFunctionAbi(builder.contractArtifact, builder.functionName); + builder.setFunctionAbi(functionAbi); + } + + // If we're not paying, e.g. this is just a read that we don't intend to submit as a TX, + // set the gas settings to default + if (!builder.paymentMethod){ + builder.setFeePaymentMethod(new NoFeePaymentMethod()); + builder.setGasSettings(GasSettings.default()); + return this.#simulateInner(builder.build()); + } + if (builder.gasSettings) { + return this.#simulateInner(builder.build()); + } + + // If we're paying, e.g. in bananas, figure out how much AZT that is. + // Note: this may call simulate recursively, + // but it *should* set the payment method to a NoFeePaymentMethod or undefined. + // perhaps we need a `read` method on the wallet. + const equivalentAztBalance = await builder.paymentMethod.getEquivalentAztBalance(); + gasEstimator = GasEstimator.fromAztBalance(equivalentAztBalance); + builder.setGasSettings(gasEstimator.proposeGasSettings()); + + while (!gasEstimator.isConverged()) { + const result = await this.#simulateInner(builder.build()); + gasEstimator.update(result); + builder.setGasSettings(gasEstimator.proposeGasSettings()); + } + + return result; + +} + + +``` From f3699a0890ccf0df7dc4b35a62c02e00b9e56aa6 Mon Sep 17 00:00:00 2001 From: Mitchell Tracy Date: Thu, 13 Jun 2024 15:21:11 -0400 Subject: [PATCH 4/9] draft design complete --- README.md | 1 + ...6973-refactor-base-contract-interaction.md | 920 +++++++++--------- 2 files changed, 448 insertions(+), 473 deletions(-) diff --git a/README.md b/README.md index 621096c..7dcf047 100644 --- a/README.md +++ b/README.md @@ -129,6 +129,7 @@ Fill in bullets for each area that will be affected by this change. - [ ] L1 Contracts - [ ] Prover - [ ] Economics +- [ ] Fees - [ ] P2P Network - [ ] DevOps diff --git a/in-progress/6973-refactor-base-contract-interaction.md b/in-progress/6973-refactor-base-contract-interaction.md index a51ccf7..5297bb7 100644 --- a/in-progress/6973-refactor-base-contract-interaction.md +++ b/in-progress/6973-refactor-base-contract-interaction.md @@ -1,18 +1,43 @@ -| | | -| -------------------- | --------------------------------- | -| Issue | [title](github.com/link/to/issue) | -| Owners | @you | -| Approvers | @alice @bob | -| Target Approval Date | YYYY-MM-DD | - +| | | +| -------------------- | --------------------------------------------------------------------------------------------- | +| Issue | [Refactor Contract Interactions](https://github.com/AztecProtocol/aztec-packages/issues/6973) | +| Owners | @just-mitch | +| Approvers | @PhilWindle @LHerskind | +| Target Approval Date | 2024-06-21 | ## Executive Summary -This is a refactor of the `BaseContractInteraction` class to simplify the API and improve the user (developer) experience. +This is a refactor of the API for interacting with contracts to improve the user (developer) experience, focused on aztec.js. The refactored approach mimics Viem's API, with some enhancements and modifications to fit our needs. +In a nutshell, by being more verbose in the API, we can remove a lot of complexity and make the code easier to understand and maintain; this also affords greater understanding and control over the lifecycle of their contracts and transactions. + +Key changes: +- the wallet is the central point of interaction to simulate/prove/send transactions instead of `BaseContractInteraction` +- unified api for deploying contracts, deploying accounts, and calling functions +- idempotent simulations +- enables more sophisticated gas estimation mechanism + +### Major Interface Update + +Remove `BaseContractInteraction` and its subclasses. + +Define the following: + +```ts +export interface UserAPI { + getTxExecutionRequest(userRequest: UserRequest): Promise; + simulate(userRequest: UserRequest): Promise; + read(userRequest: UserRequest): Promise; + prove(userRequest: UserRequest): Promise; + send(userRequest: UserRequest): Promise; +} + +export type Wallet = UserAPI & AccountInterface & PXE & AccountKeyRotationInterface; +``` + ### Account Creation ```ts @@ -22,36 +47,54 @@ const salt = 42; // Returns the wallet and deployment options. // Does not modify PXE state. -const { wallet: aliceWallet, deploymentOptions } = await getSchnorrWallet(pxe, secretKey, salt); +const { wallet: aliceWallet, deploymentArgs } = await getSchnorrWallet(pxe, secretKey, salt); + +const aliceContractInstance = getContractInstanceFromDeployParams( + SchnorrAccountContract.artifact, + deploymentArgs +); // Register the account in PXE, and wait for it to sync. await aliceWallet.registerAccountInPXE({ sync: true }); -// No wallet or secret passed to FPC -const paymentMethod = new PrivateFeePaymentMethod(someCoin.address, someFPC.address); +const paymentMethod = new SomeFeePaymentMethod( + someCoin.address, + someFPC.address, + // the fee payment method may need a wallet, + // e.g. for reading the balance or producing authwits + aliceWallet +); // Changes to the PXE (e.g. notes, nullifiers, auth wits, contract deployments, capsules) are not persisted. -const { request: deployAliceAccountRequest } = await aliceWallet.deploy.simulate({ +const { request: deployAliceAccountRequest } = await aliceWallet.simulate({ artifact: SchnorrAccountContract.artifact, - deploymentOptions, + instance: aliceContractInstance, + functionName: deploymentArgs.constructorName, + args: deploymentArgs.constructorArgs, paymentMethod, - skipClassRegistration: true, - skipPublicDeployment: true, - skipInitialization: false, // gasSettings: undefined => automatic gas estimation. the returned `request` will have the gasSettings set. }); -const { contractInstance } = await aliceWallet.deploy.send(deployAliceAccountRequest).wait(); +// Wallet can stop here to prompt the user to confirm the gas estimation. + +const deployAliceAccountRequestWithProof = await aliceWallet.prove(deployAliceAccountRequest); + +const sentTx = await aliceWallet.send(deployAliceAccountRequestWithProof) + +const receipt = await sentTx.wait({ + // permanently add the transient contracts to the wallet if the tx is successful. + // defaults to true. + registerOnSuccess: true +}); -debugLogger(`Alice's account deployed at ${contractInstance.address.toString()}`); ``` -### Deploy and Use Contract +### Deploy Token ```ts -const bananaCoinDeploymentOptions = { - constructorArtifact: 'constructor', +const bananaCoinDeploymentArgs: InstanceDeploymentParams = { + constructorName: 'constructor', constructorArgs: { admin: aliceWallet.getAddress(), name: 'BananaCoin', @@ -63,18 +106,33 @@ const bananaCoinDeploymentOptions = { deployer: aliceWallet.getAddress() } -const { request: deployTokenRequest } = await aliceWallet.deploy.simulate({ +const bananaCoinInstance = getContractInstanceFromDeployParams( + TokenContract.artifact, + bananaCoinDeploymentArgs +); + +const { request: deployTokenRequest } = await aliceWallet.simulate({ artifact: TokenContract.artifact, - deploymentOptions: bananaCoinDeploymentOptions, - skipClassRegistration: false, - skipPublicDeployment: false, - skipInitialization: false, + instance: bananaCoinInstance, + functionName: bananaCoinDeploymentArgs.constructorName, + args: bananaCoinDeploymentArgs.constructorArgs, + deploymentOptions: { + registerClass: true, + publicDeploy: true, + }, paymentMethod }) -const { contractInstance: bananaCoinInstance } = await aliceWallet.deploy.send(deployTokenRequest).wait(); +const deployTokenRequestWithProof = await aliceWallet.prove(deployTokenRequest); +const sentTx = await aliceWallet.send(deployTokenRequestWithProof) +const receipt = await sentTx.wait() + +``` + +### Use Token -const { result: privateBalance } = await aliceWallet.simulate({ +```ts +const { result: privateBalance } = await aliceWallet.read({ contractInstance: bananaCoinInstance, functionName: 'balance_of_private' args: {owner: aliceWallet.getAddress()}, @@ -93,33 +151,17 @@ const { request: transferRequest } = await aliceWallet.simulate({ paymentMethod, }); - -// Proving is done automatically since `request` wouldn't have it set after just simulation. -// Users can prove explicitly ahead of time with -// const { request: requestWithProof } = await aliceWallet.prove(request); -// await aliceWallet.send(requestWithProof).wait(); -await aliceWallet.send(transferRequest).wait(); +const transferRequestWithProof = await aliceWallet.prove(transferRequest); +const sentTx = await aliceWallet.send(transferRequestWithProof) +const receipt = await sentTx.wait() ``` -### Scoping Instances - -A bit of a "nice to have". - -```ts -const aliceBananaCoinInstance = aliceWallet.useContractInstanceAt(TokenContract.artifact, bananaCoinInstance.address); -const {result: privateBalance} = await aliceBananaCoinInstance.simulate.balance_of_private({ - args: {owner: aliceWallet.getAddress()}, -}); -``` - - ## Introduction Developers and users have to think too hard when deploying accounts and submitting transactions. -This is due in part to holding mutable state in the `BaseContractInteraction` class, which is a base class for all contract interactions: -it has multiple subclasses that mutate the state, so it is hard to know what has been set. +This is due in part to holding mutable state in the `BaseContractInteraction` class, which is a base class for all contract interactions; it has multiple subclasses that mutate the state, so it is hard to know what has been set. For example, the current attempt to estimate gas has a section that reads: ```ts @@ -136,8 +178,6 @@ For example, the current attempt to estimate gas has a section that reads: this.txRequest = undefined; ``` -This push for a refactor was motivated by the realization that we need a more sophisticated gas estimation mechanism where we first simulate without the FPC to get the gas cost of app logic, then plug that into a simulation with the FPC. - Doing this well also requires that simulations to be idempotent: all changes to the PXE including notes, nullifiers, auth wits, contract deployments, capsules, etc. should not be persisted until the user explicitly sends the transaction. This would fix comments seen in the `DeployMethod` class like: @@ -171,101 +211,34 @@ This would clear up concerns like: // derived ContractInteractions is confusing. We should unify the flow of all ContractInteractions. -### The old UML - -```mermaid -classDiagram - -class BaseContractInteraction { - Tx - TxExecutionRequest - Wallet -} - -class BatchCall { - FunctionCall[] -} -BatchCall -->BaseContractInteraction - -class ContractFunctionInteraction { - AztecAddress contractAddress - FunctionAbi - any[] args -} -ContractFunctionInteraction --> BaseContractInteraction - -class DeployMethod { - Fr publicKeysHash - ContractArtifact - PostDeployCtor - any[] args - string|FunctionArtifact constructorNameOrArtifact -} -DeployMethod --> BaseContractInteraction - -class DeployAccountMethod { - AuthWitnessProvider - FunctionArtifact feePaymentArtifact -} -DeployAccountMethod --> DeployMethod - - -``` - - - - ## Interface The key operations we want to perform are: -- Producing a valid Tx Request -- Simulating a Tx Request -- Estimating the gas cost of a Tx Request -- Proving a Tx Request -- Submitting a Tx Request +- Simulating/Reading +- Proving +- Submitting These operations should be accessible from a `Wallet`, which will generally be `Schnorr` or `Signerless`. This is analogous to `viem` having a `walletClient` and a `publicClient`. +The main idea in this design is to have the BaseWallet define all the functionality to perform the key operations; the specific entrypoint in use will be able to adapt the `TxExecutionRequest`s produced to its needs. -### What is a Wallet? +To accomplish this, we have the BaseWallet construct a `TxExecutionRequestBuilder` that can be passed to various `TxExecutionRequestAdapters`, such as one for the account entrypoint, one for the fee payment method, etc. -```ts -export type Wallet = AccountInterface & PXE & AccountKeyRotationInterface; +Further, we expand `TxExecutionRequest` to allow passing things that we were previously setting directly on the PXE, so simulations can be idempotent. -export interface AccountInterface extends AuthWitnessProvider, EntrypointInterface { - /** Returns the complete address for this account. */ - getCompleteAddress(): CompleteAddress; +The interfaces are as follows: - /** Returns the address for this account. */ - getAddress(): AztecAddress; - - /** Returns the chain id for this account */ - getChainId(): Fr; - - /** Returns the rollup version for this account */ - getVersion(): Fr; -} +```ts -export interface AuthWitnessProvider { - createAuthWit( - messageHashOrIntent: - | Fr - | Buffer - | { - /** The caller to approve */ - caller: AztecAddress; - /** The action to approve */ - action: ContractFunctionInteraction | FunctionCall; - /** The chain id to approve */ - chainId?: Fr; - /** The version to approve */ - version?: Fr; - }, - ): Promise; +// new +export interface ArtifactAndInstance { + artifact: ContractArtifact; + instance: ContractInstanceWithAddress; } +// modified export class TxExecutionRequest { constructor( // All these are the same: @@ -280,45 +253,96 @@ export class TxExecutionRequest { * Transient capsules needed for this execution. */ public capsules: Fr[][], + /** + * Transient contracts needed for this execution. + */ + public transientContracts: ArtifactAndInstance[], ) {} // ... } -export type TxExecutionRequestEntrypoint = Pick +// new +export interface InstanceDeploymentParams { + constructorName: string; + constructorArgs: any; + salt: Fr; + publicKeysHash: Fr; + deployer: AztecAddress; +} + +// new +export interface DeploymentOptions { + registerClass?: boolean; + publicDeploy?: boolean; +} -export interface ExecutionRequestInit { +// new +export interface UserRequest { contractInstance: ContractInstanceWithAddress; functionName: string; args: any; + deploymentOptions?: DeploymentOptions; + gasSettings?: GasSettings; paymentMethod?: FeePaymentMethod; contractArtifact?: ContractArtifact; functionAbi?: FunctionAbi; from?: AztecAddress; simulatePublicFunctions?: boolean; + executionResult?: ExecutionResult; // the raw output of a simulation that can be proven + tx?: Tx; // a proven tx to send } -export interface FeePaymentMethod { - getSetup(gasSettings: GasSettings): Promise<{ - functionCalls: FunctionCall[], - authWitnesses: AuthWitness[], - }>; +export type TxExecutionRequestAdapter = (builder: TxExecutionRequestBuilder, userRequest: UserRequest) => Promise; +export interface TxExecutionRequestComponent { + adaptTxExecutionRequest: TxExecutionRequestAdapter; +} + +export interface FeePaymentMethod extends TxExecutionRequestComponent { getEquivalentAztBalance(): Promise; } +export interface SimulationOutput { + tx: Tx; + result: DecodedReturn | []; + request: UserRequest; + executionResult: ExecutionResult; + publicOutput: PublicSimulationOutput; + privateOutput: NestedProcessReturnValues; +} -export interface EntrypointInterface { - createTxExecutionRequestEntrypoint( - functionCalls: { - appFunctionCalls: FunctionCall[]; - setupFunctionCalls: FunctionCall[]; - }, - authWitnessProvider: AuthWitnessProvider - ): TxExecutionRequestEntrypoint; +export interface UserAPI { + getTxExecutionRequest(userRequest: UserRequest): Promise; + simulate(userRequest: UserRequest): Promise; + read(userRequest: UserRequest): Promise; + prove(userRequest: UserRequest): Promise; + send(userRequest: UserRequest): Promise; +} + +export type Wallet = UserAPI & AccountInterface & PXE & AccountKeyRotationInterface; + +// Swap `EntrypointInterface` for `TxExecutionRequestComponent` +export interface AccountInterface extends AuthWitnessProvider, TxExecutionRequestComponent { + getCompleteAddress(): CompleteAddress; + getAddress(): AztecAddress; + getChainId(): Fr; + getVersion(): Fr; + +} + +// unchanged +export interface AuthWitnessProvider { + createAuthWit( + messageHashOrIntent: + | Fr + | Buffer + | { + caller: AztecAddress; + action: ContractFunctionInteraction | FunctionCall; + chainId?: Fr; + version?: Fr; + }, + ): Promise; } ``` @@ -326,21 +350,27 @@ export interface EntrypointInterface { ## Implementation +### Expose an explicit `proveTx` on PXE + +We currently don't have a way to *only* prove a transaction; everything must be simulated first. + +So we will expose a `proveTx` method on the PXE that will take a `TxExecutionRequest` and an `ExecutionResult` and return a `Tx`. + ### Drop `isFeePayer` from `FeeEntrypointPayload` -I think removing this makes the code simpler and more flexible: account contracts can just check if fee payer has been set and set it if not. +Removing this makes the code simpler and more flexible: account contracts can just check if fee payer has been set and set it if not. Concerned users can just inspect the simulation result to see if they are paying the fee. Then we just have `EntrypointPayload` as a single, concrete class. -### Translating user requests to TxExecutionRequests +### BaseWallet.getTxExecutionRequest -Consider that we have +Consider that we have, e.g.: ```ts -const { request: transferRequest } = await aliceWallet.simulate({ +{ contractInstance: bananaCoinInstance, functionName: 'transfer', args: { @@ -350,18 +380,14 @@ const { request: transferRequest } = await aliceWallet.simulate({ nonce: 0n }, paymentMethod -}); +} ``` We need to ultimately get to a `TxExecutionRequest`. -There are a few key steps to get there. - -Assume we're dealing with schnorr. +#### Translate a `contractInstance`, `functionName` and `args` to a `FunctionCall` -#### Translate the `contractInstance`, `functionName` and `args` to a `FunctionCall` - -Get the contract artifact out of the PXE using the contractInstance's contractClassId. +Define helpers somewhere as: ```ts @@ -389,149 +415,231 @@ function makeFunctionCall( }); } +``` + +#### class registration + +Define a helper somewhere as: +```ts +export const addClassRegistration: TxExecutionRequestAdapter = ( + builder: TxExecutionRequestBuilder, request: UserRequest +) => { + if (!request.contractArtifact) { + throw new Error('Contract artifact must be provided to register class'); + } + + const contractClass = getContractClassFromArtifact(request.contractArtifact); + + builder.addCapsule( + bufferAsFields( + contractClass.packedBytecode, + MAX_PACKED_PUBLIC_BYTECODE_SIZE_IN_FIELDS + )); + + const { artifact, instance } = getCanonicalClassRegisterer(); + + const registerFnAbi = findFunctionAbi(artifact, 'register'); + + builder.addAppFunctionCall( + makeFunctionCall( + registerFnAbi, + instance.address, + { + artifact_hash: contractClass.artifactHash, + private_functions_root: contractClass.privateFunctionsRoot, + public_bytecode_commitment: contractClass.publicBytecodeCommitment + } + )); +} ``` -Note that since `gasSettings` was not provided, we need to determine them. +#### public deployment -In order to do this, we need to simulate the function call without the FPC to get the gas cost of the app logic, then simulate with the FPC to get the gas cost of the entire transaction. +Define a helper somewhere as -#### Get the `TxExecutionRequestEntrypoint` +```ts + +export const addPublicDeployment: TxExecutionRequestAdapter = ( + builder: TxExecutionRequestBuilder, request: UserRequest +) => { + const { artifact, instance } = getCanonicalInstanceDeployer(); + const deployFnAbi = findFunctionAbi(artifact, 'deploy'); + builder.addAppFunctionCall( + makeFunctionCall( + deployFnAbi, + instance.address, + { + salt: request.contractInstance.salt, + contract_class_id: request.contractInstance.contractClassId, + initialization_hash: request.contractInstance.initializationHash, + public_keys_hash: request.contractInstance.publicKeysHash, + universal_deploy: request.contractInstance.deployer.isZero(), + } + )); +} +``` + +#### Entrypoints implement `TxExecutionRequestComponent` + +Below is what a default account entrypoint might look like, but other entrypoints could implement this differently ```ts -createTxExecutionRequestAccountEntrypoint( - functionCalls: { - appFunctionCalls: FunctionCall[]; - setupFunctionCalls: FunctionCall[]; - }, - authWitnessProvider: AuthWitnessProvider - ): TxExecutionRequestEntrypoint { - const appPayload = EntrypointPayload.fromFunctionCalls(functionCalls.appFunctionCalls); - const setupPayload = EntrypointPayload.fromFunctionCalls(functionCalls.setupFunctionCalls); - const abi = this.getEntrypointAbi(); - const entrypointPackedArgs = PackedValues.fromValues(encodeArguments(abi, [appPayload, setupPayload])); - const firstCallArgsHash = entrypointPackedArgs.hash(); - const argsOfCalls = [...appPayload.packedArguments, ...feePayload.packedArguments, entrypointPackedArgs]; - const functionSelector = FunctionSelector.fromNameAndParameters(abi.name, abi.parameters); - - // Does not insert into PXE - const appAuthWit = await authWitnessProvider.createAuthWit(appPayload.hash()); - const setupAuthWit = await authWitnessProvider.createAuthWit(setupPayload.hash()); - return { - functionSelector, - firstCallArgsHash, - argsOfCalls, - authWitnesses: [appAuthWit, setupAuthWit], +export class DefaultAccountEntrypoint implements TxExecutionRequestComponent { + constructor( + private address: AztecAddress, + private auth: AuthWitnessProvider, + private chainId: number = DEFAULT_CHAIN_ID, + private version: number = DEFAULT_VERSION, + ) {} + + async adaptTxExecutionRequest( + builder: TxExecutionRequestBuilder, + userRequest: UserRequest + ): Promise { + const appPayload = EntrypointPayload.fromFunctionCalls(builder.appFunctionCalls); + const setupPayload = EntrypointPayload.fromFunctionCalls(builder.setupFunctionCalls); + const abi = this.getEntrypointAbi(); + const entrypointPackedArgs = PackedValues.fromValues(encodeArguments(abi, [appPayload, setupPayload])); + + return builder + .setOrigin(this.address) + .setTxContext({ + chainId: this.chainId, + version: this.version, + gasSettings: userRequest.gasSettings, + }) + .setFunctionSelector( + FunctionSelector.fromNameAndParameters(abi.name, abi.parameters) + ) + .setFirstCallArgsHash(entrypointPackedArgs.hash()); + .setArgsOfCalls([ + ...appPayload.packedArguments, + ...feePayload.packedArguments, + entrypointPackedArgs + ]) + .addAuthWitness( + await this.auth.createAuthWit(appPayload.hash()) + ) + .addAuthWitness( + await this.auth.createAuthWit(setupPayload.hash()) + ) + } + + private getEntrypointAbi() { + return { + name: 'entrypoint', + isInitializer: false, + // ... same as before + } } } + ``` #### Fill in the `TxExecutionRequest` +The abstract `BaseWallet` can implement: + ```ts -// within alice wallet. -// This is a low level call that requires us to have resolved the contract artifact and function abi. -// It is intentionally synchronous. -#getTxExecutionRequest(requestInit: ExecutionRequestInit) { - if (!requestInit.functionAbi) { +async getTxExecutionRequest(userRequest: UserRequest): Promise { + if (!userRequest.functionAbi) { throw new Error('Function ABI must be provided'); } + if (!userRequest.gasSettings) { + throw new Error('Gas settings must be provided'); + } + if (!userRequest.paymentMethod) { + throw new Error('Payment method must be provided'); + } const builder = new TxExecutionRequestBuilder(); - builder.setOrigin(this.getAddress()); - builder.setTxContext({ - chainId: this.getChainId(), - version: this.getVersion(), - gasSettings: requestInit.gasSettings, - }); + // Add the "main" function call + builder.addAppFunctionCall( + makeFunctionCall( + userRequest.functionAbi, + userRequest.contractInstance.address, + userRequest.args + )); - const setup = requestInit.paymentMethod.getSetup(requestInit.gasSettings); + // Add stuff needed for setup, e.g. function calls, auth witnesses, etc. + await userRequest.paymentMethod.adaptTxExecutionRequest(builder, userRequest); - builder.addAuthWitnesses(setup.authWitnesses); + if (userRequest.deploymentOptions?.registerClass) { + addClassRegistration(builder, userRequest); + } - // Could also allow users to pass the artifact and short-circuit this - const appFunctionCall = makeFunctionCall( - requestInit.functionAbi, - requestInit.contractInstance.address, - requestInit.args - ); - const entrypointInfo = createTxExecutionRequestAccountEntrypoint({ - appFunctionCalls: [appFunctionCall], - setupFunctionCalls: setup.functionCalls, - }, this.account); + if (userRequest.deploymentOptions?.publicDeploy) { + addPublicDeployment(builder, userRequest); + } + + // if the user is giving us an artifact, + // allow the PXE to access it + if (userRequest.contractArtifact) { + builder.addTransientContract({ + artifact: userRequest.contractArtifact, + instance: userRequest.contractInstance, + }); + } - builder.setFunctionSelector(entrypointInfo.functionSelector); - builder.setFirstCallArgsHash(entrypointInfo.firstCallArgsHash); - builder.setArgsOfCalls(entrypointInfo.argsOfCalls); - builder.addAuthWitnesses(entrypointInfo.authWitnesses); + // Adapt the request to the entrypoint in use. + // Since BaseWallet is abstract, this will be implemented by the concrete class. + this.adaptTxExecutionRequest(builder, userRequest); return builder.build(); } ``` - - - -#### Define top-level `Simulate` +### BaseWallet.#simulateInner ```ts -// helpers somewhere - -function decodeSimulatedTx(simulatedTx: SimulatedTx, functionAbi: FunctionAbi): DecodedReturn | [] { - const rawReturnValues = - functionAbi.functionType == FunctionType.PRIVATE - ? simulatedTx.privateReturnValues?.nested?.[0].values - : simulatedTx.publicOutput?.publicReturnValues?.[0].values; - - return rawReturnValues ? decodeReturnValues(functionAbi.returnTypes, rawReturnValues) : []; -} - -// within alice wallet - -async #simulateInner(requestInit: ExecutionRequestInit): { - tx: SimulatedTx, - result: DecodedReturn | [], - request: ExecutionRequestInit, -} { - const txExecutionRequest = this.getTxExecutionRequest(initRequest); - // Call the PXE +// Used by simulate and read +async #simulateInner(userRequest: UserRequest): ReturnType { + const txExecutionRequest = await this.getTxExecutionRequest(initRequest); const simulatedTx = await this.simulateTx(txExecutionRequest, builder.simulatePublicFunctions, builder.from); const decodedReturn = decodeSimulatedTx(simulatedTx, builder.functionAbi); return { - tx: simulatedTx, + tx: simulatedTx.tx, + publicOutput: simulatedTx.publicOutput, + privateOutput: simulatedTx.privateReturnValues, + executionResult: simulatedTx.executionResult, result: decodedReturn, request: initRequest, }; } +``` -async simulate(requestInit: ExecutionRequestInit): { - const builder = new ExecutionRequestInitBuilder(requestInit); +### BaseWallet.simulate - if (!builder.functionAbi) { - const contractArtifact = builder.contractArtifact ?? await pxe.getContractArtifact(builder.contractInstance.contractClassId); - builder.setContractArtifact(contractArtifact); - const functionAbi = findFunctionAbi(builder.contractArtifact, builder.functionName); - builder.setFunctionAbi(functionAbi); - } +```ts - // If we're not paying, e.g. this is just a read that we don't intend to submit as a TX, - // set the gas settings to default - if (!builder.paymentMethod){ - builder.setFeePaymentMethod(new NoFeePaymentMethod()); - builder.setGasSettings(GasSettings.default()); - return this.#simulateInner(builder.build()); +async simulate(userRequest: UserRequest): { + tx: simulatedTx.tx, + publicOutput: simulatedTx.publicOutput, + privateOutput: simulatedTx.privateReturnValues, + executionResult: simulatedTx.executionResult, + result: decodedReturn, + request: initRequest, +} { + // If we're simulating, we need to have the payment method set. + // Users should use `read` if they just want to see the result. + if (!userRequest.paymentMethod){ + throw new Error('Payment method must be set before simulating'); } + + const builder = new UserRequestBuilder(userRequest); + + await this.#ensureFunctionAbi(builder); + if (builder.gasSettings) { return this.#simulateInner(builder.build()); } // If we're paying, e.g. in bananas, figure out how much AZT that is. - // Note: this may call simulate recursively, - // but it *should* set the payment method to a NoFeePaymentMethod or undefined. - // perhaps we need a `read` method on the wallet. + // Note: paymentMethod.getEquivalentAztBalance() may call `read` internally. const equivalentAztBalance = await builder.paymentMethod.getEquivalentAztBalance(); gasEstimator = GasEstimator.fromAztBalance(equivalentAztBalance); builder.setGasSettings(gasEstimator.proposeGasSettings()); @@ -543,251 +651,117 @@ async simulate(requestInit: ExecutionRequestInit): { } return result; - } +async #ensureFunctionAbi(builder: UserRequestBuilder): void { + // User can call simulate without the artifact if they have the function ABI + if (!builder.functionAbi) { + // If the user provides the contract artifact, we don't need to ask the PXE + if (!builder.contractArtifact) { + const contractArtifact = await this.getContractArtifact(builder.contractInstance.contractClassId); + builder.setContractArtifact(contractArtifact); + } + const functionAbi = findFunctionAbi(builder.contractArtifact, builder.functionName); + builder.setFunctionAbi(functionAbi); + } +} -``` +// helpers somewhere +function decodeSimulatedTx(simulatedTx: SimulatedTx, functionAbi: FunctionAbi): DecodedReturn | [] { + const rawReturnValues = + functionAbi.functionType == FunctionType.PRIVATE + ? simulatedTx.privateReturnValues?.nested?.[0].values + : simulatedTx.publicOutput?.publicReturnValues?.[0].values; + return rawReturnValues ? decodeReturnValues(functionAbi.returnTypes, rawReturnValues) : []; +} +``` +### BaseWallet.read -### Kitchen Sink +Like `simulate`, but without the gas estimation. ```ts +async read(userRequest: UserRequest): DecodedReturn | [] { + const builder = new UserRequestBuilder(userRequest); -// Create a new schnorr account -const aliceSecretKey = Fr.random(), + if (!builder.paymentMethod) { + builder.setFeePaymentMethod(new NoFeePaymentMethod()); + } -const signingPrivateKey = deriveSigningKey(aliceSecretKey); -const signingPublicKey = new Schnorr().computePublicKey(signingPrivateKey); + if (!builder.gasSettings) { + builder.setGasSettings(GasSettings.default()); + } + await this.#ensureFunctionAbi(builder); -const aliceDeploymentOptions = { - constructorArtifact: 'constructor', - constructorArgs: { - signing_pub_key_x: signingPublicKey.x, - signing_pub_key_y: signingPublicKey.y - }, - salt: 42, - publicKeysHash: deriveKeys(aliceSecretKey).publicKeys.hash(), - deployer: AztecAddress.ZERO + return this.#simulateInner(builder.build()); } - -const aliceContractInstance = getContractInstanceFromDeployParams( - SchnorrAccountContract.artifact, - aliceDeploymentOptions -); - -const aliceCompleteAddress = CompleteAddress.fromSecretKeyAndInstance(aliceSecretKey, aliceContractInstance); - -await pxe.registerAccount( - aliceSecretKey, - aliceCompleteAddress.partialAddress -); -await waitForAccountSynch(pxe, aliceCompleteAddress); - -const aliceAuthWitProvider = new SchnorrAuthWitnessProvider(signingPrivateKey) -const nodeInfo = await pxe.getNodeInfo(); -const aliceInterface = new DefaultAccountInterface(aliceAuthWitProvider, aliceCompleteAddress, nodeInfo) -const aliceWallet = new AccountWallet(pxe, aliceInterface); - - -// Everything above could be rolled into a single function call: -// const wallet = await getSchnorrAccount( -// pxe, -// alice.secretKey, -// deriveSigningKey(alice.secretKey), -// aliceDeploymentOptions.salt -// ).getWallet(); - -const paymentMethod = new PrivateFeePaymentMethod(someCoin.address, someFPC.address); - -const { request: deployAliceAccountRequest } = await aliceWallet.deploy.simulate({ - artifact: SchnorrAccountContract.artifact, - deploymentOptions: aliceDeploymentOptions, - paymentMethod, - skipClassRegistration: true, - skipPublicDeployment: true, -}); - -await aliceWallet.deploy.send(deployAliceAccountRequest).wait(); +``` -// Deploy BananaCoin as an instance of TokenContract +### BaseWallet.prove -const bananaCoinDeploymentOptions = { - constructorArtifact: 'constructor', - constructorArgs: { - admin: aliceWallet.getAddress(), - name: 'BananaCoin', - symbol: 'BC', - decimals: 18 - }, - salt: 43, - publicKeysHash: Fr.ZERO, - deployer: aliceWallet.getAddress() +```ts +async prove(request: UserRequest): Promise { + if (!request.executionResult) { + throw new Error('Execution result must be set before proving'); + } + const builder = new UserRequestBuilder(request); + await this.#ensureFunctionAbi(builder); + const initRequest = builder.build(); + const txExecutionRequest = await this.getTxExecutionRequest(initRequest); + const provenTx = await this.proveTx(txExecutionRequest, request.executionResult); + builder.setTx(provenTx); + return builder.build(); } +``` -const bananaCoinInstance = getContractInstanceFromDeployParams( - TokenContract.artifact, - bananaCoinDeploymentOptions -); - -// simulate to get the gas estimation -const { request: deployTokenRequest } = await aliceWallet.deploy.simulate({ - artifact: TokenContract.artifact, - deploymentOptions: bananaCoinDeploymentOptions, - skipClassRegistration: false, // injects the capsule and function call below - skipPublicDeployment: false, // injects the function call below - // skipInitialization: false, assumed false since we specified the initializerFunction - paymentMethod -}) - -// wallet.deploy does the following under the hood: - -// Use contract instances to prepare function calls -// const initializeBCFnCall = bananaCoinInstance.prepareCall({ -// functionName: 'constructor', -// args: bananaCoinDeploymentOptions.constructorArgs -// }); - -// await aliceWallet.registerContract({ -// artifact: TokenContract.artifact, -// instance: bananaCoinInstance -// }); -// note: if we are `simulating`, the contract will be transient and not actually registered - - -// prepare a capsule for contract class registration -// const encodedBytecode = bufferAsFields( -// bananaCoinClass.packedBytecode, -// MAX_PACKED_PUBLIC_BYTECODE_SIZE_IN_FIELDS -// ); - -// inject a function call to deploy the contract class, which will consume the capsule -// const tokenContractClass = getContractClassFromArtifact(TokenContract.artifact); -// const registerBCClassFnCall = getRegistererContract().prepareCall({ -// functionName: 'register', -// args: { -// artifact_hash: tokenContractClass.artifactHash, -// private_functions_root: tokenContractClass.privateFunctionsRoot, -// public_bytecode_commitment: tokenContractClass.publicBytecodeCommitment -// } -// }); - -// inject a function call to deploy the contract instance -// const deployBCInstanceFnCall = getDeployerContract().prepareCall({ -// functionName: 'deploy', -// args: { -// salt: bananaCoinInstance.salt, -// contract_class_id: bananaCoinInstance.contractClassId, -// initialization_hash: bananaCoinInstance.initializationHash, -// public_keys_hash: bananaCoinInstance.publicKeysHash, -// universal_deploy: bananaCoinInstance.deployer.isZero(), -// } -// }); - - - -const { contractInstance: bananaCoinInstance } = await aliceWallet.deploy.send(deployTokenRequest).wait(); - - - -// Now use the contract -const { result: privateBalance } = await aliceWallet.simulate({ - contractInstance: bananaCoinInstance, - functionName: 'balance_of_private' - args: {owner: aliceWallet.getAddress()}, -}); - -// Can also scope the instance to Alice's wallet -// But this requires the contract to be registered in the wallet -const aliceBananaCoinInstance = aliceWallet.useContractInstanceAt(BananaCoin, bananaCoinInstance.address); - -const {result: privateBalance} = await aliceBananaCoinInstance.simulate.balance_of_private({ - args: {owner: aliceWallet.getAddress()}, -}); - - -// Let's transfer the balance to Bob - -const bobAddress = AztecAddress.random(); - -// Gas estimation is done automatically and set on the request object -const { request: transferRequest } = await aliceWallet.simulate({ - contractInstance: bananaCoinInstance, - functionName: 'transfer', - args: { - from: aliceAddress, - to: bobAddress, - value: privateBalance, - nonce: 0n - }, - paymentMethod -}); - - -// Proving is done automatically since `request` wouldn't have it set after just simulation. -// Users can prove explicitly ahead of time with -// const { request: requestWithProof } = await aliceWallet.prove(request); -// await aliceWallet.send(requestWithProof).wait(); -await aliceWallet.send(transferRequest).wait(); - - +### BaseWallet.send +```ts +async send(request: UserRequest): Promise { + if (!request.tx) { + throw new Error('Tx must be set before sending'); + } + if (!request.tx.proof || request.tx.proof.isEmpty()) { + throw new Error('Tx must be proven before sending'); + } + const builder = new UserRequestBuilder(request); + await this.#ensureFunctionAbi(builder); + const initRequest = builder.build(); + const txExecutionRequest = await this.getTxExecutionRequest(); + const txHash = await this.sendTx(txExecutionRequest, request.tx); + return new SentTx(this.pxe, txHash, txExecutionRequest); +} +``` -///////////////////////// -/// Over on Bob's PXE /// -///////////////////////// +### Concerns -// somehow Bob gets the following: -const bananaCoinInstance: ContractInstanceWithAddress = { - version: 1, - contractClassId: getContractClassFromArtifact(TokenContract.artifact).id, - salt, // somehow bob gets this - initializationHash, // somehow bob gets this - publicKeysHash, // somehow bob gets this - address, // somehow bob gets this - deployer, // somehow bob gets this -}; +#### `UserRequest` is a kitchen sink -pxe.registerContract({ - artifact: TokenContract.artifact, - instance: bananaCoinInstance, -}); +The `UserRequest` object is a bit of a kitchen sink. It might be better to have a `DeployRequest`, `CallRequest`, etc. that extends `UserRequest`. -const { result: bobPrivateBalance } = await bobWallet.simulate({ - contractInstance: bobBananaCoinInstance, - functionName: 'balance_of_private' - args: {owner: bobWallet.getAddress()}, -}); +#### Just shifting the mutable subclass problem +Arguably the builder + adapter pattern just shifts the "mutable subclass" problem around. I think that since the entire lifecycle of the builder is contained to the `getTxExecutionRequest` method within a single abstract class, it's not nearly as bad as the current situation. -// can also use the wallet to load a particular instance -const bobBananaCoinInstance = bobWallet.useContractInstanceAt(BananaCoin, bananaCoinInstance.address); +#### Verbosity + loss of convenience -const { result: bobPrivateBalance } = await bobBananaCoinInstance.simulate.balance_of_private({ - args: {owner: bobWallet.getAddress()}, -}); +People might be concerned that we lose the ability to do: +```ts +const bananaCoin = await BananaCoin.at(bananaCoinAddress, this.aliceWallet); +bananaCoin.methods.mint_public(this.aliceAddress, this.ALICE_INITIAL_BANANAS).send().wait() +``` -const { request: transferRequest } = await bobBananaCoinInstance.simulate.transfer({ - args: { - from: bobWallet.getAddress(), - to: aliceWallet.getAddress(), - value: bobPrivateBalance, - nonce: 0n - }, - paymentMethod -}); +I think this is a good thing. It's not clear what `mint_public` does (which is create a stateful `ContractFunctionInteraction`), and it's not clear what `send().wait()` does. It's not even clear what `at` does (which asks the PXE for the underlying instance at the provided address). It's also hard to specify gas settings and payment methods: they're presently pushed into `send`, which doesn't make sense because they're needed for the simulation. -await bobBananaCoinInstance.send.transfer(transferRequest).wait(); -``` +I think we can still have a `BananaCoin` class that wraps the `UserAPI` and provides a more user-friendly interface, but it should be explicit about what it's doing. -Delve into the specifics of the design. Include diagrams, code snippets, API descriptions, and database schema changes as necessary. Highlight any significant changes to the existing architecture or interfaces. +I'd suggest we do that in a follow-up design. -Discuss any alternative or rejected solutions. ## Change Set @@ -799,22 +773,22 @@ Fill in bullets for each area that will be affected by this change. - [ ] Public Kernel Circuits - [ ] Rollup Circuits - [ ] Aztec.nr +- [x] Aztec.js - [ ] Noir - [ ] AVM -- [ ] Sequencer -- [ ] Fees +- [x] Sequencer +- [x] Fees - [ ] P2P Network - [ ] Cryptography - [ ] DevOps ## Test Plan -Outline what unit and e2e tests will be written. Describe the logic they cover and any mock objects used. +Implement the above changes and get all the tests to pass. ## Documentation Plan -Identify changes or additions to the user documentation or protocol spec. - +An enormous amount of documentation will need to be updated. Will likely need to ask DevRel for help. ## Rejection Reason From 0f74acd98f2025eb86d2a15a1063b46e23cef263 Mon Sep 17 00:00:00 2001 From: Mitchell Tracy Date: Fri, 14 Jun 2024 12:59:24 -0400 Subject: [PATCH 5/9] multicall support --- ...6973-refactor-base-contract-interaction.md | 220 ++++++++++-------- 1 file changed, 124 insertions(+), 96 deletions(-) diff --git a/in-progress/6973-refactor-base-contract-interaction.md b/in-progress/6973-refactor-base-contract-interaction.md index 5297bb7..66dcba7 100644 --- a/in-progress/6973-refactor-base-contract-interaction.md +++ b/in-progress/6973-refactor-base-contract-interaction.md @@ -12,7 +12,7 @@ This is a refactor of the API for interacting with contracts to improve the user The refactored approach mimics Viem's API, with some enhancements and modifications to fit our needs. -In a nutshell, by being more verbose in the API, we can remove a lot of complexity and make the code easier to understand and maintain; this also affords greater understanding and control over the lifecycle of their contracts and transactions. +In a nutshell, by being more verbose in the API, we can remove a lot of complexity and make the code easier to understand and maintain; this also affords greater understanding and control over the lifecycle of contracts and transactions. Key changes: - the wallet is the central point of interaction to simulate/prove/send transactions instead of `BaseContractInteraction` @@ -67,10 +67,13 @@ const paymentMethod = new SomeFeePaymentMethod( // Changes to the PXE (e.g. notes, nullifiers, auth wits, contract deployments, capsules) are not persisted. const { request: deployAliceAccountRequest } = await aliceWallet.simulate({ - artifact: SchnorrAccountContract.artifact, - instance: aliceContractInstance, - functionName: deploymentArgs.constructorName, - args: deploymentArgs.constructorArgs, + // easy multicall support + calls: [{ + artifact: SchnorrAccountContract.artifact, + instance: aliceContractInstance, + functionName: deploymentArgs.constructorName, + args: deploymentArgs.constructorArgs, + }], paymentMethod, // gasSettings: undefined => automatic gas estimation. the returned `request` will have the gasSettings set. }); @@ -112,14 +115,16 @@ const bananaCoinInstance = getContractInstanceFromDeployParams( ); const { request: deployTokenRequest } = await aliceWallet.simulate({ - artifact: TokenContract.artifact, - instance: bananaCoinInstance, - functionName: bananaCoinDeploymentArgs.constructorName, - args: bananaCoinDeploymentArgs.constructorArgs, - deploymentOptions: { - registerClass: true, - publicDeploy: true, - }, + calls: [{ + artifact: TokenContract.artifact, + instance: bananaCoinInstance, + functionName: bananaCoinDeploymentArgs.constructorName, + args: bananaCoinDeploymentArgs.constructorArgs, + deploymentOptions: { + registerClass: true, + publicDeploy: true, + }, + }], paymentMethod }) @@ -133,9 +138,11 @@ const receipt = await sentTx.wait() ```ts const { result: privateBalance } = await aliceWallet.read({ - contractInstance: bananaCoinInstance, - functionName: 'balance_of_private' - args: {owner: aliceWallet.getAddress()}, + calls: [{ + contractInstance: bananaCoinInstance, + functionName: 'balance_of_private' + args: {owner: aliceWallet.getAddress()} + }] }); @@ -276,16 +283,20 @@ export interface DeploymentOptions { publicDeploy?: boolean; } -// new -export interface UserRequest { +export interface UserFunctionCall { contractInstance: ContractInstanceWithAddress; functionName: string; args: any; deploymentOptions?: DeploymentOptions; - gasSettings?: GasSettings; - paymentMethod?: FeePaymentMethod; contractArtifact?: ContractArtifact; functionAbi?: FunctionAbi; +} + +// new +export interface UserRequest { + calls: UserFunctionCall[]; + gasSettings?: GasSettings; + paymentMethod?: FeePaymentMethod; from?: AztecAddress; simulatePublicFunctions?: boolean; executionResult?: ExecutionResult; // the raw output of a simulation that can be proven @@ -371,14 +382,16 @@ Consider that we have, e.g.: ```ts { - contractInstance: bananaCoinInstance, - functionName: 'transfer', - args: { - from: aliceAddress, - to: bobAddress, - value: privateBalance, - nonce: 0n - }, + calls: [{ + contractInstance: bananaCoinInstance, + functionName: 'transfer', + args: { + from: aliceAddress, + to: bobAddress, + value: privateBalance, + nonce: 0n + }, + }], paymentMethod } ``` @@ -417,40 +430,60 @@ function makeFunctionCall( ``` +#### main function calls + +Define a helper somewhere as: + +```ts +const addMainFunctionCall: TxExecutionRequestAdapter = ( + builder: TxExecutionRequestBuilder, call: UserFunctionCall +) => { + if (!call.functionAbi) { + throw new Error('Function ABI must be provided'); + } + builder.addAppFunctionCall( + makeFunctionCall( + call.functionAbi, + call.contractInstance.address, + call.args + )); +} +``` + #### class registration Define a helper somewhere as: ```ts -export const addClassRegistration: TxExecutionRequestAdapter = ( - builder: TxExecutionRequestBuilder, request: UserRequest +const addClassRegistration = ( + builder: TxExecutionRequestBuilder, call: UserFunctionCall ) => { - if (!request.contractArtifact) { - throw new Error('Contract artifact must be provided to register class'); - } + if (!call.contractArtifact) { + throw new Error('Contract artifact must be provided to register class'); + } - const contractClass = getContractClassFromArtifact(request.contractArtifact); + const contractClass = getContractClassFromArtifact(call.contractArtifact); - builder.addCapsule( - bufferAsFields( - contractClass.packedBytecode, - MAX_PACKED_PUBLIC_BYTECODE_SIZE_IN_FIELDS - )); + builder.addCapsule( + bufferAsFields( + contractClass.packedBytecode, + MAX_PACKED_PUBLIC_BYTECODE_SIZE_IN_FIELDS + )); - const { artifact, instance } = getCanonicalClassRegisterer(); + const { artifact, instance } = getCanonicalClassRegisterer(); - const registerFnAbi = findFunctionAbi(artifact, 'register'); + const registerFnAbi = findFunctionAbi(artifact, 'register'); - builder.addAppFunctionCall( - makeFunctionCall( - registerFnAbi, - instance.address, - { - artifact_hash: contractClass.artifactHash, - private_functions_root: contractClass.privateFunctionsRoot, - public_bytecode_commitment: contractClass.publicBytecodeCommitment - } - )); + builder.addAppFunctionCall( + makeFunctionCall( + registerFnAbi, + instance.address, + { + artifact_hash: contractClass.artifactHash, + private_functions_root: contractClass.privateFunctionsRoot, + public_bytecode_commitment: contractClass.publicBytecodeCommitment + } + )); } ``` @@ -460,8 +493,8 @@ Define a helper somewhere as ```ts -export const addPublicDeployment: TxExecutionRequestAdapter = ( - builder: TxExecutionRequestBuilder, request: UserRequest +const addPublicDeployment = ( + builder: TxExecutionRequestBuilder, call: UserFunctionCall ) => { const { artifact, instance } = getCanonicalInstanceDeployer(); const deployFnAbi = findFunctionAbi(artifact, 'deploy'); @@ -478,6 +511,7 @@ export const addPublicDeployment: TxExecutionRequestAdapter = ( } )); } + ``` #### Entrypoints implement `TxExecutionRequestComponent` @@ -544,9 +578,6 @@ The abstract `BaseWallet` can implement: ```ts async getTxExecutionRequest(userRequest: UserRequest): Promise { - if (!userRequest.functionAbi) { - throw new Error('Function ABI must be provided'); - } if (!userRequest.gasSettings) { throw new Error('Gas settings must be provided'); } @@ -556,34 +587,27 @@ async getTxExecutionRequest(userRequest: UserRequest): Promise { - const txExecutionRequest = await this.getTxExecutionRequest(initRequest); +async #simulateInner(userRequest: UserRequest): ReturnType { + const txExecutionRequest = await this.getTxExecutionRequest(userRequest); const simulatedTx = await this.simulateTx(txExecutionRequest, builder.simulatePublicFunctions, builder.from); const decodedReturn = decodeSimulatedTx(simulatedTx, builder.functionAbi); return { @@ -607,7 +631,7 @@ async #simulateInner(userRequest: UserRequest): ReturnType { privateOutput: simulatedTx.privateReturnValues, executionResult: simulatedTx.executionResult, result: decodedReturn, - request: initRequest, + request: userRequest, }; } ``` @@ -632,7 +656,7 @@ async simulate(userRequest: UserRequest): { const builder = new UserRequestBuilder(userRequest); - await this.#ensureFunctionAbi(builder); + await this.#ensureFunctionAbis(builder); if (builder.gasSettings) { return this.#simulateInner(builder.build()); @@ -653,16 +677,18 @@ async simulate(userRequest: UserRequest): { return result; } -async #ensureFunctionAbi(builder: UserRequestBuilder): void { - // User can call simulate without the artifact if they have the function ABI - if (!builder.functionAbi) { - // If the user provides the contract artifact, we don't need to ask the PXE - if (!builder.contractArtifact) { - const contractArtifact = await this.getContractArtifact(builder.contractInstance.contractClassId); - builder.setContractArtifact(contractArtifact); +async #ensureFunctionAbis(builder: UserRequestBuilder): void { + for (const call of builder.calls) { + // User can call simulate without the artifact if they have the function ABI + if (!call.functionAbi) { + // If the user provides the contract artifact, we don't need to ask the PXE + if (!call.contractArtifact) { + const contractArtifact = await this.getContractArtifact(call.contractInstance.contractClassId); + call.setContractArtifact(contractArtifact); + } + const functionAbi = findFunctionAbi(call.contractArtifact, call.functionName); + call.setFunctionAbi(functionAbi); } - const functionAbi = findFunctionAbi(builder.contractArtifact, builder.functionName); - builder.setFunctionAbi(functionAbi); } } @@ -695,7 +721,7 @@ async read(userRequest: UserRequest): DecodedReturn | [] { builder.setGasSettings(GasSettings.default()); } - await this.#ensureFunctionAbi(builder); + await this.#ensureFunctionAbis(builder); return this.#simulateInner(builder.build()); } @@ -710,7 +736,7 @@ async prove(request: UserRequest): Promise { throw new Error('Execution result must be set before proving'); } const builder = new UserRequestBuilder(request); - await this.#ensureFunctionAbi(builder); + await this.#ensureFunctionAbis(builder); const initRequest = builder.build(); const txExecutionRequest = await this.getTxExecutionRequest(initRequest); const provenTx = await this.proveTx(txExecutionRequest, request.executionResult); @@ -730,7 +756,7 @@ async send(request: UserRequest): Promise { throw new Error('Tx must be proven before sending'); } const builder = new UserRequestBuilder(request); - await this.#ensureFunctionAbi(builder); + await this.#ensureFunctionAbis(builder); const initRequest = builder.build(); const txExecutionRequest = await this.getTxExecutionRequest(); const txHash = await this.sendTx(txExecutionRequest, request.tx); @@ -744,6 +770,8 @@ async send(request: UserRequest): Promise { The `UserRequest` object is a bit of a kitchen sink. It might be better to have a `DeployRequest`, `CallRequest`, etc. that extends `UserRequest`. +Downside here is that the "pipeline" it goes through would be less clear, and components would have to be more aware of the type of request they are dealing with. + #### Just shifting the mutable subclass problem Arguably the builder + adapter pattern just shifts the "mutable subclass" problem around. I think that since the entire lifecycle of the builder is contained to the `getTxExecutionRequest` method within a single abstract class, it's not nearly as bad as the current situation. From 370edd5408e0f660d8526f2f792122cd4a51cd58 Mon Sep 17 00:00:00 2001 From: Mitchell Tracy Date: Fri, 14 Jun 2024 13:51:37 -0400 Subject: [PATCH 6/9] add dapp funded transactions --- ...6973-refactor-base-contract-interaction.md | 126 ++++++++++++++++++ 1 file changed, 126 insertions(+) diff --git a/in-progress/6973-refactor-base-contract-interaction.md b/in-progress/6973-refactor-base-contract-interaction.md index 66dcba7..4dc1609 100644 --- a/in-progress/6973-refactor-base-contract-interaction.md +++ b/in-progress/6973-refactor-base-contract-interaction.md @@ -764,6 +764,132 @@ async send(request: UserRequest): Promise { } ``` +### Dapp Funded Transactions + + +#### A `TxExecutionRequestComponent` for the Dapp + +```ts +export class DefaultDappInterface implements AccountInterface { + constructor( + private completeAddress: CompleteAddress, + private userAuthWitnessProvider: AuthWitnessProvider, + private dappEntrypointAddress: AztecAddress, + private chainId: number = DEFAULT_CHAIN_ID, + private version: number = DEFAULT_VERSION, + ) {} + + async adaptTxExecutionRequest( + builder: TxExecutionRequestBuilder, + userRequest: UserRequest + ): Promise { + if (builder.appFunctionCalls.length !== 1) { + throw new Error(`Expected exactly 1 function call, got ${calls.length}`); + } + if (builder.setupFunctionCalls.length !== 0) { + throw new Error(`Expected exactly 0 setup function calls, got ${calls.length}`); + } + const payload = EntrypointPayload.fromFunctionCalls(builder.appFunctionCalls); + const abi = this.getEntrypointAbi(); + + const entrypointPackedArgs = PackedValues.fromValues(encodeArguments(abi, [payload, this.completeAddress.address])); + const functionSelector = FunctionSelector.fromNameAndParameters(abi.name, abi.parameters); + + const innerHash = computeInnerAuthWitHash([Fr.ZERO, functionSelector.toField(), entrypointPackedArgs.hash]); + const outerHash = computeOuterAuthWitHash( + this.dappEntrypointAddress, + new Fr(this.chainId), + new Fr(this.version), + innerHash, + ); + const authWitness = await this.userAuthWitnessProvider.createAuthWit(outerHash); + + builder + .setOrigin(this.dappEntrypointAddress) + .setTxContext({ + chainId: this.chainId, + version: this.version, + gasSettings: userRequest.gasSettings, + }) + .setFunctionSelector(functionSelector) + .setFirstCallArgsHash(entrypointPackedArgs.hash) + .setArgsOfCalls([...payload.packedArguments, entrypointPackedArgs]) + .addAuthWitness(authWitness); + + } + + + createAuthWit(messageHash: Fr): Promise { + return this.authWitnessProvider.createAuthWit(messageHash); + } + + getCompleteAddress(): CompleteAddress { + return this.completeAddress; + } + + getAddress(): AztecAddress { + return this.completeAddress.address; + } + + getChainId(): Fr { + return this.chainId; + } + + getVersion(): Fr { + return this.version; + } + + private getEntrypointAbi() { + return { + name: 'entrypoint', + // ... same as before + } + } + +} +``` + +#### Create the wallet as normal + +```ts +export async function getDappWallet( + pxe: PXE, + accountAddress: AztecAddress, + dappAddress: AztecAddress, + userAuthWitnessProvider: AuthWitnessProvider, +): Promise { + const completeAddress = await pxe.getRegisteredAccount(accountAddress); + if (!completeAddress) { + throw new Error(`Account ${address} not found`); + } + const nodeInfo = await pxe.getNodeInfo(); + const entrypoint = new DefaultDappInterface(completeAddress, userAuthWitnessProvider, dappAddress); + return new AccountWallet(pxe, entrypoint); +} + + +const schnorr = new SchnorrAccountContract(signingPrivateKey); +const authWitProvider = schnorr.getAuthWitnessProvider(); +const aliceDappWrappedWallet = await getDappWallet(pxe, aliceAddress, dappAddress, userAuthWitnessProvider); + +const { request: deployAliceAccountRequest } = await aliceDappWrappedWallet.simulate({ + calls: [{ + contractInstance: bananaCoinInstance, + functionName: 'transfer', + args: { + from: aliceAddress, + to: bobAddress, + value: privateBalance, + nonce: 0n + }, + }], + paymentMethod: new NoFeePaymentMethod(), +}) + +``` + + + ### Concerns #### `UserRequest` is a kitchen sink From a60dca4f76d5708b13c8058820cbd13a0738d6e5 Mon Sep 17 00:00:00 2001 From: Mitchell Tracy Date: Fri, 14 Jun 2024 16:59:11 -0400 Subject: [PATCH 7/9] add gas estimation algo --- ...6973-refactor-base-contract-interaction.md | 133 +++++++++++++++++- 1 file changed, 132 insertions(+), 1 deletion(-) diff --git a/in-progress/6973-refactor-base-contract-interaction.md b/in-progress/6973-refactor-base-contract-interaction.md index 4dc1609..80a9fe8 100644 --- a/in-progress/6973-refactor-base-contract-interaction.md +++ b/in-progress/6973-refactor-base-contract-interaction.md @@ -320,6 +320,7 @@ export interface SimulationOutput { executionResult: ExecutionResult; publicOutput: PublicSimulationOutput; privateOutput: NestedProcessReturnValues; + error?: SimulationError; } export interface UserAPI { @@ -665,7 +666,7 @@ async simulate(userRequest: UserRequest): { // If we're paying, e.g. in bananas, figure out how much AZT that is. // Note: paymentMethod.getEquivalentAztBalance() may call `read` internally. const equivalentAztBalance = await builder.paymentMethod.getEquivalentAztBalance(); - gasEstimator = GasEstimator.fromAztBalance(equivalentAztBalance); + gasEstimator = new BinarySearchGasEstimator(equivalentAztBalance); builder.setGasSettings(gasEstimator.proposeGasSettings()); while (!gasEstimator.isConverged()) { @@ -889,6 +890,136 @@ const { request: deployAliceAccountRequest } = await aliceDappWrappedWallet.simu ``` +### Gas Estimation + +```ts + + +export interface GasEstimator { + proposeGasSettings(): GasSettings; + isConverged(): boolean; + update(simulationOutput: SimulationOutput): void; +} + +interface SearchRatios { + daToL2: number; + daTearDown: number; + l2TearDown: number; +} + +// Finds a balance that is enough to cover the gas costs of the simulation. +// Marks as converged if the simulation did not run out of gas, +// or if it is not possible to increase the balance further. +export class BinarySearchGasEstimator implements GasEstimator { + // keep the initial balance + // and the ratios of daGas to l2Gas + // as well as the ratios to teardownGas + private balance: number + + // The upper and lower bounds of the search space. + // We start at the midpoint of these bounds. + // The search space is the balance, and the ratios of daGas to l2Gas + // as well as the ratios to teardownGas. + // The goal is to find a balance that is enough to cover the gas costs of the simulation. + // Then we can just use the true gas costs for the actual transaction. + private upperBounds: SearchRatios; + private lowerBounds: SearchRatios; + private current: SearchRatios; + private mostRecentSimulation?: SimulationOutput; + private iterations: number = 0; + private maxIterations: number = 10; + private epsilon: number = 0.01; + + + + constructor(private balance: number) { + this.lowerBounds = { + daToL2: 0, + daTearDown: 0, + l2TearDown: 0, + }; + this.upperBounds = { + daToL2: 1, + daTearDown: 1, + l2TearDown: 1, + }; + this.current = { + daToL2: .5, + daTearDown: .1, + l2TearDown: .1, + }; + } + + update(simulationOutput: SimulationOutput): void { + // If the simulation ran out of DA gas + const oog = simulationOutput.getOutOfGas(); + if (!oog) { + return + } else if (oog === 'da') { + // increase the balance + this.lowerBounds.daToL2 = this.current.daToL2; + } else if (oog === 'l2') { + // increase the balance + this.lowerBounds.daToL2 = this.current.daToL2; + } else if (oog === 'da_teardown') { + // increase the balance + this.lowerBounds.daTearDown = this.current.daTearDown; + } else if (oog === 'l2_teardown') { + // increase the balance + this.lowerBounds.l2TearDown = this.current.l2TearDown; + } + // update the current balance + this.current.daToL2 = (this.lowerBounds.daToL2 + this.upperBounds.daToL2) / 2; + this.current.daTearDown = (this.lowerBounds.daTearDown + this.upperBounds.daTearDown) / 2; + this.current.l2TearDown = (this.lowerBounds.l2TearDown + this.upperBounds.l2TearDown) / 2; + } + + proposeGasSettings(): GasSettings { + return GasSettings.from({ + gasLimits: { + daGas: this.balance * this.current.daToL2, + l2Gas: this.balance * (1-this.current.daToL2), + }, + teardownGasLimits: { + daGas: this.balance * this.current.daToL2 * this.current.daTearDown, + l2Gas: this.balance * (1 - this.current.daToL2) * this.current.l2TearDown, + }, + // This should actually be informed somehow + maxFeesPerGas: { daGas: 1, l2Gas: 1 }, + inclusionFee: 0 + }); + } + +// + isConverged(): boolean { + // If the simulation did not run out of gas, we are converged. + if (!this.mostRecentSimulation?.getOutOfGas()) { + return true; + } + + // If we have reached the maximum number of iterations, we are converged. + if (this.iterations >= this.maxIterations) { + return true; + } + + // If the search space is too small, we are converged. + if (this.upperBounds.daToL2 - this.lowerBounds.daToL2 < this.epsilon) { + return true; + } + if (this.upperBounds.daTearDown - this.lowerBounds.daTearDown < this.epsilon) { + return true; + } + if (this.upperBounds.l2TearDown - this.lowerBounds.l2TearDown < this.epsilon) { + return true; + } + + return false; + + } + +} + +``` ### Concerns From 93a3422b30de9acf016e8253673a4070f993c9fc Mon Sep 17 00:00:00 2001 From: Mitchell Tracy Date: Tue, 18 Jun 2024 13:48:12 -0400 Subject: [PATCH 8/9] clarify "contract" registration and deployment --- in-progress/6973-refactor-base-contract-interaction.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/in-progress/6973-refactor-base-contract-interaction.md b/in-progress/6973-refactor-base-contract-interaction.md index 80a9fe8..197f73e 100644 --- a/in-progress/6973-refactor-base-contract-interaction.md +++ b/in-progress/6973-refactor-base-contract-interaction.md @@ -456,7 +456,7 @@ const addMainFunctionCall: TxExecutionRequestAdapter = ( Define a helper somewhere as: ```ts -const addClassRegistration = ( +const addContractClassRegistration = ( builder: TxExecutionRequestBuilder, call: UserFunctionCall ) => { if (!call.contractArtifact) { @@ -494,7 +494,7 @@ Define a helper somewhere as ```ts -const addPublicDeployment = ( +const addPublicContractDeployment = ( builder: TxExecutionRequestBuilder, call: UserFunctionCall ) => { const { artifact, instance } = getCanonicalInstanceDeployer(); @@ -591,10 +591,10 @@ async getTxExecutionRequest(userRequest: UserRequest): Promise Date: Tue, 18 Jun 2024 15:57:56 -0400 Subject: [PATCH 9/9] Add concern on simulation times. --- in-progress/6973-refactor-base-contract-interaction.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/in-progress/6973-refactor-base-contract-interaction.md b/in-progress/6973-refactor-base-contract-interaction.md index 197f73e..68ba630 100644 --- a/in-progress/6973-refactor-base-contract-interaction.md +++ b/in-progress/6973-refactor-base-contract-interaction.md @@ -1023,6 +1023,10 @@ export class BinarySearchGasEstimator implements GasEstimator { ### Concerns +#### Long time to simulate for gas estimation + +The binary search proposed in the gas estimation algorithm could add a lot of time needed for simulation. If/when we find this to be the case, we could impose an interface on FPCs to provide more information around how much gas the use of that FPC is going to cost, though the exact mechanism here is a bit unclear. + #### `UserRequest` is a kitchen sink The `UserRequest` object is a bit of a kitchen sink. It might be better to have a `DeployRequest`, `CallRequest`, etc. that extends `UserRequest`.