Skip to content
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

OIP Blockchain integration #12

Closed
wants to merge 7 commits into from
Closed

Conversation

T-Dnzt
Copy link

@T-Dnzt T-Dnzt commented Aug 9, 2018

This PR contains the blockchain integration OIP.

Status: WIP, but feedback and early review welcome.

@T-Dnzt T-Dnzt self-assigned this Aug 9, 2018
@T-Dnzt T-Dnzt added the category/ewallet Category - eWallet SDK label Aug 9, 2018

- `contract_address` (`nullable`)
- `contract_template_uuid` (`nullable`)
- `status` (`pending`|`confirmed`)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is pending means the contract is deploying?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes


### eWallet Setup

The only mandatory step to plug an eWallet to Ethereum is to generate a new Ethereum account. The public key for that Ethereum account will be used as the main identifier for the eWallet. The private key will be encrypted and stored in the database, while the encryption key is stored in environment variables.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The terms "public key", "private key", "encryption key", and "this value" could be reduced to just "public key" and "private key"? Just to reduce confusion.

Suggesting:

The only mandatory step to plug an eWallet to Ethereum is to generate a new Ethereum account. The public key for that Ethereum account will be used as the main identifier for the eWallet. The private key will be encrypted and stored in the database, while the public key is stored in environment variables.

This public key value should be stored in the Setting table, under primary_pub_key.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The suggestion is functionally different to the original text.

Could we please clarify?

  1. The private key is encrypted and stored in the database (this is the same in the original text and suggestion)
  2. Without specifying the encryption method of the private key in 1), where is the information stored that is required to decrypt the encrypted private key?
  3. Where is the corresponding public key from point 1) stored?
  4. Is the public address, which is a representation of the public key stored somewhere?


Very close to the current state of things, except transactions between eWallets happen on the blockchain/Plasma. Users are not able to get their private keys.

#### 2. Optional User custody (providers own all the private keys by default, but users may request ownership of their private keys with multiple options)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I like the wording "optional user custody", more accurate than "partial custody" that we used before.


##### From an eWallet to a user

1. A user purchases something through a provider, and that provider wnats to reward him with some loyalty ETC-20 tokens.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: wnats -> wants

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also ETC-20 -> ERC-20

For the flows below, we'll use the following use-case:

```
A end-user wants to purchase something from eWallet A using a mobile application published by (and therefore connected to) eWallet B.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: A -> An

3. eWallet B detects that the received public key doesn't match its own, meaning the transaction request is hosted by another eWallet.
4. eWallet B then makes a call to the eWallet Ethereum Name Service sending the public key received in the consumption call. A domain name will be returned by the NS.
5. eWallet B sends a request to eWallet A to consume the request on behalf of the user. It receives a consumption ID back (with status `awaiting_funds`) and whether or not the tokens need to be converted (and if yes, to which currency) - this is defined by a conversion strategy inside the eWallet. In this case, eWallet A only wants to receive `ABC`. eWallet B connects to the websocket channel for the consumption.
6. eWallet B requests a quote from the DEX in order to know how many `ZYX` are needed to send 10 `ABC` to eWallet A. An event is sent to eWallet A about the progress.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if the quote is timely enough to make the exchange. The orderbook may have already changed since the inquiry. Should the exchange still be done or cancelled?

Maybe the user needs to set an acceptance threshold that the exchange should still happen within +/- 2% of the quoted rate?

Or maybe it's possible to setup a counterparty that's willing to accept the exchange rate risk by charging a percentage fee, similar to the Plasma's fast withdrawal counterparty? :S

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes this is a very good point. Usually, an exchange doesn't provide a quote for a guaranteed price. The exchange will have indicative information on the order books what the price may be and also what the last traded prices were.

When we wrote user stories, for the ideal user flow, we required a broker persona that could provide a quote. Note that the broker could be a service from the exchange, or a third party.

- `inserted_at`
- `updated_at`

In case a blockchain wallet goes below or above the limits, a user will be notified that an action is required to keep the system secure (like moving funds in/out from/to a cold wallet.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! I think this eventually could be extended into a one-way sweep account where if a hot wallet goes beyond high_amount, it triggers an auto-transfer of the extra amount to a cold wallet.


See "Database Changes" for more information.

### Plasma Deposits and Exists
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: exists -> exits


Blockchain wallets will be the representations of Ethereum accounts in the eWallet. Two types of blockchain wallets will exist: `hot` and `cold`.

The eWallet saves the encrypted private keys of the `hot` wallets in the database, with the encryption key being stored in a file to which only the application has access. No private keys are stored with `cold` wallets and the administrators are responsible for managing them directly. Interaction with `cold` wallets will require manual action through [Metamask](https://metamask.io/) or similar tools.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From team discussion:

Env var means it's available to all process running in the container. Using a file instead means you can restrict which user and process can access it but it won't be 12-factor.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which means we should recommend the use of a file but allow the ENV variable, in case someone would like to deploy on Heroku for example.


### Plasma Deposits and Exists

Every time funds are deposited on a plasma chain or exited, a deposit/exit will be recorded in the database.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need separation of concerns in this aspect, between the ewallet (blockchain adapter) and the Watcher, as we're likely having the Watcher record these. CCing @pnowosie to follow/take over this conversation

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the Watcher takes care of it, then it probably doesn't have to be in the eWallet :)

6. eWallet B forward the details to the user's mobile application.
7. The user then needs to request a quote from the DEX directly in order to know how many of his choice of currency are needed to send 10 `ABC` to eWallet A. An event is sent to eWallet A/B about the progress.
8. The user receives the quote back from the DEX on the Mobile application and approves it. An event is sent to eWallet A/B about the progress.
9. The user deposits funds on Plasma in his choice of currency.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is an expensive and time consuming operation. It is likely, it wouldn't be undertaken for every single payment, so the user would hold funds on Plasma already. That of course requires the user to validate the child chain (this way or another - not necessarily running a Watcher themselves, but somehow ensuring that the funds aren't getting stolen, in a manner appropriate to risk tolerance etc.)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this scenario is quite unrealistic right now. A user would need to have some funds already on Plasma for a good experience. I'm hoping one day things will be at a state where deposits are potentially fast enough (sharding, Ethereum POS, etc).

I'll update it and move that step as a prerequisite

- `mint`
- `burn`
- `send_chain_transaction`
- `send_offchain_transaction`
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd not call plasma transactions offchain, better plasma_transaction or child_chain_transaction

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I called it like this to reflect the idea that the eWallet could potentially run on a different blockchain/offchain/dex system.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is then the difference between send_chain and offchain in terms of API / expected behavior

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree we should think about chain and offchain. A Plasma chain, is a blockchain.

We may want to consider that there will be multiple Plasma chains in a nested fashion. Whilst there will be be one rootchain.

In this instance, I think we are saying:

  1. send_chain_transaction = send_rootchain_transaction
  2. send_offchain_transaction = send_chain_tran
  3. In future we may need to have a send_parentchain_transaction (with the ambiguity being the parentchain may be the rootchain...)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using rootchain and chain sounds good to me, I'll update.


##### `omg_adapter`

- `create_account` (creates an Ethereum account)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

privkey handling/ signing is important here - how will all these handle that? It looks like the flow will be different for hot and cold wallets

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, good point, Will explore and expand on it.


In case a blockchain wallet goes below or above the limits, a user will be notified that an action is required to keep the system secure (like moving funds in/out from/to a cold wallet.

Table name: `child_chain_contract`
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so we've intended every Watcher instance to be "bound" to a particular child chain contract. Such watcher is validating that particular child chain and it doesn't ever "switch". So every entry in such db would determine where to speak to the appropriate Watcher.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, makes sense! What would be the process in case of a mass exit in POS? Do we need to get a new Watcher?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reckon, the watcher wouldn't change much in POS, in terms of API, but that's just a first guess, would need to think more.

Where things could start to get funny is when you actually watch several chains - either completely independent copies of a similar child chain, different independent child chains or interrelated child chains (child and grand-child chains that use the former as parent chains).

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah wait, you mean one child chain breaks or goes defunct, the wallet (or users) exits and then wants to use a new child chain... then you might either need to somehow archive that old Watchers data and ditch the old Watcher completely and use a new one, potentially speaking to the same place for Watcher's service. Or you would keep the broken Watcher running in "frozen" state, just to serve the old data and allow for exits.

Hmm... this is interesting, we'll need to think about this more

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that's what I was thinking, as I'd like the eWallet to be able to exit / re-enter automatically, hence the list of "plasma contracts". Not sure if we should let this be handled manually tho.

@ghost ghost added s1/draft Status - Drafts s2/wip 🚧 labels Aug 16, 2018

### Transaction Flows for two eWallets

For the flows below, we'll use the following use-case:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found the following section confusing to follow. Normally, I would think on terms of Alice (A) sends to Bob (B) rather than the other way round.

For example, the heading eWallet A (Full custody) -> Ethereum address with regular transaction looks like wallet A sends to an Ethereum address.

Edit: typo


In the cases below, both eWallets have funds deposited in Plasma: eWallet A has `ABC` tokens while eWallet B has `ZYX` tokens.

#### eWallet A (Full custody) -> Ethereum address with regular transaction

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will be we supporting the case where eWallet B has funds on the Plasma chain, but eWallet A wants funds on Ethereum? Might need to be clear about this.

#### eWallet A (Full custody) -> eWallet B (Full custody) with regular transaction

1. Using eWallet B mobile app, a user wants to send funds to a user of eWallet A.
2. When submitting the transaction, the user needs to specify an eWallet's public key as well as an internal wallet address.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to be careful here because the internal wallet address could potentially identify the recipient. It would be better if eWallet B calls an end point from eWallet A to for a payment request.

eWallet B can then receive a one time use correlationId and also an attestation that eWallet A will forward funds to the respective account. The latter is especially useful for the unhappy path where funds were sent but it didn't reach the recipient.


#### eWallet A (Full custody) -> eWallet B (Full custody) with transaction request

1. Using eWallet B mobile app, the user scans a QR code to pay for his purchase at eWallet A (10 `ABC`) using funds stored in eWallet B pooled wallets (`ZYX`). That QR code contains a string composed of an `ewallet_pub_key` and a transaction request ID.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest ewallet_a_pub_key for clarity.

Also, we should consider that there may be multiple Plasma chains and nested Plasma chains.


This section describes how the flows explained earlier interact with the eWallet entities.

Coming soon.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some description of the the 'unhappy paths' is required.

  • Particularly the case where a user with custody of their private key sends funds to an eWallet with full custody.
  • What happens if an underpayment or over payment is made?

3. eWallet B detects that the received public key doesn't match its own, meaning the transaction request is hosted by another eWallet.
4. eWallet B then makes a call to the eWallet Ethereum Name Service sending the public key received in the consumption call. A domain name will be returned by the NS.
5. eWallet B sends a request to eWallet A to consume the request on behalf of the user. It receives a consumption ID back (with status `awaiting_funds`) and whether or not the tokens need to be converted (and if yes, to which currency) - this is defined by a conversion strategy inside the eWallet. In this case, eWallet A only wants to receive `ABC`. eWallet B connects to the websocket channel for the consumption.
6. eWallet B requests a quote from the DEX in order to know how many `ZYX` are needed to send 10 `ABC` to eWallet A. An event is sent to eWallet A about the progress.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes this is a very good point. Usually, an exchange doesn't provide a quote for a guaranteed price. The exchange will have indicative information on the order books what the price may be and also what the last traded prices were.

When we wrote user stories, for the ideal user flow, we required a broker persona that could provide a quote. Note that the broker could be a service from the exchange, or a third party.

@T-Dnzt T-Dnzt changed the title Blockchain integration OIP Blockchain integration Nov 12, 2018
@T-Dnzt T-Dnzt added this to Inbox in eWallet Apr 22, 2019
@T-Dnzt T-Dnzt moved this from 1-Inbox to 3-In Progress in eWallet Oct 22, 2020
@T-Dnzt T-Dnzt moved this from 3-In Progress to 4-Review in eWallet Nov 18, 2020
@T-Dnzt T-Dnzt moved this from 4-Review to 1-Inbox in eWallet Dec 9, 2020
@T-Dnzt T-Dnzt closed this Mar 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category/ewallet Category - eWallet SDK s1/draft Status - Drafts s3/review
Projects
eWallet
  
1-Inbox
Development

Successfully merging this pull request may close these issues.

None yet

5 participants