Inject a clock instead of mocking currentTimeMS

[christophsturm]

time is a dependency like any other and it needs to be injected where needed.
see https://docs.oracle.com/javase/8/docs/api/java/time/Clock.html
https://github.com/robfletcher/test-clock

[mrosseel]

would indeed be better for unit testing, any idea of the impact of this change?

[christophsturm]

will probably be a big diff but it does not really change any logic, so it should be safe.
one additional issue is that we already have a class called "clock", that should probably be renamed to SecondsScheduler or something.
I can do both in separate prs. if people agree that its worthwile

[sqrrm]

While it's nice to clean up the code I wonder if it's really worthwhile right now. I see issues with current trade protocol and performance issues as more urgent. People are having problem at startup with the app not starting or being very slow and there are a few bugs out there causing people to lose money trying to initiate trades or transactions failing for unclear reasons.

This kind of refactor is probably a lot easier to understand though, and perhaps a good start rather than diving straight into the most sensitive parts of the code. All depends on how much work and the risk to stability.

[christophsturm]

I'm just writing issues as I notice problems, so others can chime in with their opinion.

[sqrrm]

Yes, that's how I understood it. I'm adding my opinion on where effort is probably best spent at this point. I think it's good that you open these issues and I don't disagree with the issue itself.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale]

This issue has been automatically closed because of inactivity. Feel free to reopen it if you think it is still relevant.