chore: import secp256k1 for amino parsing

[albttx]

Amino failed to parse crypto.PubKey

Description

This simple client isn't working to get account because amino don't know the struct BaseAccount.Pubkey crypto.PubKey

This simple main can be fixed by importing, i believe it will make dev experience easier by putting it in std rather than clients

_ "github.com/gnolang/gno/tm2/pkg/crypto/secp256k1"

package main

import (
	"fmt"

	"github.com/gnolang/gno/tm2/pkg/amino"
	rpcClient "github.com/gnolang/gno/tm2/pkg/bft/rpc/client"
	"github.com/gnolang/gno/tm2/pkg/std"
)


// Client is the TM2 HTTP client
type Client struct {
	client rpcClient.Client
}

// NewClient creates a new TM2 HTTP client
func NewClient(remote string) *Client {
	return &Client{
		client: rpcClient.NewHTTP(remote, ""),
	}
}

func (c *Client) GetAccount(address string) (std.Account, error) {
	path := fmt.Sprintf("auth/accounts/%s", address)

	queryResponse, err := c.client.ABCIQuery(path, []byte{})
	if err != nil {
		return nil, fmt.Errorf("unable to execute ABCI query, %w", err)
	}

	var queryData struct{ BaseAccount std.BaseAccount }

	if err := amino.UnmarshalJSON(queryResponse.Response.Data, &queryData); err != nil {
		return nil, err
	}

	return &queryData.BaseAccount, nil
}

func main() {
	c := NewClient("http://rpc.gnochess.com:80")
	acc, err := c.GetAccount("g1x90eh5ejc22548hjqznm2egyvn8ny36lqu460f")
	fmt.Println(acc, err)
}

runtime: goroutine stack exceeds 1000000000-byte limit
runtime: sp=0xc020780390 stack=[0xc020780000, 0xc040780000]
fatal error: stack overflow

runtime stack:
runtime.throw({0x9842fa?, 0xd4dea0?})
        /nix/store/akhjsmrrsakcnj8x3xgygvizhccbyn0v-go-1.19.3/share/go/src/runtime/panic.go:1047 +0x5d fp=0x7f681368cc18 sp=0x7f681368cbe8 pc=0x43907d
runtime.newstack()
        /nix/store/akhjsmrrsakcnj8x3xgygvizhccbyn0v-go-1.19.3/share/go/src/runtime/stack.go:1103 +0x5cc fp=0x7f681368cdd0 sp=0x7f681368cc18 pc=0x452d0c
runtime.morestack()
        /nix/store/akhjsmrrsakcnj8x3xgygvizhccbyn0v-go-1.19.3/share/go/src/runtime/asm_amd64.s:570 +0x8b fp=0x7f681368cdd8 sp=0x7f681368cdd0 pc=0x46a32b

goroutine 1 [running]:
fmt.(*fmt).padString(0xc00de1ff20?, {0x8961be, 0x7})
        /nix/store/akhjsmrrsakcnj8x3xgygvizhccbyn0v-go-1.19.3/share/go/src/fmt/format.go:108 +0x299 fp=0xc0207803a0 sp=0xc020780398 pc=0x4dac39
fmt.(*fmt).fmtS(0xc0207803f8?, {0x8961be?, 0x8961bd?})
        /nix/store/akhjsmrrsakcnj8x3xgygvizhccbyn0v-go-1.19.3/share/go/src/fmt/format.go:359 +0x3f fp=0xc0207803d8 sp=0xc0207803a0 pc=0x4db75f
fmt.(*pp).fmtString(0x8ca000?, {0x8961be?, 0x8ca000?}, 0x0?)
        /nix/store/akhjsmrrsakcnj8x3xgygvizhccbyn0v-go-1.19.3/share/go/src/fmt/print.go:474 +0x86 fp=0xc020780428 sp=0xc0207803d8 pc=0x4de566
fmt.(*pp).handleMethods(0xc00de1fee0, 0x100800?)
        /nix/store/akhjsmrrsakcnj8x3xgygvizhccbyn0v-go-1.19.3/share/go/src/fmt/print.go:65

[....] It's a stack overflow due to recursion

Contributors' checklist...

• Added new tests, or not needed, or not feasible
• Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory
• Updated the official documentation or not needed
• No breaking changes were made, or a BREAKING CHANGE: xxx message was included in the description
• Added references to related issues and PRs
• Provided any useful hints for running manual tests
• Added new benchmarks to generated graphs, if any. More info here.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (5a0ff55) 56.30% compared to head (3bef735) 56.06%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1178      +/-   ##
==========================================
- Coverage   56.30%   56.06%   -0.24%     
==========================================
  Files         422      421       -1     
  Lines       65699    65457     -242     
==========================================
- Hits        36993    36700     -293     
- Misses      25821    25891      +70     
+ Partials     2885     2866      -19     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[thehowl]

LGTM; though we may want to wait on #1117 in order to add a regression test. (start node -> try go run on the small script you made)

[moul]

I approve the fix. I'm unsure about the ideal location for the anonymous import. Perhaps it would make more sense to place it in the rpc/client. It's fine to merge it from my end.

By the way, it would be great to have a small unit test. The test should involve creating a minimal client and attempting to unmarshal something with an account key.

[piux2]

This file is intended to be used as a library and does not contain a main() function. For compatibility with Amino, each package invokes Package = amino.RegisterPackage() at the package level. This registers Amino types with the global codec, referred to as gcdc

In other word the func main() suppose to use out side tm2/pkg.

[thehowl]

This file is intended to be used as a library and does not contain a main() function. For compatibility with Amino, each package invokes Package = amino.RegisterPackage() at the package level. This registers Amino types with the global codec, referred to as gcdc

In other word the func main() suppose to use out side tm2/pkg.

@piux2 Okay, so what solution are you proposing, I don't understand?

The issue is that there is a bad dev experience if somebody tries to amino.UnmarshalJSON from a very basic example which Albert has kindly provided. I understand why we need the blank import (the amino.RegisterPackage call you mentioned); but what this PR is about is adding "sane defaults". (Obviously, part of the issue is that the error message is incredibly unhelpful, but this was fixed by #1179).

Can you make a compelling case for why the users should do the blank import, rather than having this transitively imported when importing either std or rpc/client packages? (Or any other place which makes sense, imported by Albert's sample file?)

[piux2]

This file is intended to be used as a library and does not contain a main() function. For compatibility with Amino, each package invokes Package = amino.RegisterPackage() at the package level. This registers Amino types with the global codec, referred to as gcdc
In other word the func main() suppose to use out side tm2/pkg.

@piux2 Okay, so what solution are you proposing, I don't understand?

The issue is that there is a bad dev experience if somebody tries to amino.UnmarshalJSON from a very basic example which Albert has kindly provided. I understand why we need the blank import (the amino.RegisterPackage call you mentioned); but what this PR is about is adding "sane defaults". (Obviously, part of the issue is that the error message is incredibly unhelpful, but this was fixed by #1179).

Can you make a compelling case for why the users should do the blank import, rather than having this transitively imported when importing either std or rpc/client packages? (Or any other place which makes sense, imported by Albert's sample file?)

@thehowl The short answer is

crypto.PubKey is an interface, not a struct.

We want to load all concrete type of the PubKey in account.go so that our code and can automatically pick the right concrete PubKey type when we unmarshal the std.BaseAccount

_ "github.com/gnolang/gno/tm2/pkg/crypto/secp256k1"
_ "github.com/gnolang/gno/tm2/pkg/crypto/ed25519"
_ "github.com/gnolang/gno/tm2/pkg/crypto/multisig"
_ "github.com/gnolang/gno/tm2/pkg/crypto/mock"

[thehowl]

We want to load all concrete type of the PubKey in account.go so that our code and can automatically pick the right concrete PubKey type when we unmarshal the std.BaseAccount

Alright, sorry, sometimes hard to understand your concrete proposal :) (pun very intended)

Agreed on your plan of action!

[albttx]

Thanks @piux2 i love the idea, just updated the list of imported packages

[piux2]

Thanks @piux2 i love the idea, just updated the list of imported packages

@albttx Good work!

Let's add import these in std/account.go. It is close to crypto.PubKey used in the BaseAccount
The package.go file in each directly is purely used for genproto to generate Amino compatible proto message file.

[thehowl]

ping @albttx for integrating @piux2's requested changes :)

[albttx]

@thehowl merged master, formatted and udpated :)

[albttx]

@piux2 Waiting for your approval for a merge :)