Skip to content

Conversation

laanwj
Copy link
Member

@laanwj laanwj commented Oct 3, 2012

This command is a leftover from send-to-IP transactions, which have been removed a long time ago.

Removes the last remaining GetBoolArg("-allowreceivebyip").

@jgarzik
Copy link
Contributor

jgarzik commented Oct 3, 2012

Good but incomplete. You should also remove:

  1. "reply" message handling
  2. mapRequests
  3. PushRequest*() functions

These command are a leftover from send-to-IP transactions, which have been
removed a long time ago.
Also removes CNode::mapRequests and CNode::PushRequests, as these were
only used for the mentioned commands.
@laanwj
Copy link
Member Author

laanwj commented Oct 3, 2012

Ok, even better, I think I got them all now.

@jgarzik
Copy link
Contributor

jgarzik commented Oct 3, 2012

ACK

@BitcoinPullTester
Copy link

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/529a4d4824136a00e63cd69831023b9c4dd6936b for binaries and test log.

@sipa
Copy link
Member

sipa commented Oct 7, 2012

ACK

@Diapolo
Copy link

Diapolo commented Oct 7, 2012

Same here, I think such removal pulls should get into 0.7.1.

@gmaxwell
Copy link
Contributor

ACK.

@gavinandresen
Copy link
Contributor

ACK

sipa added a commit that referenced this pull request Oct 25, 2012
@sipa sipa merged commit 344620e into bitcoin:master Oct 25, 2012
laudney pushed a commit to reddcoin-project/reddcoin-3.10 that referenced this pull request Mar 19, 2014
@laanwj laanwj deleted the 2012_10_remove_getorder branch April 9, 2014 14:18
KolbyML pushed a commit to KolbyML/bitcoin that referenced this pull request Dec 5, 2020
…shes in masternode manager

dd67066 [Test] Add feature_blockhashcache.py (random-zebra)
4031fd2 [Refactor] Remove some more chainActive accesses and cs_main locks (random-zebra)
fe1d84e [Validation] Verify MN pings with cached block hashes (random-zebra)
14d3c32 [Masternode] Use cached block hashes to create mn pings (random-zebra)
f5349b9 [Cleanup] Remove global map mapCacheBlockHashes (random-zebra)
3cdb0a0 [Cleanup] Remove GetBlockHash in masternode.h/.cpp (random-zebra)
4d41910 [Masternode] Use cached block hashes to calculate the score (random-zebra)
616039e [Validation] Cycle-back when uncaching block hashes (random-zebra)
32617a7 [RPC] Add getcachedblockhashes function (random-zebra)
0de6544 [Init] Load block hashes cache at startup (random-zebra)
2237a69 [Refactor] Cache last 200 block hashes in Masternode Manager (random-zebra)
ddf63fe [Refactor] Introduce CyclingVector class (random-zebra)
2c60025 [Cleanup] Remove not needed calls to masternode.cpp GetBlockHash (random-zebra)
aa28000 [Refactor] Cache tip height in masternode manager (random-zebra)

Pull request description:

  **Problem**:
  Validation of many objects on the second layer (masternode pings, winners/scores, etc...) requires access to chainActive to get the hashes of blocks at a certain height. This is done very often (especially for masternode pings) and means holding `cs_main` lock every time, making the process slow and possibly leading to locking order inconsistencies.

  **Solution**:
  Cache the last N block hashes in the masternode manager.
  In order to implement this efficiently, we introduce a new data structure called `CyclingVector`, which is a fixed-size vector, exposing only a setter `Set(i,  value)` which modifies the index `i % N`.
  The cache is initialized at startup and updated when blocks get connected to /  disconnected from the tip.

  The size (`CACHED_BLOCK_HASHES`) is currently set a default value of 200 making it possible to validate all masternode pings (but not all masternode winners) without accessing chainActive. In the future we could add a startup flag to let the user customize this (higher cache size would, as usual, mean faster execution but higher memory usage).

  An RPC `getcachedblockhashes` is introduced, in order to print the content of the cache. This can be used later in the functional test suite for better testing.

  This PR also removes the accesses to chainActive to get the current chain-height (using a value cached in the manager).

ACKs for top commit:
  furszy:
    tested with bitcoin#1916 and have run it for a while in mainnet, all looking good. Really nice, ACK dd67066 .
  Fuzzbawls:
    ACK dd67066

Tree-SHA512: 3ba56ddbf9fb37c96c1298773f1008f70ca8d0b9aac25754884805e96c7bd104f4e0c2403e49d5502117b6764c319ddd25c70bb53095b009855894d50531c056
KolbyML pushed a commit to KolbyML/bitcoin that referenced this pull request Dec 5, 2020
872da91 Speedup listmasternodes removing a redundant for loop over every MN on the mnnodeman. (furszy)
ea434a1 CMasternodeMan::GetMasternodeRanks removed unneeded MN extra validations and totally unnecessary extra vector load. (furszy)

Pull request description:

  Built on top of bitcoin#1904 . PR starting on 9123d22.

  `listmasternodes` RPC command was taking a considerable amount of time, checked the flow and discovered a lot of redundancies in the process. This PR aims to correct them.

   1) `CMasternodeMan::GetMasternodeRanks` is only used for an RPC call and it was checking and validating each Masternode instead of just retrieve them as it suppose to be doing (the tier two thread is the only one responsible of check and update the masternodes list).

  2) `CMasternodeMan::GetMasternodeRanks` had a totally unneeded second vector load. Looping over the entire masternodes list one more time after getting it.

  3) `listmasternodes` after calling `GetMasternodeRanks` was looking for each of the retrieved masternodes one more time using the retrieved masternode vin (yes..).

ACKs for top commit:
  random-zebra:
    ACK 872da91
  Fuzzbawls:
    ACK 872da91

Tree-SHA512: 52db4488ec429594a8fa6cacceaaf494ec0da179929b7de30f8698cbbb2d940c0d859f8a0082dce33ead5227066079011f30480434ab935906e12aa80b4199a3
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants