pendulum-chain / vortex

1 stars 0 forks source link

Phase 3 of Offramp prototype Pendulum -> Spacewalk -> Stellar Mykobo #20

Closed prayagd closed 4 months ago

prayagd commented 5 months ago

This ticket describes a webapp based version of the offramping prototype that uses Spacewalk and a SEP-24 based offramp on Stellar in order to offramp EURC.s on Pendulum.

Goal

The goal of this phase is remove the stellar secret key input field too and allow the user to offramp just by connecting their Pendulum wallet

Todo

This is a follow-up ticket to #19

prayagd commented 5 months ago

@gianfra-t @ebma The signing part of the stellar txn and tech details of it are not very clear to me, hence just mentioned as a line item, Would you be able to add the tech details there?

Here is the notion ticket, where Torsten has explained it.

prayagd commented 5 months ago

Hey team! Please add your planning poker estimate with Zenhub @b-yap @bogdanS98 @ebma @gianfra-t @TorstenStueber

ebma commented 5 months ago

@prayagd done.

@TorstenStueber do you already have something in mind for the backend server? In the past we learned that AWS Lambda function and similar are more costly than spinning up a simple service in our infrastructure so I guess we should do that. Should we add the code to this repository as well or put it somewhere else?

prayagd commented 5 months ago

one question

  1. Would we need a account were this temporary stellar account be funded from?
ebma commented 5 months ago

Yes we do. Something similar to the account you shared previously. To be on the safe side, we can merge the funds from that account into a new one so that we have a new account with a secret that was not exposed in the browser text field. I would only do this once the implementation of this is almost finished.

TorstenStueber commented 5 months ago

@ebma Lambda was more costly? I actually expect that we are able to use it for free. Do you remember where he had to pay for it?

Anyway, it's best to leave this to @zoveress to find a way to deploy it and we can just write a normal nodejs backend.

Particularly as this is a (possibly throw away) prototype, I would put the backend code in the same repository.

ebma commented 5 months ago

I think we wanted to use Lambda for our token stats API but identified that it's more costly. Though maybe just because we wanted to add an extra cache and rate limiting or something, I don't remember the details.

I would put the backend code in the same repository.

Alright, then let's add it to the same repository. We can again draw inspiration from the implementation of our existing services, like the faucet.

TorstenStueber commented 5 months ago

About the data flow between frontend and backend

There are four transactions that require that both parties (funding account and ephemeral account) sign:

I think that we can combine the first two transactions into a single one. Each of these transactions needs to be signed by both key pairs (so once in the browser and once in the backend).

It is important that the backend does not blindly sign a transaction. Out of experience it is usually easiest and safest to:

I suggest two endpoints:

TorstenStueber commented 5 months ago

Some decisions we took today:

gianfra-t commented 5 months ago

Regarding the data flow, we could make the back-end send the finalized transaction and only respond with a success or error, and delegate stellar error handling to the back-end.

Or alternatively, we can send back the transaction signed by the back-end to the client again, and maintain the error handling in the client. Any preference regarding these alternatives @TorstenStueber @ebma ?

ebma commented 5 months ago

Or alternatively, we can send back the transaction signed by the back-end to the client again, and maintain the error handling in the client.

I think it's better to keep the back-end lean and leave the transaction submission and error handling to the client.

gianfra-t commented 5 months ago

Yes I thought the same too, in fact it would be the easiest to create the tx on the backend, sign it, and then send it to the user for final co-sign.

Now, I have another doubt that is more in lines with security or potential exploits. If we at any point allow the user control of the ephemeral secret, we risk an exploit in which the user purposely shuts down the process before the clean up (could be as soon as the creation of the ephemeral account is submitted). So a malicious could do this and we would end up with many funded but irrecoverable ephemeral accounts.

TorstenStueber commented 5 months ago

In the future it would make sense to let the backend store the transactions and also submit them as it should not rely on the user to submit the transactions once Spacewalk transferred the money to the ephemeral accounts (otherwise the funding account can be drained). But this goes beyond (this version of) the prototype.

For the data transport I again recommend that not transactions but only signatures are sent. That is from our experience that it is much cleaner and safer code to recreate transactions on both sides than validating a transaction (there are a lot of many little details that need to be validated in order for this to be safe and this code is brittle if we need to change transaction details).

Transaction Envelopes

Here are some details how to do this. The TransactionBuilder does actually not create a transaction, but a transaction envelope, two different things. The envelope consists of a) a transaction and b) an array of signatures for this transaction (confusingly the TypeScript type for the envelope in the stellar-sdk is just Transaction).

This allows to attach multiple signatures to the envelope. The envelope is what is ultimately submitted to the Stellar horizon (RPC) server, not the blank transaction.

For our envelopes we need to attach two signatures, the one from the ephemeral keypair and the one from the funding account keypair. The TransactionBuilder always creates an envelope with an empty array of signatures.

The sign method of the envelope (TypeScript type Transaction) does two things: create a signature and attach it to the envelope.

We need two new operations now:

gianfra-t commented 5 months ago

Okay so we can avoid thinking about that scenario for now.

Regarding the sending of the transactions, I was thinking on using the method toXDR() either with empty or appended signatures in order to send that envelope object from the back to the UI. Later, the UI or backend can reconstruct the envelop by using TransactionBuilder.fromXDR().

With this there wouldn't be a need for the UI to build the transaction in the first place, so the backend could just create it (if needed, according to the parameters sent) and send it ready for co-sign and submission. I tested this locally and seems to work. Would this be an adequate flow for this prototype?

TorstenStueber commented 5 months ago

But this is exactly what I say we should not do: don't send transactions because they need to be validated and recreate them instead.

We could say that for the prototype we could be less secure and send the transaction and don't validate it but eventually we need to do it that way to be sufficiently safe and with shared code it is not any more complicated.

gianfra-t commented 5 months ago

Yes I understand! I thought if the response of the server could be trusted, then there was no need to validate the transaction on the UI. This was all in the context in which the server creates the transaction in the first place.

But it's true, if we will aim at validating in the future, we can recreate both and test that.

vadaynujra commented 4 months ago

Successfully tested.

ebma commented 4 months ago

Let's do one last test next Monday to check if the backend runs stable over many days and then we can merge to production and close this ticket.

vadaynujra commented 4 months ago

Tested again today and worked properly.

ebma commented 4 months ago

Let's close this ticket then and merge v3 to production then @vadaynujra?

vadaynujra commented 4 months ago

Let's go for it.

ebma commented 4 months ago

Pushed the changes on staging to production. Closing this ticket then.