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

Realm with "import encoding/json" can't be addpkg via gnokey, but manually #808

Closed
r3v4s opened this issue May 8, 2023 · 8 comments
Closed
Assignees
Labels
🐞 bug Something isn't working 📦 🤖 gnovm Issues or PRs gnovm related

Comments

@r3v4s
Copy link
Contributor

r3v4s commented May 8, 2023

Realm with "import encoding/json" can't be addpkg via gnokey, but manually

Description

certain package with import encoding/json can not be add to chain via gnokey maketx addpkg (it fails with panic unknown import path encoding/json).

but manually adding file to ~/gno/examples/gno.land/r/ works

Your environment

  • OS and version: macOS Ventura
  • version of gno: latest master branch
  • branch that causes this issue (with the commit hash): master

Steps to reproduce

// qjson.gno
package qjson

import (
	"encoding/json"
)
-home ~/.gno \
-remote 127.0.0.1:26657 \
-broadcast=true \
-chainid dev \
-gas-fee 1ugnot \
-gas-wanted 5000000 \
-memo "" \
-pkgdir ./ \
-pkgpath gno.land/r/demo/qjson \
test1

Expected behaviour

  • adding packing with gnokey maketx addpkg should work

OR

  • calling package(already added by manually) should not work

Actual behaviour

  • adding packing with gnokey maketx addpkg failed

AND

  • calling package(already added by manually) succeed

Additional

Looks like encoding/json is imported from here

@thehowl
Copy link
Member

thehowl commented May 8, 2023

I think duplicate/related with #668?

I think the way native go stdlib bindings are implemented has issues. I'll create a new issue to work on a proposal for a refactor.

@r3v4s
Copy link
Contributor Author

r3v4s commented May 8, 2023

I think duplicate/related with #668?

I think the way native go stdlib bindings are implemented has issues. I'll create a new issue to work on a proposal for a refactor.

oh yes, I forgot to tag related issue & pr. Thank you in advance

@ajnavarro ajnavarro added 🐞 bug Something isn't working 📦 🤖 gnovm Issues or PRs gnovm related labels May 15, 2023
@piux2
Copy link
Contributor

piux2 commented Jun 15, 2023

@r3v4s, what do you get when you precompile and build qjson.gno file using gno?

@r3v4s
Copy link
Contributor Author

r3v4s commented Jun 15, 2023

@r3v4s, what do you get when you precompile and build qjson.gno file using gno?

// Code generated by github.com/gnolang/gno. DO NOT EDIT.

//go:build gno

package qjson

import (
	"encoding/json"
)

@zivkovicmilos
Copy link
Member

zivkovicmilos commented Jul 7, 2023

Hey @thehowl, @r3v4s,

Do we have any updates on this?
Asking because we've had questions about JSON encoding from other implementation partners.

Just want to make sure if someone has taken up digging into this issue, and if not I'll look into it

@thehowl
Copy link
Member

thehowl commented Jul 7, 2023

@zivkovicmilos Essentially, good encoding packages need reflection. Reflection is a complex issue. #814 is a step in the right direction as it allows us to work on multiple "original" gno stdlibs without breaking precompilation and needing to have everything in package std.

Related issues where we've talked about reflection: #750, #802, #788. If you, or anyone else, wants to do a master-issue on the topic it would be appreciated :)

We could do a "dumb" implementation now, but it could only work for primitive types and maybe slices of them, through a big type switch, and support more complex types (including structs, which is often what you'll want to encode) through JSONMarshaler. OR, adapt easyjson to not use unsafe and to generate gno code.

@moul
Copy link
Member

moul commented Jul 12, 2023

Note from meeting:

  • maybe we can support reflect, without unsafe as an internal package and whitelist specific packages such as encoding/json.
  • maybe we have a unsafe_reflect package for unsafe parts and reflect without unsafe.
  • start with minimal reflect package: TypeOf, ValueOf, or at least make sure to remove capabilities to "New*" or to read private fields.

@r3v4s
Copy link
Contributor Author

r3v4s commented Nov 21, 2023

Closing issue for below reason.
@harry-hov asked me about re-producing this issue, and it seems like doesn't occur any more(with current master branch).

Doesn't seems to issue or bug anymore, just json is missing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug Something isn't working 📦 🤖 gnovm Issues or PRs gnovm related
Projects
Status: 🚀 Needed for Launch
Status: Done
Development

No branches or pull requests

6 participants