The current logic for syncing headers may lead to lots of duplicate
getheaders requests being sent: If a new block arrives while the node
is in headers sync, it will send getheaders in response to the block
announcement. When the headers arrive, the message will be of maximum
size and so a follow-up request will be sent---all of that in addition
to the existing headers syncing. This will create a second "chain" of
getheaders requests. If more blocks arrive, this may even lead to
arbitrarily many parallel chains of redundant requests.
This patch changes the behaviour to only request more headers after a
maximum-sized message when it contained at least one unknown header.
This avoids sustaining parallel chains of redundant requests.
Note that this patch avoids the issues raised in the discussion of
https://github.com/bitcoin/bitcoin/pull/6821: There is no risk of the
node being permanently blocked. At the latest when a new block arrives
this will trigger a new getheaders request and restart syncing.
Verify that results correct (match known values), consistent (encrypt->decrypt
matches the original), and compatible with the previous openssl implementation.
Also check that failed encrypts/decrypts fail the exact same way as openssl.
Wallet must come before crypto, otherwise linking fails on some platforms.
Includes a tangentially-related general cleanup rather than making the Makefile
sloppier.
The output should always match openssl's, even for failed operations. Even for
a decrypt with broken padding, the output is always deterministic (and attemtps
to be constant-time).
My changes leave all tray icon and menu creation/initialization logic
untouched. It only shows or hides the icon according to the setting.
A new checkbox was added to the OptionsDialog under the Window tab. A
bool option named "hideTrayIcon" was added to OptionsModel. This
checkbox was mapped like other all options to the OptionsModel.
A signal was added to the OptionsModel for broadcasting changes the the
hideTrayIcon option. This signal was connected to a new slot added to
BitcoinGUI named setTrayIconVisible(bool). The slot simply hides or
shows the trayIcon in BitcoinGUI according to the parameter recieved.
Changing NOP3 op name to OP_CHECKSEQUENCEVERIFY, renaming instances of OP_NOP3 in script_tests.json to CHECKSEQUENCEVERIFY
Cleaning up NOP3 comment
Re-adding test cases that were accidentally deleted, removing dupicated test case, fixing formatting
Removing re-labeling of OP_NOP3 to OP_CSV
Fixing whitespace issues
- Ban/Unban/ClearBan call uiInterface.BannedListChanged() as necessary
- Ban/Unban/ClearBan sync to disk if the operation is user-invoked
- Mark node for disconnection automatically when banning
- Lock cs_vNodes while setting disconnected
- Don't spin in a tight loop while setting disconnected
Move the version reporting to Wallet::Verify, before starting
verification of the wallet.
This removes the dependency of init on a specific wallet database
library.
A further, trivial step towards resolving #7965.
166e4b0 Notify other serviceQueue thread we are finished to prevent deadlocks. (Pavel Janík)
db18ab2 Reenable multithread scheduler test. (Pavel Janík)
I made a subclass of QMessageBox that disables the send button in
exec() and starts a timer that calls a slot to re-enable it after a
configurable delay.
It also has a countdown in the send/yes button while it is disabled
to hint to the user why the send button is disabled (and that it is
actually supposed to be disabled).
* The "ERROR" was printed far too often during normal operation for what was not an error.
* Makes the Socks5() connect failure similar to the IP connect failure in debug.log.
Before:
`2016-05-09 00:15:00 ERROR: Proxy error: host unreachable`
After:
`2016-05-09 00:15:00 Socks5() connect to t6xj6wilh4ytvcs7.onion:18333 failed: host unreachable"`
Locking for each operation here is unnecessary, and solves the wrong problem.
Additionally, it introduces a problem when cs_vNodes is held in an owning
class, to which invididual CNodeRefs won't have access.
These should be weak pointers anyway, once vNodes contain shared pointers.
Rather than using a refcounting class, use a 3-step process instead.
1. Lock vNodes long enough to snapshot the fields necessary for comparing
2. Unlock and do the comparison
3. Re-lock and mark the resulting node for disconnection if it still exists
d1d7775 Improve worst-case behavior of CScript::FindAndDelete (Patrick Strateman)
e2a30bc Unit test for CScript::FindAndDelete (Gavin Andresen)
c0f660c Replace c-style cast with c++ style static_cast. (Patrick Strateman)
ec9ad5f Replace memcmp with std::equal in CScript::FindAndDelete (Patrick Strateman)
b559914 Move bloom and feerate filtering to just prior to tx sending. (Gregory Maxwell)
4578215 Return mempool queries in dependency order (Pieter Wuille)
ed70683 Handle mempool requests in send loop, subject to trickle (Pieter Wuille)
dc13dcd Split up and optimize transaction and block inv queues (Pieter Wuille)
f2d3ba7 Eliminate TX trickle bypass, sort TX invs for privacy and priority. (Gregory Maxwell)
f0188f9 http: use std::move to move HTTPRequest into HTTPWorkItem (Wladimir J. van der Laan)
37b2137 http: Change boost::scoped_ptr to std::unique_ptr in HTTPRequest (Wladimir J. van der Laan)
f97b410 http: Add log message when work queue is full (Wladimir J. van der Laan)
091d6e0 http: Do a pending c++11 simplification (Wladimir J. van der Laan)
Use std::unique_ptr for handling work items.
This makes the code more RAII and, as mentioned in the comment, is what
I planned when I wrote the code in the first place.
08d7b56 util: switch LogPrint and error to variadic templates (Wladimir J. van der Laan)
9eaa0af tinyformat: force USE_VARIADIC_TEMPLATES (Wladimir J. van der Laan)
* CAddrDB modified so that when de-serialization code throws an exception Addrman is reset to a clean state
* CAddrDB modified to make unit tests possible
* Regression test created to ensure bug is fixed
* StartNode modifed to clear adrman if CAddrDB::Read returns an error code.
Some developers clearly don't get this and have been posting
"improvements" that create clear vulnerabilities. It should
have been better explained in the code, since the design
is somewhat subtle and getting it right is important.
This patch changes the implementation from one that stores 16 2-bit integers
in one uint32_t's, to one that stores the first bit of 64 2-bit integers in
one uint64_t and the second bit in another. This allows for 450x faster
refreshing and 2.2x faster average speed.
Change the few occurrences of the deprecated `auto_ptr` to c++11 `unique_ptr`.
Silences the deprecation warnings.
Also add a missing `std::` for consistency.
Bitwise logic combined with `<` with undefined signedness will
potentially results in undefined behavior. Fix this by defining the type
as a c++11 typed enum.
Fixes#6017.
f135e3c qt: Add transaction hash to details window title (Wladimir J. van der Laan)
17a6a21 qt: Make it possible to show details for multiple transactions (Wladimir J. van der Laan)
DumpBanList currently does this:
- with lock: take a copy of the banmap
- perform I/O (write out the banmap)
- with lock: mark the banmap non-dirty
If a new ban is added during the I/O operation, it may never be persisted to
disk.
Reorder operations so that the data to be persisted cannot be older than the
time at which the banmap was marked non-dirty.
I made a silly mistake in a database wrapper where keys
were sorted by char instead of uint8_t. As x86 char is signed
the sorting for the block index database was messed up, resulting
in a segfault due to missing records.
Add a test to catch:
- Wrong sorting
- Seeking errors
- Iteration result not complete
ActivateBestChain uses chainActive after releasing the lock; reorder operations
to move all access to synchronized object into existing LOCK(cs_main) block.
A small GUI annoyance for me has always been that it's impossible to
have multiple transaction detail windows open, for example to compare
transactions.
This patch makes the window non-modal so that it is possible to open
transaction details at will.
869cf12 dbwrapper: Move `HandleError` to `dbwrapper_private` (Wladimir J. van der Laan)
b69836d dbwrapper: Pass parent CDBWrapper into CDBBatch and CDBIterator (Wladimir J. van der Laan)
878bf48 dbwrapper: Remove CDBWrapper::GetObfuscateKeyHex (Wladimir J. van der Laan)
74f7b12 dbwrapper: Remove throw keywords in function signatures (Wladimir J. van der Laan)
Pass parent wrapper directly instead of obfuscation key. This
makes it possible for other databases which re-use this code
to use other properties from the database.
Add a namespace dbwrapper_private for private functions to be used
only in dbwrapper.h/cpp and dbwrapper_tests.
Using throw() specifications in function signatures is not only
not required in C++, it is considered deprecated for
[various reasons](https://stackoverflow.com/questions/1055387/throw-keyword-in-functions-signature).
It is not implemented by any of the common C++ compilers. The usage is
also inconsistent with the rest of the source code.
Without this patch:
- When I compile the GUI from the bitcoin directory itself, it works as
expected.
- When I build the GUI in an out-of-tree build, I cannot get it to
select tabs. When I click, say the "Receive" tab nothing happens,
the button selects but it doesn't switch the page. The rest - even
the debug window - seems to work.
See full discussion here:
https://github.com/bitcoin/bitcoin/pull/7911#issuecomment-212413442
This turned out to be caused by a mismatch in the arguments to moc,
preventing it from finding `bitcoin-config.h`. Fix this by passing
`$(DEFAULT_INCLUDES)` to it, which gets set to the appropriate
path by autoconf itself.
SetString seems to be passing the length of the wrong variable to
memory_cleanse, resulting in the last byte of the temporary buffer not being
securely erased.
This will avoid sending more pointless INVs around updates, and
prevents using filter updates to timetag transactions.
Also adds locking for fRelayTxes.
By eliminating queued entries from the mempool response and responding only at
trickle time, this makes the mempool no longer leak transaction arrival order
information (as the mempool itself is also sorted)-- at least no more than
relay itself leaks it.
Previously we would assert that if every block in vBlockHashesToAnnounce is in
chainActive, then the blocks to be announced must connect. However, there are
edge cases where this assumption could be violated (eg using invalidateblock /
reconsiderblock), so just check for this case and revert to inv-announcement
instead.
Rather than allowing CNetAddr/CService/CSubNet to launch DNS queries, require
that addresses are already resolved.
This greatly simplifies async resolve logic, and makes it harder to
accidentally leak DNS queries.
Note: Some seeds aren't actually returning an IP for their name entries, so
they're being added to addrman with a source of [::].
This commit shouldn't change that behavior, for better or worse.
Previously Bitcoin would send 1/4 of transactions out to all peers
instantly. This causes high overhead because it makes >80% of
INVs size 1. Doing so harms privacy, because it limits the
amount of source obscurity a transaction can receive.
These randomized broadcasts also disobeyed transaction dependencies
and required use of the orphan pool. Because the orphan pool is
so small this leads to poor propagation for dependent transactions.
When the bypass wasn't in effect, transactions were sent in the
order they were received. This avoided creating orphans but
undermines privacy fairly significantly.
This commit:
Eliminates the bypass. The bypass is replaced by halving the
average delay for outbound peers.
Sorts candidate transactions for INV by their topological
depth then by their feerate (then hash); removing the
information leakage and providing priority service to
higher fee transactions.
Limits the amount of transactions sent in a single INV to
7tx/sec (and twice that for outbound); this limits the
harm of low fee transaction floods, gives faster relay
service to higher fee transactions. The 7 sounds lower
than it really is because received advertisements need
not be sent, and because the aggregate rate is multipled
by the number of peers.
leveldb's buildsystem causes us a few problems:
- breaks out-of-tree builds
- forces flags used for some tools
- limits cross builds
Rather than continuing to add wrappers around it, simply integrate it into our
build.
b30fb42 test: Rename wallet.dat to wallet_test.dat (Wladimir J. van der Laan)
a25a4f5 wallet_ismine.h → script/ismine.h (Wladimir J. van der Laan)
f4eae2d test: Create test fixture for wallet (Wladimir J. van der Laan)
de39c95 test: move accounting_tests and rpc_wallet_tests to wallet/test (Wladimir J. van der Laan)
28b400f doc: update release-notes for `gettxoutsetinfo` change (Wladimir J. van der Laan)
76212bb rpc: make sure `gettxoutsetinfo` hash has txids (Wladimir J. van der Laan)
9ad1a51 crypto: bytes counts are 64 bit (Wladimir J. van der Laan)
Remove the mistaken assumption that GetKey returning false signifies
an internal database issue. It will return false when the key cannot
be deserialized into the (char,uint256) stanza, which indicates
that the cursor has reached a different kind of key.
Fixes bug #7890 introduced in #7756.
Without the newline I see "bein" where the two lines are concatenated:
Note that all inputs selected must be of standard form and P2SH scripts must *bein* the wallet using importaddress or addmultisigaddress (to calculate fees).
swap was using an incorrect condition to determine when to apply an optimization
(not swapping the full direct[] when swapping two indirect prevectors).
Rather than correct the optimization I'm removing it for simplicity. Removing
this optimization minutely improves performance in the typical (currently only)
usage of member swap(), which is swapping with a freshly value-initialized
object.
Fixes a bug in which pop_back did not call the deleted item's destructor.
Using the most general erase() implementation to implement all the others
prevents similar bugs because the coupling between deallocation and destructor
invocation only needs to be maintained in one place.
Also reduces duplication of complex memmove logic.
The key (transaction id for the following outputs) should be serialized
to the HashWriter.
This is a problem as it means different transactions in the same
position with the same outputs will potentially result in the same hash.
Fixes primary concern of #7758.
Break the circular dependency between main and txdb by:
- Moving `CBlockFileInfo` from `main.h` to `chain.h`. I think this makes
sense, as the other block-file stuff is there too.
- Moving `CDiskTxPos` from `main.h` to `txdb.h`. This type seems
specific to txdb.
- Pass a functor `insertBlockIndex` to `LoadBlockIndexGuts`. This leaves
it up to the caller how to insert block indices.
Byte counts for SHA256, SHA512, SHA1 and RIPEMD160 must be 64 bits.
`size_t` has a different size per platform, causing divergent results
when hashing more than 4GB of data.
Add a method Cursor() to CCoinsView that returns a cursor which can be
used to iterate over the whole UTXO set.
- rpc: Change gettxoutsetinfo to use new Cursor method
- txdb: Remove GetStats method - Now that GetStats is implemented in
terms of Cursor, remove it.
dde46d3 Merge script_valid and script_invalid tests (Pieter Wuille)
009b503 Get rid of expect in script_tests as it's implied by scripterror (Pieter Wuille)
76da761 Make script_error a mandatory 4th field for script_tests (Pieter Wuille)
269281b Fix some misconstructed tests (Pieter Wuille)
d03e466 Fix formatting of NOPs for generated script tests (Pieter Wuille)
c7c6641 Fix JSON pretty printing in script_tests (Pieter Wuille)
d12760b rpc-tests: handle KeyError nicely in test_framework.py (Rusty Russell)
85c807c getblockchaininfo: make bip9_softforks an object, not an array. (Rusty Russell)
We can't change "softforks", but it seems far more logical to use tags
in an object rather than using an "id" field in an array.
For example, to get the csv status before, you need to iterate the
array to find the entry with 'id' field equal to "csv":
jq '.bip9_softforks | map(select(.id == "csv"))[] | .status'
Now:
jq '.bip9_softforks.csv.status'
There is no issue with fork names being incompatible with JSON tags,
since we're selecting them ourselves.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Previously we used the CInv that would be sent to the peer announcing the
transaction as the key, but using the txid instead allows us to decouple the
p2p layer from the application logic (which relies on this map to avoid
duplicate tx requests).
The lockorder potential deadlock detection works by remembering for each
lock A that is acquired while holding another B the pair (A,B), and
triggering a warning when (B,A) already exists in the table.
A and B in the above text are represented by pointers to the CCriticalSection
object that is acquired. This does mean however that we need to clean up the
table entries that refer to any critical section which is destroyed, as it
memory address can potentially be used for another unrelated lock in the future.
Implement this clean up by remembering not only the pairs in forward direction,
but also backward direction. This allows for fast iteration over all pairs that
use a deleted CCriticalSection in either the first or the second position.
The current tests for varint only check that
serialization-deserialization is a roundtrip. That is a useful test, but
it is also good to check for some exact bit patterns, to prevent a code
change that changes the serialization format from going undetected.
As the varint functions are templated, also check with different types.
da5fdbb Test relay of version 2 transactions (Suhas Daftuar)
5cb1d8a Tests: move get_bip9_status to util.py (Suhas Daftuar)
e4ba9f6 Version 2 transactions remain non-standard until CSV activates (Suhas Daftuar)
Currently, we're keeping a timeout for each requested block, starting
from when it is requested, with a correction factor for the number of
blocks in the queue.
That's unnecessarily complicated and inaccurate.
As peers process block requests in order, we can make the timeout for each
block start counting only when all previous ones have been received, and
have a correction based on the number of peers, rather than the total number
of blocks.
Two-line patch to make it possible to shut down bitcoind cleanly during
the initial ActivateBestChain.
Fixes#6459 (among other complaints).
To reproduce:
- shutdown bitcoind
- copy chainstate
- start bitcoind
- let the chain sync a bit
- shutdown bitcoind
- copy back old chainstate
- start bitcoind
- bitcoind will catch up with all blocks during Init()
(the `boost::this_thread::interruption_point` / `ShutdownRequested()`
dance is ugly, this should be refactored all over bitcoind at some point
when moving from boost::threads to c++11 threads, but it works...)
They claimed to be testing P2SH scripts with non-push scriptSigs, but
1) they were not enabling P2SH
2) they have push-only scriptSigs
Fix this, and add a few more related cases.
Split out methods to every module, apart from 'help' and 'stop' which
are implemented in rpcserver.cpp itself.
- This makes it easier to add or remove RPC commands - no longer everything that includes
rpcserver.h has to be rebuilt when there's a change there.
- Cleans up `rpc/server.h` by getting rid of the huge cluttered list of function definitions.
- Removes most of the bitcoin-specific code from rpcserver.cpp and .h.
Continues #7307 for the non-wallet.
Ubuntu 16.04 "xenial xerus" does not come with Python 2.x by default.
It is possible to install a python-2.7 package, but this has its own
problem: no `python` or `python2` symlink (see #7717).
This fixes the following scripts to work with python 3:
- `make check` (bctest,py, bitcoin-util-test.py)
- `make translate` (extract_strings_qt.py)
- `make symbols-check` (symbol-check.py)
- `make security-check` (security-check.py)
Explicitly call the python commands using $(PYTHON) instead
of relying on the interpreter line at the top of the scripts.
The build date does only makes sense for custom/self-compiled bitcoin-core versions because we are using static build-dates for our deterministic release builds.
Having a quick option to get the current datadir is much more valuable for debug purposes.
The "feefilter" p2p message is used to inform other nodes of your mempool min fee which is the feerate that any new transaction must meet to be accepted to your mempool. This will allow them to filter invs to you according to this feerate.
Creates the generatetoaddress rpc which is virtually identical to the generate rpc except that it takes an argument for the address to mine to. It does not rely on wallet functionality.
The mining code shared by generate and generatetoaddress has been moved to another method to reduce duplication.
fad8cfb [qa] mininode: Add and use CONSTs (MarcoFalke)
fa8cd46 [qa] Move create_tx() to util.py (MarcoFalke)
fad7dc8 [qa] wallet: speed up tests (MarcoFalke)
fa3a81a [tests] Extend util_ParseMoney test case (MarcoFalke)
This implements caching of ancestor state to each mempool entry, similar to
descendant tracking, but also including caching sigops-with-ancestors (as that
metric will be helpful to future code that implements better transaction
selection in CreatenewBlock).
The work limit served to prevent the descendant walking algorithm from doing
too much work by marking the parent transaction as dirty. However to implement
ancestor tracking, it's not possible to similarly mark those descendant
transactions as dirty without having to calculate them to begin with.
This commit removes the work limit altogether. With appropriate
chain limits (-limitdescendantcount) the concern about doing too much
work inside this function should be mitigated.
While trying to find a black/white version of the Bitcoin
logo for the organization I noticed the about.png is not
entirely black - it has some reflection. Remove this to make
it the same as other icons.
Also ran the icons through `contrib/devtools/optimize-pngs.py`,
so `chevron.png` was optimized too.
0ecb340 test: Script_error checking in script_invalid tests (Wladimir J. van der Laan)
2317ad7 test: Re-introduce JSON pretty printing in test builder (Wladimir J. van der Laan)
b0ff857 test: Move non-generated script_invalid test to the correct place (Wladimir J. van der Laan)
It looks like, TorController::disconnected_cb(TorControlConnection&
conn) gets called multiple times which results in multiple event_new().
Avoid this by creating the event only once in the constructore, and
deleting it only once in the destructor (thanks to Cory Fields for the
idea).
Replaces the fix by Jonas Schnelli in #7610, see discussion there.
