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: encoding/binary (for encoding/base64) #816

Merged
merged 5 commits into from
Jun 6, 2023

Conversation

r3v4s
Copy link
Contributor

@r3v4s r3v4s commented May 10, 2023

Description

contract with import encoding/base64 is failing when gnokey maketx addpkg
'base64' imports binary but there is non in stdlibs/*

Additional

@thehowl I think this is related to my previous issue #808 and yours #814

@r3v4s r3v4s requested review from jaekwon and moul as code owners May 10, 2023 04:36
@thehowl
Copy link
Member

thehowl commented May 12, 2023

If you're adding bigEndian I think you should also add littleEndian?

Do you have an application where you need these specifically?

@schollz
Copy link
Contributor

schollz commented May 13, 2023

Do you have an application where you need these specifically?

These functions are nice-to-have so that Gno smart contracts could support encoding/decoding of various file formats. For example, I wanted both of these for a .wav file format encoder (https://github.com/schollz/gno/blob/bytebeat/examples/gno.land/p/demo/audio/riff/riff.gno) for developing web3 audio applications (like audio nfts).

If you're adding bigEndian I think you should also add littleEndian?

I'd agree it would be nice to have the package incorporate as much as possible. I've found that this works well: master...schollz:gno:feat/binary-encoding. I won't make another PR for it, but perhaps that can be followed to including the encoding/binary needed for encoding/base64

@ajnavarro ajnavarro added 🐞 bug Something isn't working 📦 🤖 gnovm Issues or PRs gnovm related 🧾 package/realm Tag used for new Realms or Packages. and removed 📦 🤖 gnovm Issues or PRs gnovm related labels May 15, 2023
@jaekwon
Copy link
Contributor

jaekwon commented May 18, 2023

Let's please add little endian too.
If any stdlib functions are modified, please note the changes with XXX as well.
Thank you!

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.

pre-approving but please add little endian too.

@r3v4s r3v4s marked this pull request as draft May 19, 2023 01:55
@github-actions github-actions bot removed the 🧾 package/realm Tag used for new Realms or Packages. label May 19, 2023
@r3v4s r3v4s changed the title feat: small part of encoding/binary for encoding/base64 feat: encoding/binary (for encoding/base64) May 19, 2023
@r3v4s
Copy link
Contributor Author

r3v4s commented May 19, 2023

Let's please add little endian too. If any stdlib functions are modified, please note the changes with XXX as well. Thank you!

thanks to your review.
I've add little endian too with test cases

@r3v4s r3v4s marked this pull request as ready for review May 19, 2023 03:16
@moul
Copy link
Member

moul commented May 19, 2023

@r3v4s
Copy link
Contributor Author

r3v4s commented May 19, 2023

And update this file please: https://github.com/gnolang/gno/blob/master/gnovm/docs/go-gno-compatibility.md#stdlibs

Should I change in this PR? or create new one??

@moul
Copy link
Member

moul commented May 19, 2023

In the same please, it's to take the habit of doing it. Thank you.

@r3v4s r3v4s requested a review from a team as a code owner May 22, 2023 01:08
schollz added a commit to schollz/gno that referenced this pull request May 22, 2023
@moul
Copy link
Member

moul commented May 28, 2023

Please, run make fmt to fix the last small details.

diff --git a/gnovm/stdlibs/encoding/binary/binary.gno b/gnovm/stdlibs/encoding/binary/binary.gno
index 7e99d14e..08f7b3f2 100644
--- a/gnovm/stdlibs/encoding/binary/binary.gno
+++ b/gnovm/stdlibs/encoding/binary/binary.gno
@@ -200,4 +200,4 @@ func (bigEndian) AppendUint64(b []byte, v uint64) []byte {
 
 func (bigEndian) String() string { return "BigEndian" }
 
-func (bigEndian) GoString() string { return "binary.BigEndian" }
\ No newline at end of file
+func (bigEndian) GoString() string { return "binary.BigEndian" }
diff --git a/gnovm/stdlibs/encoding/binary/binary_test.gno b/gnovm/stdlibs/encoding/binary/binary_test.gno
index 05b6c369..5407eb50 100644
--- a/gnovm/stdlibs/encoding/binary/binary_test.gno
+++ b/gnovm/stdlibs/encoding/binary/binary_test.gno
@@ -3,7 +3,6 @@ package binary
 import (
    "bytes"
    "encoding/binary"
-
    "testing"
 )
 
@@ -187,4 +186,4 @@ func TestbigEndianAppendUint64(t *testing.T) {
    if !bytes.Equal(b, []byte{0xfe, 0x14, 0xf5, 0xe9, 0x07, 0xd0, 0x03, 0xe8, 0x0b, 0xb8, 0x07, 0xd0, 0xd0, 0x07, 0xe9, 0xf5}) {
        t.Fatal("bigEndian.AppendUint64 failed")
    }
-}
\ No newline at end of file
+}

@moul moul merged commit e392ab5 into gnolang:master Jun 6, 2023
@moul moul deleted the feat/encoding-binary branch June 6, 2023 08:46
Doozers pushed a commit to Doozers/gno that referenced this pull request Aug 31, 2023
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: Done
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants