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

Revert to TtT contract with on-chain tribute storage #8294

Closed
rachelhamlin opened this issue May 28, 2019 · 13 comments
Closed

Revert to TtT contract with on-chain tribute storage #8294

rachelhamlin opened this issue May 28, 2019 · 13 comments

Comments

@rachelhamlin
Copy link
Contributor

rachelhamlin commented May 28, 2019

Description

Type: Feature

Summary: As a result of the issues found in #8129, we need to move storage of a user's tribute setting from IPFS to the TtT contract.

This means that we are also removing the personalized message from v1 of the feature.

Expected behavior

TtT works in 100% of scenarios.

Settings flow no longer contains option to set a personalised message.
Chat flow only displays system message requesting tribute amount, no personalized message.

image

Actual behavior

TtT is failing to send and receive information from IPFS.

@rachelhamlin rachelhamlin added feature feature requests and removed feature feature requests labels May 28, 2019
@yenda
Copy link
Contributor

yenda commented May 28, 2019

@3esmit is the simple contract already deployed? could you provide the address on testnet?

@3esmit
Copy link
Member

3esmit commented May 28, 2019

@yenda Deployed in Ropsten. Let me know if you need a different network

Version without messages and default fee of 0.1 value:
https://ropsten.etherscan.io/address/0x45d8274c1a4c6b5b956e8d0f44a9e78209b0bb77#code
browser/MessageTribute.sol :
bzz-raw://b3fc092f9b99fcc89693c9adb553acd2c01b19e7d585b2c58d600be6fe43c99f
metadata.json :
bzz-raw://cbae2ca4219b4547c6fc3e8e7a0f3a12081b2a0cca376c72b2b40dc15d5b4dd0

@yenda
Copy link
Contributor

yenda commented May 29, 2019

@3esmit I don't think there should be a default tribute amount can you remove it? @rachelhamlin

@rachelhamlin
Copy link
Contributor Author

Yes please! 0 default.

@yenda
Copy link
Contributor

yenda commented May 29, 2019

also I don't think we should have a way to set a default because it makes the reset method useless, the only place I see it useful is to remove tribute, but if instead it goes back to the default value it is misleading and probably not something someone will want. For this iteration we can make things as simple as possible. Btw idealy the method names should be getTribute(address), setTribute(uint256), removeTribute()

@3esmit
Copy link
Member

3esmit commented May 30, 2019

I did the changes as you requested. Kind reminder that I believe a default small fee is an important step against spam.

I didnt included a removeTribute() because you can use setTribute(0). The contract with default fee needed a resetFee function so it would start using the default again. As there is no default, the remove/reset tribute is not required.

This can be a boomer when we have separated Wallet and Identity, so I started thinking on how one wallet can pay for a chat only identity tribute to talk.
So I did two versions, one with MessageSigned capability, meaning that is possible of an externally owned account which don't hold any funds to sign a message and get it deployed by any other address.
This is possible using an ethereum signed message support in the contract, however I added a TTL to the message, to prevent it being reused in future, and also prevent user from creating a too large TTL.

Im also in the research how to make an "gas anonymizer", so users dont tie wallet to chat identity, this seems to be possible by using a mixer to pay to the gas relayer. This research is happening in other swarm (Gas Abstraction/Identity/ERC725).

The first version, more simple, is available here:
https://ropsten.etherscan.io/address/0xc61aa0287247a0398589a66fcd6146ec0f295432#contracts
https://rinkeby.etherscan.io/address/0x326e29956334a352e8d6f24e22d781020311eba6#contracts
https://goerli.etherscan.io/address/0x326e29956334a352e8d6f24e22d781020311eba6#contracts
Sources:
https://github.com/status-im/contracts/blob/aa2d12c253b99b2d3d335db7d37446b2458f1293/contracts/communication/MessageTribute.sol
MessageTribute.sol: bzz-raw://af585a5421934776033d257b2f511412d6ac28e1c3fe259d91b88aa38164e826
metadata.json: bzz-raw://4ed0b73f2fe48042ae34f201e6b74a48c533aaf5d424b0101cd29c3f525fcf40

The second version, with message signature support, is available here:
https://ropsten.etherscan.io/address/0x048887496c59c55ca703d167becb9112ec32c18f#contracts
https://rinkeby.etherscan.io/address/0x084dc950f387675044a1404842805678da3fc5a8#contracts
https://goerli.etherscan.io/address/0x084dc950f387675044a1404842805678da3fc5a8#contracts
Sources:
https://github.com/status-im/contracts/blob/9b916795a89416d63318799cf7bb6711f643bd33/contracts/communication/MessageTribute.sol
MessageTribute.sol: bzz-raw://5a8f467bcdc22628f363302b783d6a8c03252ca8d9161e5fb777699f90b224f9
metadata.json: bzz-raw://1870ccff990c1ff63a603d2bcf0f4c6181f2d7350f4831ce2d7a8faa49beb155

The third version, with gas anonymizer support, would feature a similar change, an overload of setTribute function which contains the incentive to a random gas relayer to commit the transaction inchain and get paid in an zk-snarks mixer.

@rachelhamlin
Copy link
Contributor Author

Thank you @3esmit!

@yenda
Copy link
Contributor

yenda commented May 31, 2019

@3esmit Is there a way to use the simple contract and later on add the possibility to use the MessageSigned capability in another one while still using this one as registry so that old versions of status still have access?
Also if I set tribute to talk using the MessageSigned capability to which wallet are the tributes going to be paid? If I understand correctly I am still sending the fee to the signer which supposedly doesn't have an account at this address?

@yenda
Copy link
Contributor

yenda commented May 31, 2019

@3esmit thanks for the detailed post btw!

Another question, do we really need to emit a log for the simple contract?

@3esmit
Copy link
Member

3esmit commented Jun 3, 2019

The upgradability is not implemented on this contract. Migration process can be defined in UI, but would be painful. New contract can read from old contract to skip migration of already defined values in older contract.

I can deploy this contract and use a UpgradableInstance.sol (Proxy Contract, uses delegatecall) pointing to it, so we can have upgradability.

@3esmit
Copy link
Member

3esmit commented Jun 3, 2019

The use of events is not necessary, but they might be important for updating the UI accordingly to the changes.

As a common practice, meaningful state changes on the contract should fire events.
Events can improve the performance of the dapp, in an example, the dapp could load values of a list of address in background and cache them in local database, and subscribe to the according events that would update this cache database.
When user interacts with this addresses the values would be locally available and would not have to be downloaded from lightserving nodes, improving UX.
Also, events can be filtered when they have "indexed" parameters, this allow Dapp to be notified by the blockchain node.
For example in ERC20 event Transfer(address indexed _from, address indexed _to, uint256 _value)
The wallet could subscribe to Transfer when from is the address of user, or when to is address of user, can accordingly display an notification in smartphone of this moving of funds, without having to check every transaction in blockchain.
Its important to be bolded that events are ephemeral, they cannot be read from the contract, only from transactions, and they are fired automatically by the node when the new block have its transactions validated, so very old events would not be fired with a fast sync, only with slow sync, and this is just one of the caveats of events.
See this post about EVM events and problems https://hackernoon.com/ethereum-dapp-infrastructure-2b4f1e6bfa38

However for this version of Tribute-to-Talk, indeed events are not necessary, as the values will be read from the contract always on demand, because we never know what is the new contact user will try to message, so we cannot cache that data locally, and once it's paid, there is no need of keeping sync on it.

For recurring payments/subscription model this events might be interesting depending on the features of the contract, for example, if the subscription value was changed, this could be grabbed from an event, instead of every time reading all subscriptions to see if they changed value.

@rachelhamlin
Copy link
Contributor Author

The upgradability is not implemented on this contract. Migration process can be defined in UI, but would be painful. New contract can read from old contract to skip migration of already defined values in older contract.

Does this have implications for #8047?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants