c3055bb Add help-console command to Qt debug console (Luke Mlsna)
Pull request description:
This PR would close issue #9195 by adding documentation for the debug console features (mainly nested commands) which were added in [PR #7783](https://github.com/bitcoin/bitcoin/pull/7783).
The following changes were made to QT debug console code:
- Added a line to the initial message text at the top of the debug console:
> For more information on using this console type **help-console**.
- Added a pseudo-command `help-console` which is hooked after parsing the request, but before actually executing the RPC thread. It prints the following text to the console as if it were a valid RPC response.
> This console accepts RPC commands using the standard syntax.
> example: getblockhash 8
> This console can also accept RPC commands using bracketed syntax.
> example: getblockhash(8)
> A space or a comma can be used to separate arguments for either syntax.
> example: sendtoaddress \<address\> \<amount\>
> sendtoaddress,\<address\>,\<amount\>
> Commands may be nested when specified with the bracketed syntax.
> example: getblockinfo(getblockhash(0),true).
> Result values can be queried with a non-quoted string in brackets.
> example: getblock(getblockhash(0) true)[height]
This seemed like a reasonably sane way to introduce a fake RPC help command, but
Tree-SHA512: 35d73dcef9c4936b8be99e80978169f117c22b94f4400c91097bf7e0e1489060202dcd738d9debdf4c8a7bd10709e2c19d4f625f19e47c4a034f1d6019c0e0f2
f94c2ec Fix for mismatched extern definition in wallet test classes which was breaking msvc linking. (Aaron Clauson)
Pull request description:
The extern declarations are using a raw pointer which doesn't match the source variable's std::unqiue_ptr.
Tree-SHA512: 1500ffa1282ee6565741bf2f8f9def68a355346c8e4a667e1167d3d550eb0b732e8cba52fbef380a92c860e686d36da877d800ee5f2fc1e4dd8fd88fbe170644
ac1cf8d Trivial: Improve #endif comments (danra)
Pull request description:
Improve the #endif comments for the '#if HAVE_DECL_BSWAP_XX == 0' preprocessor conditions, so each shows the full condition which it closes.
Tree-SHA512: f533311fa48cb2f46f6490b6c965ad5f8861dcfad70c56d70e31fa989b422880c78b2dd6f24f648b19d3a22f767606e0de5cf1cb71445012b42c97ac2149295e
28f8b66 Diagnose unsuitable outputs in lockunspent(). (Eelis)
Pull request description:
Fixes#2667.
This is a simplified version of pull request #3574, which was abandoned by its author.
I added some tests as well.
Tree-SHA512: e63e00dec8b1b232079380183805cb0b0b18c78ea6bea769837949aab984689d7f68b2ccfe66b1873517b040b9e616ce0eb058575c3d4382aa8c26eebcf1f14e
63c2d83 Explicitly state assumption that state.m_chain_sync.m_work_header != nullptr in ConsiderEviction (practicalswift)
Pull request description:
Explicitly state assumption that `state.m_chain_sync.m_work_header != nullptr` in `ConsiderEviction(…)`.
Static analyzer (and humans!) will see the null-check in ...
```
else if (state.m_chain_sync.m_timeout == 0 || (state.m_chain_sync.m_work_header != nullptr && ...
```
... and infer that `state.m_chain_sync.m_work_header` might be set to `nullptr` when reaching `else if (state.m_chain_sync.m_timeout > 0 && time_in_seconds > state.m_chain_sync.m_timeout)` and thus flag `state.m_chain_sync.m_work_header->GetBlockHash().ToString()` as a potential null pointer dereference.
This commit makes the tacit assumption of `state.m_chain_sync.m_work_header != nullptr` explicit.
Code introduced in 5a6d00c6de ("Permit disconnection of outbound peers on bad/slow chains") which was merged into master four days ago.
Friendly ping @sdaftuar :-)
Tree-SHA512: 32e5631025b7ba7556a02c89d040fbe339c482a03f28d0dbc9871c699e1f8ac867619b89c5fd41fdcfcf0dc4d7c859295b26ccd988572145cc244261aec18ce9
5ff01c236 [docs] Add instructions for lcov coverage report generation (James O'Beirne)
Pull request description:
After rediscovering the `lcov` report generation recipe one too many times, it seemed prudent to write some doc.
Tree-SHA512: 20e1b5f51ecd39e14bd67986a2c1578fb7da03a50625366eaca35b201db66aef99cd4a5456df3aaca5d2d66b18ed7d2e8eb8f3bd9c7aaf9af48164d9bac38931
fad63eb [logging] Don't incorrectly log that REJECT messages are unknown. (John Newbery)
Pull request description:
Reject messages are logged to debug.log if NET debug logging is enabled.
Because of the way the `ProcessMessages()` function is structured,
processing for REJECT messages will also drop through to the default
branch and incorrectly log `Unknown command "reject" from peer-?`. Fix
that by exiting from `ProcessMessages()` early.
without this PR:
```
2018-05-03T17:37:00.930600Z received: reject (21 bytes) peer=0
2018-05-03T17:37:00.930620Z Reject message code 16: spammy spam
2018-05-03T17:37:00.930656Z Unknown command "reject" from peer=0
```
with this PR:
```
2018-05-03T17:35:04.751246Z received: reject (21 bytes) peer=0
2018-05-03T17:35:04.751274Z Reject message code 16: spammy spam
```
Tree-SHA512: 5c84c98433ab99e0db2dd481f9c2db6f87ff0d39022ff317a791737e918714bbcb4a23e81118212ed8e594ebcf098ab7f52f7fd5e21ebc3f07b1efb279b9b30b
2c084a6 net: Minor accumulated cleanups (Thomas Snider)
Pull request description:
From now-derelict larger changes I had been working on, here are a series of DRY refactors/cleanups. Net loss of 35 lines of code - a small step in the good fight.
In particular I think operator!= should only ever be implemented as a negation of operator==. Lower chance for errors, and removes the possibility of divergent behavior.
Tree-SHA512: 58bf4b542a4e8e5bc465b508aaa16e9ab51448c3f9bee52cd9db0a64a5c6c5a13e4b4286d0a5aa864934fc58064799f6a88a40a87154fd3a4bd731a72e254393
46e7f800b Limit the number of IPs we use from each DNS seeder (e0)
Pull request description:
A risk exists where a malicious DNS seeder eclipses a node by returning an enormous number of IP addresses. In this commit we mitigate this risk by limiting the number of IP addresses addrman learns to 256 per DNS seeder.
As discussed with @theuni
Tree-SHA512: 949e870765b1470200f2c650341d9e3308a973a7d1a6e557b944b0a2b8ccda49226fc8c4ff7d2a05e5854c4014ec0b67e37a3f2287556fe7dfa2048ede1f2e6f
c887f87 Extend #11583 to include the most common message generated by non-contributing peers (port scanners?) 37% of the log default log entries for a node that has been up for ~24hrs was "version handshake timeout..." (Clem Taylor)
Pull request description:
37% of the default log entries for a node that has been up for ~24hrs was "version handshake timeout..."
Tree-SHA512: dceeee5d55a9ff7570174aeb63faac9beda239087220522adefef7ed11e0eeffa008ca28726011247c8834c1a222d37817baf895635ab874a95ebc435959070e
660f5f1 net: don't retry failed oneshot connections forever (Cory Fields)
Pull request description:
As introduced by (my suggestion, sorry, in) #11512, failed dns resolves end up as oneshots. But failed oneshots are re-added as oneshots, so we need to make sure that we're not queuing these up forever after failed resolves.
Rather than trying to differentiate, I think we should just not re-add failed oneshots and be done with it.
Maybe @sipa can shed a light on what the original intention was.
Tree-SHA512: 2dfe35dabfb6354c315cf6f8ae42971765d36575e685662caae7ed8f9dea9472c6fb1fd5e62ec35301550b74b6613a54265e90fca2a6618544f78dacaac4d4fd
fix 12329 backport
Signed-off-by: Pasta <pasta@dashboost.org>
fix 12329 backport
Signed-off-by: Pasta <pasta@dashboost.org>
fix 12329 backport
Signed-off-by: Pasta <pasta@dashboost.org>
96dbd38 net: initialize socket to avoid closing random fd's (Cory Fields)
Pull request description:
An excellent spot by @david60.
Even if it isn't causing the fd issue we're looking for, this should be fixed.
Tree-SHA512: 062a8f2cdd39d895213e1263dbd7b8391473ddaea2f93c82c211a9bb6ea6744d48a6c84c8ff804b16b865d14145492635c500a9fd138d0988fee5e4f719ebb91
d3a185a net: Move misbehaving logging to net logging category (Wladimir J. van der Laan)
Pull request description:
This moves the error messages for misbehavior (when available) into the line that reports the misbehavior, as well as moves the logging to the `net` category.
This is a continuation of #11583 and avoids serious-looking errors due to misbehaving peers. As it is impossible to correlate the `peer=X` numbers to specific incoming connections now without enabling the `net` category, it doesn't really help to see these messages by default.
To do this, Misbehaving() gains an optional `message` argument.
E.g. change:
2018-01-18 16:02:27 Misbehaving: x.x.x.x:62174 peer=164603 (80 -> 100) BAN THRESHOLD EXCEEDED
2018-01-18 16:02:27 ERROR: non-continuous headers sequence
to
2018-01-18 16:02:27 Misbehaving: x.x.x.x:62174 peer=164603 (80 -> 100) BAN THRESHOLD EXCEEDED: non-continuous headers sequence
When there is a category for "important" net messages (see #12219 ), we should move it there.
Tree-SHA512: 51c97e9a649bf5409f2fd4625fa1243a036e9c9de6037bb064244207408c2e0eb025e3af80866df673cdc006b8f35dc4078d074033f0d4c6a73bbb03949a269f
be9f38c Do not make it trivial for inbound peers to generate log entries (Matt Corallo)
Pull request description:
Based on #11580 because I'm lazy.
We should generally avoid writing to debug.log unconditionally for
inbound peers which misbehave (the peer being about to be banned
being an exception, since they cannot do this twice).
Tree-SHA512: 8e59c8d08d00b1527951b30f4842d010a4c2fc440503ade112baa2c1b9afd0e0d1c5c2df83dde25183a242af45089cf9b9f873b71796771232ffb6c5fc6cc0cc
6eddd43 Fix warnings when building with DEBUG_ADDRMAN (practicalswift)
Pull request description:
Fix warnings when building with `DEBUG_ADDRMAN`.
Warnings prior to this commit:
```
addrman.cpp:390:24: warning: comparison of integers of different signs: 'size_type' (aka 'unsigned long') and 'int' [-Wsign-compare]
if (vRandom.size() != nTried + nNew)
~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~
addrman.cpp:411:52: warning: comparison of integers of different signs: 'int' and 'size_type' (aka 'unsigned long') [-Wsign-compare]
if (info.nRandomPos < 0 || info.nRandomPos >= vRandom.size() || vRandom[info.nRandomPos] != n)
~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~
addrman.cpp:419:25: warning: comparison of integers of different signs: 'size_type' (aka 'unsigned long') and 'int' [-Wsign-compare]
if (setTried.size() != nTried)
~~~~~~~~~~~~~~~ ^ ~~~~~~
addrman.cpp:421:23: warning: comparison of integers of different signs: 'size_type' (aka 'unsigned long') and 'int' [-Wsign-compare]
if (mapNew.size() != nNew)
~~~~~~~~~~~~~ ^ ~~~~
4 warnings generated.
```
Tree-SHA512: 0316faecfe95066d2c9a0b6b3960086e43824f21a67086a895ea45fbce1327f8d6df5945fe923c2dbe4efce430bc1384d515d317c3930d97d24965e507cf734d
2b839ab Update chainparams comment for more info on service bits per dnsseed (Matt Corallo)
62e7642 Fall back to oneshot for DNS Seeds which don't support filtering. (Matt Corallo)
51ae766 Use GetDesireableServiceFlags in static seeds, document this. (Matt Corallo)
fb6f6b1 bluematt's testnet-seed now supports x9 (and is just a static list) (Matt Corallo)
Pull request description:
4440710 broke inserting entries into addrman from dnsseeds which
did not support service bits, as well as static seeds. Static seeds
were already being filtered by UA for 0.13.1+ (ie NODE_WITNESS), so
simply changing the default service bits to include NODE_WITNESS
(and updating docs appropriately) is sufficient. For DNS Seeds, not
supporting NODE_WITNESS is no longer useful, so instead use
non-filtering seeds as oneshot hosts irrespective of named proxy.
I've set my testnet-seed to also support x9, though because it is simply a static host, it may be useful to leave the support off so that it is used as a oneshot to get addresses from a live node instead. I'm fine with either.
Tree-SHA512: 3f17d4d2b0b84d876981c962d2b44cb0c8f95f52c56a48c6b35fd882f6d7a40805f320ec452985a1c0b34aebddb1922709156c3ceccd1b9f8363fd7cb537d21d
ce2418f [gui] reset addrProxy/addrSeparateProxyTor if colon char missing (Cristian Mircea Messel)
Pull request description:
If addrProxy or addrSeparateProxyTor do not have a colon in the string
somewhere in the QSettings storage, then attempting to open the options
dialog will cause the entire program to crash.
Fixes#11209
Tree-SHA512: 2d9e6987cf05af3f41033290b61d00920f7fe4a65bea7efd96ed417a8ca7866d248f091e09947cc8aad3a6a4aa8b7777211cfff7f379a62188be50df2c46d4b2
fa6c3dea420b6c50c164ccc34f4e9e8a7d9a8022 p2p: Clarify control flow in ProcessMessage() (MarcoFalke)
Pull request description:
`ProcessMessage` is effectively a massive switch case construct. In the past there were attempts to clarify the control flow in `ProcessMessage()` by moving each case into a separate static function (see #9608). It was closed because it wasn't clear if moving each case into a function was the right approach.
Though, we can quasi treat each case as a function by adding a return statement to each case. (Can be seen as a continuation of bugfix #13162)
This patch does exactly that.
Also note that this patch is a subset of previous approaches such as #9608 and #10145.
Review suggestion: `git diff HEAD~ --function-context`
Tree-SHA512: 91f6106840de2f29bb4f10d27bae0616b03a91126e6c6013479e1dd79bee53f22a78902b631fe85517dd5dc0fa7239939b4fefc231851a13c819458559f6c201
3830b6e net: use CreateSocket for binds (Cory Fields)
df3bcf8 net: pass socket closing responsibility up to caller for outgoing connections (Cory Fields)
9e3b2f5 net: Move IsSelectableSocket check into socket creation (Cory Fields)
1729c29 net: split socket creation out of connection (Cory Fields)
Pull request description:
Requirement for #11227.
We'll need to create sockets and perform the actual connect in separate steps, so break them up.
#11227 adds an RAII wrapper around connection attempts, as a belt-and-suspenders in case a CloseSocket is missed.
Tree-SHA512: de675bb718cc56d68893c303b8057ca062c7431eaa17ae7c4829caed119fa3f15b404d8f52aca22a6bca6e73a26fb79e898b335d090ab015bf6456cf417fc694
Sometimes the node we ask for mnlistdiff is so fast to reply that we receive the message back before we reset `last_mnlistdiff`. To fix this we should reset it before sending the message, not after.
* Check MNs up to 24 blocks deep when verifying `dstx`
* Handle DSTX-es more like regular txes and not like "other" invs
* Try asking for a DSTX too when trying to find missing tx parents
* Check DSTX-es when chainlock arrives
`HasChainLock` was always `false` in `IsExpired` because tip is updated before the corresponding chainlock is received
* Apply `Handle DSTX-es more like regular txes` idea to `AlreadyHave()`
* Alternative handling of DSTX+recentRejects
Co-authored-by: Alexander Block <ablock84@gmail.com>
927f4ff5a GUI: Receive: Remove option to reuse a previous address (Luke Dashjr)
Pull request description:
This was justified by the need to "resent" an invoice, but now that we have the request history, that need should be gone.
Tree-SHA512: 4ade4eb84a21bbbd8dcc3a2c9580d416e113284b5bdf350c22051c233101fe0ee31659c54a7a46e7136f9c999acb61efbbb3f97aeb2fa7b2b1e1daec02ca0837
5e0ba8f8c [wallet] getreceivedbyaddress should return error if address is not mine (John Newbery)
ea0cd24f7 [tests] Tidy up receivedby.py (John Newbery)
Pull request description:
Two commits:
- First commit tidies up the `receivedby.py` test (and speeds it up by factor of two)
- Second commit changes getreceivedbyaddress to return error if the address is not found in wallet, and adds test to `receivedby.py`
Tree-SHA512: e41342dcbd037a6b440cbe4ecd3b8ed589e18e477333f0d866f3564e948e0f5231e497d5ffb66da4e6680eb772d9f0cf839125098bb68b92d04a5ee35c6c0a81
11413646b [trivial] (whitespace only) fix getblockchaininfo alignment (John Newbery)
bd9c18171 [rpc] Add initialblockdownload to getblockchaininfo (John Newbery)
Pull request description:
Exposing whether the node is in IBD would help for testing, and may be useful in general, particularly for developers.
First discussed in #10357 here: https://github.com/bitcoin/bitcoin/pull/10357#pullrequestreview-59963870
> ... we could simplify this (and possibly other) tests by just adding a way to know if a node is in IBD. I'd like to do that, but I'm not sure it makes sense to complicate this PR with discussion over how that information should be made available. Eg it's not clear to me that the notion of being in IBD is worth exposing to the casual user, versus a hidden rpc call or something, since the definition has changed over time, and may continue to change in the future. But I still do agree that at least for testing purposes it would be far simpler to expose the field somehow...
This PR currently implements the simplest way of doing this: adding an `initialblockdownload` field to `getblockchaininfo`. Other approaches we could take:
1. add a new debug RPC method that exposes `IBD` and potentially other information.
2. add a parameter to `getblockchaininfo`, eg `debug_info`, which would cause it to return debug information including IBD
3. add a query string to the url `?debug=true` which would cause RPCs to return additional debug information.
I quite like the idea of (3). Feedback on these and other approaches very much welcomed!
@sdaftuar @laanwj
Tree-SHA512: a6dedd47f8c9bd38769cc597524466250041136feb33500644b9c48d0ffe4e3eeeb2587b5bbc6420364ebdd2667df807fbb50416f9a7913bbf11a14ea86dc0d4
203a4aa31 Fix CTxMemPoolEntry::UpdateAncestorState: modifySigOps param type int -> int64_t (donaloconnor)
Pull request description:
CTxMemPoolEntry::CTxMemPoolEntry's modifySigOps parameter is int while update_ancestor_state::modifySigOpsCost is int64_t. This issue was raised in #11165. It looks like the function paramaters were not changed in commit 72abd2c
This will avoid unexpected truncation of int64_t -> int
Tree-SHA512: 314c703f217e104336456859066d18fb0d12c4f9f32835e17490a6f29eb05951184095039e4e57edacef8ad35dd75c6d97d9af656a52209dd0c3779b4ffa0914
