Add Payment Hash to response and better error messages #51
Conversation
jonathanlei
changed the title
| @@ -71,10 +71,20 @@ export default async function (req: Request, res: Response) { | |||
|
|
|||
| try { | |||
| let result; | |||
| let paymentHash; | |||
| try { | |||
There was a problem hiding this comment.
Move the signing out of the the try. That way you dont have to create the variable paymentHash.
There was a problem hiding this comment.
I moved it out, the purpose of having it there is that the hash should be different before and after submitting right? I could have an error statement catching signing errors too.
There was a problem hiding this comment.
The hash will be the same before and after
There was a problem hiding this comment.
then we don't need signing
There was a problem hiding this comment.
Signing is used to add the signature field but that doesnt change the hash as far as I know.
There was a problem hiding this comment.
The signature is among the data used to compute a transaction's identifying hash. Adding or changing the signature will change the transaction hash. Relatedly, you can learn about transaction malleability here. Generally speaking, I don't consider any given transaction to have a hash yet if it is not signed (yet).
The hash is the same before and after submitting, if you are submitting a signed transaction.
jonathanlei
changed the title
There was a problem hiding this comment.
I think this is fine, although I have not tested it. Note that after a transaction is signed, it is possible to compute the transaction's hash. Submitting the transaction doesn't change its hash. So the extra assignment should be unnecessary; though I also don't think it would hurt.
High Level Overview of Change
Add Payment Hash to server response
Add more error code logging.
Add env.example and better local testing instructions instead of command line instruction.
Type of Change
X] New feature (non-breaking change which adds functionality)