-
Notifications
You must be signed in to change notification settings - Fork 50
Sync with the main repo #131
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
+2,026
−1,328
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, we would check to see if we were in IBD and ignore getheaders requests accordingly. However, the IBD criteria -- an optimization mostly targeted at behavior when we have peers serving us many blocks we need to download -- is difficult to reason about in edge-case scenarios, such as if the network were to go a long time without any blocks found and nodes getting restarted during that time. To make things simpler to reason about, just use nMinimumChainWork as our anti-DoS threshold; as long as our chain has that much work, it should be fine to respond to a peer asking for our headers (and this should allow such a peer to request blocks from us if needed).
Expect responses to a getheaders iff the node has a chain with more than nMinimumChainWork
Co-authored-by: Pieter Wuille <[email protected]>
The 'Fragment' type was previously named 'Nodetype'. For clarity, name the variables the same. -BEGIN VERIFY SCRIPT- sed -i 's/nodetype/fragment/g' src/script/miniscript.* -END VERIFY SCRIPT- Co-authored-by: Pieter Wuille <[email protected]>
Co-authored-by: Pieter Wuille <[email protected]>
Co-authored-by: Pieter Wuille <[email protected]>
This helps to have finer-grained descriptor parsing errors.
This makes IsSane clearer. It is useful to differentiate between 'potential non-malleable satisfactions are valid' and 'such satisfactions exist' for testing. Co-authored-by: Pieter Wuille <[email protected]>
Co-authored-by: Pieter Wuille <[email protected]>
Parse also key hashes using the Key type. Make this target the first of the 4 Miniscript fuzz targets in a single `miniscript` file. Co-authored-by: Pieter Wuille <[email protected]>
Co-authored-by: Pieter Wuille <[email protected]>
…ddr cache randomizer
When in IBD, if the honest chain is only known by inbound peers, then we must eventually sync from them in order to learn it. This change allows us to perform initial headers sync and fetch blocks from inbound peers, if we have no blocks in flight. The restriction on having no blocks in flight means that we will naturally throttle our block downloads to any such inbound peers that we may be downloading from, until we leave IBD. This is a tradeoff between preferring outbound peers for most of our block download, versus making sure we always eventually will get blocks we need that are only known by inbound peers even during IBD, as otherwise we may be stuck in IBD indefinitely (which could have cascading failure on the network, if a large fraction of the network managed to get stuck in IBD).
Use of this macro enables precondition checks for iterators and functions of the library. It's use is recommended in debug builds. See: https://www.boost.org/doc/libs/1_78_0/libs/multi_index/doc/tutorial/debug.html.
This test can now be run even with the Bitcoin Core wallet disabled.
-BEGIN VERIFY SCRIPT- sed -i -E -e '/^([a-z]+ )?Mutex [a-z]/ s/Mutex/GlobalMutex/' $(git grep -lE '^([a-z]+ )?Mutex [a-z]') -END VERIFY SCRIPT-
…section in dev notes that was added in 2015 by commit b8c06ef in PR 7003, as that potential issue would now be caught by the test/lint/lint-format-strings.py script run by the CI
fa243e9 Remove no-op TIME_INIT on deser (MarcoFalke) Pull request description: Split out from bitcoin/bitcoin#24697 ACKs for top commit: laanwj: ACK fa243e9 fanquake: ACK fa243e9 Tree-SHA512: 3b92578a291279d04ac1b274807a6e4ee7a342e3527cc03d90223a1dbc4961668ddb572e40aff85171600a5a3cb2572188c0d75f757a3db8a441c1103eb66e84
…efault mempool 687adda test: add BIP-125 rule 5 testcase with default mempool (James O'Beirne) 6120e8e test: allow passing sequence through create_self_transfer_multi (James O'Beirne) Pull request description: Currently, we only test rule 5 of BIP-125 (replacement transactions cannot evict more than 100 transactions) by changing default mempool parameters to allow for more descendants. The current test works on a single transaction graph that has over 100 descendants. This patch adds a test to exercise rule 5 using the default mempool parameters. The case is a little more sophisticated: instead of working on a single transaction graph, it uses a replacement transaction to "unite" several UTXOs which join independent transaction graphs. The total number of transactions in these graphs sum to more than the max allowable replacement. I think the difference in transaction topology makes this a worthwhile testcase to have, setting aside the fact that this testcase works without having to use atypical mempool params. See also: [relevant discussion from IRC](https://www.erisian.com.au/bitcoin-core-dev/log-2022-05-27.html#l-126) ACKs for top commit: laanwj: Code review ACK 687adda LarryRuane: ACK 687adda Tree-SHA512: e589aeaf9d6f137d546b7809f8795d6f6043d87b15e97c2efe85b42ce8b49d977ee7d79440c542ca4b0b5ca2de527488029841a1ffc0d96c5771897df4b3f324
…a wtx lookup and add exception for db write error 57fb37c wallet: CommitTransaction, remove extra wtx lookup and add exception for a possible db write error. (furszy) Pull request description: Two points for `CWallet::CommitTransaction`: 1) The extra wtx lookup: As we are calling to `AddToWallet` first, which returns the recently added/updated wtx pointer, there is no need to look up the wtx again few lines later. We can just use it. 2) The db write error: `AddToWallet` can only return a nullptr if the db write fails, which inside `CommitTransaction` translates to an exception throw cause. We expect everywhere that `CommitTransaction` always succeed. ------------------------------------------------ Extra note: This finding generated another working path for me :) It starts with the following question: why are we returning a nullptr from `AddToWallet` if the db write failed without removing the recently added transaction from the wallet's map?.. Can led to a wallet invalid state where the inputs of this new transaction are not marked dirty, while the transaction that spends them still exist on the in-memory wallet tx map. -- I'm writing it here to gather some feedback first and not forget it, will create a follow-up PR in the coming days 🚜 -- ACKs for top commit: achow101: ACK 57fb37c jonatack: ACK 57fb37c ryanofsky: Code review ACK 57fb37c. Seems like a clear improvement. Better to fail earlier with a better error message if the failure is going to happen anyway Tree-SHA512: 80e59c01852cfbbc70a5de1a1c2c59b5e572f9eaa08c2175112cb515256e63fa04c7942f92a513b620d6b06e66392029ebe8902287c456efdbee58a7a5ae42da
…, drop LogPrint-vs-LogPrintf section in dev notes 433b525 Add LogPrintLevel to lint-format-strings, drop LogPrint-vs-LogPrintf section in dev notes (Jon Atack) Pull request description: added by #7003 in 2015, as that potential issue would now be caught by the `test/lint/lint-format-strings.py` script run by the CI. ACKs for top commit: MarcoFalke: cr ACK 433b525 w0xlt: ACK bitcoin/bitcoin@433b525 Tree-SHA512: 91a2ac76689ed4f1f638e07c16d2ec8952fb013cc8bb896780fbd9333abd084281ce99afdc9de715d07a9abb4dce5dd67edf5e347aff466c6ef339ccc4158679
b1f662b doc: Fix command in "OpenBSD Build Guide" (Hennadii Stepanov) Pull request description: Fixed `pkg_add sqlite3` command. ACKs for top commit: theStack: ACK b1f662b Tree-SHA512: b1dd1baa238f76dadfb188b46bc72f993cc88ea4651cf0836cd85348429baa15228e9cd4c15e588675c9f340692118952302a8629f45d7dc275cc086917c11ca
nTime is always initialized on deserialization or default-initialized with TIME_INIT, so special casing 0 does not make sense.
292828c [test] Test addr cache for multiple onion binds (dergoegge) 3382905 [net] Seed addr cache randomizer with port from binding address (dergoegge) f10e80b [net] Use ConnectedThroughNetwork() instead of GetNetwork() to seed addr cache randomizer (dergoegge) Pull request description: The addr cache id randomizer is currently supposed to be seeded with the network of the inbound connection and the local socket (only the address is used not the port): https://github.com/bitcoin/bitcoin/blob/a8098f2cef53ec003edae91100afce564e9c6f23/src/net.cpp#L2800-L2804 For inbound onion connections `CNode::addr.GetNetwork()` returns `NET_UNROUTABLE` and `CNode::addrBind` is set to `127.0.0.1:<onion bind port>`. This results in the same addr cache for all inbound connections on 127.0.0.1 binds. To avoid the same addr cache across all onion and other 127.0.0.1 binds, we should seed the addr cache randomizer with the correct network for inbound onion connections (using `CNode::ConnectedThroughNetwork()`) as well as the port of `CNode::addrBind`. ACKs for top commit: sipa: utACK 292828c mzumsande: Code Review ACK 292828c naumenkogs: utACK 292828c Tree-SHA512: d0be13bab6bc121c2926d4b168687f6c2ed4ce0c9dd19be71eb4886adeba8afc3daacdc4e232a0ba3b03a89d69b618abc5595b69abd1ad0c476d825bc6ea1f9f
All other places calculate "now - nLastTry", which is safe and correct to do when nLastTry is 0. So do the same here.
…E_MODE` when debugging 06e18e0 build: use BOOST_MULTI_INDEX_ENABLE_SAFE_MODE when debugging (fanquake) Pull request description: Use of this macro enables precondition checks for iterators and functions of the library. It's use is recommended in debug builds. See https://www.boost.org/doc/libs/1_78_0/libs/multi_index/doc/tutorial/debug.html for more info. There is also a `BOOST_MULTI_INDEX_ENABLE_INVARIANT_CHECKING` macro: > When this mode is in effect, all public functions of Boost.MultiIndex will perform post-execution tests aimed at ensuring that the basic internal invariants of the data structures managed are preserved. ACKs for top commit: laanwj: Concept and code review ACK 06e18e0 Tree-SHA512: 7ee489eccda81c7dbca9210af6d3007d5b2c704b645139d2714c077af157789dd9478c29d0d212e210e96686ea83713aaf3d458e879122b3cde64f3e3e3789d2
…deError fa74b63 test: Fix wait_for_debug_log UnicodeDecodeError (MacroFake) Pull request description: Fix the intermittent `UnicodeDecodeError` when the debug log is truncated on an (multi-byte) unicode character by treating everything as bytes. Also, remove the `ignore_case` option and the`re.search+re.escape` wrap. All of this is unused and doesn't exist on raw byte strings. Fixes bitcoin/bitcoin#24575 ACKs for top commit: jonatack: ACK fa74b63 brunoerg: ACK fa74b63 Tree-SHA512: c67c9355073e784fa8d9d48b8e79ff0c98f5ae9cd4d704ad12a76d2604733946054bc74b8ab346aa2184db23d740b85c8c13eb892d76cba92e42ebfd73f2f1bf
This allows us to use `p2p_port()` with `set_test_params()`.
…checks 8888bd4 Remove redundant nLastTry check (MarcoFalke) 00001e5 Remove redundant nTime checks (MarcoFalke) Pull request description: Split out from bitcoin/bitcoin#24697 because it makes sense on its own. ACKs for top commit: dergoegge: re-ACK 8888bd4 naumenkogs: utACK 8888bd4 Tree-SHA512: 32c6cde1c71e943c76b7991c2c24caf29ae467ab4ea2d758483a0cee64625190d1a833b468e8eab1f834beeb2c365af96552c14b05270f08cf63790e0707581d
…d `desig` to ignore-words d575413 doc: add `desig` to ignore-words (brunoerg) c06cc41 doc: fix typo in kernel/context.h (brunoerg) Pull request description: This PR fixes a typo in `kernel/context.h` (libary => library) and add `desig` to ignore-words since it's a valid word, see: https://github.com/bitcoin/bitcoin/blob/b9416c3847cd347238a9d75d949327f69e187d79/src/net.cpp#L1105-L1117 ACKs for top commit: fanquake: ACK d575413 Tree-SHA512: 2d548c737b8184d0243445c7503f3f68256ecb0970bd834d52de099de3cd8c8b9c140e2b77d55e2542fbd45b1d21cbdee639f5b2ef8138c37b8b72e5211029c3
…getaddr_caching.py ea54ba2 [test] Fix port collisions caused by p2p_getaddr_caching.py (dergoegge) f9682e7 [test_framework] Set PortSeed.n directly after initialising params (dergoegge) Pull request description: This PR fixes the issue mentioned [here](bitcoin/bitcoin#25096 (comment)), to avoid port collisions between nodes spun up by the test framework. Top commit has no ACKs. Tree-SHA512: ec9159f0af90db636f7889d664c24e1430cf2bcb3c02a9ab2dcfe531b2a4d18f6e3a0f8ba73071bdf2f7db518df9d5d86a9cd06695e67644d20fe4515fac32b7
ce893c0 doc: Update developer notes (Anthony Towns) d285291 sync.h: Imply negative assertions when calling LOCK (Anthony Towns) bba87c0 scripted-diff: Convert global Mutexes to GlobalMutexes (Anthony Towns) a559509 sync.h: Add GlobalMutex type (Anthony Towns) be6aa72 qt/clientmodel: thread safety annotation for m_cached_tip_mutex (Anthony Towns) f24bd45 net_processing: thread safety annotation for m_tx_relay_mutex (Anthony Towns) Pull request description: This changes `LOCK(mutex)` for non-global, non-recursive mutexes to be annotated with the negative capability for the mutex it refers to, to prevent . clang applies negative capabilities recursively, so this helps avoid forgetting to annotate functions. This can't reasonably be used for globals, because clang would require every function to be annotated with `EXCLUSIVE_LOCKS_REQUIRED(!g_mutex)` for each global mutex; so this introduces a trivial `GlobalMutex` subclass of `Mutex`, and reduces the annotations for both `GlobalMutex` to `LOCKS_EXCLUDED` which only catches trivial errors (eg (`LOCK(x); LOCK(x);`). ACKs for top commit: MarcoFalke: review ACK ce893c0 🐦 hebasto: ACK ce893c0 Tree-SHA512: 5c35e8c7677ce3d994a7e3774f4344adad496223a51b3a1d1d3b5f20684b2e1d5cff688eb3fbc8d33e1b9940dfa76e515f9434e21de6f3ce3c935e29a319f529
fa7a711 test: Fix out-of-range port collisions (MacroFake) Pull request description: Otherwise the test will fail if two tests running in parallel use the same port. See bitcoin/bitcoin#25096 (comment) and bitcoin/bitcoin#25312 ACKs for top commit: dergoegge: ACK fa7a711 - This gets rid of some rather arbitrary choices for ports in some of our functional tests that can cause port collisions across test runs, resulting in intermittent failures. Tree-SHA512: ac73da8a498230b992ab12e1ee3c4ff3d868cd63c00d2c71537d156cb7c8f8be8598ec574646b17c5a44ae3ac5bb54bf29d300f054a36cec6f6ce8054a0da0a4
…Coins()` return `void` 1f653dc qt, wallet, refactor: Make `WalletModel::sendCoins()` return `void` (Hennadii Stepanov) Pull request description: Currently, the `WalletModel::sendCoins()` function always returns the same value. Also dead and noop (calling `processSendCoinsReturn(OK)`) code has been removed. The other `return` statements have been removed from the `WalletModel::sendCoins()` function in bitcoin/bitcoin#17154 and bitcoin/bitcoin#17165. ACKs for top commit: kristapsk: cr ACK 1f653dc furszy: Code review ACK 1f653dc shaavan: Code Review ACK 1f653dc w0xlt: Code Review ACK bitcoin-core/gui@1f653dc Tree-SHA512: 2b59495a7fc10b4de30fcc63fc3af92d50406e16031112eb72494736dce193ac1fbac0802623496cf81edcd16766e1647d9c4f3a607b3eb84cc50e273b999c04
…settings e47c6c7 Reset settings.json when GUI options are reset (Ryan Ofsky) 99ccc02 Add release notes about unified bitcoin-qt and bitcoind persistent settings (Ryan Ofsky) 504b06b Migrate -lang setting from QSettings to settings.json (Ryan Ofsky) 9a016a3 Migrate -prune setting from QSettings to settings.json (Ryan Ofsky) f067e19 Migrate -proxy and -onion settings from QSettings to settings.json (Ryan Ofsky) a09e3b7 Migrate -listen and -server settings from QSettings to settings.json (Ryan Ofsky) d2ada6e Migrate -upnp and -natpmp settings from QSettings to settings.json (Ryan Ofsky) 1dc4fc2 Migrate -spendzeroconfchange and -signer settings from QSettings to settings.json (Ryan Ofsky) a7ef6d5 Migrate -par setting from QSettings to settings.json (Ryan Ofsky) 284f339 Migrate -dbcache setting from QSettings to settings.json (Ryan Ofsky) Pull request description: If a setting like pruning, port mapping, or a network proxy is enabled in the GUI, it will now be stored in the bitcoin persistent setting file in the datadir and shared with bitcoind, instead of being stored as Qt settings which end up in the the windows registry or platform specific config files and are ignored by bitcoind. This PR has been split off from bitcoin/bitcoin#15936 so some review of these commits previously took place in that PR. ACKs for top commit: furszy: Code review ACK e47c6c7 hebasto: ACK e47c6c7 Tree-SHA512: 076ea7c7efe67805b4a357113bfe1643dce364d0032774106de59566a0ed5771d57a5923920085e03d686beb34b98114bd278555dfdf8bb7af0b778b0f35b7d2
GUIX hashes on arm64:
|
jarolrod
approved these changes
Jun 15, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 708323f
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Sync with the main repo up to the latest bitcoin/bitcoin@37633d2, which includes bitcoin-core/gui#602.
No conflicts :)
Guix builds on
arm64
: