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

fix: update to stacks.js 4.0.0 (removes username checking) #2333

Merged
merged 2 commits into from
Apr 26, 2022

Conversation

janniks
Copy link
Contributor

@janniks janniks commented Mar 25, 2022

Try out this version of the Hiro Wallet - download extension builds.

scope

  • updates stacks.js deps to 4.0.0
  • fixes test issues and switches to node test environment

@vercel
Copy link

vercel bot commented Mar 25, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/blockstack/stacks-wallet-web/ES6gaTMMoU8WKJrBpXMfYxS2vTzB
✅ Preview: https://stacks-wallet-web-git-remove-username-checking-blockstack.vercel.app

@vercel vercel bot temporarily deployed to Preview March 25, 2022 14:57 Inactive
@fbwoolf fbwoolf force-pushed the dev branch 2 times, most recently from dbf0f5f to e20a2ea Compare March 25, 2022 16:58
@vercel vercel bot temporarily deployed to Preview March 25, 2022 20:21 Inactive
@vercel vercel bot temporarily deployed to Preview March 28, 2022 15:24 Inactive
@vercel vercel bot temporarily deployed to Preview March 28, 2022 15:44 Inactive
@vercel vercel bot temporarily deployed to Preview March 28, 2022 15:56 Inactive
@janniks janniks force-pushed the remove-username-checking branch from 7d34d86 to f7263cf Compare March 28, 2022 17:22
@vercel vercel bot temporarily deployed to Preview March 28, 2022 17:22 Inactive
@vercel vercel bot temporarily deployed to Preview March 29, 2022 11:25 Inactive
@janniks janniks force-pushed the remove-username-checking branch from 3b6d323 to 99c0673 Compare March 29, 2022 11:46
@vercel vercel bot temporarily deployed to Preview March 29, 2022 11:46 Inactive
@vercel vercel bot temporarily deployed to Preview March 29, 2022 12:44 Inactive
@vercel vercel bot temporarily deployed to Preview March 29, 2022 16:35 Inactive
@markmhendrickson markmhendrickson linked an issue Apr 7, 2022 that may be closed by this pull request
@markmhendrickson
Copy link
Collaborator

markmhendrickson commented Apr 7, 2022

@kantai @larrysalibra @hstove @kyranjamie I'd be curious whether any of you see any immediate dangers to removing BNS-related queries from authentication in general per this PR.

@janniks is working on this approach as a way to resolve a variety of BNS-related issues that have cropped up, especially regarding expired, renewed and "legacy" Stacks 1.0 names. And this seems like a possibly efficient way to resolve them all at once.

@kyranjamie
Copy link
Collaborator

Where can we see the changes made to auth?

I can't think of any immediate dangers. Auth does a lot so if we can decouple any BNS features of it, then great. I don't fully understand why auth needs to look up/match BNS names in the first place.

@aulneau
Copy link
Contributor

aulneau commented Apr 7, 2022

I agree that name resolution should be decoupled from authentication. It's easy enough to expose functions to resolve names to addresses, I personally don't think it needs to be included in an authentication response.

@vercel vercel bot temporarily deployed to Preview April 8, 2022 15:33 Inactive
@vercel vercel bot temporarily deployed to Preview April 8, 2022 15:36 Inactive
@vercel vercel bot temporarily deployed to Preview April 8, 2022 15:49 Inactive
@vercel vercel bot temporarily deployed to Preview April 11, 2022 13:28 Inactive
@janniks janniks force-pushed the remove-username-checking branch from efd0912 to 54e2da7 Compare April 11, 2022 13:29
@vercel vercel bot temporarily deployed to Preview April 11, 2022 13:30 Inactive
@vercel vercel bot temporarily deployed to Preview April 11, 2022 15:20 Inactive
@vercel vercel bot temporarily deployed to Preview April 11, 2022 15:48 Inactive
@janniks janniks force-pushed the remove-username-checking branch from 66716ea to ced2efa Compare April 11, 2022 15:53
@vercel vercel bot temporarily deployed to Preview April 11, 2022 15:53 Inactive
@vercel vercel bot temporarily deployed to Preview April 20, 2022 23:48 Inactive
@vercel vercel bot temporarily deployed to Preview April 21, 2022 00:01 Inactive
@vercel vercel bot temporarily deployed to Preview April 21, 2022 00:08 Inactive
@vercel vercel bot temporarily deployed to Preview April 21, 2022 00:17 Inactive
@janniks janniks marked this pull request as ready for review April 21, 2022 01:00
@janniks janniks changed the title WIP: remove username checking fix: remove username checking and update to stacks.js 4.0.0 Apr 21, 2022
@janniks janniks changed the title fix: remove username checking and update to stacks.js 4.0.0 fix: update to stacks.js 4.0.0 (removes username checking) Apr 21, 2022
@janniks janniks requested a review from kyranjamie April 21, 2022 01:02
@pradel
Copy link
Contributor

pradel commented Apr 21, 2022

It's happening 🔥 can't wait to see this one merged so we can finally add Hiro wallet support to Sigle!

Copy link
Collaborator

@kyranjamie kyranjamie left a comment

Choose a reason for hiding this comment

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

LGTM 🥳

@markmhendrickson
Copy link
Collaborator

image

@vercel vercel bot temporarily deployed to Preview April 21, 2022 14:07 Inactive
@janniks
Copy link
Contributor Author

janniks commented Apr 21, 2022

in 0f3a2b9 I had to skip a test (not sure why this fails), if somebody wants to look into it. may be related to switching to node as a test environment.

in 0f3a2b9 I also added --forceExit to jest, because some tests are leaking (traceable w/ --detectOpenHandles, after updating jest to latest)


handing back off to #userx, feel free to merge, or just use as a test-app for 4.0.0 stacks.js w/ connect.
(needed for arbitrary message signing)

@hstove
Copy link
Contributor

hstove commented Apr 21, 2022

Yeah, I do like this! In general I'm a fan of supporting a more "virtual" auth flow. On ETH there are no auth checks, so you can "log in" as any address you want to an app. This has some nice benefits - like I can login to an app using my vault address while on my phone, which doesn't have that key. If an app wants to strictly verify ownership (which is a more edge use-case, like gating premium features) they can always use other APIs for that.

@kyranjamie
Copy link
Collaborator

kyranjamie commented Apr 25, 2022

Before we merge, can we rebase this PR into 1 (or a few) commits 🙏🏼

@janniks janniks force-pushed the remove-username-checking branch from 0f3a2b9 to c4dd676 Compare April 25, 2022 13:49
@vercel vercel bot temporarily deployed to Preview April 25, 2022 13:49 Inactive
@janniks janniks force-pushed the remove-username-checking branch from c4dd676 to c5a3886 Compare April 25, 2022 14:08
@vercel vercel bot temporarily deployed to Preview April 25, 2022 14:11 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants