Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pay fees natively during account contract initialization #5190

Closed
Tracked by #4998
just-mitch opened this issue Mar 13, 2024 · 6 comments · Fixed by #5601
Closed
Tracked by #4998

Pay fees natively during account contract initialization #5190

just-mitch opened this issue Mar 13, 2024 · 6 comments · Fixed by #5601
Assignees

Comments

@just-mitch
Copy link
Contributor

No description provided.

@spalladino
Copy link
Collaborator

Today account contracts are initialized by calling into their initializer method directly, meaning they don't go through a traditional entrypoint that can handle fee payment. So account contract initialization does not pay fees at all today. To fix this, we have two options:

Add fee payment to initializer. Along with the initialization arguments for the account contract (eg the authorization ECDSA public key), we can supply a FeePaymentPayload with instructions on how to pay for the initialization fee. This has a problem though: adding a FeePaymentPayload to the initializer means it becomes part of the initialization hash that needs to be committed to when the contract address is precalculated. We probably don't want this, since we want flexibility in deciding how (and specially, how much) the fee will be paid, as it depends on the moment in which the tx is sent. Doing this requires adding a new macro aztec(noinitargscheck) that disables the init args check, and instead manually calls assert_initialization_args_match_address_preimage with the arguments we want to constrain. The FeePaymentPayload should be checked via an authwit, as usual, to prevent someone else from initializing the contract and paying a ridiculous amount of money without the owner's authorization.

Add a canonical initializer contract. This contract does not need to be enshrined, but it should be well-known. This contract should provide a common entrypoint that takes care of initialization and fee payment in the account contract. The logic should be something along the lines of:

contract AccountInitializer:
  entrypoint(target_address, init_args, fee_payload):
    private_call(target_address, "constructor", init_args)
    private_call(target_address, "entrypoint", app_payload: empty, fee_payload)

Note that this approach, while a lot cleaner since it does not need the dangerous noinitargscheck, will not work unless we can read a nullifier emitted earlier in the same tx (which is not yet supported), since the entrypoint should have an init check to prevent it from being called before initialization. Alternatively, we could flag the entrypoint as noinitcheck, and "manually" assert that the account contract has been initialized based on whether we can read the initialized data (eg the ECDSA pub key).

@alexghr alexghr self-assigned this Mar 26, 2024
@alexghr
Copy link
Contributor

alexghr commented Mar 26, 2024

Personally, I'm leaning towards the second option as it seems cleaner and safer. The noinitargscheck macro looks like it could lead to a bunch of bugs.

One problem I see is that the fee is designed to go at the beginning of the execution, marking itself as 'non-revertibe'. If we follow the "init, then pay the fee" flow then the initialisation becomes part of the non-revertible part of a tx. This becomes a problem if an account contract in the future requires public initialisation as that would be tagged for the setup phase, meaning it either has to be whitelisted by the sequencer or the sequencer takes on risk that init would take significant compute and eventually revert.

Ideally we'd setup the fee first inside AccountInitializer, then run the account contract's init function as part of private app logic, but how do we validate the authwitness? We don't have an account contract to validate authwits.

I'll come back here once I have a solution

@spalladino
Copy link
Collaborator

Ideally we'd setup the fee first inside AccountInitializer, then run the account contract's init function as part of private app logic, but how do we validate the authwitness? We don't have an account contract to validate authwits.

I'm afraid that if you go down this route you end up in a situation where you need a method in the account contract to validate a fee payload before initialization, which means you manually need to reconstruct the init args from the address preimage, which means you end up implementing something very similar to the first option.

I'd suggest restricting account contracts to only have private constructors if they are meant to be deployed this way. And if it needs a public constructor, then have it initialized from another account who pays the fee.

@alexghr
Copy link
Contributor

alexghr commented Mar 26, 2024

Coming back with an update after thinking through this a bit more and chatting with @spalladino on slack. We propose two flows.

Account contract initialiser

This would be a new protocol contract that would be responsible for initialising individual account contracts. Like in the comment above:

contract AccountInitializer:
  entrypoint(target_address, init_args, fee_payload):
    private_call(target_address, "constructor", init_args)
    private_call(target_address, "entrypoint", app_payload: empty, fee_payload)

The AccountInitializer first initialises the target account contract, then it calls into its entrypoint to setup fee payment as normally.

Pros:

  • simple flow
  • the bulk of the logic is kept into a dedicated protocol contract
  • it reuses existing fee payment flow

Cons:

  • it breaks the fee mental model: the account contract initialisation becomes part of private setup (and so its side effects will be part of endNonRevertible)
  • it requires reading the initialisation nulliffier emitted in the same tx -- is there an issue for this somewhere?

The initialiser would run as part of the private setup phase meaning whatever nullifiers/notes it emits become part of the non-revertible set. I don't immediately see an issue with this, a fee still gets paid for the data.

Security

A malicious user could craft an account contract that enqueues public functions to be executed as part of the public setup phase. In this case the sequencer protects itself by whitelisting which contract instances/classes are allowed to run in setup and would reject the unrecongised account contract.

Pop capsule

We can take Palla's initial suggestion, but instead of adding the macro aztec(noinitargscheck), we pass the FeePayload as a capsule.

contract SomeAccountContract:
  constructor(pub_key: PublicKey):
    # get the fee and check authwit
    fee_payload = FeePayload::deserialize(pop_capsule)
    assert(is_valid(context, fee_payload.hash(), pub_key))
    # execute fee, capture hwm
    fee_payload.execute_calls(context)
    context.capture_min_revertible_side_effect_counter()
    # init storage in app logic
    storage.pub_key.set(pub_key)

Pros:

  • it keeps everything contained in the account contract
  • it maintains the fee mental modal
  • it reuses existing fee functions
  • it does not require reading a nullifier emitted in the same tx

Cons:

  • capsules
  • each account contract will need to have a copy of the above code ☝️ (or we could put in the AccountActions shared struct)
  • storage is initialised at the end (in private app logic). If anything reads account storage in private setup they will not find the pub key. We could set the pub key at the top of function but again, it will become part of private setup/endNonRevertible

Security

Same security implications apply. If a malicious account contract tries to inject code into public setup, it would get rejected by the sequencer through the fact that it's not on the approved fee payment contracts.


PS: I'm starting to think that maybe the naming we chose for revertible/non-revertible side effects might not have been the best. If an action in either phase reverts then the tx "fails" but in one case it soft-reverts and goes on chain and pays a partial fee while in the other it hard-reverts and is rejected by the sequencer.

@spalladino
Copy link
Collaborator

Thanks for the writeup! Just one more comment from me:

If a malicious account contract tries to inject code into public setup, it would get rejected by the sequencer through the fact that it's not on the approved fee payment contracts.

I don't think this is a security issue inherent of this task. Anyone can deploy a contract that behaves like this, regardless of what we do with account initialization.

@alexghr
Copy link
Contributor

alexghr commented Apr 9, 2024

After a bit of back and forth on capsules we've agreed to go the account contract initialiser route with one difference. Instead of deploying a new protocol contract whose sole responsibility is to intialise account contracts, we instead are using the canonical multi call entrypoint to do it.

The multi call contract will receive a payload with the following function calls:

  • (optional) register contract class id
  • (optional) publicly register the account contract instance
  • initialise the new account contract
  • execute the fee payload

In order to achieve this we have modified the entrypoint interface to accept transient authwits alongside function calls so that the yet-to-be-deployed-account can create an authwit authorizing the fee payload.

alexghr added a commit that referenced this issue Apr 10, 2024
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
3 participants