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): create distributions for maid addrs #1248

Merged
merged 4 commits into from Feb 5, 2024

Conversation

iancoleman
Copy link
Contributor

@iancoleman iancoleman commented Jan 31, 2024

This PR changes the faucet startup process. At startup the faucet creates distributions of tokens using the snapshot of maid addresses + balances + public keys.

Each distribution is a combination of a transfer (from the faucet) and an encrypted secret key (randomly generated) that enables that transfer to be spent. The maid owner can decrypt the secret key using their maid private key, which is then used as a wallet key for spending the transfer.

Distributions created by the faucet are saved to $HOME/.local/share/safe_snapshot/distributions/<address> and contain the messagepack-encoded hex for the distribution.

Note:

  • This doesn't enable access to the distributions yet, this is to be done in future work by exposing a faucet server endpoint that returns the distribution for a particular maid address. I'm trying to keep each PR relatively small.

  • The startup process for the faucet server has become much longer while it creates these distributions. Creating 1200 initial distributions took about 11 minutes.

    • Should we move this into a separate thread and have it run in the background?
    • Should we have a feature flag for local testnets that doesn't use the 'live' snapshot list and uses a fake test-only list of a single address?

Future work will be:

  • allow users to fetch their distribution from the cli for their maid address, and automatically send that distribution to their wallet.

  • enable the faucet to receive new maid address+publickey pairs and create new distributions on the fly for those.

  • expose the full snapshot and distribution list to enable audit of the process. Need to explore what an audit would require.

  • find a simple / automatic way to retain address + publickeys that have been submitted while the faucet is running, stored in maid_address_pubkeys.csv for future use.

  • consider adding a test maid address to this so we can test the claim process easily.

Description

Summary generated by Reviewpad on 31 Jan 24 03:07 UTC

This pull request includes changes to the Cargo.lock, Cargo.toml, and faucet_server.rs files. It updates dependencies and adds new ones, including "ecies", "libsecp256k1", "libsecp256k1-core", "libsecp256k1-gen-ecmult", "libsecp256k1-gen-genmult", "rmp-serde", and "serde_bytes". It also adds structs and functions related to distributing tokens to addresses using public and secret keys.

@reviewpad reviewpad bot requested a review from bochaco January 31, 2024 03:07
@reviewpad reviewpad bot added Medium Medium sized PR waiting-for-review labels Jan 31, 2024
@joshuef
Copy link
Contributor

joshuef commented Jan 31, 2024

Should we move this into a separate thread and have it run in the background?

Sounds good if that can be done easily?

Should we have a feature flag for local testnets that doesn't use the 'live' snapshot list and uses a fake test-only list of a single address?

I think we'll need that and for the CI in general (aside from probably a test that will verify that distributions have occured perhaps?) (That might be one for a follow up PR?)

snapshot: Snapshot,
pubkeys: HashMap<MaidAddress, MaidPubkey>,
) {
for (addr, amount) in snapshot {
Copy link
Contributor

Choose a reason for hiding this comment

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

Here I think we can push each iteration into its own task and then use futures::join_all to collect and ensure that's happening.

And then if this whole function is tokio::spawned we'll get a fast startup 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seemed simpler to just put the whole distribute function in a thread rather than do a bunch of spawning, because transfers can't happen in parallel (wait for wallet lock to be released).

@iancoleman
Copy link
Contributor Author

Rebased on main after #1253 was merged

@iancoleman
Copy link
Contributor Author

How to fix this failing test for unused dependencies? They are used but only in distributions feature. I can change workflows/merge.yml L32 to get the test to pass:

-run: cargo +nightly udeps --all-targets
+run: cargo +nightly udeps --all-targets --features distribution

but this feels like the wrong way to do it (maybe my feeling is wrong). Any tips or advice @grumbach @joshuef ?

@joshuef
Copy link
Contributor

joshuef commented Feb 5, 2024

@iancoleman i'm not wholly sure. I've pinged @RolandSherwin as he might have more idea, but right now we could manually ignore them via the cargo.toml: https://github.com/maidsafe/safe_network/actions/runs/7755547781/job/21151134924?pr=1248#step:5:1077

If there's no immediate better way, I'd do just that.

@joshuef
Copy link
Contributor

joshuef commented Feb 5, 2024

ah, @RolandSherwin has sorted it. Thanks!! 🙇 (was optional on the dep, @iancoleman )

@joshuef joshuef added this pull request to the merge queue Feb 5, 2024
Merged via the queue into maidsafe:main with commit 0dc5d98 Feb 5, 2024
37 of 38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Medium Medium sized PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants