status-im / team-core

Public repository for the Core team
1 stars 1 forks source link

Allow for smooth sending and receiving of SNT while maintaining privacy #5

Open andremedeiros opened 4 years ago

andremedeiros commented 4 years ago

Stakeholder: @rachelhamlin, @carlbennetts Engineer: @yenda

Introduction

We want to make it easy & painless to send SNT via Status so our marketing team can welcome new users with tokens during launch.

Current Status

Chat commands are currently disabled, which means that we don't have a flow for users to send or receive funds in a structured way. Additionally, users have no information about each other's wallets in v1 (as we have decoupled them from the Whisper key.) Lastly, contacts never exchange wallet information with each other.

The Problem

To facilitate the smooth distribution of SNT by our marketing team, we want to re-enable chat commands. In a multi-account world, this is not a straightforward problem to solve. We have taken privacy and security considerations to heart in the following options.

Part of the beauty of multi-account and the decoupled keys is that the user has a higher degree of privacy. We want to uphold that as much as possible while still facilitating transactions between casual acquaintances or even strangers.

Why Don't We Just ...

... Share the Address as Text in Chat?

Sharing the wallet address as text in chat is very inconvenient, for multiple reasons, chiefly being that copying and pasting text in mobile isn't exactly smooth. The in-chat request solution is equivalent, but just much more convenient, and follows a curated flow.

... Share the Address in Every Message? (like we do with ENS name)

Sending the ENS name with every messages was already a discussion that we've had and wasn't something we were 100% sure about. As a result, wallet address seems even more at odds, as it isn't directly related to chat unlike the ENS name which is meant to be displayed, and was ultimately added to each message for the sake of simplicity (checking if a user has an ENS name would have required a reverse lookup and to be done on every message, when the current solution only requires to check the validity of the claim for names you don't already know.) It is also spammy, as it adds unnecessary bandwidth and storage for little return.

Not only that, we can't guarantee that the wallet information is up to date, and if we want to allow the user to choose which address to share, it would have to be stored as a preference, which then would have to potentially be synced across multiple devices.

Ultimately, we don't allow custom usernames. Why would we allow custom wallets to be defined in each message? If someone wants to define a wallet in each message, they can use an ENS name, which already binds to a wallet.

... Use Hot Wallet Using Chat Private Key?

Creating this wallet when creating/recovering an account is a low effort. However, this is already how the wallet was generated in beta. Now, wallets are generated from a separate derivation path than the chat key. The driver for this is so that users can have the chat key on the phone and keep their wallets on a Keycard.

Also, only the chat key is loaded in memory (to decrypt messages), which means that we would go back to square one and share again the same private key for the chat and wallet account (so they'd essentially be the same.) The biggest concerns with that are security and privacy. It will require us to think hard about the UX to help the users understand and maintain their wallet hygiene (too many funds and it becomes a target, no privacy as everyone can link this wallet to their chat account, risk of revealing other private wallets when making transactions from/to the hot wallet), so the effort isn't even that low, as there are so many issues to take into consideration if we open that door again.

One strong argument for this is that it would enable SNT reactions, however, this feature could be done with a contract so that isn't a requirement.

Note that the address of the wallet account that is used to register the ENS name is revealed publicly because it is directly associated with the ENS name in the contrac t(as warned in the ens registration process.)

... Profile Settings / Wallet Sharing?

When you add a user as a contact, you share your wallet address in the contact request. The wallet account that you share with your contacts comes from your profile settings.

The issue with this scenario is that the user will have to know that they need to add someone as a contact to share their wallet address, and to be able to receive funds from another user. They will also need to know that they need to be added back by that person in order to be able to send funds.

As such, this solution doesn't simplify anything compared to the in-chat wallet sending / requesting flow, but could complement it in the future. In the interest of time, we stand to gain more by focusing on the in-chat flow first.

Additionally, the fact that someone added you as a contact isn't necessarily information that is directly communicated to the user at the moment it happens, but rather something that can be inferred because of the informations that the user shares with you (profile picture, contact updates.)

Proposals

In-Chat Request (Request / Send Wallet Command)

Ease of Use: High It is clear to the user that, when he wants to send funds, he makes a request and chooses a wallet. The sender signs the transaction that opens on the request. When the user wants to receive funds, the simplicity will depend on the variant.

Privacy: Reasonable You reveal your wallet address when you make a request, and since you are sending the request in a message signed with your private key (ie. you can't deny you asked me to send to this address.) It doesn't show that it is yours, but it does prove that you asked me to send funds there.

Effort / Complexity: Medium

Scenarios

Alice wants to send funds to Bob
Bob requests funds from Alice

Stage 1

Enable request command flow under a flag in 1-1 chat, using a default wallet address. Does not include design.

Stage 2

Stage 1 + allow Bob to select an account.

Stage 3

Stage 2 + Design.

Stage 4

Send command flow, remove the flag.

andremedeiros commented 4 years ago

Couple questions:

oskarth commented 4 years ago

Thanks for the write-up! I agree with proposed stages. The send command flow needs a bit more detail but this can come later.

For the system-messages, it'd be great if this was documented and pushed as a PR to relevant spec: https://github.com/status-im/specs/blob/master/status-payloads-spec.md

I also suggest we change the title to something more descriptive of the problem at hand, as this has morphed into something that has less to do with "Proof of Ownership" than it initially had.

(Changed title to clarify intent more, can probably be made more concise, feel free to edit)

yenda commented 4 years ago

Quick update for the dev call: as I'm diving into phase 1 and thinking about @3esmit proposal of using EIP681 for requests I propose the following:

The advantage is that you only need the capacity to show regular status chat to at least be able to see these message. The request being an EIP it is also understandable by any app that implements it.

yenda commented 4 years ago

Here is what it would look like:

Bob uses request command:

Bob uses send command:

yenda commented 4 years ago

If we want the request and transaction URI to be a proper standard we need to propose EIPs for them. We should discuss what these URIs have to include.

For transactions:

If we make an EIP for request based on EIP831 and EIP681 the idea would be that it can be used for uses cases other than the send command within Status. An example would be a QR code that you would scan and that would contain the information to claim some reward after scanning them.

3esmit commented 4 years ago

/send

/send could deliver a textual based like this: ethereum:tx-0xa3c4f14e049a9994a6a8235615e5ac42a5f9abbfe92dbf0f821e5d841c3f12ac

Initially it would link to etherscan, but as this get more features it can start rendering inside clients.

For the standard we should provide a way of giving the abi for the transaction data and for events we want them other to render: ethereum:tx-0xe57f30f18c43c77a91c5ef68e95037bb2d48e912e52a5ffff92c18ab56f6397b?calldata="transfer(address,uint256")&events="Transfer(address,address,uint256)"

Rendering transactions

Transactions should show minimal information about them, which could be expanded (or linked to external service) for users wanting to inspect more. Etherescan screenshot: https://user-images.githubusercontent.com/224810/68128196-058c4700-fef6-11e9-8e44-c6299696a985.png

ERC20

Note that:

  1. it should only render a box displaying ERC20 transfer (with token symbol, decimals, all beautiful) if the token address is an actual known token by user, otherwise it should only display a general box displaying the transaction details - this is crucial to avoid bad use of feature.
  2. This detection should happen using event logs, so it supports any transaction. Using the transaction data is not reliable because of smart contracts, as account contracts and multisigs.

Fields known ERC20 should show:

andremedeiros commented 4 years ago

@yenda @3esmit can we augment the original RFC comment with the added implementation details?

yenda commented 4 years ago

@andremedeiros yes shouldn't we make a PR rather than an issue to facilitate feeback integration?

@3esmit transaction URIs should include the chain-id otherwise you don't know which chain they are on, and also it will make it easier to warn the user when they are not on the same chain.

Regarding the calldata can't we always get it using the transaction and transaction receipt (or even etherescan api for simplicity)?

3esmit commented 4 years ago

@yenda agree with chainId.

I think is important to contain also the fields I mentioned, which is calldata and events. calldata does not contain the calldata itself, but the necessary data to decode the calldata (and events). Perhaps I named it bad and we can have it named method.

ethereum:tx-0xe57f30f18c43c77a91c5ef68e95037bb2d48e912e52a5ffff92c18ab56f6397b?chainId=1&method="transfer(address,uint256)"&events="Transfer(address,address,uint256)"

3esmit commented 4 years ago

Therefore with all this data, which is transactionHash, chainId, method signature and events signatures, you can decode everything related to the transaction.

Note that you should be able to describe multiple event signatures in the events field, as some events trigger multiple events. But a transaction can only call directly one method.

3esmit commented 4 years ago

The spec of /send output text is being written here https://notes.status.im/Mc8cH-HdS8qCIVd7fqetaA#

3esmit commented 4 years ago

EIP submitted as Draft. https://github.com/ethereum/EIPs/pull/2400