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

[Net] Reduce preferential Graphene/Xthin timers to 1 second each #1511

Merged
merged 1 commit into from Dec 5, 2018

Conversation

jtoomim
Copy link
Contributor

@jtoomim jtoomim commented Dec 4, 2018

Recently, the consecutive 10 second preferential download timers for Xthin/Graphene caused an orphan race on BCH mainnet:

https://www.reddit.com/r/btc/comments/a1bp0b/please_send_me_your_debuglog_files_from_the_bsv/eb2ogw0/?context=3

These 10 second timers often result in BU not downloading a block (even a tiny, 20 kB one!) from peers until 20 seconds have elapsed. Since no BU nodes support Compact Blocks, and almost no other nodes (except XT) supports both CB and Xthin, this means that any time a block is mined by BSV or ABC, no BU nodes on the network will receive it for at least 10 seconds, and when XT network bridging doesn't work (e.g. on BSV) all BU nodes will be delayed by 20 seconds. This causes a 3.27% higher orphan rate than necessary for any miners using BU.

This preferential download mechanism should probably be completely redesigned and replaced. I proposed one such idea in the reddit thread above. However, as a short-term fix, I suggest lowering the timers to 1 second instead of 10 seconds. This is long enough that a node that sees an Xthin or Graphene peer with the block will usually be able to download it in time, but short enough that orphan rates will not increase very much for BU miners. This PR implements this 1 second timer.

Based on my test results from mainnet, it seems to work as intended. My test node is probably already effectively bridging the BU and ABC subsets of the BCH network:

2018-12-04 19:48:25 AskFor block via headers direct fetch 0000000000000000001ffe762dbd324320a4112a43cac97ef7ebb0c18ba304c7 (559418) peer=19
2018-12-04 19:48:26 Preferential Graphene Block timer exceeded
2018-12-04 19:48:26 Starting Preferential Thinblock timer (1169 millis)
2018-12-04 19:48:27 Preferential Thinblock timer exceeded - downloading regular block instead
2018-12-04 19:48:27 Clearing Preferential Thinblock timer
2018-12-04 19:48:27 Clearing Preferential Graphene Block timer
2018-12-04 19:48:27 sending msg: getdata (37 bytes) peer=39
2018-12-04 19:48:27 Requesting Regular Block 0000000000000000001ffe762dbd324320a4112a43cac97ef7ebb0c18ba304c7 from peer 47.95.32.37:8333 (39)

This PR is identical to #1510, but rebased on the dev branch instead of release.

Copy link
Collaborator

@ptschip ptschip left a comment

Choose a reason for hiding this comment

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

I'm ok with this, until we get compact blocks in place...then we can disable the timers when all three protocols are turned on graphene/xthin/cmpctblocks.

Copy link
Collaborator

@sickpig sickpig left a comment

Choose a reason for hiding this comment

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

Thanks @jtoomim, appreciated.

@sickpig
Copy link
Collaborator

sickpig commented Dec 5, 2018

this a @jtoomim's idea to improve the current situation even further, he called it "download slots":

you have 2 graphene-only download slots, 2 graphene-or-xthin slots, and 2 graphene-or-xthin-or-getblock slots

if a node announces a block you don't have, you check which protocols they support

if they support graphene, there are 6 slots that could be used

you use the most selective slots first (i.e. graphene-only)

so if you have 5 graphene peers announce a block, followed by an xthin peer, you would

(a) fill up all g slots with 2 g peers

(b) full up all g+x slots with 2 peers

(c) fill up 1 g+x+gb slot with 1 g peer

(d) not have any x-supporting slots left for the xthin peer

actually, using the least-selective slots first might be better

that way, you won't waste bandwidth on getblock if you already have 2 active graphene or xthin transfers

@gandrewstone gandrewstone merged commit cdb1eb4 into BitcoinUnlimited:dev Dec 5, 2018
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