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

Update transactions after /send command executed #4870

Closed
wants to merge 1 commit into from

Conversation

dmitryn
Copy link
Contributor

@dmitryn dmitryn commented Jun 20, 2018

fixes #4865

Summary:

As a quick solution to issue described in #4865 we schedule 3 transaction state updates after funds got sent in chat – in 30, 60 and 90 seconds. These numbers are still experimental. Instead of 1 update 60 seconds later, we do 3 here – adds a bit of overhead, but brings faster feedback to the user and more confidence we get data from etherscan (additional request in 90 seconds).

status: ready

@dmitryn dmitryn self-assigned this Jun 20, 2018
@dmitryn dmitryn requested a review from goranjovic June 20, 2018 15:55
@status-github-bot status-github-bot bot added this to REVIEW in Pipeline for QA Jun 20, 2018
@status-comment-bot
Copy link

status-comment-bot commented Jun 20, 2018

@status-github-bot status-github-bot bot moved this from REVIEW to TO TEST in Pipeline for QA Jun 21, 2018
@statustestbot
Copy link

72% of end-end tests have passed

Total executed tests: 29
Failed tests: 8
Passed tests: 21

Failed tests (8)

Click to expand
1. test_username_and_profile_picture_in_chats

Tap on EditButton
Looking for EditPictureButton

E selenium.common.exceptions.NoSuchElementException: Message: 'EditPictureButton' is not found on screen, using: 'accessibility id:edit-profile-photo-button'

Device sessions:

2. test_incorrect_password

Tap on ProfileButton
Looking for UserNameText

E selenium.common.exceptions.NoSuchElementException: Message: 'UserNameText' is not found on screen, using: 'xpath://android.widget.ImageView[@content-desc="chat-icon"]/../../android.widget.TextView'

Device sessions:

3. test_delete_1_1_chat

Tap on RequestCommand
Looking for EthAsset

E selenium.common.exceptions.NoSuchElementException: Message: 'EthAsset' is not found on screen, using: 'xpath://*[@text="ETH"]'

Device sessions:

4. test_transaction_send_command_one_to_one_chat

Tap on SendCommand
Looking for EthAsset

E selenium.common.exceptions.NoSuchElementException: Message: 'EthAsset' is not found on screen, using: 'xpath://*[@text="ETH"]'

Device sessions:

5. test_transaction_send_command_wrong_password

Tap on SendCommand
Looking for EthAsset

E selenium.common.exceptions.NoSuchElementException: Message: 'EthAsset' is not found on screen, using: 'xpath://*[@text="ETH"]'

Device sessions:

6. test_send_eth_to_request_in_one_to_one_chat

Tap on RequestCommand
Looking for EthAsset

E selenium.common.exceptions.NoSuchElementException: Message: 'EthAsset' is not found on screen, using: 'xpath://*[@text="ETH"]'

Device sessions:

7. test_profile_picture

Tap on EditButton
Looking for EditPictureButton

E selenium.common.exceptions.NoSuchElementException: Message: 'EditPictureButton' is not found on screen, using: 'accessibility id:edit-profile-photo-button'

Device sessions:

8. test_sign_message_from_daap

Tap on NextButton
Looking for ConfirmPasswordInput

E AttributeError: 'NoneType' object has no attribute 'set_value'

Device sessions:

Passed tests (21)

Click to expand
1. test_sign_transaction_twice
Device sessions:

2. test_swipe_and_delete_1_1_chat
Device sessions:

3. test_one_to_one_chat_messages
Device sessions:

4. test_copy_and_paste_messages
Device sessions:

5. test_send_transaction_from_daap
Device sessions:

6. test_faucet_console_command
Device sessions:

7. test_send_eth_from_wallet_sign_now
Device sessions:

8. test_open_transaction_on_etherscan
Device sessions:

9. test_send_eth_to_request_from_wallet
Device sessions:

10. test_backup_seed_phrase_and_recover_account
Device sessions:

11. test_offline_messaging_1_1_chat
Device sessions:

12. test_contact_profile_view
Device sessions:

13. test_public_chat
Device sessions:

14. test_public_chat_management
Device sessions:

15. test_network_switch
Device sessions:

16. test_wallet_error_messages
Device sessions:

17. test_browse_link_entering_url_in_dapp_view
Device sessions:

18. test_switch_users
Device sessions:

19. test_set_up_wallet
Device sessions:

20. test_send_stt_from_wallet_via_enter_recipient_address
Device sessions:

21. test_qr_code_and_its_value
Device sessions:

@lukaszfryc lukaszfryc moved this from TO TEST to IN TESTING in Pipeline for QA Jun 21, 2018
@lukaszfryc lukaszfryc self-assigned this Jun 21, 2018
@lukaszfryc
Copy link
Contributor

@dmitryn could you rebase this PR to include the latest commits like 32bbf4e?

@lukaszfryc
Copy link
Contributor

lukaszfryc commented Jun 21, 2018

@dmitryn I noticed that when you send a transaction then go offline and go back online when transaction is confirmed on etherscan (or on another device), the status is still "Pending". It stays like this until you visit Transaction History. Maybe we could update it when user goes online?

Other things look good so far.

@dmitryn
Copy link
Contributor Author

dmitryn commented Jun 21, 2018

@lukaszfryc added transactions update request when going back online. Please check.

@status-comment-bot
Copy link

status-comment-bot commented Jun 21, 2018

@lukaszfryc
Copy link
Contributor

lukaszfryc commented Jun 21, 2018

@dmitryn there is another issue in the recent build:

  1. Device A: Send a transaction in 1-1 chat
  2. Device B: The incoming transaction message appears but the "TX not found" stays there forever. Also, when I go to transaction history, I don't see this transaction there even though it has 14 confirmations on etherscan and it's visible on the Device A.

Since it's not a Beta blocker, I suggest going back to this PR after Beta is released.

@lukaszfryc lukaszfryc moved this from IN TESTING to CONTRIBUTOR in Pipeline for QA Jun 21, 2018
@dmitryn
Copy link
Contributor Author

dmitryn commented Jun 21, 2018

@lukaszfryc if TX is not in wallet, it deserves a separate issue as chat doesn't send transactions on its own, but uses wallet for that.

@dmitryn dmitryn force-pushed the bug/update-transactions-after-send-#4865 branch from 2c59b8b to 9334066 Compare June 21, 2018 14:01
@status-comment-bot
Copy link

status-comment-bot commented Jun 21, 2018

@lukaszfryc
Copy link
Contributor

lukaszfryc commented Jun 21, 2018

@dmitryn I still can reproduce the bug here. But, everything works fine on latest nightly (08932c4) and latest release/0.9.21 (97ead90). Could you please try to debug it on your side?

@jeluard
Copy link
Contributor

jeluard commented Jun 21, 2018

@dmitryn What if the transaction stales for more than 90s?

@dmitryn
Copy link
Contributor Author

dmitryn commented Jun 21, 2018

@jeluard

What if the transaction stales for more than 90s?

If user stay in the chat, nothing would happen after 90s. Next time we would update transactions if he goes to Wallet or goes offline -> online.

@lukaszfryc
Copy link
Contributor

@dmitryn here are TF logs for the missing incoming transaction issue:

@dmitryn
Copy link
Contributor Author

dmitryn commented Jun 22, 2018

@lukaszfryc thanks for providing TF sessions. After watching it, i noticed that device B (receiver) had changed network from mainnet to ropsten before receiving chat transaction.

I could assume that device B doesn't see transaction because app pulls from wrong etherscan api (api.etherscan.io but not api-ropsten.etherscan.io), but we don't have this info in logs and i cannot reproduce it locally. I think we need to have additional logging for etherscan api requests to understand what is happening.

@status-comment-bot
Copy link

status-comment-bot commented Jun 25, 2018

@status-comment-bot
Copy link

status-comment-bot commented Jul 6, 2018

@lukaszfryc
Copy link
Contributor

@dmitryn I tested it again and the issue is still there. Here are more details:

Device A: I sent ETH in 1:1 chat and went to transaction history to monitor the transaction. I waited for 10 minutes and transaction have still 0 confirmations in Status. When I opened it on etherscan, there was 16 confirmations. I refreshed the screen but there was still 0 confirmations. When I killed the app and logged in, the wallet showed correct amount (it included transaction) but the transaction history screen was empty.

Device B: I received incoming transaction in chat but the status was "TX not found" and did not change. Wallet showed correct amount (included the transaction), but same as on Device A, the transaction history was empty.

It worked the same when a transaction was made from Wallet. I have no idea how it's related to this PR and why everything works fine on latest nightly. @dmitryn I really need your help trying to investigate it. Maybe you could try again on your device?

@dmitryn
Copy link
Contributor Author

dmitryn commented Jul 9, 2018

@lukaszfryc i haven't experienced this bug before. If transaction exists in blockchain and is shown on etherscan, it should also exist in transactions history in wallet (since wallet use etherscan API to fetch transactions history).

I could assume that etherscan web UI and etherscan API may show different data for same account due to technical issues on their end (etherscan web and api use different data sources that could be out of sync, etc)

The reason why wallet shows correct amount in your case is that we fetch balance data by web3 call (Infura API) so the data could be different from data provided by etherscan.

@lukaszfryc
Copy link
Contributor

@dmitryn the worst thing is that I can't replicate it on develop. It does not work only in this PR. I tried on different accounts including creating new ones. Are you sure it's working on your side?

@dmitryn
Copy link
Contributor Author

dmitryn commented Jul 13, 2018

@lukaszfryc i still cannot reproduce this. If you think we should not merge this PR, we can close it and wait for #5138 as it will handle transactions update better.

@lukaszfryc
Copy link
Contributor

@dmitryn ok, so let's wait for #5138

@lukaszfryc lukaszfryc closed this Jul 13, 2018
Pipeline for QA automation moved this from CONTRIBUTOR to DONE Jul 13, 2018
@rasom rasom deleted the bug/update-transactions-after-send-#4865 branch September 24, 2018 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

Pending state is shown for transaction in 1:1 chat for sender even if tx confirmed more than 100 times in
6 participants