8000 feat: add cosmosfaucet by ilgooz · Pull Request #630 · ignite/cli · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

feat: add cosmosfaucet #630

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 15 commits into from
Jan 15, 2021
Merged

feat: add cosmosfaucet #630

merged 15 commits into from
Jan 15, 2021

Conversation

ilgooz
Copy link
Member
@ilgooz ilgooz commented Jan 10, 2021 •
edited
Loading

to transfer tokens from the faucet account to the requester account.

refactoring code from the original implementation: https://github.com/allinbits/cosmos-faucet.

related to #570.

the faucet server runs via Starport's Dev server and it is reachable through /faucet endpoint.

usage:

...
faucet:
        name: user1
...

And then make a JSON POST req with following payload:
http://localhost:12345/faucet

{
    "address": "..."
}

@ilgooz ilgooz force-pushed the feature/cosmosfaucet branch 2 times, most recently from 8a63244 to e1e84b5 Compare January 12, 2021 14:05
@ilgooz ilgooz force-pushed the feature/cosmosfaucet branch from e1e84b5 to ea377ff Compare January 13, 2021 14:05
@ilgooz
enable faucet in the config.yml
284845d 
the faucet server runs via Starport's Dev server and it is reachable
through `/faucet` endpoint.

usage:

```yaml
...
faucet:
        name: user1
...
```
@ilgooz ilgooz force-pushed the feature/cosmosfaucet branch from 8d6d50c to 284845d Compare January 14, 2021 07:26
@ilgooz ilgooz changed the title feature: add cosmosfaucet feat: add cosmosfaucet Jan 14, 2021
@ilgooz ilgooz marked this pull request as ready for review January 14, 2021 08:22
@ilgooz ilgooz requested review from fadeev and lumtis as code owners January 14, 2021 08:22
@fadeev
Copy link
Contributor
fadeev commented Jan 14, 2021 •
edited
Loading

Awesome! The faucet can only send one denom, right? I'm just thinking about UX of config.yml.

Currently the interface is:

faucet:
  name: faucet
  denom: "token"
  credit: 100
  max_credit: 1000

This is fine, but I propose keeping the existing terminology:

faucet:
  name: faucet
  denom: "token"
  amount: 100
  amount_max: 1000

If we can make it multi-token, that would be even better, of course:

faucet:
  name: faucet
  coins: ["100token", "200foo]
  coins_max: ["1000token", "2000foo"]

@fadeev
Copy link
Contributor
fadeev commented Jan 14, 2021

Customizing the port number may be very useful as well, but we can handle this is a 8000 nother PR.

ilgooz added 2 commits January 14, 2021 13:23
@ilgooz
run the faucet in a different server w/ custom port support
9ccc631 
* detect on the fly changes in config.yml to reconfigure the faucet server.
@ilgooz
support multi denoms
6c6e96d 
changed `config.yml` syntax as below:

```yml
...
faucet:
    name: user1
    coins:
        - "10token"
        - "5stake"
    coins_max:
        - "33token"
        - "20stake"
...
```
@ilgooz
Copy link
Member Author
ilgooz commented Jan 14, 2021

@fadeev solved by 9ccc631 & 6c6e96d. 👍

lumtis
lumtis previously approved these changes Jan 14, 2021
View reviewed changes
8000
Copy link
Contributor
@lumtis lumtis left a comment

Choose a reason for hiding this comment

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

utACK 👍

@fadeev
Copy link
Contributor
fadeev commented Jan 14, 2021 •
edited
Loading

Great having a faucet! 🙂

I've been manually testing it, and it doesn't seem to send tokens properly. If I specify multiple coin denoms:

accounts:
  - name: user1
    coins: ["1000token", "100000000stake"]
  - name: faucet
    coins: ["100000token", "100000foo"]
faucet:
  name: faucet
  coins: ["100token", "2foo"]

It sends only token (ignores foo).

If I have only foo:

accounts:
  - name: user1
    coins: ["1000token", "100000000stake"]
  - name: faucet
    coins: ["100000token", "100000foo"]
faucet:
  name: faucet
  coins: ["2foo"]

It responds with:

{"status":"error","error":"cannot send tokens (SDK code 5): failed to execute message; message index: 0: 0uatom is smaller than 10000000uatom: insufficient funds"}

@ilgooz
Copy link
Member Author
ilgooz commented Jan 14, 2021

@fadeev actually with the current implementation, faucet does not transfer all the defined coins in the config.yml, but it only transfers a single coin type by the denom. When denom isn't specfied during the transfer request, the first coin type in the faucet.coins array is used.

{
    "address": "cosmos1hw3ydfl768324h4m58apuvplxn4y6qkma6enzy",
    "denom": "stake" (optional)
}

How do you feel about this behavior tho?

@ilgooz
Copy link
Member Author
ilgooz commented Jan 14, 2021

@fadeev 3647492 should fix the 2nd point. 👍

@fadeev
Copy link
Contributor
fadeev commented Jan 14, 2021

Thanks for making this clear for me! This behavior is somewhat confusing. I think it would be much easier to understand if faucet.coins was the amount given to a user per request by default and (optionally) coins in the request body would overwrite this value.

@ilgooz
Copy link
Member Author
ilgooz commented Jan 15, 2021

@fadeev f61fdd7 should do it. 👌

@ilgooz ilgooz requested a review from lumtis January 15, 2021 10:38
@fadeev
Copy link
Contributor
fadeev commented Jan 15, 2021

@ilgooz This works great! One last improvement before we can merge 🙏 Can you increase the default coin max to 10000000 so that users don't have to manually increase the limit.

@ilgooz
Copy link
Member Author
ilgooz commented Jan 15, 2021

@fadeev fixed by 7707535.

fadeev
fadeev approved these changes Jan 15, 2021
View reviewed changes
Copy link
Contributor
@fadeev fadeev left a comment

Choose a reason for hiding this comment

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

Woohoo! 👏 Works awesome!

@ilgooz ilgooz merged commit 936dff7 into develop Jan 15, 2021
@ilgooz ilgooz deleted the feature/cosmosfaucet branch January 15, 2021 13:12
faddat added a commit that referenced this pull request Jan 21, 2021
@faddat
Merge https://github.com/tendermint/starport into aur
25d23ea 
* https://github.com/tendermint/starport: (94 commits)
  fix: change "frontend" to "vue" in gitignore (#674)
  feat(serve): save state as genesis when the app is stopped (#666)
  fix(relayer): update version (#640)
  feat(scaffold): upgrade Stargate to SDK v0.40.1 (#663)
  feat(scaffolder): init and export genesis for scaffolded types (#661)
  refactor(templates): rm internal/ from Stargate scaffold (#659)
  refactor: move Stargate app's third_party/proto into proto/third_party (#658)
  fix(docs): typo on custom modules
  feat: integrate app's protocgen script into starport (#654)
  fix: starport-docs-tutorials Introduction.md file (#653)
  fix(template:) rm dash in proto pkg names (#649)
  fix: add d to appname (#652)
  Update install info (#650)
  docs: update docs stargate (#648)
  feat: add cosmosfaucet (#630)
  feat: import wasm with Stargate (#642)
  feat: Pi for stargate (#627)
  feat(scaffolder): run fmt on scaffolder commands (#647)
  docs: added info on config (#646)
  feat: custom keyring-backend in config (#637)
  ...
Jchicode pushed a commit to Jchicode/cli that referenced this pull request Aug 9, 2023
@ilgooz
feat: add cosmosfaucet (ignite#630)
97018a2 
* init cosmosfaucet with LP and SP compability

* enable faucet in the config.yml

the faucet server runs via Starport's Dev server and it is reachable
through `/faucet` endpoint.

usage:

```yaml
...
faucet:
        name: user1
...
```

* print err code

* docs

* fix lint

* fix lint

* run the faucet in a different server w/ custom port support

* detect on the fly changes in config.yml to reconfigure the faucet server.

* support multi denoms

changed `config.yml` syntax as below:

```yml
...
faucet:
    name: user1
    coins:
        - "10token"
        - "5stake"
    coins_max:
        - "33token"
        - "20stake"
...
```

* fix lint

* fix when max not provided for coin

* add test for cosmoscoin

* support transferring multi coins at once

* handle client side cancel

* fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fadeev fadeev fadeev approved these changes

@lumtis lumtis Awaiting requested review from lumtis

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ilgooz @fadeev @lumtis
0