feat: pay fee when deploying account contracts#5540
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
alexghr
changed the title
alexghr
force-pushed
the
04-02-feat_pay_fee_when_deploying_schnorr_account
branch
from
90f4dbc to
e951798
Compare
Benchmark resultsMetrics with a significant change:
Detailed resultsAll benchmarks are run on txs on the This benchmark source data is available in JSON format on S3 here. Values are compared against data from master at commit L2 block published to L1Each column represents the number of txs on an L2 block published to L1.
L2 chain processingEach column represents the number of blocks on the L2 chain where each block has 16 txs.
Circuits statsStats on running time and I/O sizes collected for every circuit run across all benchmarks.
Tree insertion statsThe duration to insert a fixed batch of leaves into each tree type.
MiscellaneousTransaction sizes based on how many contract classes are registered in the tx.
Transaction size based on fee payment method
Transaction processing duration by data writes.
|
alexghr
force-pushed
the
04-02-feat_pay_fee_when_deploying_schnorr_account
branch
3 times, most recently
from
51ed6a1 to
f61f732
Compare
alexghr
force-pushed
the
04-02-feat_pay_fee_when_deploying_schnorr_account
branch
3 times, most recently
from
4d98bc8 to
1bbdc57
Compare
alexghr
force-pushed
the
04-02-feat_pay_fee_when_deploying_schnorr_account
branch
from
1bbdc57 to
086911b
Compare
|
Looks like boxes use the published versions of aztec.js & accounts instead of what's in the current workspace so the their tests will fail. We can either disable them temporarily until the next release or find a way to link them against the workspace's packages. |
spalladino
left a comment
spalladino
left a comment
There was a problem hiding this comment.
Looks good! I left a few comments but we can address those later. I was about to comment on how it'd be nice to be able to publicly deploy and initialize simultaneously, when I remembered that you just mentioned that on Slack heh.
| #[aztec(private)] | ||
| #[aztec(initializer)] | ||
| fn constructor() { | ||
| let actions = AccountActions::private(&mut context, ACCOUNT_ACTIONS_STORAGE_SLOT, is_valid_impl); | ||
| actions.initialize_account_contract(); | ||
| } |
There was a problem hiding this comment.
Let's remove this: this account contract does not need initialization at all.
| ) | ||
| } | ||
|
|
||
| pub fn initialize_account_contract(self) { |
There was a problem hiding this comment.
I'd rename this to something like "execute_fee_payload_for_init", since it's not actually initializing the contract.
| let fee_payload = FeePayload::deserialize(pop_capsule()); | ||
| let fee_hash = fee_payload.hash(); |
There was a problem hiding this comment.
I think I'm starting to agree with Joe on this use case for capsules. The fee_payload should be an argument to this function and not something that shows up magically, and what we really need is a way to say "this argument is not part of the initialization hash". But we can change that in a future PR.
There was a problem hiding this comment.
I think the tasks for this would be:
- Add a
noinitargcheckattribute in Noir for the argument to be skipped - Make sure the attribute is emitted in the abi
- Compute a separate args hash for initialization preimage check if there are any noinitargcheck
- Change the address computation so it ignores args marked as
noinitargcheckwhen computing the initialization hash in the address preimage (this will be the most annoying bit I think) - Ensure the deploy method does not ignore them when actually executing the initialization
It's quite a lot unfortunately, but feels cleaner..?
There was a problem hiding this comment.
I think I'm starting to agree with Joe on this use case for capsules
lol same, but I was actually thinking of going the protocol contract route instead of adding a new macro 😅
| #[aztec(private)] | ||
| #[aztec(initializer)] | ||
| fn constructor() { | ||
| let actions = AccountActions::private(&mut context, ACCOUNT_ACTIONS_STORAGE_SLOT, is_valid_impl); | ||
| actions.initialize_account_contract(); | ||
| } |
There was a problem hiding this comment.
Let's remove this one as well
There was a problem hiding this comment.
Whoa, when was entrypoints moved to a package of its own?
There was a problem hiding this comment.
We creates the entrypoints package during the offsite. In hindsight I think a better approach would've been to expose it as aztec.js/entrypoints.
| if (!this.initializerArtifact) { | ||
| throw new Error('Account contract can not be initialized without an initializer'); | ||
| } |
There was a problem hiding this comment.
Let's extend this to support account contracts without initializers, in which case we should just publicly deploy the instance.
| const deployWallet = new SignerlessWallet(this.pxe); | ||
| const deployer = new ContractDeployer( | ||
| this.accountContract.getContractArtifact(), | ||
| deployWallet, | ||
| const args = this.accountContract.getDeploymentArgs() ?? []; | ||
| this.deployMethod = new DeployAccountMethod( | ||
| this.pxe, | ||
| this.accountContract.getAuthWitnessProvider(this.getCompleteAddress()), | ||
| encryptionPublicKey, | ||
| this.accountContract.getContractArtifact(), | ||
| args, | ||
| () => this.#register(), | ||
| ); | ||
| const args = this.accountContract.getDeploymentArgs() ?? []; | ||
| this.deployMethod = deployer.deploy(...args); |
| /** | ||
| * The function calls to execute | ||
| * @internal | ||
| */ | ||
| get function_calls() { | ||
| return this.#functionCalls; | ||
| } |
There was a problem hiding this comment.
Let's add a comment why this snake_case property is needed
| /** | ||
| * Contract interaction for deploying an account contract. Handles fee preparation and contract initialization. | ||
| */ | ||
| export class DeployAccountMethod<TContract extends ContractBase = Contract> extends BaseContractInteraction { |
There was a problem hiding this comment.
Let's rename to InitializeAccount, since it doesn't have the capability to publicly deploy.
|
Superseded by #5601 |
This PR enables accounts to pay tx fees when they're deployed. To achieve this a new deployment method was added that's used by the `AccountManager` class to optionally register/publicly deploy and initialize the target account contract. Entrypoint classes now accept authwits/packed arguments alongside the normal function calls from before. This is needed so that authwits could be created in a parent context and then passed along as transient authwits to the transaction (see `ExecutionRequestInit` wrapper type) Initializing an account contract can use any of the three existing payment methods: - using bridged gas token from L1 - paying privately through a fee payment contract - paying publicly through a fee payment contract In order to use fee payment contracts this PR adds `noinitcheck` to `spend_private_authwit` and `spend_public_authwit` because it's not possible to read the init nullifier in the current tx (protocol limitation). Instead the contract relies on the note containing the public key to exist to validate that the contract has been initialized correctly. An extra payment flow is tested as well: a third party takes the account's public keys and deploys and initializes the account while paying the associated fee. This simulates the flow where a deployment service is used that takes payment through a side chain (e.g. fiat). Breaking change: moved `DefaultMultiCallEntrypoint` to aztec.js This PR supersedes #5540 and #5543. Fix #5190 #5191 #5544.
3 participants
This PR enables account contracts to pay for tx fees when they're deployed.
Fix #5190