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

Updates get lost if app gets killed whilst doing a trade #172

Closed
rodvar opened this issue Jan 24, 2025 · 3 comments
Closed

Updates get lost if app gets killed whilst doing a trade #172

rodvar opened this issue Jan 24, 2025 · 3 comments
Assignees
Labels
In Progress Trading issue is related to the core functionality of trading

Comments

@rodvar
Copy link
Collaborator

rodvar commented Jan 24, 2025

PRE

  • taker wants to sell his bitcoin using bisq mobile
  • maker wants to buy bitcoin and he created an offer using his desktop Bisq2 app
  • let's use USD market, but could be any other

STEPS

  1. as a maker open desktop and create a USD buy offer with all the default options
  2. as a taker open the app navigate to USD market and take that new offer
  3. open the taken offer in 'My Trades' and complete the fiat payment details, click continue and then kill the app
  4. as a maker, complete the BTC address, also confirm that you have made the payment
  5. as a taker open the app again, navigate to my trade and open the trade. Observe.

Expected Result (ER)

-> you should be prompted to confirmed you received the money and then skip btc confirmation and finish the trade

Obtained Results (OR)

-> BUG: "waiting for payment confirmation" is shown. Trying to do any other network interaction also fails. (e.g. create a new offer, in a certain market, and realise offer numbers won't increase). App seems like in an invalid state.

(Tech) Notes

  • see logs below. When this bug occurs the trading state is literally the wrong one (lost the state changes)
  • "kill the app" is done by going into the backgrounded apps (swipe down -> up) and tap & hold bisq app and swiping it down -> up.
  • this bug was discovered whilst implementing Notifications: Ongoing trades #126 for the node app, but its happening for the clients as well.
  • this is most probably an app lifecycle issue, however trying to bring support for app killed services still hearing comms is probably a big challenge without support from bisq2 core jars.

Core logs on successfuly (non app-killing) case:

network.bisq.mobile.node.debug       I  INFO  b.n.p.s.c.a.MessageDeliveryStatusService - Persist MessageDeliveryStatus CONNECTING with message ID 928443a0-d24d-4f60-b106-d03ba7d8ef55
System.out              network.bisq.mobile.node.debug       I  15:20:27.344 INFO  bisq.common.fsm.Fsm - Start transition from currentState BisqEasyTradeState.SELLER_RECEIVED_FIAT_SENT_CONFIRMATION(isFinalState=false, ordinal=27)
System.out              network.bisq.mobile.node.debug       I  15:20:27.346 INFO  bisq.common.fsm.Fsm - Handle BisqEasyConfirmFiatReceiptEvent at BisqEasyConfirmFiatReceivedEventHandler
System.out              network.bisq.mobile.node.debug       I  15:20:27.347 INFO  b.n.p.s.c.ConfidentialMessageService - Creating connection to Addresses: [53882] took 2 ms
System.out              network.bisq.mobile.node.debug       I  15:20:27.346 INFO  b.c.n.AndroidEmulatorLocalhostFacade - The android app is running in the emulator. We convert the target localhost address `127.0.0.1` to `10.0.2.2`
System.out              network.bisq.mobile.node.debug       I  15:20:27.349 INFO  b.n.p.s.c.a.MessageDeliveryStatusService - Persist MessageDeliveryStatus CONNECTING with message ID 3973ae3f-fc2a-4ebb-8aaa-142fe9323b6a
TrafficStats            network.bisq.mobile.node.debug       D  tagSocket(100) with statsTag=0xffffffff, statsUid=-1
System.out              network.bisq.mobile.node.debug       I  15:20:27.362 INFO  bisq.common.fsm.Fsm - Transition completed to new state BisqEasyTradeState.SELLER_CONFIRMED_FIAT_RECEIPT(isFinalState=false, ordinal=28)

Logs from the bug:

System.out              network.bisq.mobile.node.debug       I  14:47:07.955 INFO  b.n.p.s.c.a.MessageDeliveryStatusService - Persist MessageDeliveryStatus CONNECTING with message ID f66cb0a2-e064-4c18-ab14-e720e473701a
System.out              network.bisq.mobile.node.debug       I  14:47:07.959 INFO  bisq.common.fsm.Fsm - Start transition from currentState BisqEasyTradeState.SELLER_CONFIRMED_FIAT_RECEIPT(isFinalState=false, ordinal=28)
System.out              network.bisq.mobile.node.debug       I  14:47:07.960 INFO  bisq.common.fsm.Fsm - We did not find a transition with state BisqEasyTradeState.SELLER_CONFIRMED_FIAT_RECEIPT(isFinalState=false, ordinal=28) and event BisqEasyConfirmFiatReceiptEvent. We add the event to the eventQueue for potential later processing.
System.out              network.bisq.mobile.node.debug       I  14:47:07.961 INFO  b.c.n.AndroidEmulatorLocalhostFacade - The android app is running in the emulator. We convert the target localhost address `127.0.0.1` to `10.0.2.2`
System.out              network.bisq.mobile.node.debug       I  14:47:07.961 INFO  bisq.network.p2p.node.Node - Create outbound connection to [53882] with capability version 1
System.out              network.bisq.mobile.node.debug       I  14:47:07.966 INFO  b.c.n.AndroidEmulatorLocalhostFacade - The android app is running in the emulator. We convert the target localhost address `127.0.0.1` to `10.0.2.2`
TrafficStats            network.bisq.mobile.node.debug       D  tagSocket(113) with statsTag=0xffffffff, statsUid=-1
TrafficStats            network.bisq.mobile.node.debug       D  tagSocket(113) with statsTag=0xffffffff, statsUid=-1
System.out              network.bisq.mobile.node.debug       I  14:47:07.977 INFO  b.n.p.s.c.ConfidentialMessageService - Peer is not detected offline. We wait for the connection creation has been successful and try to send the message. Request for isPeerOnline completed after 17 ms
System.out              network.bisq.mobile.node.debug       I  14:47:08.010 INFO  bisq.network.p2p.node.Node - We create an outbound connection to [53882] from a user node. node=ID: 78a79471; Addresses: 10.0.2.15:38995 @ CLEAR
System.out              network.bisq.mobile.node.debug       I  14:47:08.013 INFO  b.n.p.s.c.ConfidentialMessageService - Creating connection to Addresses: [53882] took 53 ms
System.out              network.bisq.mobile.node.debug       I  14:47:08.030 INFO  b.n.p.s.c.ConfidentialMessageService - Sent message to Addresses: [53882] after 70 ms
System.out              network.bisq.mobile.node.debug       I  14:47:08.083 INFO  b.n.p.s.c.a.MessageDeliveryStatusService - Persist MessageDeliveryStatus SENT with message ID f66cb0a2-e064-4c18-ab14-e720e473701a
@rodvar rodvar added the Trading issue is related to the core functionality of trading label Jan 24, 2025
@rodvar rodvar added this to the MVP (version 0.1.0) milestone Jan 24, 2025
@rodvar rodvar changed the title Updates get lost if app gets killed whilst doing a transaction Updates get lost if app gets killed whilst doing a trade Jan 24, 2025
@HenrikJannsen
Copy link
Contributor

I started to look into it but have not found the issue so far. You can assign it to me.

@HenrikJannsen
Copy link
Contributor

Probably fixed by bisq-network/bisq2#3166

@rodvar
Copy link
Collaborator Author

rodvar commented Feb 3, 2025

@HenrikJannsen happy to report that this bug as its described has been fixed. However, we still have some work to do:

  • Following steps as described in this bug report now shows the ER (Expected result) though interaction does not work (you can't confirm the trade, but the state is the right one now so it updates even though the app was killed).
  • note For the above to work the core service needs to be kept running (this can be done by commenting out the TODO marked lines on NodeMainPresenter onDestroying and deactivateServices methods)
  • We probably have a state re-subscription kinda issue for this scenario, cause on the desktop you can see the comms work (e.g. cancel the trade or I confirm the fiat payment the state changes in the desktop, but for the mobile user it looks like nothing happened).
  • Got a bisq core address already in use exception in the logs, but its probably because the applicationService is stopping a reinit call when its already initialized

I'll close this for now and either fix or create a new specific issue if needed. Thanks @HenrikJannsen !!!

Initializing applicationService failed
                                                                                                     java.util.concurrent.CompletionException: java.net.BindException: bind failed: EADDRINUSE (Address already in use)
               

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
In Progress Trading issue is related to the core functionality of trading
Projects
None yet
Development

No branches or pull requests

2 participants