Skip to content
This repository has been archived by the owner on Aug 1, 2022. It is now read-only.

feat(proxy): wire up Registry user registration #238

Merged
merged 9 commits into from Mar 21, 2020

Conversation

xla
Copy link
Contributor

@xla xla commented Mar 19, 2020

We replace the mock used so far for user registration with an actual integration with the Registry. Follow up to radicle-dev/radicle-registry#249.

Closes #185

@xla xla added proxy feature Something that doesn't exist yet labels Mar 19, 2020
@xla xla self-assigned this Mar 19, 2020
@xla xla added this to In progress in weekly via automation Mar 19, 2020
weekly automation moved this from In progress to Approved Mar 19, 2020
Copy link
Contributor

@MeBrei MeBrei left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Will wiring up the querying of registered users be done in a different PR?

proxy/src/registry.rs Outdated Show resolved Hide resolved
@xla
Copy link
Contributor Author

xla commented Mar 19, 2020

@MeBrei Good catch, wired it up as well as part of this PR.

ed25519::Pair::from_legacy_string(&format!("//{}", handle.to_string()), None);

use radicle_registry_client::CryptoPair;
// Give new account some dough so we can perform transactions.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Give new account some dough so we can perform transactions.
// The new account needs funds to perform transactions.

"dough" sounds cool but such language is unhelpful here, imo. Also, I suggest explaining why we are doing it, the what the code tells already.

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's a temporary measure and I allowed myself some fun with it. Will add a comment to remove the method alltogether.

proxy/src/graphql/schema.rs Outdated Show resolved Hide resolved
proxy/src/graphql/schema.rs Outdated Show resolved Hide resolved
id: juniper::ID,
) -> Result<registry::Transaction, error::Error> {
// TODO(xla): Get keypair from persistent storage.
let fake_pair = ed25519::Pair::from_legacy_string("//Alice", None);
Copy link
Contributor

Choose a reason for hiding this comment

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

You could make this function receive the seed for the ed25519 pair instead of hardcoding it to Alice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then we also need to ensure that that account has funds, will leave it until the needs arises.

@@ -247,6 +247,21 @@ impl Registry {
Ok(self.client.get_user(user_id).await?.map(|_user| handle))
}

pub async fn prepay_account(
Copy link
Contributor

Choose a reason for hiding this comment

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

This is essentially a transfer, but now you can't specify the donator.

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 method is acting like a faucet, you can bootstrap a new account from the main available pool Alice. At this point there is no reason to have more flexibility until we actually wanna test transfers in the app. Also see this as temporary aid .


futures::executor::block_on(
ctx.registry
.read()
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not fully sure as to what this is but should it be write since it's registering a user and not, say, getting/reading one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are the calls to acquire the RWLock and as we don't mutate the field itself we don't need to acquire a write lock.

@xla xla merged commit 8859037 into master Mar 21, 2020
weekly automation moved this from Approved to Done Mar 21, 2020
@xla xla deleted the xla/185-integrate-user-registration branch March 21, 2020 00:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature Something that doesn't exist yet
Projects
No open projects
weekly
  
Done
Development

Successfully merging this pull request may close these issues.

Integrate Registry user registration
4 participants