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 gno.land/pkg/gnoclient (Gno.land Go client) #1047

Merged
merged 28 commits into from
Jan 18, 2024

Conversation

moul
Copy link
Member

@moul moul commented Aug 11, 2023

This PR add a dedicated client library for interacting seamlessly with the Gno.land RPC API.
This library simplifies the process of querying or sending transactions to the Gno.land RPC API and interpreting the responses.

Closes #1324

Previous list of ideas

Note: Some content may be outdated. Please overlook for the moment, but we'll reassess at the conclusion of this PR to identify any current requirements.

  • port https://github.com/gnolang/gno/tree/master/tm2/pkg/crypto/keys/client into a simple ./gno.land/pkg/gnoclient package (Call, Send, AddPkg, Query, Eval, Package, File)
  • add unit tests so that crypto/keys/client can have less unit tests and be more focused on CLI stuff
  • create mock/testing helpers to make it more useful when writing integration tests -> ./gno.land/cmd/gnoland/*_test.go
  • port a client, i.e., gnofaucet, gnoweb, gnoblog, etc; so they use this new library instead of hardcoded manual clients
  • consider having right now underlying tm2client and gnovmclient libs that this lib will depend on, but will probably do it later and start with a monolith library
  • alternative configuration (pass existing websocket?)
  • minimal go.mod to make it light to import
  • open issue on other clients (gnoblog, discord faucet, gnomobile)
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.

Signed-off-by: Manfred Touron <94029+moul@users.noreply.github.com>
@jefft0
Copy link
Contributor

jefft0 commented Sep 11, 2023

The Client will have a Call method. Some of the required information to sign the transaction and send it to the remote node is:

  • The remote address
  • The directory of the Keybase on local storage
  • The key name in the Keybase
  • The password to use the key to sign

On one extreme, the app developer must pass all this information to the Call method each time as parameters. On the other extreme, the app developer configures the Client struct with this information (like client.SetPassword) and the Call method doesn't need these as parameters. The following diagram has one proposed design. We should discuss.
gnoclient Client

Signed-off-by: Manfred Touron <94029+moul@users.noreply.github.com>
@github-actions github-actions bot added the 📦 🌐 tendermint v2 Issues or PRs tm2 related label Sep 11, 2023
Signed-off-by: Manfred Touron <94029+moul@users.noreply.github.com>
Signed-off-by: Manfred Touron <94029+moul@users.noreply.github.com>
Signed-off-by: Manfred Touron <94029+moul@users.noreply.github.com>
Signed-off-by: Manfred Touron <94029+moul@users.noreply.github.com>
Signed-off-by: Manfred Touron <94029+moul@users.noreply.github.com>
Signed-off-by: Manfred Touron <94029+moul@users.noreply.github.com>
Signed-off-by: Manfred Touron <94029+moul@users.noreply.github.com>
Signed-off-by: Manfred Touron <94029+moul@users.noreply.github.com>
Signed-off-by: Manfred Touron <94029+moul@users.noreply.github.com>
Signed-off-by: Manfred Touron <94029+moul@users.noreply.github.com>
Signed-off-by: Manfred Touron <94029+moul@users.noreply.github.com>
Signed-off-by: Manfred Touron <94029+moul@users.noreply.github.com>
Signed-off-by: Manfred Touron <94029+moul@users.noreply.github.com>
@moul moul marked this pull request as ready for review September 14, 2023 08:47
@moul moul requested a review from a team as a code owner September 14, 2023 08:47
…fy if the password unlocks the account

Signed-off-by: Jeff Thompson <jeff@thefirst.org>
…Address, not string

Signed-off-by: Jeff Thompson <jeff@thefirst.org>
Copy link

codecov bot commented Nov 23, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (851a1ae) 55.91% compared to head (74bd901) 28.91%.

Additional details and impacted files
@@             Coverage Diff             @@
##           master    #1047       +/-   ##
===========================================
- Coverage   55.91%   28.91%   -27.00%     
===========================================
  Files         431      352       -79     
  Lines       65680    56921     -8759     
===========================================
- Hits        36722    16457    -20265     
- Misses      26080    38928    +12848     
+ Partials     2878     1536     -1342     
Flag Coverage Δ
gno.land-_test.gnokey ?
gno.land-_test.gnoland ?
gno.land-_test.pkgs ?
gnovm-_test.cmd ?
gnovm-_test.gnolang.native ?
gnovm-_test.gnolang.other ?
gnovm-_test.gnolang.pkg0 ?
gnovm-_test.gnolang.pkg1 ?
gnovm-_test.gnolang.pkg2 ?
gnovm-_test.gnolang.realm ?
gnovm-_test.gnolang.stdlibs ?
gnovm-_test.pkg ?
go-1.21.x ?
misc ?
misc-_test.genstd ?
tm2-_test.flappy ?
tm2-_test.pkg.amino ?
tm2-_test.pkg.bft ?
tm2-_test.pkg.others ?

Flags with carried forward coverage won't be shown. Click here to find out more.

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

Signed-off-by: gfanton <8671905+gfanton@users.noreply.github.com>
Signed-off-by: gfanton <8671905+gfanton@users.noreply.github.com>
jefft0 and others added 2 commits November 30, 2023 16:19
Signed-off-by: Jeff Thompson <jeff@thefirst.org>
fix: In gnoclient Render and QEval, need to check Response.Error
@moul moul marked this pull request as ready for review January 18, 2024 08:57
@moul
Copy link
Member Author

moul commented Jan 18, 2024

I believe we should merge this pull request and allow it to be iteratively improved over time.

Reasons:

  1. Berty is utilizing it in gno native. (https://github.com/gnolang/gnonative/tree/main/gnoclient)
  2. It is a non-essential library, so we can prioritize functionality over perfection in the initial versions. The current API seems satisfactory.
  3. I want to proceed with portal loop testing and either develop the monitoring oracle myself or assign it to a new contributor (I want to test the students I have identified in my city.). Merging the gnoclient is necessary for this. (Portal Loop: monitoring agent/oracle #1443)

I believe @gfanton could assist me in maintaining this library over time. He has experience maintaining similar developer-oriented wrappers and creating well-designed, idiomatic APIs.

Copy link
Member

@zivkovicmilos zivkovicmilos left a comment

Choose a reason for hiding this comment

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

This looks great 💯

I find myself having to reimplement this logic in different tools, but it will be much easier going forward to switch to this.

I think this scope is fine for v1 of the client, considering we don't have a Go one currently. I would've loved more tests and functionality, but will leave that for a future PR :)

I've gone through some of the tools I've built over the last year, and found that there is a pattern for how the client is utilized:

The work I'm doing over at #1498 will enable users to pass in a WS client (as opposed to being limited to singular HTTP requests), so abstracting the RPC implementation was a good choice ✅

gno.land/pkg/gnoclient/signer.go Show resolved Hide resolved
@moul moul requested a review from a team as a code owner January 18, 2024 09:43
@github-actions github-actions bot added 🧾 package/realm Tag used for new Realms or Packages. 📦 🤖 gnovm Issues or PRs gnovm related labels Jan 18, 2024
Signed-off-by: gfanton <8671905+gfanton@users.noreply.github.com>
@github-actions github-actions bot removed 🧾 package/realm Tag used for new Realms or Packages. 📦 🤖 gnovm Issues or PRs gnovm related labels Jan 18, 2024
Signed-off-by: gfanton <8671905+gfanton@users.noreply.github.com>
Signed-off-by: gfanton <8671905+gfanton@users.noreply.github.com>
Copy link
Member

@gfanton gfanton left a comment

Choose a reason for hiding this comment

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

Looking good for a first iteration 👍

@thehowl thehowl merged commit 17b1303 into gnolang:master Jan 18, 2024
67 of 71 checks passed
zivkovicmilos pushed a commit that referenced this pull request Mar 29, 2024
## Description

This PR adds the reference documentation to the newly added
[gnoclient](#1047).
The reference docs for gnoclient are mostly generated by using a godoc
to markdown tool, with slight modifications.

It also adds a how-to guide on how to connect to a Gno.land chain from
Go.

Along the way: 
- Adds index pages & minor doc improvements for `tm2-js` & `gno-js`
- Updates godoc comments in client
- Changes `gnoclient` function signatures to all be on the same type of
receiver (all tests pass). I realize this could've been a different PR
but its a small change with no functional/practical impact.

<details><summary>Contributors' checklist...</summary>

- [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
- [ ] Added new benchmarks to [generated
graphs](https://gnoland.github.io/benchmarks), if any. More info
[here](https://github.com/gnolang/gno/blob/master/.benchmarks/README.md).
</details>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 🌐 tendermint v2 Issues or PRs tm2 related 📦 ⛰️ gno.land Issues or PRs gno.land package related
Projects
Status: Done
Status: Done
Status: ✅ Done
Status: 🌟 Wanted for Launch
Archived in project
Development

Successfully merging this pull request may close these issues.

Validate password by signing a blank transaction. Good idea?
5 participants