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

Improve Activity - add Uniswap trade transaction #392

Open
2 tasks
AW-STJ opened this issue Oct 6, 2020 · 14 comments
Open
2 tasks

Improve Activity - add Uniswap trade transaction #392

AW-STJ opened this issue Oct 6, 2020 · 14 comments
Assignees

Comments

@AW-STJ
Copy link
Contributor

AW-STJ commented Oct 6, 2020

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

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

Sub issues:

@hboon
Copy link
Member

hboon commented Oct 6, 2020

@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 hboon transferred this issue from AlphaWallet/alpha-wallet-ios Oct 6, 2020
@hboon
Copy link
Member

hboon commented Oct 6, 2020

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

@hboon
Copy link
Member

hboon commented Oct 9, 2020

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

@SmartLayer SmartLayer changed the title Improve Activity - add trade transaction Improve Activity - add Uniswap trade transaction Oct 11, 2020
@SmartLayer
Copy link

SmartLayer commented Oct 12, 2020

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:

  1. 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.

  2. [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.

  3. 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
Copy link

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
Copy link
Member

hboon commented Oct 12, 2020

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:

  1. 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.

  1. [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

  1. 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
Copy link

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.

@colourfreak
Copy link

Are we able to make something like this?

Screenshot 2020-10-28 at 09 42 41

@colourfreak
Copy link

Screenshot 2020-10-28 at 09 55 11

@colourfreak
Copy link

Screenshot 2020-10-28 at 10 01 24

We don't need to display how much funds are in the smart contract

@hboon
Copy link
Member

hboon commented Oct 28, 2020

Are we able to make something like this?

Screenshot 2020-10-28 at 09 42 41

I'd say yes. At least that's what we want to be able to achieve with this issue.

But would be good to create separate issues for these though:

Those would be separate threads.

@SmartLayer
Copy link

Very keen on implementing this. Feel sorry for not catching up as mired in cofix. luckily they need this too. will be able to share much of the work.

@AW-STJ
Copy link
Contributor Author

AW-STJ commented Oct 29, 2020

But would be good to create separate issues for these though:

Those would be separate threads.

Done

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

No branches or pull requests

5 participants