-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Backport same PRs as done in bitcoin#11592 #3113
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
Conversation
|
Added a commit that avoids disconnecting of outbound masternode connections |
4269a53 to
5f4c69d
Compare
UdjinM6
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK to rebase&merge after #3110
UdjinM6
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs rebase
-BEGIN VERIFY SCRIPT- sed -i 's/assert_raises_jsonrpc/assert_raises_rpc_error/g' test/functional/*py test/functional/test_framework/*py -END VERIFY SCRIPT-
…kup to source file 5d465e3 Ensure backupwallet fails when attempting to backup to source file (Tomas van der Wansem) Pull request description: Previous behaviour was to destroy the wallet (to zero-length) This fixes bitcoin#11375 Tree-SHA512: bfd1738659b15e3f23b6bbdf55ec12269c62c820bf701daec19500b52bd5845bb5516733c6f76f36197eb155182a8a35dc239ad4de2ef1e59bbb0f124a455759
7104de8 [wallet] Fix leak in CDB constructor (João Barbosa) Pull request description: First commit fixes a minor leak. Second commit improves the constructor in the failure cases. Tree-SHA512: 5165413d60ed9fc28203c9fe128adbba03a9ea9e9aa3734d9ea2522dafd815ba0fb8b90fd0809dbc06eb3ad360e7764de01dadf653ade3350fe86f6b8f04bc90
…usly 478a89c Avoid opening copied wallet databases simultaneously (Russell Yanofsky) Pull request description: Make sure wallet databases have unique fileids. If they don't, throw an error. BDB caches do not work properly when more than one open database has the same fileid, because values written to one database may show up in reads to other databases. Bitcoin will never create different databases with the same fileid, but users can create them by manually copying database files. BDB caching bug was reported by @dooglus in bitcoin#11429 Tree-SHA512: e7635dc81a181801f42324b72fe9e0a2a7dd00b1dcf5abcbf27fa50938eb9a1fc3065c2321326c3456c48c29ae6504353b02f3d46e6eb2f7b09e46d8fe24388d
…eanup fafa003 qa: Remove never used return value of sync_with_ping (MarcoFalke) fa9de37 qa: Make tmpdir option an absolute path (MarcoFalke) Pull request description: This should fix issues with the multiwallet test and its symlinks when the tmpdir is a relative path. Rather than fixing os.symlink to work with paths relative to a directory descriptor, which does not work on Windows, normalize the path instead. Tree-SHA512: 189690f3d065ea2f0f48e06775c86d513d0916c7c86312432e8e16df160e65539e288c2bd53d49a4180735fa940f6fcd52b506ccd7d9815651a9b1a69850dda6
eac64bb [qa] Test nMinimumChainWork (Suhas Daftuar) 0311836 Allow setting nMinimumChainWork on command line (Suhas Daftuar) Pull request description: As discussed briefly here: https://botbot.me/freenode/bitcoin-core-dev/2017-02-28/?msg=81712308&page=4 This adds a hidden command line option for setting `nMinimumChainWork`, which allows us to test this parameter in our functional tests, as well as allowing for niche use cases like syncing nodes that are otherwise disconnected from the network. See also bitcoin#10345, which proposes a new use of `nMinimumChainWork`. Tree-SHA512: fe4d8f4f289697615c98d8760f1cc74c076110310ea0b5b875fcab78c127da9195b4eb84148aebacc7606c246e5773d3f13bd5d9559d0a8bffac20a3a28c62df
01b52ce Add comment explaining forced processing of compact blocks (Suhas Daftuar) 08fd822 qa: add test for minchainwork use in acceptblock (Suhas Daftuar) ce8cd7a Don't process unrequested, low-work blocks (Suhas Daftuar) Pull request description: A peer could try to waste our resources by sending us unrequested blocks with low work (eg to fill up our disk). Since e265200 we no longer request blocks until we know we're on a chain with more than nMinimumChainWork (our anti-DoS threshold), but we would still process unrequested blocks that had more work than our tip (which generally has low-work during IBD), even though we may not yet have found a headers chain with sufficient work. Fix this and add a test. Tree-SHA512: 1a4fb0bbd78054b84683f995c8c3194dd44fa914dc351ae4379c7c1a6f83224f609f8b9c2d9dde28741426c6af008ffffea836d21aa31a5ebaa00f8e0f81229e
…suite. 15f5d3b Switch DNSSeed-needed metric to any-automatic-nodes, not services (Matt Corallo) 5ee88b4 Clarify docs for requirements/handling of addnode/connect nodes (Matt Corallo) 57edc0b Rename fAddnode to a more-descriptive "manual_connection" (Matt Corallo) 4440710 Replace relevant services logic with a function suite. (Matt Corallo) Pull request description: This was mostly written as a way to clean things up so that the NETWORK_LIMITED PR (bitcoin#10387) can be simplified a ton, but its also a nice standalone cleanup that will also require a bit of review because it tweaks a lot of stuff across net. The new functions are fine in protocol.h right now since they're straight-forward, but after NETWORK_LIMITED will really want to move elsewhere after @theuni moves the nServices-based selection to addrman from connman. Adds HasAllRelevantServices and GetRelevantServices, which check for NETWORK|WITNESS. This changes the following: * Removes nRelevantServices from CConnman, disconnecting it a bit more from protocol-level logic. * Replaces our sometimes-connect-to-!WITNESS-nodes logic with simply always requiring WITNESS|NETWORK for outbound non-feeler connections (feelers still only require NETWORK). * This has the added benefit of removing nServicesExpected from CNode - instead letting net_processing's VERSION message handling simply check HasAllRelevantServices. * This implies we believe WITNESS nodes to continue to be a significant majority of nodes on the network, but also because we cannot sync properly from !WITNESS nodes, it is strange to continue using our valuable outbound slots on them. * In order to prevent this change from preventing connection to -connect= nodes which have !WITNESS, -connect nodes are now given the "addnode" flag. This also allows outbound connections to !NODE_NETWORK nodes for -connect nodes (which was already true of addnodes). * Has the (somewhat unintended) consequence of changing one of the eviction metrics from the same sometimes-connect-to-!WITNESS-nodes metric to requiring HasRelevantServices. This should make NODE_NETWORK_LIMITED much simpler to implement. Tree-SHA512: 90606896c86cc5da14c77843b16674a6a012065e7b583d76d1c47a18215358abefcbab44ff4fab3fadcd39aa9a42d4740c6dc8874a58033bdfc8ad3fb5c649fc
…e class 2525b97 net: stop both net/net_processing before destroying them (Cory Fields) 80e2e9d net: drop unused connman param (Cory Fields) 8ad663c net: use an interface class rather than signals for message processing (Cory Fields) 28f11e9 net: pass CConnman via pointer rather than reference (Cory Fields) Pull request description: See individual commits. Benefits: - Allows us to begin moving stuff out of CNode and into CNodeState (after bitcoin#10652 and follow-ups) - Drops boost dependency and overhead - Drops global signal registration - Friendlier backtraces Tree-SHA512: af2038c959dbec25f0c90c74c88dc6a630e6b9e984adf52aceadd6954aa463b6aadfccf979c2459a9f3354326b5077ee02048128eda2a649236fadb595b66ee3
77939f2 Fix uninitialized g_connman crash in Shutdown() (MeshCollider) Pull request description: Fixes bitcoin#11312 As @dooglus pointed out, `g_connman` is uninitialized when an invalid wallet path is passed on start up, but then dereferenced in `Shutdown()`, so this tiny PR just fixes that. Tree-SHA512: 2557133422a6e393017081450a7e6c100fe7d9ce36e628e5f5f479bc07617a7bd9a9ad4d44c0d8abadf2e3eb62a11ce9743abc27b4ae8c20f709e72df4f25a7f
…chains e065249 Add unit test for outbound peer eviction (Suhas Daftuar) 5a6d00c Permit disconnection of outbound peers on bad/slow chains (Suhas Daftuar) c60fd71 Disconnecting from bad outbound peers in IBD (Suhas Daftuar) Pull request description: The first commit will disconnect an outbound peer that serves us a headers chain with insufficient work while we're in IBD. The second commit introduces a way to disconnect outbound peers whose chains fall out of sync with ours: For a given outbound peer, we check whether their best known block (which is known from the blocks they announce to us) has at least as much work as our tip. If it doesn't, we set a 20 minute timeout, and if we still haven't heard about a block with as much work as our tip had when we set the timeout, then we send a single getheaders message, and wait 2 more minutes. If after two minutes their best known block has insufficient work, we disconnect that peer. We protect 4 of our outbound peers (who provide some "good" headers chains, ie a chain with at least as much work as our tip at some point) from being subject to this logic, to prevent excessive network topology changes as a result of this algorithm, while still ensuring that we have a reasonable number of nodes not known to be on bogus chains. We also don't require our peers to be on the same chain as us, to prevent accidental partitioning of the network in the event of a chain split. Note that if our peers are ever on a more work chain than our tip, then we will download and validate it, and then either reorg to it, or learn of a consensus incompatibility with that peer and disconnect. This PR is designed to protect against peers that are on a less work chain which we may never try to download and validate. Tree-SHA512: 2e0169a1dd8a7fb95980573ac4a201924bffdd724c19afcab5efcef076fdbe1f2cec7dc5f5d7e0a6327216f56d3828884f73642e00c8534b56ec2bb4c854a656
37886d5 Disconnect outbound peers relaying invalid headers (Suhas Daftuar) 4637f18 moveonly: factor out headers processing into separate function (Suhas Daftuar) Pull request description: Alternate to bitcoin#11446. Disconnect outbound (non-manual) peers that serve us block headers that are already known to be invalid, but exempt compact block announcements from such disconnects. We restrict disconnection to outbound peers that are using up an outbound connection slot, because we rely on those peers to give us connectivity to the honest network (our inbound peers are not chosen by us and hence could all be from an attacker/sybil). Maintaining connectivity to peers that serve us invalid headers is sometimes desirable, eg after a soft-fork, to protect unupgraded software from being partitioned off the honest network, so we prefer to only disconnect when necessary. Compact block announcements are exempted from this logic to comply with BIP 152, which explicitly permits nodes to relay compact blocks before fully validating them. Tree-SHA512: 3ea88e4ccc1184f292a85b17f800d401d2c3806fefc7ad5429d05d6872c53acfa5751e3df83ce6b9c0060ab289511ed70ae1323d140ccc5b12e3c8da6de49936
2530bf2 net: Add missing lock in ProcessHeadersMessage(...) (practicalswift) Pull request description: Add missing lock in `ProcessHeadersMessage(...)`. Reading the variable `mapBlockIndex` requires holding the mutex `cs_main`. The new "Disconnect outbound peers relaying invalid headers" code added in commit 37886d5 and merged as part of bitcoin#11568 two days ago did not lock `cs_main` prior to accessing `mapBlockIndex`. Tree-SHA512: b799c234be8043d036183a00bc7867bbf3bd7ffe3baa94c88529da3b3cd0571c31ed11dadfaf29c5b8498341d6d0a3c928029a43b69f3267ef263682c91563a3
…n invalid block (more effeciently) f3d4adf Make p2p-acceptablock not an extended test (Matt Corallo) 00dcda6 [qa] test that invalid blocks on an invalid chain get a disconnect (Matt Corallo) 015a525 Reject headers building on invalid chains by tracking invalidity (Matt Corallo) 932f118 Accept unrequested blocks with work equal to our tip (Matt Corallo) 3d9c70c Stop always storing blocks from whitelisted peers (Matt Corallo) 3b4ac43 Rewrite p2p-acceptblock in preparation for slight behavior changes (Matt Corallo) Pull request description: @sdaftuar pointed out that the version in bitcoin#11487 was somewhat DoS-able as someone could feed you a valid chain that forked off the the last checkpoint block and force you to do lots of work just walking backwards across blocks for each new block they gave you. We came up with a few proposals but settled on the one implemented here as likely the simplest without obvious DoS issues. It uses our existing on-load mapBlockIndex walk to make sure everything that descends from an invalid block is marked as such, and then simply caches blocks which we attempted to connect but which were found to be invalid. To avoid DoS issues during IBD, this will need to depend on bitcoin#11458. Includes tests from bitcoin#11487. Tree-SHA512: 46aff8332908e122dae72ceb5fe8cd241902c2281a87f58a5fb486bf69d46458d84a096fdcb5f3e8e07fbcf7466232b10c429f4d67855425f11b38ac0bf612e1
This avoids future merge conflicts
6262915 Add unit test for stale tip checking (Suhas Daftuar) 83df257 Add CConnmanTest to mutate g_connman in tests (João Barbosa) ac7b37c Connect to an extra outbound peer if our tip is stale (Suhas Daftuar) db32a65 Track tip update time and last new block announcement from each peer (Suhas Daftuar) 2d4327d net: Allow connecting to extra outbound peers (Suhas Daftuar) Pull request description: This is an alternative approach to bitcoin#11534. Rather than disconnect an outbound peer when our tip looks stale, instead try to connect to an additional outbound peer. Periodically, check to see if we have more outbound peers than we target (ie if any extra peers are in use), and if so, disconnect the one that least recently announced a new block (breaking ties by choosing the newest peer that we connected to). Tree-SHA512: 8f19e910e0bb36867f81783e020af225f356451899adfc7ade1895d6d3bd5afe51c83759610dfd10c62090c4fe404efa0283b2f63fde0bd7da898a1aaa7fb281
97932cd rpc: further constrain the libevent workaround (Cory Fields) 6b58360 rpc: work-around an upstream libevent bug (Cory Fields) Pull request description: A rare race condition may trigger while awaiting the body of a message. This may fix some reported rpc hangs/crashes. This work-around mimics what libevent does internally once a write has started, which is what usually happens, but not always due to the processing happening on a different thread: https://github.com/libevent/libevent/blob/e7ff4ef2b4fc950a765008c18e74281cdb5e7668/http.c#L373 Fixed upstream at: libevent/libevent@5ff8eb2 Tree-SHA512: b9fa97cae9da2a44101c5faf1e3be0b9cbdf722982d35541cf224be31430779c75e519c8ed18d06ab7487bfb1211069b28f22739f126d6c28ca62d3f73b79a52
This value was meant to mimic the block time in Bitcoin, so should be set to 2.5 minutes in Dash.
5f4c69d to
424ed32
Compare
|
Rebased on develop and removed the temporary commit. |
nmarley
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK
UdjinM6
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Slightly tested ACK (tried syncing on testnet and it indeed disconnected forked peers as expected)
This does the same as #3110, but for bitcoin#11592