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

Do not send excessive messages in governance sync #2124

Merged
merged 1 commit into from
Feb 4, 2019

Conversation

UdjinM6
Copy link

@UdjinM6 UdjinM6 commented Jun 16, 2018

No need to send gobject inv for a single gobject, the other node already knows it, so send votes only. Also, no need to send "fake" stats like "0 votes" when syncing gobjects and "1 object" when syncing votes. And finally, rename functions accordingly.

EDIT:
PS. Haven't assigned milestone for now - should be safe to merge in 12.3 IMO, but this PR changes the actual behaviour while (strictly saying) it doesn't fix a critical bug or smth, just removes some excessive/redundant parts, so I'm open for other opinions/arguments.

@UdjinM6 UdjinM6 changed the title Drop excessive messages in governance sync Do not send excessive messages in governance sync Jun 16, 2018
Copy link

@QuantumExplorer QuantumExplorer left a comment

Choose a reason for hiding this comment

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

utACK

src/governance.cpp Outdated Show resolved Hide resolved
Copy link

@nmarley nmarley left a comment

Choose a reason for hiding this comment

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

Probably should fix the log message before merging this (commented inline)

nmarley
nmarley previously approved these changes Jun 18, 2018
Copy link

@nmarley nmarley left a comment

Choose a reason for hiding this comment

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

utACK

@UdjinM6 UdjinM6 added this to the 12.4 milestone Jun 18, 2018
@UdjinM6
Copy link
Author

UdjinM6 commented Jun 18, 2018

I checked the code once again and I'm still not 100% sure if this PR breaks smth or not. Postponing it till 12.4, needs more testing IMO.

@UdjinM6 UdjinM6 changed the title Do not send excessive messages in governance sync [WIP] Do not send excessive messages in governance sync Jul 13, 2018
@UdjinM6 UdjinM6 dismissed stale reviews from nmarley and QuantumExplorer via 72b639c July 13, 2018 12:17
@UdjinM6 UdjinM6 removed this from the 13.0 milestone Nov 3, 2018
@UdjinM6 UdjinM6 changed the title [WIP] Do not send excessive messages in governance sync Do not send excessive messages in governance sync Jan 25, 2019
@UdjinM6 UdjinM6 added this to the 14.0 milestone Jan 25, 2019
@UdjinM6
Copy link
Author

UdjinM6 commented Jan 25, 2019

Reviving this. Rebased and slightly tested by connecting one patched node with no data directly to another patched one, seems to be working as expected.

No need to send gobject inv for a single gobject, the other node already knows it, so send votes only.
Also, no need to send "fake" stats like "0 votes" when syncing gobjects and "1 object" when syncing votes.
Rename functions accordingly.
Copy link

@nmarley nmarley left a comment

Choose a reason for hiding this comment

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

utACK

Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

utACK

@UdjinM6 UdjinM6 merged commit 68c0de4 into dashpay:develop Feb 4, 2019
@UdjinM6 UdjinM6 added the P2P Some notable changes on p2p level label Mar 20, 2019
thephez added a commit to thephez/dash-docs that referenced this pull request Apr 16, 2019
thephez added a commit to dash-docs/dash-docs that referenced this pull request Apr 17, 2019
* Guide - PrivateSend updates
 - Variable paticipant count
 - Link updates

* Guide - MN sync reorg
 - Move all content regarding old mn sync system to a subsection

* Guide - Governance sync update
 - Related to dashpay/dash#2124

* Content - Remove governance watchdog references
 - Related to dashpay/dash#2816

* Content - Governance sync update

* Content - Update Dash Core version and protocol number

* Formatting fix

* Content - Disclaimer and roadmap link

* Content - Update Sentinel info to indicate deprecation of sentinelping

* Formatting fix
thephez added a commit to thephez/dash-docs that referenced this pull request May 17, 2019
* Guide - PrivateSend updates
 - Variable paticipant count
 - Link updates

* Guide - MN sync reorg
 - Move all content regarding old mn sync system to a subsection

* Guide - Governance sync update
 - Related to dashpay/dash#2124

* Content - Remove governance watchdog references
 - Related to dashpay/dash#2816

* Content - Governance sync update

* Content - Update Dash Core version and protocol number

* Formatting fix

* Content - Disclaimer and roadmap link

* Content - Update Sentinel info to indicate deprecation of sentinelping

* Formatting fix
thephez added a commit to dash-docs/dash-docs that referenced this pull request May 17, 2019
* Guide - PrivateSend updates
 - Variable paticipant count
 - Link updates

* Guide - MN sync reorg
 - Move all content regarding old mn sync system to a subsection

* Guide - Governance sync update
 - Related to dashpay/dash#2124

* Content - Remove governance watchdog references
 - Related to dashpay/dash#2816

* Content - Governance sync update

* Content - Update Dash Core version and protocol number

* Formatting fix

* Content - Disclaimer and roadmap link

* Content - Update Sentinel info to indicate deprecation of sentinelping

* Formatting fix
@UdjinM6 UdjinM6 deleted the dropgovsyncmessages branch November 26, 2020 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2P Some notable changes on p2p level
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants