Old version doesn't support debug information for ELF produced by modern compilers
It appears like an error:
> unrecognized DWARF version in .debug_info at 6
> no debug info in ELF executable
which causes:
> No debug information available for stacktrace. You should add debug information and then run
And backtraces are not shown on linux even if debug infos here.
With this fixes output as expected:
```
std::terminate() called due to unhandled exception
Exception: type=char const*, what="HEHE"
0#: (0x5D7C19364C3F) bitcoind.cpp:114 - AppInit
1#: (0x5D7C19364C3F) bitcoind.cpp:273 - main
2#: (0x7B0B19C28150) libc_start_call_main.h:74 - __libc_start_call_main
3#: (0x7B0B19C28209) libc-start.c:128 - call_init
4#: (0x7B0B19C28209) libc-start.c:347 - __libc_start_main_impl
5#: (0x5D7C193734D5) <unknown-file> - ???
```
5730a43703f7e5a5ca26245ba3b55fbdd027d0b6 test: Add functional test for AddrFetch connections (Martin Zumsande)
c34ad3309f93979b274a37de013502b05d25fad8 net, rpc: Enable AddrFetch connections for functional testing (Martin Zumsande)
533500d9072b7d5a36a6491784bdeb9247e91fb0 p2p: Add timeout for AddrFetch peers (Martin Zumsande)
b6c5d1e450dde6a54bd785504c923adfb45c7060 p2p: AddrFetch - don't disconnect on self-announcements (Martin Zumsande)
Pull request description:
AddrFetch connections (old name: oneshots) are intended to be short-lived connections on which we ask a peer for addresses via `getaddr` and disconnect after receiving them.
This is done by disconnecting after receiving the first `addr`. However, it is no longer working as intended, because nowadays, the first `addr` a typical bitcoin core node sends is its self-announcement.
So we'll disconnect before the peer gets a chance to answer our `getaddr`.
I checked that this affects both `-seednode` peers specified manually, and DNS seeds when AddrFetch is used as a fallback if DNS doesn't work for us.
The current behavior of getting peers via AddrFetch when starting with an empty addrman would be to connect to the peer, receive its self-announcement and add it to addrman, disconnect, reconnect to the same peer again as a full outbound (no other addresses in addrman) and then receive more `addr`. This is silly and not in line with AddrFetch peer being intended to be short-lived peers.
Fix this by only disconnecting after receiving an `addr` message of size > 1.
[Edit] As per review discussion, this PR now also adds a timeout after which we disconnect if we haven't received any suitable `addr`, and a functional test.
ACKs for top commit:
amitiuttarwar:
reACK 5730a43703f7e5a5ca26245ba3b55fbdd027d0b6
naumenkogs:
ACK 5730a43703f7e5a5ca26245ba3b55fbdd027d0b6
jnewbery:
ACK 5730a43703
Tree-SHA512: 8a81234f37e827705138eb254223f7f3b3bf44a06cb02126fc7990b0d231b9bd8f07d38d185cc30d55bf35548a6fdc286b69602498d875b937e7c58332158bf9
7b52349b11 Revert "merge #855: Fix bad mn sig ipv6" (Vijay)
cd21313389 Merge bitcoin/bitcoin#21927: fuzz: Run const CScript member functions only once (MarcoFalke)
5ab49e6b57 Merge bitcoin/bitcoin#21756: Avoid calling `getnameinfo` when formatting IPv6 addresses in `CNetAddr::ToStringIP` (W. J. van der Laan)
86c874edfc Merge bitcoin/bitcoin#21983: build: Silence lupdate "unknown namespace/class" warnings (W. J. van der Laan)
Pull request description:
bitcoin backport
Top commit has no ACKs.
Tree-SHA512: dbe30f5ef22b67742c2c832d2aff76ea2ec1307f63e4aaae8a3a041d3000dbf9e2b3b0a632068326f6d7b5373ed7e7e966a2c47e67ec20527b49d43b0a2c3fe3
fa74bfc8601cd2b749deb38388e2a4636d8b86ea fuzz: Run const CScript member functions only once (MarcoFalke)
Pull request description:
Those functions should be O(N) in the input size (or maybe worse, I didn't check), so if the fuzz input dictates to run them N times, the complexity is N^2.
Fix this by calling them only once.
Can be reviewed with: `--ignore-all-space --word-diff-regex=.`
Input: https://github.com/bitcoin/bitcoin/files/6464685/clusterfuzz-testcase-minimized-input.log
Hopefully fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34101
ACKs for top commit:
practicalswift:
cr ACK fa74bfc8601cd2b749deb38388e2a4636d8b86ea: patch looks correct, rationale makes sense and patch touches only `src/test/fuzz/`
Tree-SHA512: d579f7a2103ec154bf482a872142e55a1d9e7673d33a22a4c4230186fdd1b6618846463f4e25941031cc8c4bd1ea8d06cb49ae1bb1ec4af115497f5e5de1e19c
54548bae8004a8f49d73bd29aeca8b41894214c4 net: Avoid calling getnameinfo when formatting IPv6 addresses in CNetAddr::ToStringIP (practicalswift)
c10f27fdb2d335954dd1017ce6d5800159427374 net: Make IPv6ToString do zero compression as described in RFC 5952 (practicalswift)
Pull request description:
Avoid calling `getnameinfo` when formatting IPv6 addresses in `CNetAddr::ToStringIP`.
Fixes#21466.
Fixes#21967.
The IPv4 case was fixed in #21564.
ACKs for top commit:
laanwj:
Code review ACK 54548bae8004a8f49d73bd29aeca8b41894214c4
vasild:
ACK 54548bae8004a8f49d73bd29aeca8b41894214c4
Tree-SHA512: 8404e458b29efdb7bf78b91adc075d05e0385969d1532cccaa2c7cb69cd77411c42d95fcefc4000137b9f2076fe395731c7d9844b7d42b58a6d3bec69eed6fce
88bdc4d33eaeefb5cfe41bb4aa565c0cb361a982 build: Silent lupdate "unknown namespace/class" warnings (Hennadii Stepanov)
Pull request description:
This PR removes multiple _"Qualifying with unknown namespace/class"_ warnings in `make -C src translate` output.
Also all of the `lupdate` options are moved before input files (as documented).
The remaining warnings are fixed in Qt 5.12.2 (see [QTBUG-42736](https://bugreports.qt.io/browse/QTBUG-42736)).
ACKs for top commit:
laanwj:
Tested ACK 88bdc4d33eaeefb5cfe41bb4aa565c0cb361a982
Tree-SHA512: 9c12ff5425a84758aaf8073554891ffe7eb2d75650a12be14ece364b6b7a4461626654885e35c5543462f0941abb020cc36dc64e656c85c8d6d2da36efc7ea2c
2d7534bd93ec78609e187beaea64f1d1bdb1f81a wallet: use PACKAGE_NAME instead of "Bitcoin" in rpcdump (fanquake)
14b480240539eee8d296ed1ac6ec674b34635433 wallet: use FormatFullVersion instead of CLIENT_BUILD in rpcdump (fanquake)
Pull request description:
The dumpwallet RPC is the last place we're using CLIENT_BUILD directly, rather FormatFullVersion() (which just returns it), so switch to using that. At the same time, use PACKAGE_NAME (Bitcoin Core), rather than just "Bitcoin".
ACKs for top commit:
MarcoFalke:
cr ACK 2d7534bd93ec78609e187beaea64f1d1bdb1f81a
laanwj:
Tested ACK 2d7534bd93ec78609e187beaea64f1d1bdb1f81a
achow101:
ACK 2d7534bd93ec78609e187beaea64f1d1bdb1f81a
Zero-1729:
crACK 2d7534b
Tree-SHA512: b38ee074e317448719d2a628380786ec665413515b38d9ce680c21608bc2acf6a2bf817f78f100a8310477613ae72d6969cc4f595f4f44af0896659d3ebf2671
5fe8100ff36fed6d50c2a25b028f57b25af3504c Change the wallet_backup.py test to use the restorewallet RPC command instead of restoring wallets manually. (lsilva01)
ae23faba6fc5cabc896f1175456d1018576f912d Add a new RPC command: restorewallet (lsilva01)
Pull request description:
As far as I know, there is no command to restore the wallet from a backup file.
The only way to do this is to replace the `wallet.dat` of a newly created wallet with the backup file, which is hardly an intuitive way.
This PR implements the `restorewallet` RPC command which restores the wallet from the backup file.
To test:
First create a backup file:
`$ bitcoin-cli -rpcwallet="wallet-01" backupwallet /home/Backups/wallet-01.bak`
Then restore it in another wallet:
`$ bitcoin-cli restorewallet "restored-wallet-01" /home/Backups/wallet-01.bak`
ACKs for top commit:
achow101:
re-ACK 5fe8100ff36fed6d50c2a25b028f57b25af3504c
prayank23:
tACK 5fe8100ff3
meshcollider:
utACK 5fe8100ff36fed6d50c2a25b028f57b25af3504c
Tree-SHA512: 9639df4d8ad32f255f5b868320dc69878bd9aceb3b471b49dfad500b67681e2d354292b5410982fbf18e25a44ed0c06fd4a0dd010e82807c2e00ff32e84047a1
4844b74ba73ecc6d336a52b4dc4cd144a01b0ea2 p2p: log addrman consistency checks (Jon Atack)
Pull request description:
This mini-patch picks up #22479 to log addrman consistency checks in the `BCLOG::ADDRMAN` category when they are enabled with the `-checkaddrman=<n>` configuration option for values of n greater than 0.
```
$ ./src/bitcoind -signet -checkaddrman=20 -debug=addrman
...
2021-08-13T11:14:45Z Addrman checks started: new 3352, tried 89, total 3441
2021-08-13T11:14:45Z Addrman checks completed successfully
```
This allows people to
- verify the checks are running
- see when and how often they are being performed
- see the number of new/tried/total addrman entries per check
- see the start/end of the checks
Thanks to John Newbery for ideas to improve this logging.
ACKs for top commit:
jnewbery:
Code review ACK 4844b74ba73ecc6d336a52b4dc4cd144a01b0ea2
Zero-1729:
tACK 4844b74ba73ecc6d336a52b4dc4cd144a01b0ea2
theStack:
Concept and code-review ACK 4844b74ba73ecc6d336a52b4dc4cd144a01b0ea2 ♟️
Tree-SHA512: 10b51c480d52a753ea8a59dbdd1e2c4f49067e7f4afe59d58426a8fb438f52447fe3a6090fa52132bc382d876927fa338b229c906d85668086f7f8f5bd8ed38a
0bdc299af8 Merge bitcoin/bitcoin#22359: wallet: Do not set fInMempool in transactionAddedToMempool when tx is not in the mempool (Samuel Dobson)
Pull request description:
bitcoin backports
Top commit has no ACKs.
Tree-SHA512: 97cb8d377e9f9460dfa9674a3c29b10e259336df7fc3beb7a2291d200a4cd0b1ddc257f9bbd3b9e8748b3c42f58f32019e09486c232dd51b5b38d1c031d802c8
fa6fd3dd6a4e7f30eff5963836aed43fe01af078 wallet: Properly set fInMempool in mempool notifications (MarcoFalke)
Pull request description:
A wallet method (like bumping the fee) might have set `fInMempool` to false because the transaction was removed from the mempool (See commit fa4e088cbac035b8029a10b492849540150d0622).
Avoid setting it back to true (incorrectly) in the validation interface background thread.
Fixes#22357
ACKs for top commit:
ryanofsky:
Code review ACK fa6fd3dd6a4e7f30eff5963836aed43fe01af078. Only change since last review is extending workaround to `transactionRemovedFromMempool`. Since we know this workaround is imperfect and the goal of this PR is mainly to fix CI errors, I would probably be inclined to limit the workaround to as few places as possible where we have seen actual failures, instead of adding the workaround to as many places as possible, where there is some chance it might trigger new failures. But since this workaround is so straightforward and almost looks like a real fix, probably it doesn't matter.
meshcollider:
utACK fa6fd3dd6a4e7f30eff5963836aed43fe01af078
Tree-SHA512: d690136a577f1f532aa1fee80d3f6600ff7fc61286fbf564a53d7938d5ae52d33f0dbb0fef8b8c041a4970fb424f0b9f1ee7ce791e0ff8354e0000ecc9e22b84
fafd121026c4f1e25d498983e4f88c119516552b refactor: Make CFeeRate constructor architecture-independent (MarcoFalke)
Pull request description:
Currently the constructor is architecture dependent. This is confusing for several reasons:
* It is impossible to create a transaction larger than the max value of `uint32_t`, so a 64-bit `size_t` is not needed
* Policy (and consensus) code should be arch-independent
* The current code will print spurious compile errors when compiled on 32-bit systems:
```
policy/feerate.cpp:23:22: warning: result of comparison of constant 9223372036854775807 with expression of type 'size_t' (aka 'unsigned int') is always true [-Wtautological-constant-out-of-range-compare]
assert(nBytes_ <= uint64_t(std::numeric_limits<int64_t>::max()));
```
Fix all issues by making it arch-independent. Also, fix `{}` style according to dev notes.
ACKs for top commit:
theStack:
re-ACK fafd121026c4f1e25d498983e4f88c119516552b
promag:
Code review ACK fafd121026c4f1e25d498983e4f88c119516552b.
Tree-SHA512: e16f75bad9ee8088b87e873906d9b5633449417a6996a226a2f37d33a2b7d4f2fd91df68998a77e52163de20b40c57fadabe7fe3502e599cbb98494178591833
b85fc45a15 Merge bitcoin/bitcoin#28769: build: Update `qt` package up to 5.15.11 (fanquake)
Pull request description:
## Issue being fixed or feature implemented
It fixes crash of qt app on Kubuntu 24.04
```
*** buffer overflow detected ***: terminated
#0 __pthread_kill_implementation (no_tid=0, signo=6, threadid=<optimized out>) at ./nptl/pthread_kill.c:44
#1 __pthread_kill_internal (signo=6, threadid=<optimized out>) at ./nptl/pthread_kill.c:78
#2 __GI___pthread_kill (threadid=<optimized out>, signo=signo@entry=6) at ./nptl/pthread_kill.c:89
#3 0x00007ffff764526e in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
#4 0x00007ffff76288ff in __GI_abort () at ./stdlib/abort.c:79
#5 0x00007ffff76297b6 in __libc_message_impl (fmt=fmt@entry=0x7ffff77ce765 "*** %s ***: terminated\n") at ../sysdeps/posix/libc_fatal.c:132
#6 0x00007ffff7736c19 in __GI___fortify_fail (msg=msg@entry=0x7ffff77ce74c "buffer overflow detected") at ./debug/fortify_fail.c:24
#7 0x00007ffff77365d4 in __GI___chk_fail () at ./debug/chk_fail.c:28
#8 0x00007ffff7737a67 in __readlink_chk (path=<optimized out>, buf=<optimized out>, len=<optimized out>, buflen=<optimized out>) at ./debug/readlink_chk.c:31
#9 0x0000555556988ab3 in qt_readlink(char const*) ()
#10 0x0000555556a1f18b in QLockFilePrivate::processNameByPid(long long) ()
#11 0x0000555556a1aa7b in QLockFilePrivate::lockFileContents() const ()
#12 0x0000555556a1eccf in QLockFilePrivate::tryLock_sys() ()
#13 0x0000555556a1bf01 in QLockFile::tryLock(int) ()
#14 0x0000555556a1c094 in QLockFile::tryLock(int) ()
#15 0x0000555556a1c094 in QLockFile::tryLock(int) ()
#16 0x00005555568d8dd3 in QConfFileSettingsPrivate::syncConfFile(QConfFile*) ()
#17 0x00005555568d9753 in QConfFileSettingsPrivate::sync() ()
#18 0x00005555568c8ef5 in QSettings::~QSettings() ()
#19 0x0000555555721f82 in Intro::showIfNeeded (did_show_intro=<optimized out>, prune_MiB=@0x7fffffffd2f0: 0) at qt/intro.cpp:275
#20 0x00005555556ddab6 in GuiMain (argc=3, argv=0x7fffffffd828) at qt/bitcoin.cpp:629
#21 0x00007ffff762a1ca in __libc_start_call_main (main=main@entry=0x5555556d7f00 <main(int, char**)>, argc=argc@entry=3, argv=argv@entry=0x7fffffffd828) at ../sysdeps/nptl/libc_start_call_main.h:58
#22 0x00007ffff762a28b in __libc_start_main_impl (main=0x5555556d7f00 <main(int, char**)>, argc=3, argv=0x7fffffffd828, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffd818) at ../csu/libc-start.c:360
#23 0x00005555556d7e35 in _start ()
```
Every usage of `QSettings` cause this crash on one of my PC, but not other. `-resetguisettings` doesn't help. gcc/clang - same crash. Removing data doesn't help.
## What was done?
Bump QT to the newer version.
See also related PR: https://github.com/dashpay/dash/pull/6011
## How Has This Been Tested?
Build & Run
## Breaking Changes
N/A
## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
Top commit has no ACKs.
Tree-SHA512: 3a9024a860c11aa7a48bfb36b3f29772feb6ff458ecbc0faffe774af08e6c7db5754048b0db2bf8713faf4cba861794b2855931416b8cd77389b31f1e47b8aad
8047bb6feaa9ee5d6c1edb7640baaf228450bc6b build: Update `qt` package up to 5.15.11 (Hennadii Stepanov)
Pull request description:
In the light of https://github.com/bitcoin/bitcoin/pull/28622, we probably have to patch Qt. It seems reasonable to update it up to the latest available version before doing that.
ACKs for top commit:
TheCharlatan:
ACK 8047bb6feaa9ee5d6c1edb7640baaf228450bc6b
Tree-SHA512: b4d7df2ff059b8f58c3202d913237c0d39a962748658f1ce853884dca095fbda5f56d4d68f73a1bc8da2f295e96a20927306e148b41a9f4afc42c8edb11c3729
09205b33aa74e385caa2803aa6febc18ad1efa32 net: Clarify message header validation errors (W. J. van der Laan)
955eee76803c098978cf0bbc7f1f6d3c230544e2 net: Sanitize message type for logging (W. J. van der Laan)
Pull request description:
- Use `SanitizeString` when logging message errors to make sure that the message type is sanitized. I have checked all logging in `net.cpp`.
- For the `MESSAGESTART` error don't inspect and log header details at all: receiving invalid start bytes makes it likely that the packet isn't even formatted as valid P2P message. Logging the four unexpected start bytes (as hex) should be enough.
- Update `p2p_invalid_messages.py` test to check this.
- Improve error messages in a second commit.
Issue reported by gmaxwell.
ACKs for top commit:
MarcoFalke:
re-ACK 09205b33aa74e385caa2803aa6febc18ad1efa32 only change is log message fixup 🔂
practicalswift:
re-ACK 09205b33aa74e385caa2803aa6febc18ad1efa32
Tree-SHA512: 8fe5326af135cfcf39ea953d9074a8c966b9b85a810b06a2c45b8a745cf115de4f321e72fc769709d6bbecfc5953aab83176db6735b04c0bc6796f59272cadce
6ba892126d354219b146f0c7f35d472f9c14bdac refactor + document coin selection strategy (glozow)
58ea324fdd906204bb77ea4be1c01a3ab56cf86f [docs] add doxygen comments to wallet code (glozow)
0c74716c50384677724247e05e6592f845fc8635 [docs] format existing comments as doxygen (glozow)
Pull request description:
I think it would help code review to have more documentation + doxygen comments
ACKs for top commit:
Xekyo:
ReACK 6ba892126d
achow101:
ACK 6ba892126d354219b146f0c7f35d472f9c14bdac
Tree-SHA512: 74a78d9b0e0c1d5659bed566432a5b3511511d8b2432f440565f443da7b8257a1b90e70aa7505a7f8abf618748eeb43d166e84f278bdee3d34ce5d5c37dc573a
e0d9dc18a8 scripted-diff: Merge #21836: Replace three dots with ellipsis in the UI string (Vijay)
7bd0262aa2 Merge bitcoin/bitcoin#21753: doc: add -addrinfo to tor docs (W. J. van der Laan)
6350b0cd86 Merge bitcoin/bitcoin#21710: doc: update helps for addnode rpc and -addnode/-maxconnections config options (W. J. van der Laan)
Pull request description:
bitcoin backports
Top commit has no ACKs.
Tree-SHA512: 0aafc4ac01f13418921d3cc4f13f42e7a4c4688918de3a801d9f7f7e6290d33b6e9408689866aef7db558b233375d2f8d01a0fe670e96dd09346ece634e45211
b4fcbcfb49461b96bc72fb64d6152de7c5ce00de doc: update -maxconnections config option help (Jon Atack)
79685a8992ad302833b506cc6d03aab1cc127de0 doc: update -addnode config option help (Jon Atack)
2896c6c4cc6d382d8369c037e274c08dd8e32c69 doc: update addnode rpc help (Jon Atack)
Pull request description:
Since #9319 proposed by Gregory Maxwell and released in v0.14, peers manually added through the `-addnode` config option or using the `addnode` RPC have their own separate limit of 8 connections that does not compete with other inbound or outbound connection usage and is not subject to the limitation imposed by the `-maxconnections` option.
This PR updates the `-addnode` and `-maxconnections` config options and the `addnode` RPC help docs with this information.
`-addnode` config option help
```
$ bitcoind -h | grep -A5 addnode=
-addnode=<ip>
Add a node to connect to and attempt to keep the connection open (see
the addnode RPC help for more info). This option can be specified
multiple times to add multiple nodes; connections are limited to
8 at a time and are counted separately from the -maxconnections
limit.
$ bitcoind -h | grep -A3 maxconnections=
-maxconnections=<n>
Maintain at most <n> connections to peers (default: 125). This limit
does not apply to connections manually added via -addnode or the
addnode RPC, which have a separate limit of 8.
```
`addnode` rpc help
```
$ bitcoin-cli help addnode
addnode "node" "command"
Attempts to add or remove a node from the addnode list.
Or try a connection to a node once.
Nodes added using addnode (or -connect) are protected from DoS disconnection and are not required to be
full nodes/support SegWit as other outbound peers are (though such peers will not be synced from).
Addnode connections are limited to 8 at a time and are counted separately from the -maxconnections limit.
```
ACKs for top commit:
prayank23:
ACK b4fcbcfb49
jarolrod:
ACK b4fcbcfb49461b96bc72fb64d6152de7c5ce00de
Tree-SHA512: b6d69baa6cbf6d53f91bac5b39b549d49db6c95f92ea1bdd3588a6432794a25ac2c8b3c89e2c72bb9097e61f2717c8b5ecc404745d5992b88e523db03200898f
5f23531926e1a9cf13bd69c09a7a8f638df1c32b CRegTestParams: Use `args` instead of `gArgs`. (Kiminuo)
Pull request description:
This PR is a very minor follow-up to #13311.
I believe that `gArgs` was just overlooked at the modified line.
ACKs for top commit:
MarcoFalke:
cr ACK 5f23531926e1a9cf13bd69c09a7a8f638df1c32b
Tree-SHA512: f4e4ed6b23fca60e88825b502f20a1341ee2e4429bc8a2a7e419057adb643abda11be2061fe7ee076931657736e629aff88fd2c33737c84c330dc9d64f368c30
c274574458e5921be4d1f3e86e6bba72a7cd3e65 p2p, rpc, fuzz: various tiny follow-ups (Jon Atack)
Pull request description:
- p2p: pass `Span` by value per https://github.com/bitcoin/bitcoin/pull/22143#issuecomment-853953438 as a follow-up to 8be56f0f8ecc54744
- rpc: remove duplicate `CAddress` constructor per https://github.com/bitcoin/bitcoin/pull/22043#discussion_r638535703
- fuzz: rename 3 fuzz targets changed in eba9a94b9f56be2fda623e77f19b960425ea1eb5 back to their original names per https://github.com/bitcoin-core/qa-assets/pull/63#issuecomment-855281865
ACKs for top commit:
MarcoFalke:
cr ACK c274574458e5921be4d1f3e86e6bba72a7cd3e65
practicalswift:
cr ACK c274574458e5921be4d1f3e86e6bba72a7cd3e65: patch looks correct
jarolrod:
ACK c274574458e5921be4d1f3e86e6bba72a7cd3e65
Tree-SHA512: 3672b210d30b3a91f3a6455005e4d3cb1f89621820c417c645d24b06e53459440122a1f75758e0e04c3d04eff9d6f88ef62865216aa3e42301c6df783f7c0b4a
3b5dc9e5aa Merge bitcoin/bitcoin#21745: refactor: Add missing includes in pubkey.cpp/pubkey.h (W. J. van der Laan)
1ee01c801e Merge bitcoin/bitcoin#22120: test: p2p_invalid_block: Check that a block rejected due to too-new tim… (MarcoFalke)
bf72bea014 Merge bitcoin/bitcoin#22214: refactor: Rearrange fillPSBT arguments (fanquake)
Pull request description:
bitcoin backport
Top commit has no ACKs.
Tree-SHA512: 5b4ab7e898c8e0c3f465acc084f3c2136c8c8523d5cc813f042df8591967acff17a380a4173b7c6da63f6dca76773d8ad9811d68d290f3ddd198eaa6481bbacb
71c824ed6cf70b39ca09e8b3962f452f69523af0 cleaned up and added missing "include" statements for pubkey.cpp and pubkey.h (William Bright)
Pull request description:
#### Problem:
Many symbols in the files were undefined and causing issues when I was working on building independent sections of the codebase. The hidden imports from the "secp256k1" library was a particular pain point.
The other standard and missing includes are following best practices and will help with refactoring, build process and others.
#### Changes:
Clean up and declared imports/include for `pubkey.cpp` and `pubkey.h`
ACKs for top commit:
jnewbery:
utACK 71c824ed6c
laanwj:
Code review ACK 71c824ed6cf70b39ca09e8b3962f452f69523af0
Tree-SHA512: bce605cfde24d8e3be82a596cabab7a8577fec0aef7c5e6f7a56603357046d8e8dea11ac8e3dbe79600550291be7784e35c7a55ebf40b46525b8949e4bedae96
754e802274e9373ad7e1dccb710acf74ded6e7fb test: check rejected future block later accepted (Luke Dashjr)
Pull request description:
(Luke) was unsure if the code sufficiently avoided caching a
time-too-new rejection, so wrote this test to check it. It looks like
despite only exempting BLOCK_MUTATED, it is still okay because header
failures never cache block invalidity. This test will help ensure that
if this ever changes, BLOCK_TIME_FUTURE gets excluded at the same time.
This PR re-opens https://github.com/bitcoin/bitcoin/pull/17872 which went stale and addresses the nits raised by reviewers there.
ACKs for top commit:
MarcoFalke:
review ACK 754e802274e9373ad7e1dccb710acf74ded6e7fb
Tree-SHA512: a2bbc8fffb523cf2831e1ecb05f20868e30106a38cc2e369e4973fa549cca06675a668df16f76c49cc4ce3a22925404255e5c53c4232d63ba1b9fca878509aa0
f47e8028391fbcf44fe1dbf3539f42e4185590fd Rearrange fillPSBT arguments (Russell Yanofsky)
Pull request description:
Move fillPSBT inout argument before output-only arguments. This is a nice thing to do to keep the interface style [consistent](https://google.github.io/styleguide/cppguide.html#Inputs_and_Outputs). But motivation is to work around a current limitation of the libmultiprocess code generator (which figures out order of inout parameters by looking at input list, but more ideally would use the output list).
---
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10). The commit was first part of larger PR #10102.
ACKs for top commit:
achow101:
ACK f47e8028391fbcf44fe1dbf3539f42e4185590fd
theStack:
Code-review ACK f47e8028391fbcf44fe1dbf3539f42e4185590fd
Tree-SHA512: 1787af3031ff7ed6b519f3b93054d8b257af96a3380a476a6dab0f759329039ecc5d624b785c5c2d14d594fc852dd81c626880c775c691ec9c79b7b3dbcfb257
ef5e9304cd407adab1563f24215da1b582274c20 test: update logging and docstring in rpc_blockchain.py (Jon Atack)
d548dc71e4849f638fccaea6be86ac4fa5304f01 test: replace magic values by constants in rpc_blockchain.py (Jon Atack)
78c361086fc0bf27612e8142bd33e05e37a36af6 test: assert on mediantime in getblockheader and getblockchaininfo (Jon Atack)
0a9129c588ab016eb0453b40a0cae918ca4aa6a2 test: assert on the value of getblockchaininfo#time (Jon Atack)
Pull request description:
Follow-up to #22407 improving test coverage per https://github.com/bitcoin/bitcoin/pull/22407#pullrequestreview-702077013.
ACKs for top commit:
tryphe:
untested ACK ef5e9304cd407adab1563f24215da1b582274c20
Tree-SHA512: f746d56f430331bc6a2ea7ecd27b21b06275927966aacf1f1127d8d5fdfd930583cabe72e23df3adb2e005da904fc05dc573b8e5eaa2f86e0e193b89a17a5734
a006d7d73019b8cf4d68626c019c3d69729dda69 test: add logging to wallet_listtransactions (Sebastian Falbesoner)
47915b118720c6e2b2ec9f599f25848041b42b99 test: remove unneeded/redundant code in wallet_listtransactions (Sebastian Falbesoner)
fb6c6a7938cb7c4808ad88d23bfc2b7408407b12 test: speedup wallet_listtransactions by whitelisting peers (immediate tx relay) (Sebastian Falbesoner)
Pull request description:
This PR improves the test `wallet_listtransactions.py` in three ways:
* speeds up runtime by a factor of 2-3x by using the good ol' immediate tx relay trick (`-whitelist=noban@127.0.0.1`)
* removes unneeded/redundant code
* adds log messages, mostly by turning comments into `self.log.info(...)` calls
ACKs for top commit:
jonatack:
ACK a006d7d73019b8cf4d68626c019c3d69729dda69
kristapsk:
ACK a006d7d73019b8cf4d68626c019c3d69729dda69
Tree-SHA512: a91a19f5ebc4d05f0b96c5419683c4c57ac0ef44b64eeb8dd550bd72296fd3a2857a3ba83f755fe4b0b3bd06439973f226070b5d0ce2dee58344dae78cb50290
fa45a1338adb127d69aee982920e29519bc1fed6 refactor: Remove unused validation includes (MarcoFalke)
Pull request description:
Unused includes will cause needless recompilation when headers are changed. Also, they pretend there are dependencies that don't exist.
Fix both by removing them.
ACKs for top commit:
laanwj:
Code review ACK fa45a1338adb127d69aee982920e29519bc1fed6
theStack:
ACK fa45a1338adb127d69aee982920e29519bc1fed6 ♻️
Tree-SHA512: 69190fd09184b75bce34ce3f315a1817e09ea32779f9ddc2d4790c89b0887b6cebd88aba66fa054c43c9183fc66202a556d982dd7034fc389a75802d8aaac83a
113b3feddc fix: actually use `-socketevents` (UdjinM6)
Pull request description:
## Issue being fixed or feature implemented
#6007 follow-up
## What was done?
## How Has This Been Tested?
check `socketevents` in `getnetworkinfo` response
## Breaking Changes
n/a
## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
kwvg:
ACK 113b3feddc
PastaPastaPasta:
utACK 113b3feddc
Tree-SHA512: 50dcbdfe1f34e42e71078b585cfed2cd6b07f5f08c8296c7205367043e42e676c2eca47fa5193fdb9071eef202b01ba6e44ae2e3affb59a4e94196ecb6eb4350
e0ad143e08 chore: dashify file list exception for liner (Konstantin Akimov)
98a2dad78c fix: wrong permission for various files accordingly new linter (Konstantin Akimov)
c8c58a1810 Merge bitcoin/bitcoin#25015: test: Use permissions from git in lint-files.py (MacroFake)
f226e8dc1f Merge bitcoin/bitcoin#24762: lint: Start to use py lint scripts (MarcoFalke)
85013e99d6 Merge bitcoin/bitcoin#21873: test: minor fixes & improvements for files linter test (MarcoFalke)
dce79f5c8e Merge bitcoin/bitcoin#21740: test: add new python linter to check file names and permissions (W. J. van der Laan)
Pull request description:
## Issue being fixed or feature implemented
Backports from bitcoin v22+ for a new linter for filenames and file permissions
## What was done?
backports:
- bitcoin/bitcoin#21740
- bitcoin/bitcoin#21873
- bitcoin/bitcoin#24762
- bitcoin/bitcoin#25015
## How Has This Been Tested?
Run new linter: `test/lint/lint-files.py`
## Breaking Changes
N/A
## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
ACKs for top commit:
PastaPastaPasta:
utACK e0ad143e08
Tree-SHA512: e0ccc1655628fb34a40875b8da28145d44c94868efd9934457568afbe9b75db3954fef80870119476d7bdc9c3424a9b222ede6e47333ea432e1ef63f2218e938
908fb7e2ec37fe68675d38dbfee4df9f861bb2b5 test: Use permissions from git in `lint-files.py` (laanwj)
48d2e80a7479a44b0ab09e87542c8cb7a8f72223 test: Don't use shell=True in `lint-files.py` (laanwj)
Pull request description:
Improvements to the `lint-files.py` script:
- Avoid use of `shell=True`.
- Check the permissions in git's metadata instead of in the filesystem. This stops the umask or filesystem from interfering. It's also more efficient as it only needs a single call to `git ls-files`.
(what triggered this change was `File "..." contains a shebang line, but has the file permission 775 instead of the expected executable permission 755.` errors running the script locally).
ACKs for top commit:
vincenzopalazzo:
re-tACK 908fb7e2ec
Tree-SHA512: 2eaf868c55a9c3508b12658a5b3ac429963fd0551e645332d0ac54be56fefccee95115e4667386df24b46b545593cb0d0bf8c6cecab73f9cb19d37ddf704c614
fae211c0ae0dd90876a3390eb21449b7b0bb45c4 lint: Start to use py lint scripts (MarcoFalke)
fa82e890e7950fe5ba6d4fa88fcd922cc929dc47 Move lint script and data file to avoid lint- prefix (MarcoFalke)
Pull request description:
ACKs for top commit:
fjahr:
tACK fae211c0ae0dd90876a3390eb21449b7b0bb45c4
Tree-SHA512: f8272a1bab9efb8203cac121710baae68f01f79e520ad71ff15aa516d19763d61c088b411b019de105a6a30e7ee3c274814d59963f6ac22ba1084560fb601f45
2227fc4e6203064b14e99bcf453601bd263a0196 test: minor fixes & improvements for files linter test (windsok)
Pull request description:
Couple of minor fixes & improvements for files linter test added in #21740
- Use a context manager when opening files, so that files are closed are we are done with them
- Use the `-z` flag when shelling out to `git ls-files` so that we can catch newlines and other weird control characters in filenames.
From the `git ls-files` manpage:
```
-z \0 line termination on output and do not quote filenames. See OUTPUT below for more information.
Without the -z option, pathnames with "unusual" characters are quoted as explained for the configuration variable
core.quotePath (see git-config(1)). Using -z the filename is output verbatim and the line is terminated by a NUL byte.
```
ACKs for top commit:
MarcoFalke:
cr ACK 2227fc4e6203064b14e99bcf453601bd263a0196
practicalswift:
cr ACK 2227fc4e6203064b14e99bcf453601bd263a0196: patch looks correct
Tree-SHA512: af059a805f4a7614162de85dea856052a45ab531895cb0431087e7fc9e037513fa7501bb5eb2fe43238adf5f09e77712ebdbb15b1486983359ad3661a3da0c60
46b025e00df40724175735eb5606ac73067cb3b8 test: add new python linter to check file names and permissions (windsok)
6f6bb3ebc7cb8e17a5dfc8ef55aa2d3f2dc6bdea test: fix file permissions on various scripts (windsok)
Pull request description:
Adds a new python linter test which tests for correct filenames and file permissions in the repository.
Replaces the existing tests in the `test/lint/lint-filenames.sh` and `test/lint/lint-shebang.sh` linter tests, as well as adding some new and increased testing. This increased coverage is intended to catch issues such as in #21728 and https://github.com/bitcoin/bitcoin/pull/16807/files#r345547050
Summary of tests:
* Checks every file in the repository against an allowed regexp to make sure only lowercase or uppercase alphanumerics (a-zA-Z0-9), underscores (_), hyphens (-), at (@) and dots (.) are used in repository filenames.
* Checks only source files (*.cpp, *.h, *.py, *.sh) against a stricter allowed regexp to make sure only lowercase alphanumerics (a-z0-9), underscores (_), hyphens (-) and dots (.) are used in source code filenames. Additionally there is an exception regexp for directories or files which are excepted from matching this regexp (This should replicate the existing `test/lint/lint-filenames.sh` test)
* Checks all files in the repository match an allowed executable or non-executable file permission octal. Additionally checks that for executable files, the file contains a shebang line.
* Checks that for executable `.py` and `.sh` files, the shebang line used matches an allowable list of shebangs (This should replicate the existing `test/lint/lint-shebang.sh` test)
* Checks every file that contains a shebang line to ensure it has an executable permission
Additionally updates the permissions on various files to comply with the new tests.
Fixes#21729
ACKs for top commit:
practicalswift:
cr re-ACK 46b025e00df40724175735eb5606ac73067cb3b8: patch still looks correct
kiminuo:
code review ACK 46b025e00df40724175735eb5606ac73067cb3b8 if `contrib/gitian-descriptors/assign_DISTNAME` permission change is deemed OK.
laanwj:
Code review ACK 46b025e00df40724175735eb5606ac73067cb3b8
Tree-SHA512: 1c8201a2cee0d9cbce15652b68cec9a6458a8b493fcd5392f98560aca0b1a12e668baab65a47100f116f626dadc3f591deb47f7368468c6a46c6c712c2533455
