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

Check if user has downgraded to an older version #4829

Merged
merged 2 commits into from
Nov 24, 2020

Conversation

chimp1984
Copy link
Contributor

Check if user has downgraded to an older version. If so require shutdown
and do not read or write persisted data.

We had recently a case where a user downgraded from 1.4.2 to 1.3.9 and
this caused failed trades and the wallet funds have been missing due to
some complexities of the wallet wegwit upgrade. The fund could be recovered
but it took quite some effort.
As downgrade is never tested and can lead to all kind of weird bugs we
should prevent that users accidentally can do it.
If there is valid reason to downgrade they can remove the version file.

I would suggest that we add that to 1.5.0 as it is a critical update which would likely cause major issues if users downgrade. But up to maintainers if they consider risk/benefit ration in favor to add it to the release.

and do not read or write persisted data.

We had recently a case where a user downgraded from 1.4.2 to 1.3.9 and
this caused failed trades and the wallet funds have been missing due to
some complexities of the wallet wegwit upgrade. The fund could be recovered
but it took quite some effort.
As downgrade is never tested and can lead to all kind of weird bugs we
should prevent that users accidentally can do it.
If there is valid reason to downgrade they can remove the version file.

public static boolean hasDowngraded(String lastVersion) {
return lastVersion != null && Version.isNewVersion(lastVersion, Version.VERSION);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this can't really be tested by downgrading from an older pkg, I hacked BisqSetup.java Line 531:

public static boolean hasDowngraded(String lastVersion) {
	return true;
    // return lastVersion != null && Version.isNewVersion(lastVersion, Version.VERSION);
}

It worked as expected, showing me a shutdown message & button, but I got an NPE after clicking the shutdown button. I am not sure this is avoidable or very important, but think you'd want to know.

Nov-21 13:28:17.176 [JavaFX Application Thread] ERROR bisq.core.app.BisqSetup: Downgrade from version 1.5.0 to version 1.5.0 is not supported 
Nov-21 13:28:21.649 [Thread-3] INFO  bisq.core.app.BisqExecutable: Start graceful shutDown 
Nov-21 13:28:21.659 [Thread-3] INFO  b.c.a.AvoidStandbyModeService: Stopped 
Nov-21 13:28:21.659 [Thread-3] INFO  b.core.offer.OpenOfferManager: Remove open offers at shutDown. Number of open offers: 0 
Nov-21 13:28:21.660 [Thread-3] INFO  bisq.core.app.BisqExecutable: OpenOfferManager shutdown completed 
Nov-21 13:28:21.660 [Thread-3] INFO  bisq.core.app.BisqExecutable: WalletsSetup shutdown completed 
Nov-21 13:28:21.664 [JavaFX Application Thread] INFO  b.n.p2p.network.NetworkNode: Shutdown immediately because no connections are open. 
Nov-21 13:28:21.666 [Thread-3] INFO  b.n.p2p.network.TorNetworkNode: Tor has not been created yet. We cancel the torStartupFuture. 
Nov-21 13:28:21.666 [Thread-3] ERROR b.n.p2p.network.TorNetworkNode: Shutdown torNetworkNode failed with exception: null 
java.lang.NullPointerException
	at bisq.network.p2p.network.TorNetworkNode.torNetworkNodeShutDown(TorNetworkNode.java:187)
	at bisq.network.p2p.network.TorNetworkNode.shutDown(TorNetworkNode.java:154)
	at bisq.network.p2p.P2PService.doShutDown(P2PService.java:276)
	at bisq.network.p2p.peers.Broadcaster.doShutDown(Broadcaster.java:81)
	at bisq.network.p2p.peers.Broadcaster.shutDown(Broadcaster.java:68)
	at bisq.network.p2p.P2PService.shutDown(P2PService.java:244)
	at bisq.core.app.BisqExecutable.lambda$gracefulShutDown$4(BisqExecutable.java:245)
	at com.sun.javafx.binding.ExpressionHelper$SingleChange.fireValueChangedEvent(ExpressionHelper.java:181)
	at com.sun.javafx.binding.ExpressionHelper.fireValueChangedEvent(ExpressionHelper.java:80)
	at javafx.beans.property.BooleanPropertyBase.fireValueChangedEvent(BooleanPropertyBase.java:104)
	at javafx.beans.property.BooleanPropertyBase.markInvalid(BooleanPropertyBase.java:111)
	at javafx.beans.property.BooleanPropertyBase.set(BooleanPropertyBase.java:145)
	at bisq.core.btc.setup.WalletsSetup.shutDown(WalletsSetup.java:344)
	at bisq.core.app.BisqExecutable.lambda$gracefulShutDown$5(BisqExecutable.java:260)
	at bisq.core.offer.OpenOfferManager.shutDown(OpenOfferManager.java:232)
	at bisq.core.app.BisqExecutable.gracefulShutDown(BisqExecutable.java:234)
	at bisq.desktop.app.BisqApp.lambda$stop$1(BisqApp.java:148)
	at java.base/java.lang.Thread.run(Thread.java:834)
Nov-21 13:28:26.668 [JavaFX Application Thread] ERROR b.n.p2p.network.TorNetworkNode: A timeout occurred at shutDown 

Copy link
Contributor

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@ripcurlx ripcurlx merged commit 94453cf into bisq-network:release/v1.5.0 Nov 24, 2020
@chimp1984 chimp1984 deleted the prevent-downgrade branch November 26, 2020 15:17
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

Successfully merging this pull request may close these issues.

3 participants