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

feat: add std.CreatePackage #715

Closed
wants to merge 3 commits into from

Conversation

mconcat
Copy link

@mconcat mconcat commented Apr 7, 2023

Description

Dynamically create a package from an existing package code, similar to Solidity new keyword.

Design decisions:

  • Add std.CreatePackage native binding. The CreatePackage takes three arguments, the template package path(gno code to duplicate from), the new path for the package, and the initial arguments for init() function for the package.
  • Use Store.GetMemPackage() to retrieve an existing package code. Unlike Store.GetPackage(), this methods returns the raw strings(without the realm state) for a package code, the same type that user provides when creating a package.
  • After the package is deployed, iterate through the result PackageNode.FileSet.Files[i].Decls[j].(*FuncDecl).NameExpr.Name. Filter all exported(first letter capitalized non-method) functions.
  • Create a gnolang struct with methods that internally calls std.CallPackage(temporary name) targeting for corresponding exported functions. It will spin off new machine, load realm, and execute the function with the provided arguments. Will be implemented in a separated PR.
  • std.CreatePackage will return this dynamically created gnolang interface, PackageInterface, in a form of interface{}.
  • The user(contract invoked std.CreatePackage) will type assert the interface with appropriate GRC interface. The type assertion logic will automatically check if the new package satisfies the interface(and panic if not).
  • The package struct could be stored in a variable or a mapping to be invoked later.

User ergonomic example:

package tokenfactory

import (
  "gno.land/r/std/grc20"
)

var tokens map[string]grc20.Interface

func init() {
  tokens = make(map[string]grc20.Interface)
}

func DeployTokenContract(tokenName, tokenSymbol string, maxSupply bigint) string {
  tokenContract := std.CreatePackage("std/grc20", tokenName, maxSupply).(grc20.Interface)
  tokens[tokenName] = tokenContract
  return tokenContract.PackageName()
}

func SendToken(tokenName string, from, to string, amount bigint) {
  tokenContract := tokens[tokenName]
  tokenContract.Send(from, to, amount)
  // Internally calls std.CallPackage("tokenfactory/{tokenName}/grc20", "Send", from, to, amount)
}

The same UX can be used for dynamically importing a package - std.ImportPackage() can return a PackageInterface that maps function call to exported functions in the package. For statically importing a package, typical golang style top level import is more secure. ref: #566

How has this been tested?

Please complete this section if you ran tests for this functionality, otherwise delete it

@moul
Copy link
Member

moul commented Apr 8, 2023

Thank you for sharing your solution. It's great to see that you've implemented it in a lightweight and effective manner. I appreciate your effort.

As we move forward, we should consider how we can use similar rules and filters to whitelist paths (#384). Nevertheless, your implementation is an excellent starting point.

Would it be possible for you to include unit tests in your pull request? Thank you.

Copy link
Member

@moul moul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking due to big addition and multiple considerations.

@jaekwon
Copy link
Contributor

jaekwon commented Apr 18, 2023

Thank you for the submission. It is pretty cool.

But I think it would be best if we moved away from template package generation. Can't we just use a factory pattern, say?

std.CallPackage() violates type-checking and I don't think should be offered to the VM.

Is this returning a DeclaredType with methods, to satisfy the GRC interface?

I think what you're trying to do here is pretty cool, and I wonder if we can maintain it as a fork or +gotag in a separate file so it doesn't get included in applications that don't want the dynamic features. OTOH the dynamism is cool and worth exploring.

I like the idea that we try to get developers to write beautiful re-usable modules without having to resort to code template/copy-paste or .Eval() or .CallPackage(); if they can be expressed purely within the language of Go (example; factory pattern), it would create more portable and intuitive solutions; and be safer too.

For example, there may come a day where we need to quarantine or freeze or attend to buggy modules, and we would want to be able to scan all the code and identify which modules depend on the broken one. I think the intent of the dynamism of .CallPackage() would make this more difficult, right?

@mconcat
Copy link
Author

mconcat commented Apr 19, 2023

Having a typecheck on function call between realm boundaries(or any function call, in general) is necessary to avoid any type checking, I also think we should support it. The dynamism of wrapping the inter package function call is purely for developer convenience and could be moved out from the std.

I think we still need a way to create and invoke other packages in a form of golang function call, namely, having a synchronous function call with immediate return value, while preserving the execution context. This will enable complex contract patterns that are already usable (for example in Solidity) like proxy or factory contracts. Using template is basically "forking" a code from an existing package which is more safe than deploying static bytecode in Solidity.

Since go/gno does not support first class packages, the way we can implement such typesafe function call to other package is to mimic a package-value. To construct a such proxy struct for calling other packages without using std functions like CallPackage() we may need to utilize go2gno to encapsulate the pseudo-package in the golang level.

EDIT: is package types considered a first class value? if so, we can simply return a package value to represent a foreign package, and route any function call to it to the target package.

@moul moul changed the title feat: CreatePackage feat: add std.CreatePackage Apr 19, 2023
// Returns if the name is an exported identifier,
// according to the golang's canonical exported name rule,
// namely, starting with a capitalized alphabet.
func (name Name) IsExportedName() bool {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
func (name Name) IsExportedName() bool {
func (name Name) IsExported() bool {

@jaekwon
Copy link
Contributor

jaekwon commented Apr 19, 2023

Having a typecheck on function call between realm boundaries(or any function call, in general) is necessary to avoid any type checking, I also think we should support it. The dynamism of wrapping the inter package function call is purely for developer convenience and could be moved out from the std.

I think we still need a way to create and invoke other packages in a form of golang function call, namely, having a synchronous function call with immediate return value, while preserving the execution context. This will enable complex contract patterns that are already usable (for example in Solidity) like proxy or factory contracts. Using template is basically "forking" a code from an existing package which is more safe than deploying static bytecode in Solidity.

See #566 (comment).

We want to discourage the usage of templates. There is probably a better way to implement what is needed purely in Gno/Go, like a factory constructor. So that's what we should encourage, not bad patterns from the EVM.

Since go/gno does not support first class packages, the way we can implement such typesafe function call to other package is to mimic a package-value. To construct a such proxy struct for calling other packages without using std functions like CallPackage() we may need to utilize go2gno to encapsulate the pseudo-package in the golang level.

There is a PackageValue already, so there is a "right" way to implement the notion of a package value that can be used dynamically. I think it's a worthy experimental fork to maintain, but not something that should go into mainline gno.land, at least not for the usual smart contracts.

EDIT: is package types considered a first class value? if so, we can simply return a package value to represent a foreign package, and route any function call to it to the target package.

Yes exactly what I meant. But we explicitly don't want this for gno.land, and instead want to support patterns with all package dependencies declared up front in the code, and we don't want to support Call.

It might be best to maintain an experimental fork of Gno to keep much of the complexity separate. We can flesh out the PackageValue/PackageType op_erations in this project, but it would be best if we don't directly support it in this project, so as to dis-incentivize solidity patterns and encourage pure go ones instead.

@jaekwon jaekwon self-requested a review April 19, 2023 23:40
// - Array and Struct are moveable across realm only if they are composed of moveable types(recursively applied). The value will be copied to the callee realm.
// - Slice are moveable across realm only if they are referring to moveable types(recursively applied)(not implemented yet).
// - Any other cases require some complex inter-realm object management scheme, low priority.
func (tv *TypedValue) IsMoveableAcrossRealm() bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a misnomer because there is already a notion of what is passable between realms as already implemented... that is, one can already call a realm function from another realm, passing in objects, as long as objects aren't sharing persistence. What is implemented here conflicts with that.

I think what you maybe want is IsMoveableAcrossChains or IsIBCArgument, and for that for now I think it should be limited to a list of stringified primitives only, no structs;

and then we need to have a whole series of conversations to evolve that into something that either supports the EVM ABI, or some other interop standard.

Copy link
Contributor

@jaekwon jaekwon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to continue with this path of development, I think it would be best done on a fork of Gno. These are the kinds of experiments we want to see around Gno.

Different ideas will emerge around dynamism of packages and calls, so while some things like implementation of Op* for PackageValues and other common logic can live in Gno, but the details of the implementation of Call* or CreatePackage should live in the fork.

@jaekwon jaekwon mentioned this pull request Apr 19, 2023
@jaekwon jaekwon closed this Apr 20, 2023
@jaekwon jaekwon mentioned this pull request Apr 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants