add helper for parsing AnchorTransaction #111
Conversation
| @@ -57,3 +57,15 @@ describe("SEP-10 helpers", () => { | |||
| expect(isValid).toBeFalsy(); | |||
| }); | |||
| }); | |||
|
|
|||
| describe("Server helpers", () => { | |||
| it("should parse a JSON AnchorTransaction", () => { | |||
There was a problem hiding this comment.
Can you add a test for MGI incomplete transactions?
There was a problem hiding this comment.
ya, do you have a json string by chance? I'm having trouble generating one
There was a problem hiding this comment.
hey @Ifropc were you able to get one?
| transaction: string, | ||
| ): AnchorTransaction => { | ||
| const parsed = JSON.parse(transaction); | ||
| if (parsed.kind === "withdrawal") { |
There was a problem hiding this comment.
should this method recognize withdrawal-exchange and deposit-exchange kinds as well? I know the task is about Sep-24 but in case we wanna make it Sep-6 friendly as well we could easily adapt it by checking parsed.kind?.includes("withdrawal") instead of the direct === comparison. Wdyt?
There was a problem hiding this comment.
hmm I like the idea but imo I think separating the parse functions would be better bc of the types. The types for sep6 and sep24 transactions are similar but have slight differences. I think if we'd want to add a parse method for sep6 it should be a separate method to make it clear what the method is returning 🤔
right now it's returning AnchorTransaction, if it was returning AnchorTransaction | Sep6Transaction I think it would be a bit messy / too general
imo we can add in a separate PR if it's needed
if you feel strongly though lmk!
| export const parseAnchorTransaction = ( | ||
| transaction: string, | ||
| ): AnchorTransaction => { | ||
| const parsed = JSON.parse(transaction); |
There was a problem hiding this comment.
I believe JSON.parse() alone can throw, should we wrap it with try catch just in case we receive something not well formatted from the server?
There was a problem hiding this comment.
And throw a different error in this case, something like TransactionIsNotValidJSONError() or similar.
Too conservative?
There was a problem hiding this comment.
ya I was on the fence if the try-catch should be handled here or by the function using this method.
But I think you've nudged me over the edge :)
4684622
|
spoke with @Ifropc offline, going to merge this in to unblock it and will add the MGI test in a subsequent PR tracking it here https://stellarorg.atlassian.net/browse/WAL-1409 |
* fix broken test (#93) * add sep10 sign challenge txn helper (#95) * add sep10 helper method * name change * correct resp * create server module * add npm publish gha (#96) * WAL-1064 - add anchor platform integration tests (#99) * create workspace (#101) * create workspace * try * try * try * try * cleanup * update anchor platform docker run * fix * fix * upgrade babel/traverse * upgrade browserify-sign * add keypair package in a second workspace (#102) * add km package * fix tests * cleanup * add second CD (#109) * fix stellar-sdk imports (#112) * fix imports * fix * preparing for 1.4.0 release stuff (#110) * fix * fix webpack process * webpack fix (#113) * webpack fix * cleanup * add helper for parsing AnchorTransaction (#111) * add helper for parsing AnchorTransaction * use kind * add try-catch --------- Co-authored-by: Alec Charbonneau <aleccharb21@gmail.com>
ticket