-
Notifications
You must be signed in to change notification settings - Fork 1
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
[Draft] Implement phase 3 of offramp prototype #22
[Draft] Implement phase 3 of offramp prototype #22
Conversation
…n/pendulum-pay into spacewalk-mykobo-prototype
by changing the used polyfills
by also defining the target to `esnext`
✅ Deploy Preview for pendulum-pay ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me overall 👍
signer-service/src/config/express.js
Outdated
// enable CORS - Cross Origin Resource Sharing | ||
app.use(cors()); | ||
|
||
const allowedOrigins = ['http://localhost:5173', 'https://pendulum-pay.netlify.app']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we remove this if it's not used? Might cause confusions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's. I intended to leave it for production but it is probably unnecessary for the prototype.
signer-service/src/index.js
Outdated
const FUNDING_PUBLIC_KEY = process.env.FUNDING_PUBLIC_KEY; | ||
const FUNDING_SECRET = process.env.FUNDING_SECRET; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Do we really need two separate variables here if the public key can simply be derived from the secret?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right. I didn't think about this. Let me make a small refactor so only secret is needed.
exports.createStellarTransaction = async (req, res, next) => { | ||
try { | ||
let { signature, sequence } = await buildCreationStellarTx(FUNDING_SECRET, req.body.accountId, req.body.maxTime); | ||
return res.json({ signature, sequence, success: true, public: FUNDING_PUBLIC_KEY }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: success: true
is redundant if we never return success: false
. In case something fails we just return 500
anyways.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True, and we are not checking for success on the UI, so I will just remove the field.
Follow-up to #18. Only merge once #18 is merged.
Closes #20.
Addresses comments from previous PR.
Server signature process
The service exposes two endpoints,
/v1/stellar/create
and/v1/stellar/payment
.The first will provide the signature for the creation and trust opening operation of the specified ephemeral account's address.
The second will provide two signatures: One for a payment transaction of a specified amount and destination, and another one for a trust closing and merge operation.
Sequence passing
For the first signing operation, the sequence number of the funding account will be provided from the service to the UI, which will load the account with this sequence number and public key to create the corresponding mirror transaction.
For the second call, the UI provides the sequence number to the service, which it will use to create the transactions and send the signatures.
Potential bugs
Several processes querying the same signing service could produce a situation in which the sequence number specified by the signing service is not valid if more than two signatures are created in parallel, at least for one of them.
This can be partially avoided by keeping a global sequence state on the service, but still the problem exists that the corresponding transactions must be executed by the UI in the order by which the service signed them, which is not certain since the UI may execute at different speeds due to network issues.