search

kalepail / stellar-quest-bounties

Stellar Quest Bounties is an extension of the traditional, series based Stellar Quest challenges allowing seasoned and passionate Stellar Questers to continue their journey of education and earning during the "lean times" between Stellar Quest series.
https://quest.stellar.org/bounties
23 stars 27 forks source link

JavaScript SEP-0010 Server Reference Implementation #131

Closed fpbrault closed 2 years ago

fpbrault commented 2 years ago

Link the bounty file

https://github.com/tyvdh/stellar-quest-bounties/blob/main/bounties/level-2/sep10-javascript-server.md

Mark your progress

Provide relevant details

Repo: https://github.com/fpbrault/stellar-sep-0010-server

Demo: https://stellar.beign.es/ Swagger: https://stellar.beign.es/api

fpbrault commented 2 years ago

Using Nest JS for this, since I wanted to learn working with it, and it seems like a good fit for this project.

fpbrault commented 2 years ago

Added a link to the server (and Swagger API). I'm using heroku free, so the server will shutdown if idle for some time. Should take at most 30 seconds to restart though.

Most functionality is present. Waiting for memo and client_domain support to be added in js-stellar-sdk, but I may decide to implement it on my own if it takes too long.

I've also added a protected endpoint (https://stellar.beign.es/profile) to verify that the JWT token works.

fpbrault commented 2 years ago

I've added support for memo/muxed accounts since v9.0.1 of the Stellar JS SDK has been released :)

fpbrault commented 2 years ago

client_domain support has been added to the sdk so I'll be able to finish this soon :)

fpbrault commented 2 years ago

This is pretty much completed at this point.

stellar.beign.es runs on the public network, but my dev instance is on the testnet: powerful-everglades-30138.herokuapp.com

rahimklaber commented 2 years ago

I checked it out and aside from a few minor issues, I think its good to go.

There are two minor problems:

  1. the swagger docs is incomplete at some places. It doesn't show the responses that the server sends.
  2. When posting the xdr, the server responds with response code 201 instead of 200.

There is something else which I'm not sure is a problem: I am using the Java SDK to verify the challenge tx. Verifying the challenge tx with a muxed account fails with an error saying that the tx is not signed by the server account. This is probably just a bug in the Java SDK though.

fpbrault commented 2 years ago

I checked it out and aside from a few minor issues, I think its good to go.

There are two minor problems:

  1. the swagger docs is incomplete at some places. It doesn't show the responses that the server sends.
  2. When posting the xdr, the server responds with response code 201 instead of 200.

There is something else which I'm not sure is a problem: I am using the Java SDK to verify the challenge tx. Verifying the challenge tx with a muxed account fails with an error saying that the tx is not signed by the server account. This is probably just a bug in the Java SDK though.

There actually was a problem with how my code was handling muxed accounts! I fixed that, and improved the swagger docs to add reponses (including examples)

@rahimklaber Tell me what you think :)

silence48 commented 2 years ago

I've looked at this code, and it looks good. Still would like to look at it a bit deeper. The code installs and runs with a little coercing (probably from windows) I'd like to have some docs on the swagger auth... The end points all seem to work, the details, and stellar.toml endpoint work perfect. I was able to generate a tx on the GET /auth endpoint, but was unable to verify it on the POST /auth after signing it; though maybe I did something wrong. will review further and advise.

fpbrault commented 2 years ago

I've looked at this code, and it looks good. Still would like to look at it a bit deeper. The code installs and runs with a little coercing (probably from windows) I'd like to have some docs on the swagger auth... The end points all seem to work, the details, and stellar.toml endpoint work perfect. I was able to generate a tx on the GET /auth endpoint, but was unable to verify it on the POST /auth after signing it; though maybe I did something wrong. will review further and advise.

Has the account you used to generate the challenge funded? My code retrieves the account at verification to check its thresholds, so you'll get an error if the account cannot be found! However, this tells me I should maybe add a specific error message for this situation, since right now it only shows "Signatures are not valid or do not meet the required threshold!" instead of something like "Account cannot be retrieved, make sure it has been funded" or something in that style...

BlackBadPinguin commented 2 years ago

This Implementation looks really good, the Authentication works fine, the API responses are also all according to Spec. For me, everything worked fine (on testnet). There are good instructions on how to set it up in the Repo, also the Code is very well documented. I also tried your muxed Account fix and indeed it is fixed right and according to Spec. So I think the same as @rahimklaber it is good to go.

rahimklaber commented 2 years ago

Trying to verify the challenge tx (with tha java sdk) when using muxed accounts still fails with an exception : Exception in thread "main" org.stellar.sdk.InvalidSep10ChallengeException: Transaction not signed by server: GCQBVSARJXE4MCDOD7CZ2BNIB34KQ7KM4VO3OCHJXCJQW3IOYWOGWDLP. (with Sep10Challenge.readChallengeTransaction).

I also tried verifying the signature with KeyPair.verify which also doesn't work. There is no issue when using normal accounts.

I'll try it in another language, but it will probably be some time. Could someone else try to verify the challenge tx? Not sure if I'm doing something wrong.

fpbrault commented 2 years ago

@rahimklaber I don't think the java sdk supports muxed accounts for SEP-10 right now: https://github.com/stellar/java-stellar-sdk/issues/360

BlackBadPinguin commented 2 years ago

For me it worked with the js sdk.

rahimklaber commented 2 years ago

Alright, good to go for me then. 👍

fpbrault commented 2 years ago

Should I set this to review completed?

rahimklaber commented 2 years ago

Yes, I think you can do that. 👍

ElliotFriend commented 2 years ago

@silence48 I don't see an address for you in the ADDRESSES.yml file. Can you add one?

ElliotFriend commented 2 years ago

68c15d081c5d06769de2b83779a7baf67b3a19a2a90bf3064af2f002b4bcaebe

ElliotFriend commented 2 years ago

@fpbrault For accounting purposes, what's your Discord user id? If you'd rather not share that here, you can send me a DM on discord. I'm ElliotFriend there 👍🏻