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

stellar-accounts-as-a-service by orsab #154

Closed
3 tasks done
orsab opened this issue Jan 19, 2022 · 7 comments
Closed
3 tasks done

stellar-accounts-as-a-service by orsab #154

orsab opened this issue Jan 19, 2022 · 7 comments

Comments

@orsab
Copy link
Contributor

orsab commented Jan 19, 2022

Link the bounty file

https://github.com/tyvdh/stellar-quest-bounties/blob/main/bounties/level-2/stellar-accounts-as-a-service.md

Mark your progress

  • 🔵  Started working
  • 🟢  Ready for review
  • 🟣  Review completed

Provide relevant details

No response

@orsab orsab changed the title stellar-accounts-as-a-service stellar-accounts-as-a-service by orsab Jan 19, 2022
@github-actions github-actions bot added review and removed work labels Jan 20, 2022
@orsab
Copy link
Contributor Author

orsab commented Jan 20, 2022

@LorDDark6660
Copy link
Contributor

I am missing in description that i have to rename example file to .example.env

IMG_20220129_085004

@orsab
Copy link
Contributor Author

orsab commented Jan 29, 2022 via email

@orsab
Copy link
Contributor Author

orsab commented Jan 29, 2022 via email

@DieKautz
Copy link
Contributor

DieKautz commented Feb 1, 2022

Nice work!

I really like the structuring of your routings and the horizon feedback when endpoints are called successfully.

When playing around with the endpoints I stumbled upon some exceptions. They are caught by the app, but the response on the api side is hard to handle (e.g. using an invalid JWT token the response will be in html).
Also maybe you should add more descriptive errors in general like:

  • invalid payment amounts (negative, 0, too many decimals)
  • invalid destinations (especially if they don't exist on the ledger yet (create them?)

edit: I see there is a very well made error catching in place, shouldn't the messages be displayed to the api user?

When dealing with really small and precise amounts like 0.1 and one stroop the displayed balance at /info has some floating point errors.

What do you think about crazy usernames like with unicode and stuff?

@orsab
Copy link
Contributor Author

orsab commented Feb 1, 2022

Hi @DieKautz, thank you for your code review. Just fixed some bugs like: error show, floating point...
I'm not antagonist, as a result, I'm not pressure users to create good usernames. Sometimes crazy username with unicode is very good practice in security 😉

@github-actions github-actions bot added award and removed review labels Feb 3, 2022
@ElliotFriend
Copy link
Collaborator

c6dfd6696fa140475f15fc03e759408a4e0a70faf6f1184899996bc5993d8a2b

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants