moul commented 1 year ago

Context

When a smart contract goes live, it may need to be updated or changed for various reasons.

To enable smart contract upgradability, it is important to address challenges such as data loss, immutability, security, interoperability, data migration, and governance.

Gno smart contracts use human-readable package URLs, and packages have a hash address derived from their URL. The expected URL structure is $zone.land/{p,r}/$user/$dir[/$subdir] on gno.land and potentially other instances.

Current proposal and open questions

The solution will be a blend of core features and recommended patterns, as shown in https://github.com/gnolang/gno/pull/125.

For version management, a Git-like approach can be used, where each version has a content-deterministic hash and a named version. The hash is immutable, while the version can be changed, much like a Git tag or Unix symlink.

Publishing a package to gno.land/r/manfred/pkg creates an immutable package at gno.land/r/manfred/pkg@hash, with a symlink for access without a version suffix. Previous versions are only accessible via gno.land/r/manfred/pkg@previous-hash. Alternatively, a counter per path can be used, allowing access via /v1, /v2, and so on.

While the webUI allows access to various URLs, there is a question of how to update existing contracts that depend on prior versions of a dependency. Possible solutions include updating the import paths via gno.mod or the source, using a BumpDep VM call, or patching .gno files.

Regarding data state, the question is whether to share it between contract versions based on variable name and type, or keep it independent and require developers to write patterns.

Additional challenges exist around UX (#688), transparency, and governance. A potential solution is to implement a security layering system similar to kernel security rings.

In our efforts to support upgradeability, we could consider incorporating a built-in feature to pause or deprecate previous versions. Additionally, we could investigate the feasibility of automatically generating and sharing release notes while browsing packages.

One last consideration is the possibility of adding an UpdatePkg VM call to allow the addition of new files to existing packages.

Potentially related with package metadata (https://github.com/gnolang/gno/issues/498). Related with upgrading policy for stdlibs (https://github.com/gnolang/gno/issues/239). Related with security UX (#688). Related with upgrade pattern examples (#125).

anarcher commented 1 year ago

I'm wondering what to do with persisted values from previous versions. If each new version creates new persisted values, will we need to do some sort of migration?

moul commented 1 year ago

Indeed, it is an option, and it may lead to some intriguing migration workflows, such as:

// contract foo

var users []User
var paused bool
func ListUsers() {...} 
func AddUser() { if paused {panic()}}

// contract bar

var users []User
func init() {users = foo.ListUsers()}

To enhance migration capabilities, the p/demo/avl package could enable contract B to modify the state of contract A without necessitating an update of contract A, similar to how deletion is handled in SQLite and can be used with tape archives (tar).

An additional option, even with independent states between versions, is to implement patterns through data-oriented contracts and dynamic whitelisted intermediary contracts.

My suggestion would be to weigh the potential drawbacks of sharing state between versions and impose appropriate restrictions. However, if we are unable to identify a viable solution, then it may be prudent to maintain independent states and concentrate our efforts on crafting great migration patterns and powerful structures.

anarcher commented 1 year ago

Personally, I don't think it's easy to plan ahead for the next version and add paused functionality to the current version.

It would be nice to have some sort of switch in vmkeeper to disallow changes to that realm, e.g. PausePkg FreezePkg?

func init() {users = foo.ListUsers()}

If this could be used as a kind of migration, it would be very useful. :-)

moul commented 1 year ago

Related with #709.

thehowl commented 1 year ago

I think that the way that we handle contract versioning should first of all differ between importing packages and realms. I think that if a package or a realm imports another package, the import should be pretty much identical to what happens in Go: the import path is "gno.lang/p/demo/foo", the package is saved as an import in gno.mod, which gnoland is aware of at the time of publishing, and the specific version in gno.mod is always used.

For realms, due to their nature of state preservation and of potentially being end-user programs, I don't think version locking like for packages is a good idea, with the larger issue being data upgrades and security fixes.

As a realm owner, I want to be able to patch my realm if there is an issue in the code which seriously harms the functionality

For example: if after having published a package I realise that using a method in a certain way means that a user is managing to profit ugnots off of a realm's bug, I want to be able to patch this and not have the user able to version-lock to the previous version with the bug.
As a realm owner, I want to be able to modify data structures (possibly restricted to an "append-only" mode for public APIs) without having headaches about backwards-compatibility
For example: suppose an old version of the code had a structure like:
```
type Post struct {
   Author string
   Content string
}
```
The new version adds a field.
```
type Post struct {
   Author  string
   Content string

   modifiedAt int64
}
```
Through upgrade mechanisms, modifiedAt is transparently added and initialised when upgrading the realm. However, say we have a realm user which is calling our realm's API, for instance, first doing a foo.SubmitPost(bar.Post{Author: "x", Content: "y"}) and then getting back the data using a foo.GetLatestPost(). While we could implement an auto-upgrade mechanism, whereby the function upgrading bar's internal var posts []Post is called again on the newly submitted data, but then this grows more complex if we account for foo.GetLatestPost(), where the data has to be effectively downgraded to the version being locked!

My proposal for realm upgrades is the following:

A realm user always uses the latest version of the realm being called.
When a realm is published, its exported API is "locked" for compatibility in a similar fashion to the Go 1 compat promise, ie. it may be only added to in subsequent package upgrades. (Following Go's example, a short list of allowed upgrades could be:
- New (exported) symbols (functions/methods/types/consts...)
- Additional fields in structs (unsure on this one -- may need to forbid to intialise imported struct types using unkeyed fields)
- Note that this means that in some edge cases, the importers of the realm may be erroneous or even fail to run following an upgrade, mostly for the cases that the go 1 compat promise specifies as well (ie. "Methods. As with struct fields, it may be necessary to add methods to types. Under some circumstances, such as when the type is embedded in a struct along with another type, the addition of the new method may break the struct by creating a conflict with an existing method of the other embedded type. We cannot protect against this rare case and do not guarantee compatibility should it arise.")
On package upgrades, variables are upgraded:
- "Automatically" when possible and not specified otherwise (ie. convertible types such as []byte-string, int-uint64, or non-destructive changes in complex data structures, such as [24]byte -> [25]byte or [24]byte -> []byte, struct{a int} -> struct{ a int; b int })
- If a change is destructive, then a manual upgrade is required (ie. struct{a int; b int} -> struct{a int}, []byte -> [8]byte) and gnoland forbids publishing the package until there is an upgrade function.
- Manual upgrade: a special function func upgrade(), similar to init (ie. can be declared multiple times, and with different signatures) which takes in the old type (if named type, doesn't have to be matching as long as the underlying types are identical)
```
var posts [8]Post
var usernames []string
// func upgrade(oldVariableName OldVariableType) (newVariableName NewVariableType)
// newVariableName is omitted if == to old
func upgrade(posts []Post) [8]Post { return [8]Post(posts[:8]) }
// another example: variable renaming!
func upgrade(users []string) (usernames []string) { return users }
```
  The reason why upgrade doesn't touch the variables directly is because shadowing makes the top-level posts inaccessible inside the function - I'm not 100% fond of this idea as I would rather see it take the old value and update the top-level variables, while specifying what the old variable name is and allowing variable renames. Ideally these are either deleted by the package owner after the realm upgrade is done, or their functioning is tied to the stateVersion pattern (see below)
init functions are called again every time a package is upgraded, and after upgrade is called. However, their functioning and mechanism can be controlled in a similar way to how database upgrades are often done in more common RBDMS, ie:
```
var stateVersion int64
```

func init() { stateMigrations := []func(){ 0: func() {...}, // .... }

if stateVersion < len(stateMigrations)-1 {
    for _, mig := range stateMigrations[stateVersion:] {
        if mig != nil {
            mig()
        }
    }
}

}


This way we can control code to run at every upgrade, or only on a certain upgrade, etc.
- Backwards-incompatible version changes are allowed by specifying `/v2` like in Go, which are to all effects new realms. But a solution to bridge state data between the two states can be found by using an `internal` realm, ie `internal/v1tov2`, which can be used by the two realms to make sure that they can work on the same data at the same time. 

Corollary: a realm owner, if they update dependencies and the data type of an imported package is destructively changed, will also have to write an upgrade function for that variable if present in the state.

This is just something I'd been thinking upon for the past few weeks, but I think it is a sound proposal to allow upgrades effectively in a manner that is useful for all parties involved in contract creation and usage. Let me know what y'all think!

moul commented 1 year ago

Related discussion on Reddit w/ @progger: https://www.reddit.com/r/Gnoland/comments/13f43zf/comment/jl815x7/

tbruyelle commented 1 year ago

@thehowl Interesting points!

Don't you think it will be difficult and potentially error prone to handle automatic upgrade / detect destructive/non-destructive changes ? Personally I would force the user to provide an upgrade if there's any changes in the variables.

func upgrade(posts []Post) [8]Post { return [8]Post(posts[:8]) }
func upgrade(users []string) (usernames []string) { return users }

I like this kind of API but I fear it can be too restrictive. Say your new data format is a concatenation of both Post and users, with this API you don't have access to them in the same function. Moreover after multiple upgrades, it becomes probably difficult/impossible to determine what upgrade function belongs to what version (at least for the dev/reviewer).

I prefer the second version in the init, but I wonder how do you set the stateVersion, manually ? Maybe we can just store somewhere the number of upgrades of a realm, then, during an upgrade, the VM looks for an upgrades []func() variable in the realm, and executes all the func() with an index equal or higher than the number of upgrades. That means the loop you wrote in the init is actually executed automatically by the VM after an upgrade.

So from a dev perspective, he only needs to provide that variable to handle upgrades:

var upgrades = []func(){
                // probably nothing at index 0 because there's no upgrade for version 0
        1: func() {
                   // upgrade for version 1 
                },
        2: func() { 
                   // upgrade for version 2 
                },
}

But then I wonder how do you access the old format of the realm data ? In such function, you only have access to the new format right ?

thehowl commented 1 year ago

Good points @tbruyelle. I'll get the "easier" one out of the way;

I prefer the second version in the init, but I wonder how do you set the stateVersion, manually ? Maybe we can just store somewhere the number of upgrades of a realm, then, during an upgrade, the VM looks for an upgrades []func() variable in the realm, and executes all the func() with an index equal or higher than the number of upgrades.

Well, my idea with the stateVersion pattern is that, when the state is updated, stateVersion == len(upgrades). So the "current" state version is simply determined by the number of upgrades in the array; the sub-slice upgrades[stateVersion:] represents the "missing upgrades" after each upgrade, instead. Though I would advise against having sequential upgrades like this as the default way to do upgrades, hardcoded in the VM which automatically checks for such an upgrades variable; the reason for this is that I think it's useful to have other kinds of systems for when codebases grow larger; one small improvement, for instance, is that of going from using IDs to UNIX timestamps for identifying upgrades (works better for collaboration, as the IDs become less ambiguous); this is similar to what I've seen in RoR and Laravel for db upgrades.

But then I wonder how do you access the old format of the realm data ? In such function, you only have access to the new format right ?

Yeah, which is why I was proposing using the init-stateVersion paradigm only for non-variable upgrades.

I think another proposal could be: only have init-stateVersion upgrades, but then have reflection on a realm's state which can also access a previous version's state. It's possibly a good idea: I can imagine some code like the below which to me looks potentially powerful and a good solution; but it does feel more cumbersome to write "simple" upgrades

import "reflect"

var stateVersion int
var var1 int

func init() {
  // ...
  lv := reflect.RealmHistory().Last()
  var1 = lv.Value("oldVar1").Interface().(int)
  // ...
}

As for the original func upgrade proposal...

Don't you think it will be difficult and potentially error prone to handle automatic upgrade / detect destructive/non-destructive changes ? Personally I would force the user to provide an upgrade if there's any changes in the variables.

I think that from a usability perspective, some variable upgrades I shouldn't need to specify how they happen. Ie. if I upgrade struct{ a int } to struct{ a int; b string }, I think that if I don't say anything, Gno should just initialize b to the zero value, in all old variables. I do think though that you have a point; maybe the default should happen only in "obvious" circumstances? (int8 -> int16 is obvious; but even int8 -> uint8 and viceversa is not, even if all values can be non-destructively converted)

Say your new data format is a concatenation of both Post and users, with this API you don't have access to them in the same function. Moreover after multiple upgrades, it becomes probably difficult/impossible to determine what upgrade function belongs to what version (at least for the dev/reviewer).

Yes, I think that eventually old upgrade functions would have to be "cleaned up" in the source code; or if anything relegated to files which also have a mechanism similar to //go:build to make sure that they are only executed on certain versions of the realm. I do think this is already becoming more cumbersome than it's worth, and right now I'm actually leaning towards a combination of obvious upgrades and reflection-based upgrades as the better solution

ajnavarro commented 1 year ago

I would propose a simpler solution, and it is keeping the migration to be done by the realm itself. We can do it using semantic versioning on the Realm path as Go does:

we have our realm v1: gno.land/r/myorg/myrealmname
we update our realm to a new version that is not compatible with the old one: gno.land/r/myorg/myrealmname/v2
Calls to the realm gno.land/r/myorg/myrealmname will be redirected to the latest version, folder v2 in that case.
v2 realm implementation can call previous realm calls to get data from a specific user and do some processing to remove it from the previous realm version, and add it using the new realm format:

// this is inside v2 package

package myrealmname

import "gno.land/r/myorg/myrealmname/v2/compat" import old "gno.land/r/myorg/myrealmname"

func MyRealmCall(id string, filters string) string { result := old.MyRealmCall(id) // Note that this call is old and it does not have filtering options if result != "" { // This user had the data stored on the previous realm version, getting it fr := compat.Filter(result, filters) // Compatibility layer
// TODO add more code here to migrate old data to the new realm
}

// New code getting information from this realm

}



WDYT? I think that using this approach is diverging less from the standard Go World and will be easier to understand for newcomers.

moul commented 1 year ago

Update on p/ vs. r/:

Technical Distinctions:

Realms (r/) can possess states and assets.
Realms are visible in the realm stack (e.g., PrevRealm, CurrentRealm).

Conceptual Comparisons:

Think of r/ as analogous to an app store, where each app doubles as an API for users.
Conversely, p/ is reminiscent of platforms like NPM or GitHub, acting as a repository of packages tailored for developers.

This subtle distinction implies variances in how we approach versioning and package upgrades. For instance, barring security concerns that may warrant marking a package as deprecated, it's generally acceptable to retain an outdated version of a p/ dependency. The only foreseeable issue is when passing structs from p/ to another contract due to inconsistencies in the struct versions. This can be addressed by implementing interfaces and the anticipation that primary packages remain static given their simplicity and completeness.

Package Deployment & Versioning:

In p/, packages deploy in the format p/<namespace>/<packagename>.
A DAO feature will allow for nominating a package with a single keyword (e.g., p/multierr). The DAO will likely endorse packages that are minimalistic and exhibit completion. Such packages, anticipated to be the de facto standard library of the gno ecosystem, should undergo stringent review, especially concerning security.

For p/, a space exists where developers upload packages in the p/<namespace>/<packagename> structure. They can either manage versioning independently, or the system might automatically append a version suffix (like /v1, /v2). However, we should steer clear of version upgrade mechanisms like "go get -u" from Go. The rationale is that even if a version seems outdated, it might be due to the newer version offering unused features.

For r/, there's a necessity to simplify versioning. Automatic version suffixes and intuitive interfaces for highlighting the latest versions would be beneficial. Moreover, as the ecosystem evolves, incorporating migration tools might be pivotal, especially if existing design patterns don't support seamless data transition during a r/ update.

iuricmp commented 12 months ago

Proxy

I found @ajnavarro 's Proxy idea simple to absorb as a dev. "The latest uploaded version is the source of the truth". It is similar to the Proxy Upgrade Pattern used in OpenZeppelin.

User ---- tx ---> Proxy (/r/myorg/myrealmname) ----------> Implementation_v0 (r/myorg/myrealmname@somve_versioning_0)
                                 |
                                  ------------> Implementation_v1 (r/myorg/myrealmname@somve_versioning_1)
                                 |
                                  ------------> Implementation_v2 (r/myorg/myrealmname@somve_versioning_2)

Limitations

About the upgrade limitations mentioned by @thehowl here I think it's totally acceptable due to the complexity of its nature. See that OpenZeppelin implemented a similar restriction approach to apply some limits over the state but allow function signature changes.

Due to technical limitations, when you upgrade a contract to a new version you cannot change the storage layout of that contract. This means that, if you have already declared a state variable in your contract, you cannot remove it, change its type, or declare another variable before it. In our Box example, it means that we can only add new state variables after value (the previous state variable). Fortunately, this limitation only affects state variables. You can change the contract’s functions and events as you wish.

Versioning

For r/, there's a necessity to simplify versioning.

Indeed. I suggest a simple mechanism to attach a timestamp to the file name when creating or updating it, preserving its myrealmname only to be used as "Proxy". Pattern: realm_name/@creation_date.

User ---- tx ---> Proxy (/r/myorg/myrealmname) ----------> Implementation_v0 (r/myorg/myrealmname/@1665128003)
                                 |
                                  ------------> Implementation_v1 (r/myorg/myrealmname/@1691393603)
                                 |
                                  ------------> Implementation_v2 (r/myorg/myrealmname/@1694072003)

moul commented 8 months ago

Was discussing with @kristovatlas; here is an update on how it could work.

Someone writes p/<handle>/mypkg/v1, the first version of a pure package. Then, they write p/<handle>/mypkg/v2. The v1 version remains unchanged and cannot be deleted or upgraded. We can add a way for the author to inform users about v1. Additionally, we can develop tools to facilitate consistent versioning, such as automatically adding a /vX suffix.

From a chain perspective, we can assist packages by implementing a changelog, a concept of "latest" version, and a shortcut like a symbolic link (ln -s). For example, the author can choose which version is displayed when importing without specifying a version: p/<handle>/mypkg.

If a contract imports p/ without specifying a version, we need to modify the contract or use gno.mod to fix the version during upload (addpkg). This allows for implicit versioning during development, but explicit imports in the contract.

A DAO can elect packages to become more official and create an alias from p/mypkg to p/<handle>/mypkg/v2, effectively creating a "stdlibs extension" called p/single-short-keyword. These aliases are not intended to be upgradeable, unless a migration strategy is devised for previously used contracts. It is likely that vanity aliases will never be updated.

Regarding r/, r/<handle>/mydapp functions as an app store, and apps should support new features or be capable of upgrading their imports. We plan to allow developers to upgrade their r/ contracts, potentially by utilizing aliases so that each version remains available but only one is active. Data migration features will also be implemented on the chain to facilitate migrations in a single transaction.

We are considering the introduction of different contract "classes." One class could be "flexible," allowing authors to easily change their contract. Another class could be "fused," where privileges are permanently dropped. A third class could be managed by a DAO, involving slow governance decisions.

In addition to upgrade and versioning features, we will focus on making these aspects transparent, providing automated disclaimers for recent changes, contracts, and outdated versions.

harry-hov commented 8 months ago

Here's what I think:

Versioning

I propose using SemVer(without pre-releases and metadata) for realm and packages.

p/{handle}/mypkg@v1.0.0 r/{handle}/myrealm@v1.0.0

Importing

You can just import any package/realm simply by using import path, version will be resolved and locked by gno.mod file

Note:

Makes gno.mod mandatory.
gno.mod will be uploaded and respected onchain too.
replace directive in gno.mod will be for local testing only (means you won't be able to publish package/realm with gno.mod file containing replace directive)
Deprecated realms cannot be imported

Example:

// foo.gno
package foo

import "gno.land/p/{handle}/mypkg"

[...]

// gno.mod
module gno.land/r/{handle}/foo

require (
    gno.land/p/{handle}/mypkg v1.0.0
)

Upgrading

Packages

New version of packages can be publised without any hard restrictions. It will not affect old/existing versions.

means p/{handle}/mypkg@v1.0.0 and p/{handle}/mypkg@v2.0.0 can exist together.

Realms

For realms, new versions can be publised but it will mark old/existing versions as deprecated. Realms importing old version of realm will also be marked as deprecated.

e.g: r/{handle}/myrealm@v1.0.0 deprecated r/{handle}/myrealm@v2.0.0 deprecated r/{handle}/myrealm@v3.0.0 available (cos latest)

When publishing upgrade to the realm, they need to provide migrate.gno file to migrate the state. This file will be excecuted only once (while upgrading).

e.g.

// migrate.gno
// r/users : v1.0.0 -> v2.0.0
package migrate

import (
    v100 "gno.land/r/{handle}/users"
)

// type User struct {
//  Address std.Address
//  Username    string
//  + Name string // added in upgrade
// }

func Migrate() {
    oldUsers :=  v100.Users
    // var Users *[]User // global var, aka state
    for i := range oldUsers {
        Users := append(Users, &User{
            Address: oldUsers[i].Address
            Username: oldUsers[i].Username
            Name: oldUsers[i].Username // set Name to Username
        })   
    }
}

Few more options to consider:

Deprecated or read-only? (the wording)
Have some kind of limitation on number of realms upgradation. e.g realms can be upgraded once a month?
Have some kind of governance, proposal, or voting system for realm upgradation.

ajnavarro commented 8 months ago

The v1 version remains unchanged and cannot be deleted or upgraded.

@moul If I understood correctly, I don't think this is a good idea. Even when a new major version is released, we should allow bug-fix releases for previous major versions, for security reasons. We can retract versions using gno.mod if needed, see below.

Adding to @harry-hov comment, Go modules spec is providing the needed tools to deprecate and/or detract previous versions:

Retract: https://go.dev/ref/mod#go-mod-file-retract
Deprecate: https://go.dev/ref/mod#go-mod-file-module-deprecation

When publishing upgrade to the realm, they need to provide migrate.gno file to migrate the state. This file will be excecuted only once (while upgrading).

@harry-hov the problem I see with this approach is, who is gonna pay the cost of the migration?

wwqiu commented 8 months ago

Even when a new major version is released, we should allow bug-fix releases for previous major versions, for security reasons.

@ajnavarro I think that the immutability of smart contracts post-deployment is a sensible design choice, as it ensures transparency and plays a crucial role in establishing trust among users. When it comes to security vulnerabilities, deploying new versions of contracts offers a viable solution for addressing these issues. Making contracts upgradable for security reasons introduces the potential for malicious actors to exploit this flexibility, effectively trading one set of security concerns for another.

ajnavarro commented 8 months ago

@wwqiu, my point aligns with what I've previously mentioned about generating new versions and addressing problems in older ones.

From what I've gathered based on @moul's input, once a new major version (e.g., v2.x.x) is introduced, the addition of earlier major versions (e.g., v1.x.x) becomes prohibited. However, my argument is in favor of continuing to allow the inclusion of any version that developers need.

It goes without saying that, as with all package managers, version numbers are fixed and cannot be altered.

When we say that a contract is upgradable, we talk about creating a new version for that contract, not modifying previous versions.

wwqiu commented 8 months ago

When we say that a contract is upgradable, we talk about creating a new version for that contract, not modifying previous versions. @ajnavarro I understand your point. But still, my question remains the same: could this potentially introduce new security issues, and has there been an assessment in this regard? If not, I lean towards keeping it simple.

thehowl commented 8 hours ago

We won't be implementing specific versioning features for Gno.land's mainnet.

Getting versioning right is complex and hard to do while we still don't have clear examples of packages that have had to go through a migration strategy. For now, many upgrade patterns are possible manually, which can fit different use cases:

https://github.com/gnolang/gno/tree/master/examples/gno.land/r/x/manfred_upgrade_patterns

One thing we can likely look to do before we launch are private packages: #2191.

These are simple to implement and can possibly guarantee some degree of beta testing on-chain. Although, it should be noted, these aren't meant for any kind of quick development or early iterations: those who intend to work on realms still in development should deploy them on machines using gnodev - which is going in the direction of replacing the portal loop - see #2866.

I'll be closing this issue as it contains some discussions which can be useful for context; but ultimately of relative importance when we'll finally have a proposal for versioning which considers the real-life use cases and scenarios faced by users on the live gno.land chain.

gnolang / gno

[RFC] Contract versioning (upgrade/proxy) #694

Context

Current proposal and open questions

Proxy

Limitations

Versioning

Versioning

Importing

Upgrading

Packages

Realms