# Contract upgradeability

## Purpose
A contract should be able to change its own implementation to fix bugs, add functionality, etc. The change of the implementation shouldn't change either the address or the storage of the contract, allowing to swap just the code without breaking state or composability.

## Proposal
We'll describe here a proposal using shared mutable to be able to update contract class ids, which encode vks and bytecode.

### Triggering updates
The contract instance deployer can store the updated contract class ids in shared mutables:

#[storage]
struct Storage<Context> {
    updated_class_ids: Map<AztecAddress, SharedMutable<ContractClassId, UPDATE_DELAY, Context>, Context>,
}

The updates can be triggered by calling the following function the deployer, which will allow msg_sender to update its own class id:

#[public]
fn update(new_contract_class_id: ContractClassId) {
    let address = context.msg_sender();

    // Is it possible for this to fail?
    assert(
        context.nullifier_exists(address.to_field(), context.this_address()),
        "msg.sender is not deployed",
    );

    assert(
        context.nullifier_exists(new_contract_class_id.to_field(), REGISTERER_CONTRACT_ADDRESS),
        "New contract class is not registered",
    );

    storage.updated_class_ids.at(address).schedule_value_change(new_contract_class_id);

    let event = ContractInstanceUpdated {
        DEPLOYER_CONTRACT_INSTANCE_UPDATED_MAGIC_VALUE,
        address,
        new_contract_class_id,
    };

    let payload = event.serialize();
    context.emit_unencrypted_log(payload);
}

The update will come into effect after UPDATE_DELAY blocks.

### How to read this shared mutable from circuits

Both the AVM and the private kernels will need to read the shared mutable state to find out what the current value of the updated_contract_class_id is.
Shared mutable writes a hash (``) in public storage of ScheduledValueChange and ScheduledDelayChange. We can read that hash from the public data tree and provide the preimage of the hash to reconstruct ScheduledValueChange, which holds what the current value should be:

pub(crate) struct ScheduledValueChange<T> {
    pre: T,
    post: T,
    // Block at which `post` value is used instead of `pre`
    block_of_change: u32,
}

### Changes to Bytecode validation

In public, we can query the current value of the SharedMutable hash directly from the public data tree, since we are executing at the tip of the chain.
- If the leaf doesn't exist, then bytecode validation works as it did before updates
- If the leaf exists, we then verify the ScheduledValueChange hint, and given the current block and change.block_of_change we decide what the current value (`pre` or `post`) is for updated_class_id.
- If updated_class_id is zero, then bytecode validation works as it did before updates
- If updated_class_id is nonzero, we perform membership of the bytecode against that updated class id instead of the original class id.

In any case, we need to still verify membership the initialization nullifier for the contract address, that is unchanged.

### Changes to VK checks

In private, we can't query the current value of the SharedMutable hash. Instead we perform a historical read of the appropiate slot for the SharedMutable hash.

Set the appropiate value for max_block_number for the TX. All txs that interact with an updatable contract need to set a max_block_number. Initially, all contracts are updatable except protocol contracts.

The appropiate value for max_block_number follows the same logic as on noir's shared mutable implementation. It could be simplified down a bit if we don't allow delay changes in updates.

After setting the max_block_number, using the historical value for updated_class_id works the same as in public. If we have an updated_class_id for the called address, we verify the VK against that one. If we don't have it we verify the VK normally against the address.

### Changes to the archiver

The archiver now needs to listen to unencrypted logs about updates performed to contracts. The events need to be stored in the store as objects since reorgs need to delete those update objects, and blocks mined need to add them. When necessary, the current class id of the contract can be computed by the archiver given the current block number and the updates that have been applied to the contract.

### Changes to the simulator

Before executing any contract, PXE's simulator must verify that the bytecode being executed matches the latest implementation. Latest here means the current class ID given the block the PXE is synced to. This is important because the current block might contain new note types that only the latest implementation can understand.

### Backwards compatibility

Any contract update must be compatible with previous versions in storage layout and note shape. This is because the newer implementation might be requested to process notes or to access storage structs from past implementations.

### Changes to Aztec.js interface

The PXE stores the contract instances and classes in a local database. When a contract is updated, in order to interact with it we need to pass the new artifact to the PXE, since the protocol doesn't publish artifacts.

My initial proposal consists on not changing anything in the Aztec.js interface, and instead leverage the current flows that we have. This would be the example flow to update a contract.

#[aztec]
contract Updatable {
...

    #[private]
    fn update_to(new_class_id: ContractClassId) {
        ContractInstanceDeployer::at(DEPLOYER_CONTRACT_ADDRESS).update(new_class_id).enqueue(
            &mut context,
        );
    }
...

const contract = await UpdatableContract.deploy(wallet, ...args)
      .send()
      .deployed();
const updatedContractClassId = (await getContractClassFromArtifact(UpdatedContractArtifact)).id;
await contract.methods.update_to(updatedContractClassId).send().wait();

Now, when the update has happened, calling `at` with the new contract artifact will automatically update the contract instance in the PXE if it's outdated:

// 'at' will call PXE updateContract if outdated
const updatedContract = await UpdatedContract.at(address, wallet);

I think this flow would be ideal for dapps since a new version of the dapp would be published with the new artifact. Whenever a user interacts with the new version of the dapp it'll call `at` and the PXE would automatically get updated.

If you try to call `at` with a different contract that is not the current version, it'll fail

// throws when trying to update to RandomContract 
// since the current one is UpdatedContract
await RandomContract.at(address, wallet);

However we'll probably need feedback from real apps to see if they need some specific methods in aztec.js for contract updates.

## Possible improvements

### Contract immutability
We can make some contracts be immutable by injecting a boolean in the address preimage. We currently compute it like this:

    pub fn compute(public_keys: PublicKeys, partial_address: PartialAddress) -> AztecAddress {
        let public_keys_hash = public_keys.hash();

        let pre_address = poseidon2_hash_with_separator(
            [public_keys_hash.to_field(), partial_address.to_field()],
            GENERATOR_INDEX__CONTRACT_ADDRESS_V1,
        );

We could add this boolean to the pre_address.
The contract instance deployer can check that boolean, requiring some hints (the public keys and partial address) to be passed to verify that the derivation matches the original address. The private kernels, when calling an immutable contract can skip setting the max_block_number property to the tx if that boolean is false.

### Upgrades as an opcode
The triggering of contract upgrades can be promoted to an AVM opcode, instead of being a public method of the contract instance deployer. I see some problems with this approach:
- It would be a complex gadget, that needs to emulate the behavior of SharedMutable
- We'd only see gas improvements when upgrading contracts, which is not that common of an operation to require an opcode IMO
- Contract updated logs would be emitted by the contract that is being updated, instead of the deployer protocol contract. This might be painful?

---

# Comments

*Because hackmd comments are horrible*

## Palla

- About this comment:

        // Is it possible for this to fail?
        assert(
            context.nullifier_exists(address.to_field(), context.this_address()),
            "msg.sender is not deployed",
        );

Yeah, I believe that could happen if an undeployed contract enqueues a call to upgrade from private-land.

- Given how easy it is to upgrade a contract just by issuing a call, we should add in aztec-nr a check that `call` does not go to any of the precompiles, and add a separate `system_call` method for those.
- [Alvaro] True, we probably can do that purely in aztec noir though, since the precompiles have identifiable contract addresses

- AVM has a `GETCONTRACTINSTANCE` opcode that can be used to retrieve the class id. We need to review its usages (if any) and make sure we're querying the deployer in those cases.

- How are we going to decide on the value for `UPDATE_DELAY`? Should we allow each contract to set its own delay somehow?
- [Mike] This seems reasonable to me. We already have the chaos of contracts being able to specify their own delays for their own sharedMutable state variables, so we might as well embrace it? Hmm... perhaps we want to enforce a minimum delay, though? Otherwise some very popular, and highly-composable contract could ruin the ecosystem, if it sets a delay of 1 block: then every tx in the network would be forced to adhere to 1 block ttl.
- [Mike] I'd be happy with it being quite a long minimum delay, with the messaging: "If you want to use contract upgrades to batch bugs very quickly, you cannot. Use a pause mechanism instead". Hmm... but then if a pause mechanism with a short delay is implemented by a popular contract, we're in the same boat of forcing every tx to have a short delay. Hmmm. We've already made our bed here. Although, perhaps aztec.nr should enforce a minimum delay for contracts' sharedMutable state variables too. Dear reader, you might be able to tell that I'm re-remembering all of the details of sharedMutable from last year, as I type this, and am adding very little value.
- [Alvaro] Shared mutables have configurable delays. We can expose an API in the deployer contract to change the delay, and assert it's within reasonable bounds.
## Nico

- >Given how easy it is to upgrade a contract just by issuing a call, we should add in aztec-nr a check that `call` does not go to any of the precompiles, and add a separate `system_call` method for those.

Agreed, make it an opcode or w/e but not a regular call.

- >How are we going to decide on the value for UPDATE_DELAY? Should we allow each contract to set its own delay somehow?

SharedMutable already supports changing the delay, which is non-trivial but not terribly complicated. I think it'd be nice to have this. Yes, you can artificially add a delay by timelocking the call that performs the upgrade, but that's not very visible - I'd rather have that at the base layer for something as critical as this. We can have a default value (e.g. 5 days?) and let people change it. We can also add a minimum to prevent timing issues, and a maximum to prevent people accidentally nuking the mechanism for themselves.