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.

Sign up for GitHub

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

Jump to bottom

RPC publish optional work_watch flag to add to work_watcher #2168

Merged
merged 10 commits into from
Aug 11, 2019
Merged

RPC publish optional work_watch flag to add to work_watcher #2168

merged 10 commits into from
Aug 11, 2019

Conversation

argakiig
Copy link
Contributor

@argakiig argakiig commented Jul 22, 2019 •
edited
Loading

Adds flag work_watch, default true, to rpc publish allowing for blocks processed via RPC call to not be eligible for dropping from active ensuring the block is not dropped from active_transactions until confirmed. Messages will continue to originate from the block as long as the block is not confirmed

With this change publishes originating from the local node will not be dropped ensuring your blocks are always broadcast and can propagate the network

publishes from rpc send, publish and wallet send, publish will no lon…
350e15f 
…ger be eligible to drop due to bandwidth limiting
@argakiig argakiig added the bug label Jul 22, 2019
@argakiig argakiig added this to the V20.0 milestone Jul 22, 2019
@argakiig argakiig self-assigned this Jul 22, 2019
wezrule
wezrule reviewed Jul 22, 2019
View reviewed changes
nano/node/network.cpp Outdated Show resolved Hide resolved
cryptocode
cryptocode approved these changes Jul 22, 2019
View reviewed changes
Copy link
Contributor

@cryptocode cryptocode left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM regarding throttling mechanism, though it seems like the tcp write queue sometimes drops before message dropping does (at least during beta spam). Not sure how easy it is to pick what to drop there, since it's just a bytearray queue.

SergiySW
SergiySW reviewed Jul 22, 2019
View reviewed changes
nano/node/network.cpp Outdated Show resolved Hide resolved
@argakiig argakiig added rpc semantics change documentation This item indicates the need for or supplies updated or expanded documentation labels Jul 30, 2019
@argakiig argakiig changed the title Prevent dropping local block publishes RPC publish optional work_watch flag to add to work_watcher Jul 30, 2019
Merge branch 'master' into always_publish_local
348ed80 
# Conflicts:
#	nano/node/json_handler.cpp
update node.process_local to handle work_watcher
@argakiig argakiig merged commit 3d5c2d2 into nanocurrency:master Aug 11, 2019
@zhyatt zhyatt mentioned this pull request Aug 20, 2019
Closed
@guilhermelawless guilhermelawless mentioned this pull request Aug 26, 2019
Merged
@argakiig argakiig deleted the always_publish_local branch September 24, 2019 16:50
@zhyatt zhyatt added major This item indicates the need for or supplies a major or notable change enhancement and removed beta testing wanted bug labels Oct 24, 2019
@zhyatt zhyatt added rpc Changes related to Remote Procedure Calls semantic Change to node APIs (separate label) which impacts interpretation of data, integrations impacted. and removed rpc semantics change labels Mar 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wezrule wezrule wezrule approved these changes

@guilhermelawless guilhermelawless guilhermelawless approved these changes

@SergiySW SergiySW SergiySW approved these changes

@cryptocode cryptocode cryptocode approved these changes

Assignees

@argakiig argakiig

Labels
documentation This item indicates the need for or supplies updated or expanded documentation enhancement major This item indicates the need for or supplies a major or notable change rpc Changes related to Remote Procedure Calls semantic Change to node APIs (separate label) which impacts interpretation of data, integrations impacted.
Projects
None yet
Milestone
V20.0
Development

Successfully merging this pull request may close these issues.
6 participants
@argakiig @wezrule @guilhermelawless @SergiySW @cryptocode @zhyatt