In spite of the name FindLatestBefore used std::lower_bound to try
to find the earliest block with a nTime greater or equal to the
the requested value. But lower_bound uses bisection and requires
the input to be ordered with respect to the comparison operation.
Block times are not well ordered.
I don't know what lower_bound is permitted to do when the data
is not sufficiently ordered, but it's probably not good.
(I could construct an implementation which would infinite loop...)
To resolve the issue this commit introduces a maximum-so-far to the
block indexes and searches that.
For clarity the function is renamed to reflect what it actually does.
An issue that remains is that there is no grace period in importmulti:
If a address is created at time T and a send is immediately broadcast
and included by a miner with a slow clock there may not yet have been
any block with at least time T.
The normal rescan has a grace period of 7200 seconds, but importmulti
does not.
0c50909 testcases: explicitly specify transaction version 1 (John Newbery)
b7e144b Add test cases to test new bitcoin-tx functionality (jnewbery)
61a1534 Add all transaction output types to bitcoin-tx. (jnewbery)
1814b08 add p2sh and segwit options to bitcoin-tx outscript command (Stanislas Marion)
There is still a call to ActivateBestChain with cs_main if a peer
requests the block prior to it being validated, but this one is
more specifically-gated, so should be less of an issue.
AcceptToMemoryPool has several classes of return false statements.
- return state.Invalid or state.DoS directly itself
- return false and set fMissingInputs (state is valid)
- return false and state is set by failed CheckTransaction
- return false and state is set by failed CheckInputs.
This commit patches the last case where the state variable was reused for additional calls to CheckInputs to identify witness stripping as cause of validation failure. After this commit, it should be the case that if !fMissingInputs, state is always Invalid if AcceptToMemoryPool returns false.
4e7e2e1 Update RPC argument names (John Newbery)
481f289 rpc: Named argument support for bitcoin-cli (Wladimir J. van der Laan)
9adb4e1 rpc: Argument name consistency (Wladimir J. van der Laan)
8d713f7 rpc: Named arguments for rawtransaction calls (Wladimir J. van der Laan)
37a166f rpc: Named arguments for wallet calls (Wladimir J. van der Laan)
78b684f rpc: Named arguments for mining calls (Wladimir J. van der Laan)
b8ebc59 rpc: Named arguments for net calls (Wladimir J. van der Laan)
2ca9dcd test: Add test for RPC named arguments (Wladimir J. van der Laan)
fba1a61 rpc: Named arguments for misc calls (Wladimir J. van der Laan)
286ec08 rpc: Add 'echo' call for testing (Wladimir J. van der Laan)
495eb44 rpc: Named arguments for blockchain calls (Wladimir J. van der Laan)
6f1c76a rpc: Support named arguments (Wladimir J. van der Laan)
5865d41 authproxy: Add support for RPC named arguments (Wladimir J. van der Laan)
5113474 wallet: Use CDataStream.data() (Wladimir J. van der Laan)
e2300ff bench: Use CDataStream.data() (Wladimir J. van der Laan)
adff950 dbwrapper: Use new .data() method of CDataStream (Wladimir J. van der Laan)
a2141e4 streams: Remove special cases for ancient MSVC (Wladimir J. van der Laan)
af4c44c streams: Add data() method to CDataStream (Wladimir J. van der Laan)
20449ef Don't overpay fee if we have selected new coins that result in a smaller transaction. (Alex Morcos)
42f5ce4 Try to reduce change output to make needed fee in CreateTransaction (Alex Morcos)
032ba3f RPC help documentation for addnode peerinfo. (Gregory Maxwell)
90f13e1 Add release notes for addnode changes. (Gregory Maxwell)
50bd12c Break addnode out from the outbound connection limits. (Gregory Maxwell)
On repeated calls to SelectCoins we try to meet the fee necessary for the last transaction, the new fee required might be smaller, so increase our change by the difference if we can.
Once we've picked coins and dummy-signed the transaction to calculate fee, if we don't have sufficient fee, then try to meet the fee by reducing change before resorting to picking new coins.
Previously addnodes were in competition with outbound connections
for access to the eight outbound slots.
One result of this is that frequently a node with several addnode
configured peers would end up connected to none of them, because
while the addnode loop was in its two minute sleep the automatic
connection logic would fill any free slots with random peers.
This is particularly unwelcome to users trying to maintain links
to specific nodes for fast block relay or purposes.
Another result is that a group of nine or more nodes which are
have addnode configured towards each other can become partitioned
from the public network.
This commit introduces a new limit of eight connections just for
addnode peers which is not subject to any of the other connection
limitations (including maxconnections).
The choice of eight is sufficient so that under no condition would
a user find themselves connected to fewer addnoded peers than
previously. It is also low enough that users who are confused
about the significance of more connections and have gotten too
copy-and-paste happy will not consume more than twice the slot
usage of a typical user.
Any additional load on the network resulting from this will likely
be offset by a reduction in users applying even more wasteful
workaround for the prior behavior.
The retry delays are reduced to avoid nodes sitting around without
their added peers up, but are still sufficient to prevent overly
aggressive repeated connections. The reduced delays also make
the system much more responsive to the addnode RPC.
Ban-disconnects are also exempted for peers added via addnode since
the outbound addnode logic ignores bans. Previously it would ban
an addnode then immediately reconnect to it.
A minor change was also made to CSemaphoreGrant so that it is
possible to re-acquire via an object whos grant was moved.
Performing signing in the inner loop has terrible performance
when many passes through are needed to complete the selection.
Signing before the algorithm is complete also gets in the way
of correctly setting the fee (e.g. preventing over-payment when
the fee required goes down on the final selection.)
Use of the dummy might overpay on the signatures by a couple bytes
in uncommon cases where the signatures' DER encoding is smaller
than the dummy: Who cares?
In order to do this, we must call ActivateBestChain prior to
responding getdata requests for blocks which we announced using
compact blocks.
For getheaders responses we dont need code changes, but do note
that we must reset the bestHeaderSent so that the SendMessages call
re-announces the header in question.
While we could do something smarter for getblocks, calling
ActivateBestChain is simple and more obviously correct, instead of
doing something more similar to getheaders.
See-also the BIP clarifications at
https://github.com/bitcoin/bips/pull/486
The meaning is clear from the context, and we're inconsistent here.
Also save typing when using named arguments.
- `bitcoinaddress` -> `address`
- `bitcoinprivkey` -> `privkey`
- `bitcoinpubkey` -> `pubkey`
The [JSON-RPC specification](http://www.jsonrpc.org/specification) allows passing parameters as an Array, for by-position
arguments, or an Object, for by-name arguments.
This implements by-name arguments, but preserves full backwards compatibility. API using by-name arguments are
easier to extend, and easier to use (no need to guess which argument goes where).
Named are mapped to positions by a per-call structure, provided through the RPC command table.
Missing arguments will be replaced by null, except if at the end, then the argument is left out completely.
Currently calls fail (though not crash) on intermediate nulls, but this should be improved on a per-call basis later.
05a55a6 Added EVENT_CFLAGS to test makefile to explicitly include libevent headers. (Karl-Johan Alm)
280a559 Added some simple tests for the RAII-style events. (Karl-Johan Alm)
7f7f102 Switched bitcoin-cli.cpp to use RAII unique pointers with deleters. (Karl-Johan Alm)
e5534d2 Added std::unique_ptr<> wrappers with deleters for libevent modules. (Karl-Johan Alm)
Make a more conservative notion of whether the node is caught up to the rest of the network and only count transactions as fee estimation data points if the node is caught up.
All decisions about whether the transactions are valid data points are made at the time the transaction arrives. Updating on blocks all the time will now cause stale fee estimates to decay quickly when we restart a node.
Fee estimation can just check its own mapMemPoolTxs to determine the same information. Note that now fee estimation for block processing must happen before those transactions are removed, but this shoudl be a speedup.
This was an oversight, where blocks and mempool tracking were ignored during IBD, but transactions that arrived during IBD but were included in blocks after IBD were not ignored.
Once priority estimation was removed, not all transactions in the mempool are tracked in the fee estimation mempool tracking. So there is no error if a transaction is not found for removal.
We were marking coins FRESH before being sure they were not overwriting dirty undo data. This condition was never reached in existing code because undo data was always flushed before UpdateCoins was called with new transactions, but could have been exposed in an otherwise safe refactor.
Clarify in the comments the assumptions made in ModifyNewCoins.
Add ability to undo transactions to UpdateCoins unit test.
Thanks to Russ Yanofsky for suggestion on how to make logic clearer and fixing up the ccoins_modify_new test cases.
Surprisingly this hasn't been causing me any issues while testing, probably
because it requires lots of large blocks to be flying around.
Send/Recv corks need tests!
f8d43b8 Avoid rollingMinimumFeeRate never being able to decay below half (Alex Morcos)
eab8e1b fix a bug if the min fee is 0 for FeeFilterRounder (Alex Morcos)
- Drop the interruption point directly after the pnode allocation. This would
be leaky if hit.
- Rearrange thread creation so that the socket handler comes first
5394b39 Wallet: Split main logic from InitLoadWallet into CreateWalletFromFile (Luke Dashjr)
fb0c934 Wallet: Let the interval-flushing thread figure out the filename (Luke Dashjr)
8562792 GUI/RPCConsole: Include importmulti in history sensitive-command filter (Luke Dashjr)
ff77faf Qt/RPCConsole: Use RPCParseCommandLine to perform command filtering (Luke Dashjr)
a79598d Qt/Test: Make sure filtering sensitive data works correctly in nested commands (Luke Dashjr)
629cd42 Qt/RPCConsole: Teach RPCParseCommandLine how to filter out arguments to sensitive commands (Luke Dashjr)
e2d9213 Qt/RPCConsole: Make it possible to parse a command without executing it (Luke Dashjr)
1755c04 Qt/RPCConsole: Truncate filtered commands to just the command name, rather than skip it entirely in history (Luke Dashjr)
d80a006 Qt/RPCConsole: Add signmessagewithprivkey to list of commands filtered from history (Luke Dashjr)
afde12f Qt/RPCConsole: Refactor command_may_contain_sensitive_data function out of RPCConsole::on_lineEdit_returnPressed (Luke Dashjr)
de8980d Bugfix: Do not add sensitive information to history for real (Luke Dashjr)
9044908 Qt/RPCConsole: Don't store commands with potentially sensitive information in the history (Jonas Schnelli)
fc95daa Qt/RPCConsole: Save current command entry when browsing history (Jonas Schnelli)
This commit add testcases to test the following functions in bitcoin-tx:
- add a pay to non-standard script output
- add a P2SH output
- add a P2WSH output
- add a P2WSH wrapped in a P2SH output
- add a pay to pub key output
- add a P2WPKH output
- add a P2WPKH wrapped in a P2SH output
- add a bare multisig output
- add a multisig in P2SH output
- add a multisig in a P2WSH output
- add a multisig in a P2WSH wrapped in as P2SH output
This commit enhances bitcoin-tx so all remaining standard TXO types can be created:
- Pay to Pub Key
- Multi-sig
- bare multi-sig
- multi-sig in Pay To Script Hash
- multi-sig in Pay to Witness Script Hash
- multi-sig in Pay to Witness Script Hash, wrapped in P2SH
- Pay to Witness Pub Key Hash
- Pay to Witness Pub Key Hash, wrapped in P2SH
- Pay to Witness Script Hash
- Pay to Witness Script Hash, wrapped in P2SH
If the mempool is not completely full, treat the difference between
the maximum size and the actual usage as available for the coin cache.
This also changes the early flush trigger from (usage > 0.9 * space)
to (usage > 0.9 * space && usage > space - 100MB). This means we're not
permanently leaving 10% of the space unused when the space is large.
cee1612 reduce number of lookups in TransactionWithinChainLimit (Gregory Sanders)
af9bedb Test for fix of txn chaining in wallet (Gregory Sanders)
5882c09 CreateTransaction: Don't return success with too-many-ancestor txn (Gregory Sanders)
0b2294a SelectCoinsMinConf: Prefer coins with fewer ancestors (Gregory Sanders)
749be01 Move GetWarnings() into its own file. (Gregory Maxwell)
e3ba0ef Eliminate data races for strMiscWarning and fLargeWork*Found. (Gregory Maxwell)
c63198f Make QT runawayException call GetWarnings instead of directly access strMiscWarning. (Gregory Maxwell)
813ede9 [qa] Update compactblocks test for multi-peer reconstruction (Suhas Daftuar)
7017298 Allow compactblock reconstruction when block is in flight (Suhas Daftuar)
Defers to pre-defined version if found (e.g. protobuf). For protobuf case, the definitions are identical and thus include order should not affect results.
c5c92c4 Update python tests for default tx version=2 (BtcDrak)
dab207e Preserve tx version=1 for certain tests (BtcDrak)
c5d746a tiny test fix for mempool_tests (Alex Morcos)
1f0ca1a Bump default transaction version to 2 (BtcDrak)
67dac4e Add unit tests for the CuckooCache (Jeremy Rubin)
c9e69fb Add CuckooCache implementation and replace the sigcache map_type with it (Jeremy Rubin)
SQUASHME: Change cuckoocache to only work for powers of two, to avoid mod operator
SQUASHME: Update Documentation and simplify logarithm logic
SQUASHME: OSX Build Errors
SQUASHME: minor Feedback from sipa + bluematt
SQUASHME: DOCONLY: Clarify a few comments.
This change removes a mapValue.erase("version") statement which deletes a
mapValue entry that never existed. The statement was mistakenly added in commit
865c3a2383 in 2010 and is harmless but confusing.
8225239 Merge #433: Make the libcrypto detection fail the newer API.
12de863 Make the libcrypto detection fail the newer API.
2928420 Merge #427: Remove Schnorr from travis as well
8eecc4a Remove Schnorr from travis as well
a8abae7 Merge #310: Add exhaustive test for group functions on a low-order subgroup
b4ceedf Add exhaustive test for verification
83836a9 Add exhaustive tests for group arithmetic, signing, and ecmult on a small group
20b8877 Add exhaustive test for group functions on a low-order subgroup
80773a6 Merge #425: Remove Schnorr experiment
e06e878 Remove Schnorr experiment
04c8ef3 Merge #407: Modify parameter order of internal functions to match API parameter order
6e06696 Merge #411: Remove guarantees about memcmp-ability
40c8d7e Merge #421: Update scalar_4x64_impl.h
a922365 Merge #422: Restructure nonce clearing
3769783 Restructure nonce clearing
0f9e69d Restructure nonce clearing
9d67afa Update scalar_4x64_impl.h
7d15cd7 Merge #413: fix auto-enabled static precompuatation
00c5d2e fix auto-enabled static precompuatation
91219a1 Remove guarantees about memcmp-ability
353c1bf Fix secp256k1_ge_set_table_gej_var parameter order
541b783 Fix secp256k1_ge_set_all_gej_var parameter order
7d893f4 Fix secp256k1_fe_inv_all_var parameter order
git-subtree-dir: src/secp256k1
git-subtree-split: 8225239f490f79842a5a3b82ad6cc8aa11d5208e
1a6eacb [QA] add fundrawtransaction test on a locked wallet with empty keypool (Jonas Schnelli)
c24a4f5 [Wallet] Bugfix: FRT: don't terminate when keypool is empty (Jonas Schnelli)
f36349e qt: Remove on_toggleNetworkActiveButton_clicked from RPCConsole (Wladimir J. van der Laan)
297cc20 qt: layoutAboutToChange signal is called layoutAboutToBeChanged (Wladimir J. van der Laan)
This resolves an issue where a wallet transaction which failed to
relay previously because it couldn't make it into the mempool
will not try again until restart, even though mempool conditions
may have changed.
Abandoned and known-conflicted transactions are skipped.
Some concern was expressed that there may be users with many
unknown conflicts would waste a lot of CPU time trying to
add them to their memory pools over and over again. But I am
doubtful these users exist in any number, if they do exist
they have worse problems, and they can mitigate any performance
issue this might have by abandoning the transactions in question.
This is another violation of the one definition rule, as the type
for mapOrphanTransactionsByPrev did not match the one in
net_processing.cpp anymore. As it now depends on a custom Iterator,
it seems too much hassle to correctly expose it to the tests.
Instead, this commit just removes the one test it was referenced in.
Add more comprehensive unit tests for CCoinsViewCache. Right now it is hard to
refactor caching code or fix bugs in the caching logic because you have to try
to mentally enumerate all the different states the cache might be in to make
sure a change doesn't cause unintended consequences. The new tests explicitly
enumerate relevant cache states, documenting and verifying the behavior in each
state, so it will be safer and easier to make changes to the caching code in
the future.
dd0df81 Document ConnectBlock connectTrace postconditions (Matt Corallo)
2d6e561 Switch pblock in ProcessNewBlock to a shared_ptr (Matt Corallo)
2736c44 Make the optional pblock in ActivateBestChain a shared_ptr (Matt Corallo)
ae4db44 Create a shared_ptr for the block we're connecting in ActivateBCS (Matt Corallo)
fd9d890 Keep blocks as shared_ptrs, instead of copying txn in ConnectTip (Matt Corallo)
6fdd43b Add struct to track block-connect-time-generated info for callbacks (Matt Corallo)
81e3228 Make CTransaction actually immutable (Pieter Wuille)
42fd8de Make DecodeHexTx return a CMutableTransaction (Pieter Wuille)
c3f5673 Make CWalletTx store a CTransactionRef instead of inheriting (Pieter Wuille)
a188353 Switch GetTransaction to returning a CTransactionRef (Pieter Wuille)
042f9fa qt: Show progress overlay when clicking spinner icon (Wladimir J. van der Laan)
827d9a3 qt: Replace NetworkToggleStatusBarControl with generic ClickableLabel (Wladimir J. van der Laan)
76faa3c Rename the remaining main.{h,cpp} to validation.{h,cpp} (Matt Corallo)
e736772 Move network-msg-processing code out of main to its own file (Matt Corallo)
87c35f5 Remove orphan state wipe from UnloadBlockIndex. (Matt Corallo)
Sorry for the churn on this, but the current message (introduced in #9073)
isn't acceptable:
$ src/bitcoin-cli getinfo
rpc: couldn't connect to server
(make sure server is running and you are connecting to the correct RPC port: -1 unknown)
Putting the error code after the words "RPC port" made me wonder whether
there was a port configuration issue.
This changes it to:
$ src/bitcoin-cli getinfo
error: couldn't connect to server: unknown (code -1)
(make sure server is running and you are connecting to the correct RPC port)
As orphan state is now "network state", like in
d6ea737be1,
UnloadBlockIndex is only used during init if we end up reindexing
to clear our block state so that we can start over. However, at
that time no connections have been brought up as CConnman hasn't
been started yet, so all of the network processing state logic is
empty when its called.
2c8c57e Document cs_main status when calling into PNB or PNBH (Matt Corallo)
58a215c Use ProcessNewBlockHeaders in CMPCTBLOCK processing (Matt Corallo)
a8b936d Use exposed ProcessNewBlockHeaders from ProcessMessages (Matt Corallo)
63fd101 Split ::HEADERS processing into two separate cs_main locks (Matt Corallo)
4a6b1f3 Expose AcceptBlockHeader through main.h (Matt Corallo)
This was misnamed, resulting in a warning message and missing
functionality. I'm not sure what the change in behavior will be here,
this needs testing.
Also remove connection to non-existing slot "test".
This was used for testing if the signal arrived. It is no
longer necessary.
Fixes:
2016-12-01 10:04:06 GUI: QObject::connect: No such signal PeerTableModel::layoutAboutToChange() in qt/rpcconsole.cpp:518
2016-12-01 10:04:06 GUI: QObject::connect: (receiver name: 'RPCConsole')
2016-12-01 10:04:06 GUI: QObject::connect: No such slot RPCConsole::test() in qt/rpcconsole.cpp:781
2016-12-01 10:04:06 GUI: QObject::connect: (receiver name: 'RPCConsole')
deec83f init: Get rid of fServer flag (Wladimir J. van der Laan)
16ca0bf init: Try to aquire datadir lock before and after daemonization (Wladimir J. van der Laan)
0cc8b6b init: Split up AppInit2 into multiple phases (Wladimir J. van der Laan)
Before daemonization, just probe the data directory lock and print an
early error message if possible.
After daemonization get the data directory lock again and hold on to it until exit
This creates a slight window for a race condition to happen, however this condition is harmless: it
will at most make us exit without printing a message to console.
$ src/bitcoind -testnet -daemon
Bitcoin server starting
$ src/bitcoind -testnet -daemon
Error: Cannot obtain a lock on data directory /home/orion/.bitcoin/testnet3. Bitcoin Core is probably already running.
When generating a new service key, explicitly request a RSA1024 one.
The bitcoin P2P protocol has no support for the longer hidden service names
that will come with ed25519 keys, until it does, we depend on the old
hidden service type so make this explicit.
See #9214.
Output instances of "BloomFilter" changed to "Bloom filter", in accordance with Wikipedia standard notation:
https://en.wikipedia.org/wiki/Bloom_filter
also to sync with the majority of cases in the self-same file
c7be56d net: push only raw data into CConnman (Cory Fields)
2ec935d net: add CVectorWriter and CNetMsgMaker (Cory Fields)
b7695c2 net: No need to check individually for disconnection anymore (Cory Fields)
fedea8a net: don't send any messages before handshake or after requested disconnect (Cory Fields)
d74e352 net: Set feelers to disconnect at the end of the version message (Cory Fields)
This fixes one of the last major layer violations in the networking stack.
The network side is no longer in charge of message serialization, so it is now
decoupled from Bitcoin structures. Only the header is serialized and attached
to the payload.
CVectorWriter is useful for overwriting or appending an existing byte vector.
CNetMsgMaker is a shortcut for creating messages on-the-fly which are suitable
for pushing to CConnman.
Bring up the modal progress overlay when the user clicks the spinner
icon in the task bar.
I think this is the intuitive thing to do when that icon is clicked.
ed998ea qt: Avoid OpenSSL certstore-related memory leak (Wladimir J. van der Laan)
5204598 qt: Avoid shutdownwindow-related memory leak (Wladimir J. van der Laan)
e4f126a qt: Avoid splash-screen related memory leak (Wladimir J. van der Laan)
693384e qt: Prevent thread/memory leak on exiting RPCConsole (Wladimir J. van der Laan)
47db075 qt: Plug many memory leaks (Wladimir J. van der Laan)
Also, send reject messages earlier in SendMessages(), so that disconnections are
processed earlier.
These changes combined should ensure that no message is ever sent after
fDisconnect is set.
This way we're not relying on messages going out after fDisconnect has been
set.
This should not cause any real behavioral changes, though feelers should
arguably disconnect earlier in the process. That can be addressed in a later
functional change.
d2b88f9 Move orphan-conflict removal from main logic into a callback (Matt Corallo)
97e2802 Erase orphans per-transaction instead of per-block (Matt Corallo)
ec4525c Move orphan processing to ActivateBestChain (Matt Corallo)
Make splash screen queue its own deletion when it receives the finished
command, instead of relying on WA_DeleteOnClose which doesn't work under
these circumstances.
Make ownership of the QThread object clear, so that the RPCConsole
can wait for the executor thread to quit before shutdown is called. This
increases overall thread safety, and prevents some objects from leaking
on exit.
None of these are very serious, and are leaks in objects that are
created at most one time.
In most cases this means properly using the QObject parent hierarchy,
except for BanTablePriv/PeerTablePriv which are not QObject,
so use a std::unique_ptr instead.
f004e67 Minor change to comment above new NODE_WITNESS service flag to keep it consitent with existing comment structure. Helps with readability. (Greg Walker)
Make sure that the count is a zero modulo the new mask before
scaling, otherwise the next time until a measure triggers
will take only 1/2 as long as accounted for. This caused
the 'min time' to be potentially off by as much as 100%.
When a BIP152 HB-mode peer is in the least preferred position and
disconnects, they will not be by ForNode on the next loop. They
will continue to sit in that position and prevent deactivating
HB mode for peers that are still connected.
There is no reason for them to stay in the list if already gone,
so drop the first element unconditionally if there are too many.
Fixes issue #9163.
037159c Remove block-request logic from INV message processing (Matt Corallo)
3451203 [qa] Respond to getheaders and do not assume a getdata on inv (Matt Corallo)
d768f15 [qa] Make comptool push blocks instead of relying on inv-fetch (mrbandrews)
- Do sorting for date, amount and confirmations column as longlong, not
unsigned longlong.
- Use `UserRole` to store our own data. This makes it treated as
ancillary data prevents it from being displayed.
- Get rid of `getMappedColumn` `strPad` - these are no longer necessary.
- Get rid of hidden `_INT64` columns.
- Start enumeration from 0 (otherwise values are undefined).
In 0.13 orphan transactions began being treated as implicit
INVs for their parents. But the resulting getdata were
not getting the witness flag.
This fixes issue #9182 reported by chjj and roasbeef on IRC.
Make a copy of the boost time-point to wait for, otherwise the head of
the queue may be deleted by another thread while this one is waiting,
while the boost function still has a reference to it.
Although this problem is in non-test code, this is not an actual problem
outside of the tests because we use the thread scheduler with only one
service thread, so there will never be threads fighting at the head of
the queue.
The old boost fallback escapes this problem because it passes a scalar
value to wait_until instead of a const object reference.
Found by running the tests in LLVM-4.0-master asan.
This further decouples "main" and "net" processing logic by moving
orphan processing out of the chain-connecting cs_main lock and
into its own cs_main lock, beside all of the other chain callbacks.
Once further decoupling of net and main processing logic occurs,
orphan handing should move to its own lock, out of cs_main.
Note that this will introduce a race if there are any cases where
we assume the orphan map to be consistent with the current chain
tip, however I am confident there is no such case (ATMP will fail
without DoS score in all such cases).
ae22357 Replace CValidationState param in ProcessNewBlock with BlockChecked (Matt Corallo)
7c98ce5 Remove pfrom parameter from ProcessNewBlock (Matt Corallo)
e2e069d Revert "RPC: Give more details when "generate" fails" (Matt Corallo)
dac53b5 Modify getblocktxn handler not to drop requests for old blocks (Russell Yanofsky)
55bfddc [qa] Fix stale data bug in test_compactblocks_not_at_tip (Russell Yanofsky)
47e9659 [qa] Fix bug in compactblocks v2 merge (Russell Yanofsky)
e2b3fb3 Optimize vInOutPoints insertion a bit (Matt Corallo)
eecffe5 Remove redundant duplicate-input check from CheckTransaction (Matt Corallo)
b2e178a Add deserialize + CheckBlock benchmarks, and a full block hex (Matt Corallo)
d59a518 Use fixed preallocation instead of costly GetSerializeSize (Pieter Wuille)
25a211a Add optimized CSizeComputer serializers (Pieter Wuille)
a2929a2 Make CSerAction's ForRead() constexpr (Pieter Wuille)
a603925 Avoid -Wshadow errors (Pieter Wuille)
5284721 Get rid of nType and nVersion (Pieter Wuille)
657e05a Make GetSerializeSize a wrapper on top of CSizeComputer (Pieter Wuille)
fad9b66 Make nType and nVersion private and sometimes const (Pieter Wuille)
c2c5d42 Make streams' read and write return void (Pieter Wuille)
50e8a9c Remove unused ReadVersion and WriteVersion (Pieter Wuille)
It is unexpected behavior for `ToString` to raise an exception. It
is expected to do a best-effort attempt at formatting but never fail.
Catch the exception and simply print unknown inv types as hexadecimal.
Fixes#9110.
4441018 Every main()/exit() should return/use one of EXIT_ codes instead of magic numbers (UdjinM6)
bd0de13 Fix exit codes: - `--help`, `--version` etc should exit with `0` i.e. no error ("not enough args" case should still trigger an error) - error reading config file should exit with `1` (UdjinM6)
d4833ff Bump the protocol version to distinguish new banning behavior. (Suhas Daftuar)
88c3549 Fix compact block handling to not ban if block is invalid (Suhas Daftuar)
c93beac [qa] Test that invalid compactblocks don't result in ban (Suhas Daftuar)
Dbwrapper used GetSerializeSize() to compute the size of the buffer
to preallocate. For some cases (specifically: CCoins) this requires
a costly compression call. Avoid this by just using fixed size
preallocations instead.
To get the advantages of faster GetSerializeSize() implementations
back that were removed in "Make GetSerializeSize a wrapper on top of
CSizeComputer", reintroduce them in the few places in the form of a
specialized Serialize() implementation. This actually gets us in a
better state than before, as these even get used when they're invoked
indirectly in the serialization of another object.
The CSerAction's ForRead() method does not depend on any runtime
data, so guarantee that requests to it can be optimized out by
making it constexpr.
Suggested by Cory Fields.
Remove the nType and nVersion as parameters to all serialization methods
and functions. There is only one place where it's read and has an impact
(in CAddress), and even there it does not impact any of the recursively
invoked serializers.
Instead, the few places that need nType or nVersion are changed to read
it directly from the stream object, through GetType() and GetVersion()
methods which are added to all stream classes.
Given that in default GetSerializeSize implementations created by
ADD_SERIALIZE_METHODS we're already using CSizeComputer(), get rid
of the specialized GetSerializeSize methods everywhere, and just use
CSizeComputer. This removes a lot of code which isn't actually used
anywhere.
For CCompactSize and CVarInt this actually removes a more efficient
size computing algorithm, which is brought back in a later commit.
The current getblocktxn implementation drops and ignores requests for old
blocks, which causes occasional sync_block timeouts during the
p2p-compactblocks.py test as reported in
https://github.com/bitcoin/bitcoin/issues/8842.
The p2p-compactblocks.py test setup creates many new blocks in a short
period of time, which can lead to getblocktxn requests for blocks below the
hardcoded depth limit of 10 blocks. This commit changes the getblocktxn
handler not to ignore these requests, so the peer nodes in the test setup
will reliably be able to sync.
The protocol change is documented in BIP-152 update "Allow block responses
to getblocktxn requests" at https://github.com/bitcoin/bips/pull/469.
The protocol change is not expected to affect nodes running outside the test
environment, because there shouldn't normally be lots of new blocks being
rapidly added that need to be synced.
The stream implementations had two cascading layers (the upper one
with operator<< and operator>>, and a lower one with read and write).
The lower layer's functions are never cascaded (nor should they, as
they should only be used from the higher layer), so make them return
void instead.
9027680 net: handle version push in InitializeNode (Cory Fields)
7588b85 net: construct CNodeStates in place (Cory Fields)
440f1d3 net: remove now-unused ssSend and Fuzz (Cory Fields)
5c2169c drop the optimistic write counter hack (Cory Fields)
ea33268 net: switch all callers to connman for pushing messages (Cory Fields)
3e32cd0 connman is in charge of pushing messages (Cory Fields)
b98c14c serialization: teach serializers variadics (Cory Fields)
Three categories of modifications:
1)
1 instance of 'The Bitcoin Core developers \n',
1 instance of 'the Bitcoin Core developers\n',
3 instances of 'Bitcoin Core Developers\n', and
12 instances of 'The Bitcoin developers\n'
are made uniform with the 443 instances of 'The Bitcoin Core developers\n'
2)
3 instances of 'BitPay, Inc\.\n' are made uniform with the other 6
instances of 'BitPay Inc\.\n'
3)
4 instances where there was no '(c)' between the 'Copyright' and the year
where it deviates from the style of the local directory.
- `--help`, `--version` etc should exit with `0` i.e. no error ("not enough args" case should still trigger an error)
- error reading config file should exit with `1`
Slightly refactor AppInitRPC/AppInitRawTx to return standard exit codes (EXIT_FAILURE/EXIT_SUCCESS) or CONTINUE_EXECUTION (-1)
The changes here are dense and subtle, but hopefully all is more explicit
than before.
- CConnman is now in charge of sending data rather than the nodes themselves.
This is necessary because many decisions need to be made with all nodes in
mind, and a model that requires the nodes calling up to their manager quickly
turns to spaghetti.
- The per-node-serializer (ssSend) has been replaced with a (quasi-)const
send-version. Since the send version for serialization can only change once
per connection, we now explicitly tag messages with INIT_PROTO_VERSION if
they are sent before the handshake. With this done, there's no need to lock
for access to nSendVersion.
Also, a new stream is used for each message, so there's no need to lock
during the serialization process.
- This takes care of accounting for optimistic sends, so the
nOptimisticBytesWritten hack can be removed.
- -dropmessagestest and -fuzzmessagestest have not been preserved, as I suspect
they haven't been used in years.
This allows future software that would relay compact blocks before
full validation to announce only to peers that will not ban if the
block turns out to be invalid.
f5b960b Move nTimeBestReceived updating into net processing code (Matt Corallo)
d8670fb Move all calls to CheckBlockIndex out of net-processing logic (Matt Corallo)
d6ea737 Remove network state wipe from UnloadBlockIndex. (Matt Corallo)
fc0c24f Move MarkBlockAsReceived out of ProcessNewMessage (Matt Corallo)
65f35eb Move FlushStateToDisk call out of ProcessMessages::TX into ATMP (Matt Corallo)
59ac5c5 net: Use deterministic randomness for CNode's nonce, and make it const (Cory Fields)
aff6584 net: constify a few CNode vars to indicate that they're threadsafe (Cory Fields)
Check for unreasonable alloc size in LockedPool rather than lancing through new
Arenas until we improbably find one worthy of the quixotic request or the system
can support no more Arenas.
- Use the python standard logging library
- Run all tests and report all failing test-cases (rather than stop after one test case fails)
- If output is different from expected output, log a contextual diff.
Refer to the right file in the top-level README.md.
Having only one file with test documentation saves some confusion about
where things are documented.
444c673 bench: Add benchmark for lockedpool allocation/deallocation (Wladimir J. van der Laan)
6567999 rpc: Add `getmemoryinfo` call (Wladimir J. van der Laan)
4536148 support: Add LockedPool (Wladimir J. van der Laan)
f4d1fc2 wallet: Get rid of LockObject and UnlockObject calls in key.h (Wladimir J. van der Laan)
999e4c9 wallet: Change CCrypter to use vectors with secure allocator (Wladimir J. van der Laan)
GetTotalBlocksEstimate is no longer used and it was the only thing
the checkpoint tests were testing.
Since checkpoints are on their way out it makes more sense to remove
the test file than to cook up a new pointless test.
This introduces a 'minimum chain work' chainparam which is intended
to be the known amount of work in the chain for the network at the
time of software release. If you don't have this much work, you're
not yet caught up.
This is used instead of the count of blocks test from checkpoints.
This criteria is trivial to keep updated as there is no element of
subjectivity, trust, or position dependence to it. It is also a more
reliable metric of sync status than a block count.
Fixes newly initialized bloom filters being
constructed with isEmpty(false), which still
works but loses the possible speedup when
checking for key membership in an empty filter.
This will result in many more calls to CheckBlockIndex when
connecting a list of headers (eg in ::HEADERS messages processing)
but its only enabled in debug mode, and that should mostly just be
during IBD, so it should be OK.
UnloadBlockIndex is only used during init if we end up reindexing
to clear our block state so that we can start over. However, at
that time no connections have been brought up as CConnman hasn't
been started yet, so all of the network processing state logic is
empty when its called.
Additionally, the initialization of the recentRejects set is moved
to InitPeerLogic.
This splits the output comparison for `bitcoin-tx` into two steps:
- First, check for data mismatch, parsing the data as json or hex
depending on the extension of the output file
- Then, check if the literal string matches
For either of these cases give a different error.
This prevents wild goose chases when e.g. a trailing space doesn't match
exactly, and makes sure that both test output and examples are valid
data of the purported format.
Recent discussion (in IRC meetings, and e.g. #8989) has shown a
preference for the default confirm target for smartfees to be 6 instead
of 2, to avoid overpaying fees for questionable gain.
6 is also a compromise between the GUI's pre-#8989 value of 25 and the
bitcoind `-txconfirmtarget` default of 2. These were unified in #8989,
but this has made the (overly expensive) default of 2 as GUI default.
```
getmemoryinfo
Returns an object containing information about memory usage.
Result:
{
"locked": { (json object) Information about locked memory manager
"used": xxxxx, (numeric) Number of bytes used
"free": xxxxx, (numeric) Number of bytes available in current arenas
"total": xxxxxxx, (numeric) Total number of bytes managed
"locked": xxxxxx, (numeric) Amount of bytes that succeeded locking. If this number is smaller than total, locking pages failed at some point and key data could be swapped to disk.
}
}
Examples:
> bitcoin-cli getmemoryinfo
> curl --user myusername --data-binary '{"jsonrpc": "1.0", "id":"curltest", "method": "getmemoryinfo", "params": [] }' -H 'content-type: text/plain;' http://127.0.0.1:8332/
```
Add a pool for locked memory chunks, replacing LockedPageManager.
This is something I've been wanting to do for a long time. The current
approach of locking objects where they happen to be on the stack or heap
in-place causes a lot of mlock/munlock system call overhead, slowing
down any handling of keys.
Also locked memory is a limited resource on many operating systems (and
using a lot of it bogs down the system), so the previous approach of
locking every page that may contain any key information (but also other
information) is wasteful.
This change is needed to prevent sync_blocks timeouts in the mempool_reorg
test after the sync_blocks update in the upcoming commit
"[qa] Change sync_blocks to pick smarter maxheight".
This change was initially suggested by Suhas Daftuar <sdaftuar@chaincode.com>
in https://github.com/bitcoin/bitcoin/pull/8680#r78209060
0334430 Add some missing includes (Pieter Wuille)
4100499 Return shared_ptr<CTransaction> from mempool removes (Pieter Wuille)
51f2783 Make removed and conflicted arguments optional to remove (Pieter Wuille)
f48211b Bypass removeRecursive in removeForReorg (Pieter Wuille)
nMaxInbound might very well be 0 or -1, if the user prefers to keep
a small number of maxconnections.
Note: nMaxInbound of -1 means that the user set maxconnections
to 8 or less, but we still want to keep an additional slot for
the feeler connection.
Add getNetworkActive()/setNetworkActive() method to client model.
Send network active status through NotifyNetworkActiveChanged.
Indicate in tool tip of gui status bar network indicator whether network activity is disabled.
Indicate in debug window whether network activity is disabled and add button to allow user to toggle network activity state.
Added the function SetNetworkActive() which when called with argument set to false disconnects all nodes and sets the flag fNetworkActive to false. As long as this flag is false no new connections are attempted and no incoming connections are accepted. Network activity is reenabled by calling the function with argument true.
