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

docs: add gnolang/faucet #2122

Closed
wants to merge 30 commits into from
Closed

Conversation

leohhhn
Copy link
Contributor

@leohhhn leohhhn commented May 16, 2024

Description

Closes: #2120

This PR introduces the following changes to the docs (text copied over from the issue linked above):

  • Adding a gnolang/faucet reference page in the Gno Tooling section,
  • Adding a tutorial in the new Gno Infra section for setting up and running a faucet.

Matching docs PR (linked in the issue that this PR was created from)

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.

@leohhhn leohhhn self-assigned this May 16, 2024
@leohhhn
Copy link
Contributor Author

leohhhn commented May 16, 2024

Posted some questions here to clarify things not found in the reference docs

@leohhhn
Copy link
Contributor Author

leohhhn commented May 16, 2024

Crossposting from the issue for visibility:

  • What kind of security against DDoS do we have implemented in the faucet?
  • What are the request types the faucet can take?
  • How do you actually send batch requests to the faucet?

What is the difference between these flags?

-config string                  the path to the command configuration file [TOML]
-faucet-config string           the path to the faucet TOML configuration, if any

@leohhhn leohhhn marked this pull request as ready for review May 16, 2024 10:36
@leohhhn leohhhn requested a review from moul as a code owner May 16, 2024 10:36
@zivkovicmilos zivkovicmilos self-requested a review May 16, 2024 11:31
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.

Please link to the accompanying docs.gno.land PR in the description, and add adequate labels 🙏

Also, please update the PR description to have a bit more details

@leohhhn leohhhn added the 📖 documentation Improvements or additions to documentation label May 17, 2024
- `gnoland` & `gnokey` installed
- A Gno.land keypair generated using [`gnokey`](../gno-tooling/cli/gnokey.md)

## Premining funds to an address
Copy link
Member

Choose a reason for hiding this comment

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

We should drop this section completely, and not reference genesis_balances.txt anywhere -- we have adequate genesis balances commands for premining accounts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've removed this section and added a prerequisite: 6aeb1f5


## Premining funds to an address

Before setting up the faucet, we need to make sure that the address will be used
Copy link
Member

Choose a reason for hiding this comment

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

This is not a correct English sentence:

we need to make sure that the address will be used to serve the funds contain enough testnet funds

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This section was completely removed as per #2122 (comment)


```json
{
"To": "g1juz2yxmdsa6audkp6ep9vfv80c8p5u76e03vvh"
Copy link
Member

Choose a reason for hiding this comment

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

This is outdated, please reference the latest gnolang/faucet request structure

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A request without an Amount specified is actually still valid, however I have added further explanations on the amount field and how to use it: e33f6e3

}
```

You can test this out buy running the following `curl` command:
Copy link
Member

Choose a reason for hiding this comment

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

Typo here, buy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch, fixed it: 15028c7

in the `--remote` flag in the faucet. If your node is listening on a separate
address, make sure to match it accordingly when running the faucet.

## Making faucet requests
Copy link
Member

Choose a reason for hiding this comment

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

Error handling was not covered in this section, please cover it 🙏

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added possible errors a user can have while setting up the faucet or when making requests:
e33f6e3

@@ -0,0 +1,46 @@
---
id: gno-tooling-tm2-faucet
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure what the use of this file is, it can become outdated very quickly (help output)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed it: fbb91ed

@@ -0,0 +1,173 @@
---
id: setting-up-a-faucet
Copy link
Member

Choose a reason for hiding this comment

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

This tutorial doesn't cover:

  • gnofaucet
  • gnofaucet vs gnolang/faucet
  • different things I've noted as part of the comments

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking at the original issue, this tutorial was meant to cover setting up a faucet with gnolang/faucet, which it does.

I suggest two options:

  1. We keep the scope of this PR to cover gnolang/faucet, and open a separate issue for documenting gnofaucet. Ideally, we would have some reference docs or at least a basic README about it before I start working on it.
  2. I'm unfamiliar with the gnofaucet functionalities and codebase - understanding the details of it are a requirement for me to be able to write a high-quality tutorial. Possibly we can jump on a quick 15 min call and you can run me through the main functionalities to speed up the process, and get this done asap?

I have fixed up all of the other comments as requested, and added a section mentioning that the faucet can be extended and referenced gnofaucet as an example: d2388e7

WDYT?

cc @Kouteki

Copy link
Contributor

Choose a reason for hiding this comment

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

I like option 1 better - it lets us publish this PR and make incremental progress.

Copy link
Member

Choose a reason for hiding this comment

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

@leohhhn

We discussed on a call that gnofaucet is most definitely in the scope of this PR, and should be added -- please add it and ping me when it's up in the PR 🙏

Copy link
Contributor Author

@leohhhn leohhhn Jun 6, 2024

Choose a reason for hiding this comment

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

Frankly speaking, I do not remember discussing this on a call.

However, I did take about 2h to read through the code of gnofaucet and write up a section on it. Please check it out.

I've also added a mention of the Faucet Hub.

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.

Please, read the comments carefully and address changes, because I get the feeling no effort has been put into writing this tutorial. There is no "meat', it's lacking clear explanations on how things work (the most important part for the faucet operators)

Comment on lines +19 to +20
- A Gno.land keypair generated using [`gnokey`](../gno-tooling/cli/gnokey.md)
containing funds for the faucet to serve
Copy link
Member

Choose a reason for hiding this comment

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

These are 2 separate requirements -- please make them clear.

Also, please link some kind of guide on how to actually prefund an account

```

In the `config.toml` file, you will be able to configure a few parameters:
- ChainID of the node to connect to
Copy link
Member

Choose a reason for hiding this comment

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

This is the chain ID of the network, not the node


In the `config.toml` file, you will be able to configure a few parameters:
- ChainID of the node to connect to
- Faucet listener address
Copy link
Member

Choose a reason for hiding this comment

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

Please expand on this, it's not immediately clear that this is the serve address of the faucet that others can use to initiate faucet requests

Comment on lines +50 to +51
- Mnemonic phrase to use for generating the account(s) to serve funds from
- The number of accounts to generate from the mnemonic
Copy link
Member

Choose a reason for hiding this comment

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

Please explain better how these 2 things play together into the faucet

Comment on lines +69 to +71
We will keep it simple and go with the basic configuration. After inputting the
mnemonic phrase from which your faucet address is derived from, you are ready to
run the faucet.
Copy link
Member

Choose a reason for hiding this comment

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

But how is the default configuration one where you don't specify a mnemonic?


When requesting a drip from the faucet, you can face the following errors:
- If the address provided is empty or has an invalid checksum - `invalid beneficiary address`
- If the amount requested is empty, not in the `<amount>ugnot` format, or is larger
Copy link
Member

Choose a reason for hiding this comment

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

This sentence is hanging

Comment on lines +223 to +224
- `captcha-secret` - where you can provide your reCaptcha v2 secret key
- `is-behind-proxy` - which enables or disables the IP throttling functionality.
Copy link
Member

Choose a reason for hiding this comment

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

Can you please standardize the formatting in this doc?
Some points have ., some don't

Comment on lines +239 to +241
`5` requests per minute from a single IP address. To configure this, change the
hardcoded `maxRequestsPerMinute` value in the [`throttle.go`](https://github.com/gnolang/gno/blob/master/contribs/gnofaucet/throttle.go)
file in the `gnofaucet` folder.
Copy link
Member

Choose a reason for hiding this comment

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

This is terrible advice, please delete this

id: setting-up-a-faucet
---

# Setting up a faucet for your Gno network
Copy link
Member

Choose a reason for hiding this comment

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

This tutorial doesn't explain probably the one thing it was supposed to -- how the faucet actually works with subaccounts and funding

@Kouteki
Copy link
Contributor

Kouteki commented Oct 8, 2024

@leohhhn is this PR still relevant?

@leohhhn
Copy link
Contributor Author

leohhhn commented Oct 8, 2024

@Kouteki

Closing this but we can reuse it later for the faucet/gnofaucet readme.

@leohhhn leohhhn closed this Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📖 documentation Improvements or additions to documentation
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[docs] Add gnolang/faucet
3 participants