Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat(gnovm): improved native bindings (#859)
This PR provides an initial transition and groundwork to move from native bindings as "package injections", whereby an injector function is called after package parsing and loading to provide built-in natives, to the new design described in #814. ## Status The PR is currently ready for review. - [x] Fix all tests and ensure precompiling has no issues - [x] Find out if the errors that arise when running the tests with `DEBUG=1` are bugs introduced by this PR or not - [x] Create temporary exceptions for linked types (like `std.{Coins,Address,Realm}`), so that all normal native bindings run exclusively on the new system, awaiting `//gno:link` (see #814). - [x] Finding a permanent place for ExecContext (see comment on `gnovm/stdlibs/context.go`); suggestions welcome (tm2/vm?) - [x] Include code generation in Makefiles and add a CI check to make sure PRs do generate code when necessary - [x] And some unit tests for the new functionality / potential edge cases. - [x] make sure all injected functions have `// injected` as a comment Next steps, after merging this PR, include: - Supporting precompilation of native functions by linking directly to the Go source in stdlibs/, removing the necessity of `stdshim`. - Supporting the test-only tests/stdlibs/ directory in `gno doc`; as well as documenting native functions explicitly by adding the `// injected` comment after their declaration. - Making all of the native-only packages defined in tests/imports.go native bindings; so that the special packages and native values are restricted only to ImportModeNativePreferred, while everything else uses documentable native bindings also for tests. ## Implementation Summary A new API has been added to `gnolang.Store`: `NativeStore`. This is a simple function which resolves a package and a name of a function to a native function.[^1] This is used to resolve native bindings first and foremost in `preprocess.go`, which also adds information about the native binding in `NativePkg` and `NativeName`, two new fields in `*gnolang.FuncValue`. The purpose of `NativePkg` and `NativeName` is to 1. enable assigning native functions to other function types, including function parameters or func type variables and 2. being able to easily and effectively persist the FuncValue during realm de/serialization. This way, `op_call.go` can recover the value of `nativeBody` if it is nil. On the other side, examples of the new stdlibs can be seen in the `gnovm/stdlibs` directory, where everything except a few functions in package `std` have been ported to the new system. The code generator in `misc/genstd` parses the AST of both the Go and Gno files, and ultimately generates `native.go`, which contains the porting code from the Gno function call to Go and back. To support the test contexts, which needs to have different function implementation or entirely new functions, an additional stdlibs directory was added to the `tests` directory. This is still code generated with the same generator and follows the same structure. When stdlibs are loaded for tests, the code in tests/stdlibs/, if it exists, is loaded with "higher priority" compared to normal `stdlibs` code.[^2] ### Suggested order for reviewing (and some notes) 1. `pkg/gnolang` for the core of the changes, starting from `values.go`, `preprocess.go`, `op_call.go`, `store.go`, `realm.go` which show how NativePkg, NativeName and nativeBody interact * `gnovm/pkg/gnolang/op_eval.go` adds the type of the evaluated expression, as I think it's useful to understand in what terms something is being evaluated; specifically I added it to better distinguish between `DEBUG: EVAL: (*gnolang.CallExpr) std<VPBlock(3,0)>.TestCurrentRealm()` and `DEBUG: EVAL: (*gnolang.SelectorExpr) std<VPBlock(3,0)>.TestCurrentRealm`. I think it's useful, but we can remove it if we prefer to keep it simpler. * `gnovm/pkg/gnolang/op_expressions.go` adds a new debug print pretending we're popping and pushing the value stack. This is to make clear what's happening in the debug logs (we're really swapping the value at the pointer), and I've made it `v[S]` so it's clear that the log is not coming from `PushValue/PopValue`. * I removed a goto-loop in `gnovm/pkg/gnolang/values.go` because I don't see it as beneficial when it's essentially just doing a for loop; if the concern was inlining, even with goto the function is too "costly" for the go compiler to inline. 2. `misc/genstd` contains the code generator and the template it uses to generate the files, ie. `gnovm/stdlibs/native.go` and `gnovm/tests/stdlibs/native.go`. 3. `stdlibs` and `tests/stdlibs` changes show the new stdlib directories, which genstd uses to generate code. The `tests/stdlibs` is available only in test environments, and there are also some overriding definitions to those in normal stdlibs. * `gnovm/stdlibs/encoding/base64/base64.gno` removes a check on `strconv.IntSize`. This is because `strconv.IntSize` is dependent on the system and thus is non-deterministic behaviour on the Gnovm. I don't know if `int` in Gno is == Go's `int` or it is precisely defined to be 64-bits; in any case I don't think it should be dependent on the system as this wouldn't work in a blockchain scenario. 4. Changes to `tm2` and `gnovm/tests/imports.go` mostly relate to wiring up the new NativeStore to the gnovm where it's used. 5. Changes to `examples` and `tests/files` (except for float5) are all updates to golden tests. <details><summary>Checklists...</summary> <p> ## Contributors Checklist - [x] Added new tests, or not needed, or not feasible - [x] Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory - [x] Updated the official documentation or not needed - [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message was included in the description - [x] Added references to related issues and PRs - [x] Provided any useful hints for running manual tests ## Maintainers Checklist - [ ] Checked that the author followed the guidelines in `CONTRIBUTING.md` - [ ] Checked the conventional-commit (especially PR title and verb, presence of `BREAKING CHANGE:` in the body) - [ ] Ensured that this PR is not a significant change or confirmed that the review/consideration process was appropriate for the change </p> </details> [^1]: Currently, the only native bindings that are supported with the new system are top-level functions. I think we can keep it this way in the spirit of [KISS](https://en.wikipedia.org/wiki/KISS_principle); maybe think of extending it if we find appropriate usage. [^2]: See gnolang.RunMemPackageWithOverrides --------- Co-authored-by: Thomas Bruyelle <thomas.bruyelle@gmail.com> Co-authored-by: jaekwon <jae@tendermint.com>
- Loading branch information