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

Add missing network capability for trade statistics hash update #3551

Merged
merged 1 commit into from
Nov 4, 2019

Conversation

ripcurlx
Copy link
Contributor

@ripcurlx ripcurlx commented Nov 4, 2019

I think we missed to add the new statistics hash capability to the core network capabilities.
At least this worked for me in my local nodes when I recognized some missing trade statistics.

@ripcurlx
Copy link
Contributor Author

ripcurlx commented Nov 4, 2019

@chimp1984 @sqrrm Could you review if my finding is correct?

@sqrrm
Copy link
Member

sqrrm commented Nov 4, 2019

I think you're right. I don't have the time right now to test it though.

@ripcurlx
Copy link
Contributor Author

ripcurlx commented Nov 4, 2019

I think you're right. I don't have the time right now to test it though.

It did solve the problem on two of my nodes. Unfortunately during my release smoke test I got two runtime trade stats for 1st of Nov and I thought everything is right now and we just didn't have any trades the day before because everyone was updating and too afraid to trade during this transitional period.

@ripcurlx
Copy link
Contributor Author

ripcurlx commented Nov 4, 2019

Tested it now on three different live nodes and it seems to work now. Just waiting for an ACK from someone else. Also we need to decide if we want to shoot out another release soon (latest on Thursday).

Copy link
Member

@freimair freimair left a comment

Choose a reason for hiding this comment

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

Ack.

Assuming that without this fix, a fresh client would not get all trade stats on startup, I just fired up a fresh 1.2.2 instance and compared the trade stats with my alway-on instance. All trades are there. 26 trades on 2019-11-01.

Copy link
Contributor

@chimp1984 chimp1984 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 33c9dd1 into bisq-network:master Nov 4, 2019
@ripcurlx ripcurlx deleted the add-missing-capability branch December 18, 2019 08:34
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.

None yet

4 participants