Improve Activity - add Uniswap trade transaction

AW-STJ commented 4 years ago

Can we add a trade transaction, as displayed by the wallet in the screenshots below:

cc @colourfreak, to guide on the best design for us to represent swaps and trades in activity

Sub issues:

[ ] #398 Support Trade
[ ] #399 Update 'Approve' activity

hboon commented 4 years ago

@AW-STJ assuming that the information to display is similar to the screenshot on the right, this requires some TokenScript work. Moving to TokenScript repo.

hboon commented 4 years ago

I haven't checked how the swap/transfer is done, the TokenScript-examples repo might be more appropriate?

hboon commented 4 years ago

Unassigning Tomek first. Weiwu and James can assign him back. Would be nice to have system design first before graphical.

SmartLayer commented 4 years ago

Unassigning Tomek first. Weiwu and James can assign him back. Would be nice to have system design first before graphical.

Yes, the following is what I agreed with James on the phone, so can you check if you agree with this too:

Currently, each Ethereum event can be mapped to a TS activity. Let's keep it. Even if there are new graphical designs, like grouping trades in the same transaction, the system design remains. That is, we do not create TS activites whose triggering condition is a set of events in one same transaction. A tx can be considered as a bag of activities.
[For pending transactions] There will be a mapping mechanism to map transaction data model to anticipated events, therefore allowing pending transactions to be displayed in the same way as transactions included in the blockchain. However, once the transaction is included in the blockchain, the mapping is replaced by the real transaction/event relationship. The detail of that mapping mechanism is to be designed.
That mapping mechanism should not map transaction data structure directly to activities†; instead, that mechanism should map transaction data to inputs and outputs, which are in turn mapped to activities. This allows the wallet to know how to modify the "available balance" and "future balance" of tokens, for pending transactions.

† for transactions that have inputs and outputs of course. If a transaction doesn't produce any token or reduce any token, such as a voting transaction, then rule 3 doesn't apply.

SmartLayer commented 4 years ago

regarding this specific task

[ ] research why the emitting of the token is not in the event (shouldn't it be ERC20)? (1h)
[ ] depend on the result decide action.
[ ] update UNI token's activity

hboon commented 4 years ago

Yes, the following is what I agreed with James on the phone, so can you check if you agree with this too:

Makes sense, some thoughts:

Currently, each Ethereum event can be mapped to a TS activity. Let's keep it. Even if there are new graphical designs, like grouping trades in the same transaction, the system design remains. That is, we do not create TS activites whose triggering condition is a set of events in one same transaction. A tx can be considered as a bag of activities.

If you are saying that without grouping activities via the transaction they are part of: I think [1] it would be good if that's possible, but it might not be. In this case I believe that there is only 1 event emitted and the transaction information we get from Etherscan are:

From the "normal" transactions API:

{
    "timeStamp" : "1601950354",
    "gas" : "224637",
    "contractAddress" : "",
    "txreceipt_status" : "1",
    "gasUsed" : "186820",
    "blockHash" : "0x33cb8281386e9c24aeb4ae080259ce129f259c66d9f310394edb967f8e6cbfed",
    "value" : "0",
    "confirmations" : "38711",
    "blockNumber" : "10999432",
    "isError" : "0",
    "hash" : "0xdfe89ded2490f6917c6aa83fd4b4d9790ac40521c441b419980f94abf9ae14bc",
    "gasPrice" : "77000001459",
    "cumulativeGasUsed" : "12419473",
    "transactionIndex" : "128",
    "input" : "0x18cbafe50000000000000000000000000000000000000000000000004563918244f4000000000000000000000000000000000000000000000000000000a15a75f894067e00000000000000000000000000000000000000000000000000000000000000a0000000000000000000000000fcabe3451ac8edfb8fb6b9274c2e095d9ccc8082000000000000000000000000000000000000000000000000000000005f7bd70a00000000000000000000000000000000000000000000000000000000000000030000000000000000000000001f9840a85d5af5bf1d1762f925bdaddc4201f9840000000000000000000000006b175474e89094c44da98b954eedeac495271d0f000000000000000000000000c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2",
    "to" : "0x7a250d5630b4cf539739df2c5dacb4c659f2488d",
    "nonce" : "32",
    "from" : "0xfcabe3451ac8edfb8fb6b9274c2e095d9ccc8082"
},

From the ERC20 interactions API:

{
    "tokenSymbol" : "UNI",
    "gasPrice" : "77000001459",
    "hash" : "0xdfe89ded2490f6917c6aa83fd4b4d9790ac40521c441b419980f94abf9ae14bc",
    "from" : "0xfcabe3451ac8edfb8fb6b9274c2e095d9ccc8082",
    "to" : "0xf00e80f0de9aea0b33aa229a4014572777e422ee",
    "blockHash" : "0x33cb8281386e9c24aeb4ae080259ce129f259c66d9f310394edb967f8e6cbfed",
    "gas" : "224637",
    "transactionIndex" : "128",
    "input" : "deprecated",
    "tokenDecimal" : "18",
    "tokenName" : "Uniswap",
    "timeStamp" : "1601950354",
    "gasUsed" : "186820",
    "confirmations" : "38757",
    "contractAddress" : "0x1f9840a85d5af5bf1d1762f925bdaddc4201f984",
    "nonce" : "32",
    "cumulativeGasUsed" : "12419473",
    "value" : "5000000000000000000",
    "blockNumber" : "10999432"
},

Event:

{
   "amount" : "5000000000000000000",
   "to" : "0xf00e80f0DE9aEa0B33aA229a4014572777E422EE",
   "name" : "Transfer",
   "from" : "0xfCABe3451aC8EDfB8FB6b9274C2E095D9cCC8082",
}

Can we get it just by decoding the transaction data? Apparently it's possible since Zerion displays it. I wonder how it deals with slippage.

[For pending transactions] There will be a mapping mechanism to map transaction data model to anticipated events, therefore allowing pending transactions to be displayed in the same way as transactions included in the blockchain. However, once the transaction is included in the blockchain, the mapping is replaced by the real transaction/event relationship. The detail of that mapping mechanism is to be designed.

Do you think we should take a look at failed transactions too (or at least a cursory look to see how/if it fits) #393

That mapping mechanism should not map transaction data structure directly to activities†; instead, that mechanism should map transaction data to inputs and outputs, which are in turn mapped to activities. This allows the wallet to know how to modify the "available balance" and "future balance" of tokens, for pending transactions.

What if it's a swap that invokes more than 1 pair? Eg. ETH -> USDT -> UNI?

[1] Qualifying it with because I'm not sure :)

SmartLayer commented 4 years ago

What if it's a swap that invokes more than 1 pair? Eg. ETH -> USDT -> UNI?

In designing, we will simply assume there is always more than 1 pair at work in each transaction, since new defi project gives you some token rewards (pun intended) in the process. We can always scale down to 1 pair but if we designed for 1 pair we can't scale up.

hboon commented 4 years ago

Perhaps this is also related:

https://github.com/AlphaWallet/alpha-wallet-ios/issues/2179#issuecomment-707599080

The transaction: https://ropsten.etherscan.io/tx/0x2eea5339e788b15b8cbfccb5117956781712afba0eeea4293d9bd89fe573e497 The event logs: https://ropsten.etherscan.io/tx/0x2eea5339e788b15b8cbfccb5117956781712afba0eeea4293d9bd89fe573e497#eventlog