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: faucet server and cli DBC read #486

Merged
merged 4 commits into from Jul 10, 2023
Merged

Conversation

grumbach
Copy link
Member

@grumbach grumbach commented Jul 6, 2023

Description

Summary generated by Reviewpad on 06 Jul 23 02:59 UTC

This pull request adds a new feature to the codebase. It includes changes to the Cargo.lock, sn_cli/src/subcommands/wallet.rs, sn_node/Cargo.toml, sn_node/src/bin/faucet/faucet_server.rs, and sn_node/src/bin/faucet/main.rs files.

The changes in the Cargo.lock file include the addition of the ascii and chunked_transfer packages.

The sn_cli/src/subcommands/wallet.rs file now includes a new command, Read, which reads a DBC from stdin and deposits it to the local wallet.

In the sn_node/Cargo.toml file, the path for the faucet binary has been changed from src/bin/faucet.rs to src/bin/faucet/main.rs.

A new file, sn_node/src/bin/faucet/faucet_server.rs, has been added. This file contains the implementation of the faucet server, which listens on port 8000 and sends tokens to any request.

The sn_node/src/bin/faucet.rs file has been renamed to sn_node/src/bin/faucet/main.rs.

@reviewpad reviewpad bot requested a review from bochaco July 6, 2023 02:59
@reviewpad reviewpad bot added medium Pull request is medium waiting-for-review labels Jul 6, 2023
Comment on lines 17 to 34
/// Run the faucet server.
///
/// This will listen on port 8000 and send tokens to any request.
///
/// # Example
///
/// ```bash
/// # run faucet server
/// cargo run --features="local-discovery" --bin faucet --release -- server
///
/// # query faucet server for DBC at `get local wallet address`
/// curl "localhost:8000/`cargo run --features="local-discovery" --bin safe --release wallet address | tail -n 1`" > dbc_hex
///
/// # feed DBC to local wallet
/// cat dbc_hex | cargo run --features="local-discovery" --bin safe --release wallet read
///
/// # balance should be updated
/// ```
Copy link
Member Author

Choose a reason for hiding this comment

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

Docs are here for now, might be useful to have them where we need them

Copy link
Contributor

Choose a reason for hiding this comment

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

will this work w/o local-discovery?

Copy link

@willief willief Jul 12, 2023

Choose a reason for hiding this comment

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

I tried with local-discovery and got this error

willie@gagarin:~/projects/maidsafe/safe_network$ cargo run  --features="local-discovery" --bin faucet --release -- server
    Finished release [optimized] target(s) in 0.25s
     Running `target/release/faucet server`
🔗 Connected to the Network                                                                                                                                thread 'main' panicked at 'Faucet wallet shall be created successfully.: Bincode(Custom("deserialized bytes don't encode a group element"))', /home/willie/projects/maidsafe/safe_network/sn_transfers/src/dbc_genesis.rs:108:10
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

And a similar result without local-discovery

willie@gagarin:~/projects/maidsafe/safe_network$  cargo run  --bin faucet --release -- server
    Finished release [optimized] target(s) in 0.24s
     Running `target/release/faucet server`
⠐ Connecting to The SAFE Network...                                                                                                                                 The client still does not know enough network nodes.
🔗 Connected to the Network                                                                                                                                         thread 'main' panicked at 'Faucet wallet shall be created successfully.: Bincode(Custom("deserialized bytes don't encode a group element"))', /home/willie/projects/maidsafe/safe_network/sn_transfers/src/dbc_genesis.rs:108:10
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

@grumbach grumbach enabled auto-merge July 6, 2023 04:20
@@ -39,6 +40,8 @@ pub enum WalletCmds {
/// variable to a path you would prefer to work with.
#[clap(verbatim_doc_comment)]
Deposit,
/// Read a DBC from stdin and deposit it to the local wallet.
Read,
Copy link
Contributor

Choose a reason for hiding this comment

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

Hey Anselme,

I'm not sure about adding this as another command in addition to deposit. The same functionality could be added to the deposit command.

Unless there is something distinctly different about this that I'm missing?

Copy link
Contributor

@jacderida jacderida Jul 6, 2023

Choose a reason for hiding this comment

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

I do like being able to read from stdin btw. We had that in the old CLI. However, I don't think it merits having another separate command.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree, it should be Deposit. If I understand correctly, deposit reads files in a path provided as arg. Would it make sense to read from stdin if the path argument is not provided?

Copy link
Member Author

Choose a reason for hiding this comment

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

Since the path wasn't an argument this is what I did:

# read from default path (unchanged)
safe wallet deposit

# read from stdin (new feature)
safe wallet deposit --stdin

And removed the Read
Good one @jacderida!

@reviewpad reviewpad bot added large Pull request is large and removed medium Pull request is medium labels Jul 10, 2023
@grumbach grumbach added this pull request to the merge queue Jul 10, 2023
Merged via the queue into maidsafe:main with commit e4899b5 Jul 10, 2023
20 checks passed
@grumbach grumbach deleted the faucet_server branch July 10, 2023 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
large Pull request is large waiting-for-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants