-
-
Notifications
You must be signed in to change notification settings - Fork 114
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
Zebedee send attachment #1742
base: master
Are you sure you want to change the base?
Zebedee send attachment #1742
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At a glance, this looks solid. The only thing that caught my eye is the lack of validation.
Also, I don't think we need to worry about the gametag/name changing too much once we breakout send/receive stuff. In anticipation of that, it might be better to not even do receives in the same form and just let them use the lightning address attachment so that we don't have to do another quirky migration in the near future.
I mostly suspect this wallet attachment will be largely unused in which case we don't want it to get in the way of improving other stuff. lmk what you think
wallets/zebedee/index.js
Outdated
help: `you can get an API key from [Zebedee Dashboard](${DASHBOARD_URL}) from \n\`Project->API->Live\``, | ||
clientOnly: true, | ||
requiredWithout: 'gamerTagId', | ||
validate: string() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect we could do a little better for validation.
Same for the gamer tag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can see the zebedee api key today is an alphanumeric 32 characters long string, however there is nothing in the doc that implies this is and always be its shape, so i just added a reasonable min and max length.
Are you happy with that?
i think you are probably right, the receiver concoction can be added later if there is demand, the ln address will work. |
Description
Zebedee send
and receiveattachmentScreenshots
Additional Context
For sending attachments, I just use the Zebedee dashboard APIs, nothing strange there.
But for receiving, I had to get creative since, as far as I know, Zebedee doesn’t have a receive-only API.
What I did was ask the user for their userId to fetch the invoice through the public LNURLp endpoint. However, since the userId is hard to find (I think you can only get it through the API), I added the option to input the ZBD Gamertag (or LN address), which is then converted to the userID. This way, the attachment doesn’t break if the user changes their Gamertag.
Checklist
Are your changes backwards compatible? Please answer below:
yes
On a scale of 1-10 how well and how have you QA'd this change and any features it might affect? Please answer below:
8
For frontend changes: Tested on mobile, light and dark mode? Please answer below:
yes
Did you introduce any new environment variables? If so, call them out explicitly here:
no