Multiple contracts per account

[turbolent]

Context

Currently account code may only declare up to one contract, and multiple contract interfaces.

There is one function, fun AuthAccount.setCode(_ code: [UInt8], ... initializerArguments), which updates the account's code,
and (if any) instantiates the contract and stores it in the world state.

The function is used both for initial contract deployment and for contract updates.
For the latter use case, this means that the existing contract instance is overwritten / lost.

Update this API to add support for adding, updating, and removing multiple contracts and contract interfaces, independently.

Proposal

• Add new types:

  • Contract:

    A contract that can be deployed to an account.
    The initializer checks the code and aborts if:

    • The code does not contain exactly one contract or contract interface
    • The contract or contract interface name in the code does not match the given name
    • The code/program is invalid

    pub struct Contract {
        pub let name: String
        pub let code: [UInt8]
    
        init(name: String, code: [UInt8])
    }

  • DeployedContract:

    A contract that is deployed at a given account.

    pub struct DeployedContract {
        pub let contract: Contract
        pub let address: Address
    }

  • AuthAccountContracts:

    The deployed contracts of an authorized account.
    Allows introspection, and also addition and removal.

    • fun add(contract: Contract, ... initializerArguments): DeployedContract

      • Adds the given contract to the account.
        The initializer arguments are passed to the initializer of the contract (if any)
      • Fails if a contract/contract interfaces with the name of the contract/contract interface n the given code already exists in the account

    • fun update__experimental(contract: Contract): DeployedContract

      • Experimental
      • Updates the code for the contract/contract interface in the given code in the account
      • Does not run the initializer of the contract/contract interface in the given code again. The contract instance in the world state stays as is
      • Fails if no contract/contract interface in the account with the name of the contract/contract interface in the given code exist
      • Returns the contract object for the updated contract

    • fun get(name: String): DeployedContract?

      • Returns the contract object for the contract/contract interface in the account with the given name, if any
      • Returns nil if no contract/contract interface with the given name exists in the account

    • fun delete(name: String): DeployedContract?

      • Deletes the contract/contract interface from the account which has the given name, if any
      • Returns the deleted contract object, if any
      • Returns nil if a contract/contract interface with the given name does not exist in the account

• Add the following member to AuthAccount:

  let contracts: AuthAccountContracts

• Add special function migrate to composites, the migration function, which is called automatically as part of AuthAccount.contracts.update:

  • For now, the function must know what version is being upgraded from,
    it is not passed any version information
  • migrate is a special function, like init and destroy;
    A user declaration fun migrate() won't be allowed.
    This prevents unintentionally implementing a migration function
  • migrate behaves similar to init

Future Possibilities

• The type use for code, [UInt8], could be replaced with a dedicated Code type,
  e.g. to support other encodings
• The code could be passed as a transaction argument, and support to import declarations from transaction arguments which have the type Code could be added. This would allow use of the contract after it is added/updated (the static analysis needs to have access to the type declarations in the passed code).
• Add a destructor for contracts

Open Questions

• What is the exact behaviour of migrate?
  • If initialization of constant fields is allowed, but the field existed in a previous version, then this could lead to accidental loss of resources
• How are imports and dependencies between individual account code "pieces" handled?

Definition of Done

• Sema, Interpreter, Runtime: Implement support for importing identifiers from a location from separate programs
• Sema: Add new Account API
• Interpreter, Runtime: Implement new Account API
• Tests
• Documentation

[joshuahannan]

It seems to me like migrate would need to be something optional that the user could write themselves. If they are updating the contract functions but not changing any of the fields, then migrate is not necessary, but if they are changing or adding new fields, then they need a migrate function to make sure that the state from the old account gets migrated to the correct places in the new account.

[joshuahannan]

Also, should we add an unsafe for updateAccount? Probably still good to remind users that if they make a misatake updating, it could cause a lot of problems

[dete]

I think that for the first version, we should just model migrate() as a function without special capabilities. Yes, it can't be called explicitly, but, for safety sake, I would suggest we don't give it the ability to mess with constant fields. (This does mean that there is a weird edge case around how constant fields are handled when you upgrade a smart contract, but there are lots of other weird edge cases around contract upgrading, also!) If people complain, we can try to think of ways to address their issues.

I'm not sure I understand the question about imports. I always think of the import statements more like defining a set of aliases, rather than building some kind of lasting connection. In other words, I think of the first block below as shorthand for the second block (even though I don't think the second block should be legal syntax).

import Foo from 0x304293402934

let myValue: Foo.SomeType = Foo.someFunc()

let myValue: 0x304293402934.Foo.SomeType = 0x304293402934.Foo.someFunc()

Under this mindset, imports in one contract don't impact imports on another contract. They are completely separate (but, of course, welcome to import the same Contract definitions.)

[cybercent]

I think most of the updates can be done without a migrate function.

When updating contract interfaces we might need a deprecated tag.
We'll tag the function inside the interface and add the new declaration (that might change param count, return type etc), giving some time to contracts that import this contract to update.
The deprecated tag could have a block height param saying from what block forward the function can't be called anymore.

To update resources, I imagine the owner would need to sign, so it won't be possible to update all resources at once. In this case, instead of a global migrate(), each resource could declare a a local migrate block:

migrate(oldResource, newResource) {
   newResource.foobar = oldResource.foo + oldResource.bar + 10
}

where the developer could copy some attributes from the old resource if needed. The function will swap the 2 resources automatically and check their types.
The developer could manage the versioning by declaring a version field in the resource.

[turbolent]

We had a working session for this and discussed the API and behaviour more.
Here are the changes I'll make to the proposal above:

• Add an initializer to Contract : init(name: String, code: [UInt8])

  • The initializer checks the code, and aborts the program if it is not valid

• Add a new type DeployedContract:

  struct DeployedContract {
      let name: String
      let code: [UInt8]
      let address: Address
  }
  

• Change the addContract function parameter type to Contract and return a DeployedContract

  fun addContract(contract: Contract, ... initializerArguments): DeployedContract

• Rename the function updateContract temporarily to updateContract__experimental until it is safe to use

  • Alternatives considered: unsafe / alpha / mvp / WIP / unstable
  • not unsafe like unsafeRandom, just first version

• Instead of combining contracts into one program, check them independently

  • → Add support for importing contracts separately from account
  • → requires refactoring how account code is stored, not just one code blob per account, now multiple code blobs indexed by type name
  • → Needs migration functionality in EN, need a way to copy access old data in read mode ⇒ check
  • migration of existing deployed code should be straight forward

[turbolent]

Further refined the API and updated the description

[turbolent]

There's a problem with the current API:

There are Contracts, which are not deployed yet, and DeployedContract, which is a Contract deplpoyed at a specific address.

The Contract constructor is supposed to checks the given code. However, checking needs a location (address/account the contract is deployed to), which is not known yet. So a temporary address must be used.

Once account.contracts.add is called however, the existing checker can't be reused, because it is for a temporary location, not for the location that the contract is deployed to.

Ideas/Options:

• We keep the Contract and DeployedContract distinction, and we check twice (bad performance)
• We don't have an "undeployed contract" type Contract, and only have the DeployedContract type. It can't can only be obtained by calling account.contracts.add (or get), it cannot be constructed
• Other?

[dete]

I'm surprised that the checker can't run without knowing the deployment address. I would assume that everything that the deployment address would impact would be a dynamic check (like accessing storage), not a static check. Is it because there is some sugar for importing other contracts from "the same account"?

I personally think that getting rid of the undeployed contract type is totally fine.

[joshuahannan]

I'm with Dete, I think getting rid of the undeployed type is fine. 🙆‍♂️

[turbolent]

The checker requires a location because all types are qualified with it. That way there is no possibility for a type confusion. So checking the code first with a temporary a location creates types that are not the same as the types that will be declared at the account the contract will be deployed.
It hasn't to do with dynamic checks (like accessing storage), or importing other contracts from the same account.

Given we're all OK with just having the deployed contract type, I'll update the API.

Thank you all for the giving feedback so quickly, this unblocks me 👍

[turbolent]

What do we want the behaviour be for scenarios like: removing a contract, but continuing to use it (the contract singleton); removing and then adding another contract with the same name. For the update function we know about the issues and call it update__experimental, but remove + add is effectively also an update.

Should we maybe forbid the use of the contract singleton after it was removed? (dynamic check)

[joshuahannan]

So you're saying we could have a flag that basically says that if some code in a transaction tries to access a contract that has been removed, it fails, even if a new contract with the same name and functionality have been deployed? Or only if it was removed? It seems like we should still allow it if it is the same contract. Might be easier to just disallow though. Doesn't really matter to me.

[turbolent]

Given that remove + add is just as problematic as remove, because the add could have totally different code and would be a different object (! – or we would need to replace the contract singleton variable with the newly instantiated contract object), I'd say be conservative and disallow it in both cases.

[joshuahannan]

Yeah disallowing it sounds fine with me. Good to be safe