Sync with the main repo #36
Merged
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
fixes HelpExampleRpc for
- createmultisig
adds missing HelpExampleRpc for
- deriveaddresses
- estimatesmartfee
- getdescriptorinfo
Just treat it the same as the other BSDs. Fixes #17379.
- Update the enumerator examples to snake_case per CPP Core Guidelines https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Renum-caps - Clarify that in this project enumerators may be snake_case, PascalCase, or ALL_CAPS, and to use what is seems appropriate.
The preferred Homebrew prefix for Apple Silicon is /opt/homebrew. Therefore, if we do not use pkg-config to detect packages, we should set the CPPFLAGS and LDFLAGS variables for them explicitly.
i.e. peers where getpeerinfo#relaytxes is false
i.e. peers where getpeerinfo#addr_relay_enabled is false
The places where we need to lookup information for a XOnlyPubKey currently implement a hack which makes both serializations of the full pubkey in order to try the CKeyIDs for the lookup functions. Instead of duplicating this everywhere it is needed, we can consolidate the CKeyID generation into a function, and then have wrappers around GetPubKey, GetKey, and GetKeyOrigin which takes the XOnlyPubKey, retrieves all of the CKeyIDs (using the new GetKeyIDs() function in XOnlyPubKey), and tries their respective underlying lookup function.
A circular dependency is added because policy now depends on txmempool and txmempool depends on validation. It is natural for [mempool] policy to rely on mempool; the problem is caused by txmempool depending on validation. #22677 will resolve this.
This should have no effect in practice, since we only ever call PreChecks once per transaction.
No behavior change. While we're looking through the descendants and calculating how many transactions we might replace, quit early, as soon as we hit 100. Since we're failing faster, we can also separate the loops - yes, we loop through more times, but this helps us detangle the different BIP125 rules later.
Commit 181a120 introduced an initialization order bug: CAddrMan's m_asmap must be set before deserializing peers.dat. Restore that ordering. review hint: use `git diff --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space`
CAddrMan::m_asmap is now set directly in AppInitMain() so CConnMan::SetAsmap() is no longer required.
The author is also needed for the verify-commits.py script.
An instance of the WalletView class without walletModel data member being set is invalid. So, it is better to set it in the constructor.
The walletModel member is set in the WalletView constructor now.
Reviewer hint: use `git diff --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space` Co-authored-by: Amiti Uttarwar <amiti@uttarwar.org>
Reviewer hint: use `git diff --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space` Co-authored-by: Amiti Uttarwar <amiti@uttarwar.org>
… NetBSD fdd7144 system: skip trying to set the locale on NetBSD (fanquake) Pull request description: Just treat it the same as the other BSDs. Fixes #17379. ACKs for top commit: laanwj: Code review ACK fdd7144 practicalswift: cr ACK fdd7144 Tree-SHA512: 5fe0a66f014279ad2683b548692a36af493377fb92d1f28b15dc4feef871190fe08ef40dcc4f5ba21a525fe365c42fb429fe4be0673a1e96db163af587c23204
fade9a1 Remove confusing CAddrDB (MarcoFalke) fa7f77b Fix addrdb includes (MarcoFalke) fa3f5d0 Move addrman includes from .h to .cpp (MarcoFalke) Pull request description: Split out from #22762 to avoid having to carry it around in (an)other rebase(s) ACKs for top commit: ryanofsky: Code review ACK fade9a1 lsilva01: Code Review ACK bitcoin/bitcoin@fade9a1 Tree-SHA512: 7615fb0b6235d0c1e6f8cd6263dd18c4d95890567c2b797fe6fce6cb12cc85ce6eacbe07dbb6d81b05d179ef03b42edfd61c940e35a1044ce6d363b54c2dae5c
fa0c194 cirrus: Enable tests on windows (MarcoFalke) fadecbd test: Fix tests on Windows (MarcoFalke) Pull request description: Only a cherry-picked list. `--extended` can be enabled in a follow-up. ACKs for top commit: hebasto: ACK fa0c194, tested locally on Windows 10 Pro 20H2 (build 19042.1165): Tree-SHA512: 47cfbcef7ce5fe0c62b77a1e45ace513c4f0109b1fcfaec94faf9e488fe9430d6ba0e852230021d432847eb1389a4e4b7cf39bf17f7b09bae36af3079e0d7399
Init should only concern itself with the initialization order, not the detailed initialization logic of every module. Also, inlining logic into a method that is ~800 lines of code, makes it impossible to unit test on its own.
No need to have a function that is only called in one place
f78cc90 ci: Fix merge_script in MSVC task (Hennadii Stepanov) Pull request description: The new `merge_script` in the MSVC build task does not really exit early when the task is triggered by a non-pr. In the current code https://github.com/bitcoin/bitcoin/blob/e4aa9b15b9f80a08076ad329b473fe9107d9e65e/.cirrus.yml#L104 the `exit 0` command exits from the PowerShell call, not the recent `merge_script`. This cause the next lines https://github.com/bitcoin/bitcoin/blob/e4aa9b15b9f80a08076ad329b473fe9107d9e65e/.cirrus.yml#L105-L107 are executed unconditionally. Here is an excerpt from [CI task log](https://api.cirrus-ci.com/v1/task/4578647416766464/logs/merge.log) for the ["Merge #22915: Remove confusing CAddrDB " commit](bitcoin/bitcoin@8966499): ``` ... C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build>call powershell.exe -NoLogo -EncodedCommand ZwBpAHQAIAByAGUAcwBlAHQAIAAtAC0AaABhAHIAZAA= HEAD is now at 8966499 Merge bitcoin/bitcoin#22915: Remove confusing CAddrDB C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build>if 0 NEQ 0 exit /b 0 C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build>call powershell.exe -NoLogo -EncodedCommand aQBmACAAKAAkAGUAbgB2ADoAQwBJAFIAUgBVAFMAXwBQAFIAIAAtAGUAcQAgACQAbgB1AGwAbAApACAAewAgAGUAeABpAHQAIAAwADsAIAB9AA== C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build>if 0 NEQ 0 exit /b 0 C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build>call powershell.exe -NoLogo -EncodedCommand ZwBpAHQAIABmAGUAdABjAGgAIAAkAGUAbgB2ADoAQwBJAFIAUgBVAFMAXwBSAEUAUABPAF8AQwBMAE8ATgBFAF8AVQBSAEwAIAAkAGUAbgB2ADoAQwBJAFIAUgBVAFMAXwBCAEEAUwBFAF8AQgBSAEEATgBDAEgA From https://github.com/bitcoin/bitcoin * branch HEAD -> FETCH_HEAD C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build>if 0 NEQ 0 exit /b 0 C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build>call powershell.exe -NoLogo -EncodedCommand ZwBpAHQAIABtAGUAcgBnAGUAIABGAEUAVABDAEgAXwBIAEUAQQBEAA== Already up to date. C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build>if 0 NEQ 0 exit /b 0 ``` This PR fixes this issue, and makes `merge_script` log more readable. ACKs for top commit: MarcoFalke: Concept ACK f78cc90 Tree-SHA512: c88b115f99f9019a4100a10df051e32c05487612c13105d10873b9cf38965eeca731604d36610ae750cb1f93ba77ce97dca7599fe4984181210d0753be4eb9a0
…eger-overflow:addrman.cpp facb534 test: Add missing suppression signed-integer-overflow:addrman.cpp (MarcoFalke) Pull request description: Steps to reproduce: [crash-d5f88bd8d0d460ffbab217b856b8582600c00503.log](https://github.com/bitcoin/bitcoin/files/7130854/crash-d5f88bd8d0d460ffbab217b856b8582600c00503.log) ``` $ FUZZ=addrman ./src/test/fuzz/fuzz ./crash-d5f88bd8d0d460ffbab217b856b8582600c00503.log INFO: Running with entropic power schedule (0xFF, 100). INFO: Seed: 1257085025 INFO: Loaded 1 modules (379531 inline 8-bit counters): 379531 [0x562577b768a8, 0x562577bd3333), INFO: Loaded 1 PC tables (379531 PCs): 379531 [0x562577bd3338,0x56257819dbe8), ./src/test/fuzz/fuzz: Running 1 inputs 1 time(s) each. Running: ./crash-d5f88bd8d0d460ffbab217b856b8582600c00503.log addrman.cpp:80:14: runtime error: signed integer overflow: 2105390 - -9223372036854775808 cannot be represented in type 'long' #0 0x5625752f0179 in CAddrInfo::IsTerrible(long) const addrman.cpp:80:14 #1 0x56257531917d in CAddrMan::GetAddr_(std::vector<CAddress, std::allocator<CAddress> >&, unsigned long, unsigned long, std::optional<Network>) const addrman.cpp:874:16 #2 0x562574f0251b in CAddrMan::GetAddr(unsigned long, unsigned long, std::optional<Network>) const ./addrman.h:259:9 #3 0x562574eff7ad in addrman_fuzz_target(Span<unsigned char const>) test/fuzz/addrman.cpp:295:26 SUMMARY: UndefinedBehaviorSanitizer: signed-integer-overflow addrman.cpp:80:14 in ACKs for top commit: practicalswift: cr ACK facb534 Tree-SHA512: 6368c48be8762c793f760d86caaf37a10caffa08f6903f3667dd08f7f67fade10f385fbffc451ddcbeeecc9fd02526ed97ab9de13398a75fffa55976a99af6b9
…amework fa0b916 scripted-diff: Use generate* from TestFramework (MarcoFalke) Pull request description: This is needed for #22567. By using the newly added `generate*` member functions of the test framework, it paves the way to make it easier to implicitly call `sync_all` after block generation to avoid intermittent issues. ACKs for top commit: jonatack: ACK fa0b916 Tree-SHA512: e74a324b60250a87c08847cdfd7b6ce3e1d89b891659fd168f6dd7dc0aa718d0edd28285374a613f462f34f4ef8e12c90ad44fb58721c91b2ea691406ad22c2a
…sses e699883 doc: Add IPv6 address to zmq example (nthumann) 8abe570 test: Add IPv6 test to zmq (nthumann) ded449b zmq: Enable IPv6 on listening socket (nthumann) Pull request description: This PR adds support for listening on IPv6 addresses with bitcoinds ZMQ interface, just like the RPC server. Currently, it is not possible to specify an IPv6 address, as the `ZMQ_IPV6` [socket option](http://api.zeromq.org/master:zmq-setsockopt#toc27) is not set and therefore the ZMQ initialization fails, if one does so. The absence of this option has also been noted [here](bitcoin/bitcoin#15198 (comment)). With this PR one can e.g. set `-zmqpubhashblock=tcp://[::1]:28333` to listen on the IPv6 loopback address. ACKs for top commit: laanwj: Code review ACK e699883 theStack: Tested ACK e699883 🌱 Tree-SHA512: 43c3043d8d5c79794d475926259c1be975b694db4fcc1f7750a9a28e242f0fa1b531735a63ea5777498003aa5834f6243f39742d0f3941f2f37593d0c7890700
Now that we require libevent >=2.0.21, remove backwards compatibility code for older versions.
fa7db1c [test] checks descendants limtis for second generation Package descendants (ritickgoenka) Pull request description: This PR adds a new functional test to test the new descendant limits for packages that were proposed in #21800. ``` +----------------------+ | | | M1 | | ^ ^ | | M2 ^ | | . ^ | | . ^ | | . ^ | | . ^ | | M24 ^ | | ^ | | P1 | | ^ | | P2 | | | +----------------------+ ``` This test is for checking a transaction to fail its descendant count limits because of a combination of mempool descendants, package direct descendants, and package indirect descendants. In this test, P1 has M1 as a mempool ancestor, P2 has no in-mempool ancestors, but when combined P2 has M1 as an ancestor and M1 exceeds descendant_limits (23 in-mempool descendants + 2 in-package descendants, a total of 26 including itself) ACKs for top commit: ryanofsky: Code review ACK fa7db1c. Only were suggested changes since last review: simplifying test and dropping P3 transaction as John suggested, and adding assert_equal I suggested glozow: ACK fa7db1c jnewbery: ACK fa7db1c Tree-SHA512: d1eb993550ac8ce31cbe42e17c6522a213ede66970d5d9391f31a116477ab5889fefa6ff2df6ceadd63a28c1be1ad893b0e8e449247e9ade2ca61dc636508d68
…build fa309ee bench: Fix 32-bit compilation failure in addrman bench (MarcoFalke) fae0295 ci: Switch multiprocess to i686 build (MarcoFalke) Pull request description: Building for i686 with clang helps to catch bugs early for: * The OSS-Fuzz i686 clang libFuzzer build * The arm 32-bit native clang build Fixes #22889 ACKs for top commit: hebasto: ACK fa309ee Tree-SHA512: 581820d319aae2fcd4dd44979ee3d4164a575f0438476890aa2a7447f1392a5da26766cd6ab954530499b54f66eec2417bdeefdd7efb19bc27dd679cd2b9d0ce
…pat code 6045a14 util: remove libevent <= 2.0.18 back-compat code (fanquake) Pull request description: Now that we require libevent >=2.0.21, remove backwards compatibility code for older versions. ACKs for top commit: kristapsk: ACK 6045a14 Tree-SHA512: 49a237ee3cef78b105f8ea91dc3e541fe700fe3a3d02a88f85ec91772068ffbe508dbe196a4d693399b2bcf903251b9bc2573f04cb8f2e21a2ea481f35bfde32
…g macro to fix duration, improve BCLog::LogMsg() f530202 Make unexpected time type in BCLog::LogMsg() a compile-time error (Martin Ankerl) bddae7e Add util/types.h with ALWAYS_FALSE template (MarcoFalke) 498b323 log, timer: improve BCLog::LogMsg() (Jon Atack) 8d2f847 sync: inline lock contention logging macro to fix time duration (Jon Atack) Pull request description: Follow-up to #22736. The first commit addresses the issue identified and reported by Martin Ankerl in bitcoin/bitcoin#22736 (comment) to fix the lock contention duration reporting. The next three commits make improvements to the timer code in `BCLog::LogMsg()` and add `util/types.h` with an `ALWAYS_FALSE` template, that springboard from bitcoin/bitcoin#22736 (comment) by Marco Falke. ACKs for top commit: martinus: re-ACK f530202. I ran a fully synced node for about a day. My node was mostly idle though so not much was going on. I [wrote a little script](https://github.com/martinus/bitcoin-stuff/blob/main/scripts/parse-debuglog-contention-single.rb) to parse the `debug.log` and summarize the output to see if anything interesting was going on, here is the result: theStack: ACK f530202 Tree-SHA512: 37d093eac5590e1b5846ab5994d0950d71e131177d1afe4a5f7fcd614270f977e0ea117e7af788e9a74ddcccab35b42ec8fa4db3a3378940d4988df7d21cdaaa
…ts on Windows c427a58 doc: Set PYTHONUTF8=1 for functional tests on Windows (Hennadii Stepanov) Pull request description: The `PYTHONUTF8` environment variable is defined in [PEP 540](https://www.python.org/dev/peps/pep-0540/), and it is actually used in our CI: https://github.com/bitcoin/bitcoin/blob/5e3380b9f59481fc18e05b9d651c3c733abe4053/.cirrus.yml#L89 This PR documents such usage to avoid users' [errors](bitcoin/bitcoin#22922 (comment)). ACKs for top commit: MarcoFalke: cr ACK c427a58 Tree-SHA512: 441b8cecfe47d548cfe403b0e1cd0aef25c1a70ff556434ead1f1e26372919931ac6f208a4ed6fd8dcca46e8709245e4fb06f95259a43c8e1221473ce1ee497b
853c4ed [net] Remove asmap argument from CNode::CopyStats() (John Newbery) 9fd5618 [asmap] Make DecodeAsmap() a utility function (John Newbery) bfdf4ef [asmap] Remove SanityCheckASMap() from netaddress (John Newbery) 07a9ecc [net] Remove CConnman::Options.m_asmap (John Newbery) Pull request description: These small cleanups to the asmap code are the first 4 commits from #22910. They're minor improvements that are independently useful whether or not 22910 is merged. ACKs for top commit: naumenkogs: ACK 853c4ed theStack: Concept and code-review ACK 853c4ed 🗺️ fanquake: ACK 853c4ed Tree-SHA512: 64783743182592ac165df6ff8d18870b63861e9204ed722c207fca6938687aac43232a5ac4d8228cf8b92130ab0349de1b410a2467bb5a9d60dd9a7221b3b85b
…icy/rbf 32748da whitespace fixups after move and scripted-diff (glozow) fa47622 scripted-diff: rename variables in policy/rbf (glozow) ac761f0 MOVEONLY: fee checks (Rules 3 and 4) to policy/rbf (glozow) 9c2f9f8 MOVEONLY: check that fees > direct conflicts to policy/rbf (glozow) 3f033f0 MOVEONLY: check for disjoint conflicts and ancestors to policy/rbf (glozow) 7b60c02 MOVEONLY: BIP125 Rule 2 to policy/rbf (glozow) f8ad2a5 Make GetEntriesForConflicts return std::optional (glozow) Pull request description: This PR does not change behavior. It extracts the BIP125 logic into helper functions (and puts them in the policy/rbf* files). This enables three things - I think each one individually is pretty good: - Implementation of package RBF (see #22290). I want it to be as close to BIP125 as possible so that it doesn't become a distinct fee-bumping mechanism. Doing these move-only commits first means the diff is mostly mechanical to review, and I just need to create a function that mirrors the single transaction validation. - We will be able to isolate and test our RBF logic alone. Recently, there have been some discussions on discrepancies between our code and BIP125, as well as proposals for improving it. Generally, I think making this code more modular and de-bloating validation.cpp is probably a good idea. - Witness Replacement (replacing same-txid-different-wtxid when the witness is significantly smaller and therefore higher feerate) in a BIP125-similar way. Hopefully it can just be implemented with calls to the rbf functions (i.e. `PaysForRBF`) and an edit to the relevant mempool entries. ACKs for top commit: mjdietzx: ACK 32748da theStack: Code-review ACK 32748da 📐 MarcoFalke: review ACK 32748da 🦇 Tree-SHA512: d89985c8b4b42b54861018deb89468e04968c85a3fb1113bbcb2eb2609577bc4fd9bf254593b5bd0e7ab059a0fa8192d1a903b00f77e6f120c7a80488ffcbfc0
… feature_csv_activation.py fa676db test: pep-8 whitespace (MarcoFalke) faed284 test: Avoid intermittent test failure in feature_csv_activation.py (MarcoFalke) Pull request description: Otherwise there will be disconnects if the test runs longer than the default peertimeout (60s): ``` node0 2021-09-05T20:28:30.973116Z (mocktime: 2021-09-01T07:17:29Z) [net] [net.cpp:1323] [InactivityCheck] socket receive timeout: 393061s peer=0 ``` Fix that by skipping `InactivityCheck` via a large `-peertimeout`. ACKs for top commit: fanquake: ACK fa676db Tree-SHA512: 061c0585a805aa2f8e55c4beedd4b8498a2951f33d60aa3632dda0a284db3a627d14a23dbd57e8a66c69a1612f39418e3a755c8ca97f6ae1105c0d70f0d1a801
67c85bd doc: Added hyperlink for doc/build (pradumnasaraf) Pull request description: Added hyperlink for the doc/build directory (https://github.com/bitcoin/bitcoin/tree/master/doc). This will be convenient for visitors to redirect.  Top commit has no ACKs. Tree-SHA512: e920487d5b5bc77ae4ad63f47c400834b8ca5b4b199392ff9b8f2166a620e12b842b7dcc4b04e9b2f65903dfbf421625aae41e7d59ae23306a94604106013eca
…d or corrupted fa55c3d Raise InitError when peers.dat is invalid or corrupted (MarcoFalke) fa4e2cc Inline ReadPeerAddresses (MarcoFalke) fa5aeec Move LoadAddrman from init to addrdb (MarcoFalke) Pull request description: peers.dat is silently erased when it can not be parsed or when it appears corrupted. Fix that by notifying the user. This might help in the following examples: * The user provided the database, but picked the wrong one. * A future version of Bitcoin Core wrote the file and it can't be read. * The file was corrupted by a logic bug in Bitcoin Core. * The file was corrupted by a disk failure. ACKs for top commit: jonatack: Code review re-ACK fa55c3d per `git range-diff eb1f570 fa59c6d fa55c3` and verified the new tests fail on master, except "Check mocked addrman is valid", as expected prayank23: tACK bitcoin/bitcoin@fa55c3d vasild: ACK fa55c3d Tree-SHA512: 78264a78ee570a3c3262cf9c8542b5ffaffa5f52da1eef66c8c381f346989272967cfe1769c573502d9d7d3f7ad68c3ac3b2ec734185d2e4e7595b7122b14196
jarolrod
approved these changes
Sep 11, 2021
There was a problem hiding this comment.
ACK cf892d5
Performed my own sync up to commit 053a5fc. I then compared the log of my sync and the upstream core repo to check that the only additions are QML changes. For the purposes of proper documentation, here's a pastebin showing the diff: test sync log
I then checked the same for this PR and the upstream core repo. I confirmed that the only additions are qml changes. For documentation, here's a pastebin showing the diff: pr sync log
I then compared my sync with this PR branch. I confirmed that the only difference between my sync and this PR is the merge commit itself.
--- ./log_test 2021-09-11 02:29:02.000000000 -0400
+++ ./log_pr 2021-09-11 02:29:22.000000000 -0400
@@ -1,4 +1,4 @@
-6d830e802 Merge branch 'upstream-commit-36' into test-sync-36
+cf892d5e9 Sync with the main repo up to 053a5fc7d912d597cd6dc7376b479420d1eae1c0
053a5fc7d Merge bitcoin/bitcoin#22762: Raise InitError when peers.dat is invalid or corrupted
384d07601 Merge bitcoin/bitcoin#22880: doc: Added hyperlink for doc/build
60881158c Merge bitcoin/bitcoin#22907: test: Avoid intermittent test failure in feature_csv_activation.py
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 bitcoin-core/gui@053a5fc.