88af502 test: Add createrawtransaction functional tests (João Barbosa)
27c6199 test: Add multidict to support dictionary with duplicate key (laanwj) (João Barbosa)
320669a rpc: Validate replaceable type in createrawtransaction (João Barbosa)
Pull request description:
This was motivated by the `Invalid parameter, duplicated address` test.
Credit to @laanwj for `multidict` implementation.
Tree-SHA512: a87139ae11004b73b467db1e8a072b75e23a0622b173a5668eed383b3575d8abc709817ddd2dfdc53f55afc90750fb61331199ad5de38c1ef6d482f2bc220f74
a3f5657 Add test cases covering the relevant key length boundaries: 64 bytes +/- 1 byte for HMAC-SHA256 and 128 bytes +/- 1 byte for HMAC-SHA512 (practicalswift)
Pull request description:
* Add test cases covering the relevant key length boundaries: 64 bytes +/- 1 byte for HMAC-SHA256 and 128 bytes +/- 1 byte for HMAC-SHA512.
* ~~Avoid creating a one-past-the-end pointer in the case of key length 64 (HMAC-SHA256) and key length 128 (HMAC-SHA512).~~
* ~~Avoid performing a noop memset call (zero length argument) in the case of key length 64 (HMAC-SHA256) and key length 128 (HMAC-SHA512).~~
Tree-SHA512: 48ff9ab79d41aab97b5b8f6496cc08a39955a07eb424f74ada6440d3b168b6204d3527fa677e175c47e40142f9d62c7456ae162e5a2f5b557e90fb353beef1d0
529b866 Test datadir in conf file exists (MeshCollider)
Pull request description:
Provoked by Nick ODell's discovery here: https://bitcoin.stackexchange.com/questions/64189/when-running-bitcoind-i-keep-getting-boostfilesystemspace-operation-not-p/64210#64210
If a custom data directory is specified using `-datadir` argument, its existence is checked before the conf file is loaded. But if the conf file then specifies a different non-existent `datadir`, that isn't tested, and results in esoteric errors like:
EXCEPTION: N5boost10filesystem16filesystem_errorE
boost::filesystem::space: Operation not permitted
This just adds a check for the datadir existence at the end of `ReadConfigFile()`
Tree-SHA512: e488618c40aa356263f94040ae00aa4be98038abef66e8674b01032d22a5553a7fafcb8fe2d1f095865b39fb138c07b7a94415a00ef837573f92f95af065f712
* Refactor PS a bit and make it so that the expected flow is for a mixing tx to time out, however there is in effect no max on the number of participants.
Signed-off-by: Pasta <pasta@dashboost.org>
* revert maxParticipants bump, to be bumped with next breaking change
Signed-off-by: Pasta <pasta@dashboost.org>
* revert min participants to 3 on testnet / devnet, regtest at 2
Signed-off-by: Pasta <pasta@dashboost.org>
* nit extraneous
Co-Authored-By: thephez <thephez@users.noreply.github.com>
Co-authored-by: thephez <thephez@users.noreply.github.com>
5a7c09a test: Add tests for `-debuglogfile` with subdirs (Anthony Towns)
4158734 doc: Update release notes for `-debuglogfile` (Wladimir J. van der Laan)
2323242 test: Add test for `-debuglogfile` (Wladimir J. van der Laan)
cf5f432 Add `-debuglogfile` option (Wladimir J. van der Laan)
Pull request description:
This patch adds an option to configure the name and/or directory of the debug log file.
The user can specify either a relative path, in which case the path is relative to the (network specific) data directory. They can also specify an absolute path to put the log anywhere else in the file system.
Alternative to #11741 that gets rid of the concept of a "log directory" by specifying the path for the specific kind of log, the debug log. Which happens to be the only kind of log we have at this point*, but a hypothetical new kind of log (say, an audit log) would get a new option. This has more flexibility than specifying a directory which has to contain all of them.
\* excluding `db.log` which is internally generated by the wallet database library, but that one moves along with `-walletdir`.
Tree-SHA512: 4434d0e598dc23504e5c9e67fdbaef56db4f0fd490f9f54fd503e69d4dda9b5b69c539e1794ed841e72161b7b1dc3374d2f1193dd431b057566750e56fd8f24b
b4058ed Fix code constness in CBlockIndex::GetAncestor() overloads (Dan Raviv)
Pull request description:
Make the non-const overload of `CBlockIndex::GetAncestor()` reuse the const overload implementation instead of the other way around. This way, the constness of the const overload implementation is guaranteed. The other way around, it was possible to implement the non-const overload in a way which mutates the object, and since that implementation would be called even for const objects (due to the reuse), we would get undefined behavior.
Tree-SHA512: 545a8639bc52502ea06dbd924e8fabec6274fa69b43e3b8966a7987ce4dae6fb2498f623730fde7ed0e47478941c7f8baa2e76a12018134ff7c14c0dfa25ba3a
9b80fc1 Prefix leveldb debug logging (Wladimir J. van der Laan)
Pull request description:
Add leveldb: prefix to leveldb debug logging lines.
leveldb debug messages come in various scary flavors such as:
2017-11-30 08:26:31 leveldb: Recovering log #26
2017-11-30 08:26:31 leveldb: Level-0 table #28: started
2017-11-30 08:26:31 leveldb: Level-0 table #28: 597 bytes OK
2017-11-30 08:26:31 leveldb: Delete type=0 #26
2017-11-30 08:26:31 leveldb: Delete type=3 #24
so it's reasonably important to mark them as coming from leveldb internals and not from consensus validation wallet or such.
This is consistent with the `libevent:` prefix for libevent messages.
(this only affects `-debug=leveldb` or `-debug=1` otherwise you won't see them in the first place)
Tree-SHA512: 074eba00d39c6378b9e14d03aa2c551b4e3fc7bffdd5a1a0ba2498b44bcc77071d47735be09493286056053f0cca625c5cbaad7ad1ddb9d902d30e7cd316d9d2
d31e5c1 Fix shutdown in case of errors during initialization (Wladimir J. van der Laan)
Pull request description:
PR #10286 introduced a few steps which are not robust to early shutdown in initialization.
Stumbled upon this with #11781, not sure if there are other scenarios that can trigger it, but it's good to harden against this in any case.
E.g.
```
$ src/bitcoind -debuglogfile=/dfdf
Error: Could not open debug log file /dfdf
Program received signal SIGSEGV, Segmentation fault.
UnregisterValidationInterface (pwalletIn=0x0) at /.../bitcoin/src/validationinterface.cpp:82
82 g_signals.m_internals->BlockChecked.disconnect(boost::bind(&CValidationInterface::BlockChecked, pwalletIn, _1, _2));
(gdb) bt
#0 UnregisterValidationInterface (pwalletIn=0x0) at /.../bitcoin/src/validationinterface.cpp:82
#1 0x00005555555a11fc in Shutdown () at /.../bitcoin/src/init.cpp:196
#2 0x00005555555961cc in AppInit (argc=<optimized out>, argv=<optimized out>) at /.../bitcoin/src/bitcoind.cpp:183
#3 0x0000555555596249 in main (argc=0, argv=0x555555ecf200) at /.../bitcoin/src/bitcoind.cpp:19
```
Tree-SHA512: 7dd9570a9803514a17781bfadf1edde47e96df4e852cce2f423cab422e005fb94d44e777af1a6ea5167b04a4d889e848ae7a61a7e0e94232247ddea32ee70fc8
8b2c733 clarify abortrescan rpc use (Gregory Sanders)
Pull request description:
`-rescan` during startup doesn't apply since RPC has not warmed up by that point
Tree-SHA512: 0b6738ed9429ca41edbfa23bb3e2da23bb16fed9e55c1803ab140b2efc49ef3295bb8b7eeb8c24df94204c5b3fb25d0d57cbc8d1be11856a5763c9fde71bceba
9d811dc Document partial validation in ConnectBlock() (Suhas Daftuar)
Pull request description:
`ConnectBlock()` relies on validation that happens in `ContextualCheckBlock()` and
`ContextualCheckBlockHeader()`. This has implications for implementing consensus
changes and handling software upgrade to ensure that nodes upgrading their
software end up enforcing all the consensus rules.
Tree-SHA512: 36a252af2221b0e5d5d6f8d5f8b16f8b566ca0db2d56242130a5523302c8757599ac234594a6a946c1689b260d18a32c2c7f8c3831304e78b9832e2ce5ac435a
e1a8ec5 Fix: Open files read only if requested (Andras Elso)
Tree-SHA512: 047951bf3fa1272764596d2efed0a541105a6d3057789ba4218cd1bbce8a802cab0feac0ed3d28f6eab6b2e54e294d46f729dd0162d6d209e319d716c0567352
abbd230 Move RPC registration out of AppInitParameterInteraction (Russell Yanofsky)
Pull request description:
Move to AppInitServers. This doesn't have any effects on bitcoin behavior. It was just strange to have this unrelated code in the middle of parameter interaction.
Tree-SHA512: 373e18f2ef8d21999ad36295d69326128a3086044acfc8ed537abd5497c8d3620b9832f7f6aa87c0c0939bb5e0d92be8a3c006b5997e9e6fa20334f5610c89bc
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
