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

More permissive contract upgrade checker for 1.0 network upgrade #2865

Closed
3 tasks done
j1010001 opened this issue Oct 11, 2023 · 6 comments
Closed
3 tasks done

More permissive contract upgrade checker for 1.0 network upgrade #2865

j1010001 opened this issue Oct 11, 2023 · 6 comments
Assignees
Labels

Comments

@j1010001
Copy link
Member

j1010001 commented Oct 11, 2023

Why: Contract update checker ensures developers can't break on-chain data with backwards-incompatible contract update. It is possible that the rules might need to be relaxed for upgrade to C1.0 to be possible.

Note: Discussed at the Q4 OKR onsite: https://www.notion.so/dapperlabs/Cadence-OKRs-Q4-2023-111411c14603498aae1f2f24d03314e4?pvs=4#c8d21cf2649540f587fd71df8ace95c4

Tasks:

@turbolent
Copy link
Member

We already have some "data points", i.e. the protocol core contracts, and the FT and NFT contracts.

@bluesign
Copy link
Contributor

I was checking contract update logic recently, actually it is pretty simple. Maybe listing all restrictions and going over them one by one has some benefit also. Some of them are must-have in any case.

@bjartek
Copy link
Contributor

bjartek commented Nov 10, 2023

Would it make sense for emulator only add/update methods to specify what version they are? So I can deploy v0.4 or v1?

Or specify if it is valid or not?

@j1010001 j1010001 modified the milestones: OKR-23-Q4, C1.0-Launch-M1 Nov 14, 2023
@j1010001 j1010001 changed the title More permissive contract upgrade checker for 1.0 spork More permissive contract upgrade checker for 1.0 network upgrade Nov 17, 2023
@turbolent
Copy link
Member

We had some discussions around this, and an alternative to updating the contract update checker could be to remove the requirement of contract updates to pass the checker – instead of statically checking contract updates on-chain, the checker would rather be just an off-chain tool that developers can run by themselves. The contract update functionality would be reduced to just updating the given code as-is. Developers would need to make sure the code is updated correctly.

We would have to go over all rules of the contract update checker (https://cadence-lang.org/docs/1.0/contract-upgrades) and ensure that such changes are gracefully handled at run-time, by ensuring appropriate test cases exist, by adding missing test cases, and by adding missing graceful recovery where missing.

This would allow the network upgrade for Cadence 1.0 to update all contracts, core contracts and staged "user" contracts, to be updated as-is, in one step.

@turbolent
Copy link
Member

Cadence 1.0 introduces resource destruction events, which allow referring to fields of the reference.
For example, when the following resource is emitted, the event contains the value of the field answer.

resource R {
    let answer: Int
    init(answer: Int) {
        self.answer = answer
    }

    event ResourceDestroyed(answer: Int = self.answer)
}

However, if the field answer got added through a contract update, there might be stored resource in the state which do not have a value for the field yet (the contract update only updates the code, but not stored values).

Though accessing a field which does not have a value is gracefully handled by Cadence, by aborting the program with a user error, this is not acceptable when the resource destruction event is emitted / the resource is destroyed.

Rejecting any member (field) access in a resource destruction event (e.g. self.answer) would render those events useless.

@dsainati1 noted that we could consider disabling the contract update checker only temporarily for the Cadence 1.0, and re-enabling it again afterwards.

Also, as discussed in #2170, we might define the semantics of resource destruction events to be that they are not guaranteed to be emitted, but when possible. This could potentially also result in more kinds of expressions that might abort and are currently rejected to be allowed again.

@j1010001
Copy link
Member Author

j1010001 commented Feb 26, 2024

Changing the scope, removing from tasklist: "collect feedback from community projects (data for checker rules update) - getting their updated contracts, diffing them and evaluating how that affects the upgrade checker."

We will release first version of the upgrade checker asap and deal with any reported issues by opening separate issue per reported problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants