Build: Completed Bank Card Payment Validator#31
Build: Completed Bank Card Payment Validator#31emmaodia wants to merge 5 commits intoiExecBlockchainComputing:masterfrom
Conversation
|
@sulliwane Kindly review my PR. Thanks. |
|
|
||
| // dwolla api key | ||
| const APIKEY = process.env.DWOLLA_APP_KEY; | ||
| const SECRET = process.env.DWOLLA_APP_SECRET |
There was a problem hiding this comment.
Where are these envvar coming from? I don't see them set anywhere. Should they come from a dataset? You cannot expect the worker to have them "by default"
| throw new Error(results.error); | ||
| } | ||
|
|
||
| .then(res => const response = { |
There was a problem hiding this comment.
this then isn't attached to anything.
| request: { | ||
| type: 'GET', | ||
| } | ||
| } |
There was a problem hiding this comment.
Where are these values (amount, correlationId, request) used? This is confusing
| } | ||
| } ); | ||
|
|
||
| var iexeccallback = ethers.utils.defaultAbiCoder.encode(['uint256', 'string', 'uint256'], [timestamp, details, value]); |
There was a problem hiding this comment.
You are following the same format as the pricefeed, but I don't see any of the variables timestamp, details, value being set anywhere. Shouldn't this be changed to consider the results of the dwolla call?
| * TOOLS * | ||
| *****************************************************************************/ | ||
|
|
||
| const Client = require("dwolla-v2").Client; |
There was a problem hiding this comment.
Is dwolla-v2 available? I don't see it being installed in the dockerfile.
Amxx
left a comment
Amxx
left a comment
There was a problem hiding this comment.
App code is mostly clean, but I fell like this is probably not working:
- JS Packages are not installed in the dockerfile
- Envvar are not set (might require a dataset)
- Output format is not capturing the response provided by the Web2 api.
Also, no smart contract is present to show how this can be processed onchain. There is no link to any run of this app on kovan. Was it tested at all?
It's a good first step, but it's not yet ready.
Thank you @Amxx for the review. I will work on the changes you suggested. |
|
@emmaodia Hey! We will close the bounties end of this week. If you want to have a chance to win, please fix the issues raised by the reviewer! Thanks! |
|
Update made: emmaodia@e64158d @Amxx @sulliwane Thank you. |
Build: TLS Notary DOracle
3 participants
No description provided.