[26.x] Changes for rc3

fanquake commented at 3:51 pm on November 14, 2023: member

Currently backports:

Also includes changes for rc3, and reintegrating the release-notes.

doc: rewrite explanation for -par=

The negative bound for script threads comes from the machine which
generates the man pages, so may only be correct for that machine. Any
other placeholder value will also be wrong for some machines. Fix this
be removing the value. This also fixes help2man incorrectly bolding the
value, as if it were a paramater.

Closes #28850.

Github-Pull: #28858
Rebased-From: d799ea26edfd63434b6d1cf55500de184dc67fac

5845331a6c

fanquake added this to the milestone 26.0 on Nov 14, 2023

DrahtBot commented at 3:51 pm on November 14, 2023: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	hebasto, TheCharlatan

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

DrahtBot added the label Backport on Nov 14, 2023

jonatack commented at 5:00 pm on November 14, 2023: member

there is currently nothing further marked for backport to 26.x

Would suggest backporting #28155 as mentioned in #28155 (comment).

Am rebasing and updating #28248 today, which fixes a number of bugs including the regression reported in https://github.com/bitcoin/bitcoin/pull/27213/files#r1291926369 and described in #28248 (comment). If that isn’t fixed in v26.0, it may be a good idea to add a warning to the notable changes section of the release notes where the changed behavior is described, as the way nodes have until now been able to ensure connections to various networks, via addnode, is the behavior that is impacted.

maflcko commented at 8:35 pm on November 14, 2023: member

Would suggest backporting

No strong opinion, but I think that backports at this stage should either fix a regression or be clear bugfixes. Other behavior changes are better left for the next release, because they could themselves change behavior in a buggy way, see also the previous comment #28248 (comment)

jonatack commented at 8:46 pm on November 14, 2023: member

Yes, the first suggested one in #28155 (comment), i.e. https://github.com/bitcoin/bitcoin/commit/94e8882d820969ddc83f24f4cbe1515a886da4ea, is a bug fix.

Am updating #28248 to punt for later on two of the seven, in favor of the clearest ones and the regression.

fanquake renamed this:
~~[26.x] rc3 or finalize~~
[26.x] Final Changes
on Nov 16, 2023

fanquake marked this as ready for review on Nov 16, 2023

fanquake force-pushed on Nov 16, 2023

DrahtBot added the label CI failed on Nov 16, 2023

fanquake force-pushed on Nov 16, 2023

jonatack commented at 3:32 pm on November 16, 2023: member

I see the release notes have been added since my feedback above.

If the regression in v26.0 is left unfixed, it would be a good idea to add a warning to the “Notable Changes” section of the release notes where the changed behavior is described, as the way nodes have until now been able to ensure connections to various networks and particularly the ones with fewer peers, via addnode, is the behavior that is impacted. The regression was first reported in August in https://github.com/bitcoin/bitcoin/pull/27213/files#r1291926369.

Edit: I extracted #28895 from #28248 for the regression. Its first commit can be backported and it has 4 ACKs at this time of writing.

DrahtBot removed the label CI failed on Nov 16, 2023

doc: regenerate example bitcoin.conf 7dda4991a8

pool: make sure PoolAllocator uses the correct alignment

This changes the PoolAllocator to default the alignment to the given type. This makes the code simpler, and most importantly
fixes a bug on ARM 32bit that caused OOM: The class CTxOut has a member CAmount which is an int64_t and on ARM 32bit int64_t
are 8 byte aligned which is larger than the pointer alignment of 4 bytes. So for CCoinsMap to be able to use the pool, we
need to use the alignment of the member instead of just alignof(void*).

Github-Pull: #28913
Rebased-From: ce881bf9fcb7c30bb1fafd6ce38844f4f829452a

bcc183ccce

pool: change memusage_test to use int64_t, add allocation check

If alignment of the PoolAllocator would be insufficient, then the test would fail. This also catches the issue with ARM 32bit,
where int64_t is aligned to 8 bytes but void* is aligned to 4 bytes. The test adds a check to ensure the pool has allocated
a minimum number of chunks

Github-Pull: #28913
Rebased-From: d5b4c0b69e543de51bb37d602d488ee0949ba185

1488648104

ci: Update apt cache

Github-Pull: #28925
Rebased-From: a6cc059ea50b3629a287f66be3658e766b6ddd47

437a5316e5

ci: Switch from `apt` to `apt-get`

Github-Pull: #28925
Rebased-From: 710da28c72259e1054bb63d2d81a8f3aea4c9000

5e0bcc1977

in doc/release-notes.md:238 in ec90cd706f outdated

235+  suitable for RPC `sendrawtransaction`. (#28414)
236+
237+- It's now possible to use [Miniscript](https://bitcoin.sipa.be/miniscript/) inside Taproot leaves for descriptor wallets. (#27255)
238+
239+GUI changes
240+-----------

hebasto commented at 12:55 pm on November 20, 2023:

I’m sorry for that late comment. Mind adding the following stuff:

0- The transaction list in the GUI to no longer provide a special category for "payment to yourself". Now transactions that have both inputs and outputs that affect the wallet are displayed on separate lines for spending and receiving. (gui#119)
1
2- A new menu option allows migrating a legacy wallet based on keys and implied output script types stored in BerkeleyDB (BDB) to a modern wallet that uses descriptors stored in SQLite. (gui#738)
3
4- The PSBT operations dialog marks outputs paying your own wallet with "own address". (gui#740)
5
6- The ability to create legacy wallets is being removed. (gui#764)

?

fanquake commented at 11:37 am on November 22, 2023:

Sure. Added to the next push.

fanquake renamed this:
~~[26.x] Final Changes~~
[26.x] Changes for rc3
on Nov 22, 2023

hebasto commented at 11:45 am on November 22, 2023: member

Suggesting to backport #28905 as well to ensure CI pass.

p2p: do not make automatic outbound connections to addnode peers

to allocate our limited outbound slots correctly, and to ensure addnode
connections benefit from their intended protections.

Our addnode logic usually connects the addnode peers before the automatic
outbound logic does, but not always, as a connection race can occur.  If an
addnode peer disconnects us and if it was the only one from its network, there
can be a race between reconnecting to it with the addnode thread, and it being
picked as automatic network-specific outbound peer.  Or our internet connection
or router, or the addnode peer, could be temporarily offline, and then return
online during the automatic outbound thread.  Or we could add a new manual peer
using the addnode RPC at that time.

The race can be more apparent when our node doesn't know many peers, or with
networks like cjdns that currently have few bitcoin peers.

When an addnode peer is connected as an automatic outbound peer and is the only
connection we have to a network, it can be protected by our new outbound
eviction logic and persist in the "wrong role".

Examples on mainnet using logging added in the same pull request:

2023-08-12T14:51:05.681743Z [opencon] [net.cpp:1949] [ThreadOpenConnections]
[net:debug] Not making automatic network-specific outbound-full-relay connection
to i2p peer selected for manual (addnode) connection: [geh...odq.b32.i2p]:0

2023-08-13T03:59:28.050853Z [opencon] [net.cpp:1949] [ThreadOpenConnections]
[net:debug] Not making automatic block-relay-only connection to onion peer
selected for manual (addnode) connection: kpg...aid.onion:8333

2023-08-13T16:21:26.979052Z [opencon] [net.cpp:1949] [ThreadOpenConnections]
[net:debug] Not making automatic network-specific outbound-full-relay connection
to cjdns peer selected for manual (addnode) connection: [fcc...8ce]:8333

2023-08-14T20:43:53.401271Z [opencon] [net.cpp:1949] [ThreadOpenConnections]
[net:debug] Not making automatic network-specific outbound-full-relay connection
to cjdns peer selected for manual (addnode) connection: [fc7...59e]:8333

2023-08-15T00:10:01.894147Z [opencon] [net.cpp:1949] [ThreadOpenConnections]
[net:debug] Not making automatic feeler connection to i2p peer selected for
manual (addnode) connection: geh...odq.b32.i2p:8333

Finally, there does not seem to be a reason to make block-relay or short-lived
feeler connections to addnode peers, as the addnode logic will ensure we connect
to them if they are up, within the addnode connection limit.

Fix these issues by checking if the address is an addnode peer in our automatic
outbound connection logic.

Github-Pull: #28895
Rebased-From: cc627169206fe902157806d88fcaf2b05701c38d

55af112565

ci: Avoid toolset ambiguity that MSVC can't handle

This change is required to work with the new windows-2022 image version
20231115 properly.

Github-Pull: #28905
Rebased-From: 91d5bd8ac9a28725c735f8e6900bc85673bb190a

5eaa179f27

fanquake force-pushed on Nov 22, 2023

in doc/release-notes.md:240 in 740a85ce5f outdated

237+- It's now possible to use [Miniscript](https://bitcoin.sipa.be/miniscript/) inside Taproot leaves for descriptor wallets. (#27255)
238+
239+GUI changes
240+-----------
241+
242+- The transaction list in the GUI to no longer provide a special category for "payment to yourself". Now transactions that have both inputs and outputs that affect the wallet are displayed on separate lines for spending and receiving. (gui#119)

hebasto commented at 12:30 pm on November 22, 2023:

nit:

0- The transaction list in the GUI no longer provides a special category for "payment to yourself". Now transactions that have both inputs and outputs that affect the wallet are displayed on separate lines for spending and receiving. (gui#119)

I apologize for the wrong Grammar in my initial suggestion.

fanquake commented at 4:30 pm on November 22, 2023:

No worries, fixed up.

hebasto approved

hebasto commented at 12:30 pm on November 22, 2023: member

ACK 740a85ce5f25f2282e32f148d61f92301bef7179, I’ve verified backports locally, also reviewed changes related to the -par option and rc2 –> rc3 as well.

fanquake force-pushed on Nov 22, 2023

hebasto approved

hebasto commented at 5:08 pm on November 22, 2023: member

re-ACK f868852d3d7321aa522d6b168f33bef1e5eba2dc.

build: Fix regression in "ARMv8 CRC32 intrinsics" test

The `vmull_p64` is a part of the Crypto extensions from the ACLE. They
are optional extensions, so they get enabled with a `+crypto` for
architecture flags.

Github-Pull: #28919
Rebased-From: 228d6a2969e4fcee573c9df7aad31550eab9c8d4

6045f38dc8

build: bump version to v26.0rc3 3db4d1cff2

doc: update manual pages for v26.0rc3 3b6c7f240c

doc: update release notes for v26.0rc3

Few further changes are expected, so reintegrate the release-notes.

2f86d30533

fanquake force-pushed on Nov 22, 2023

hebasto approved

hebasto commented at 5:26 pm on November 22, 2023: member

re-ACK 2f86d3053314c382dfce5cf314e98811ed433859, only #28919 backported since my recent review.

TheCharlatan approved

TheCharlatan commented at 5:39 pm on November 22, 2023: contributor

ACK 2f86d3053314c382dfce5cf314e98811ed433859

fanquake merged this on Nov 22, 2023

fanquake closed this on Nov 22, 2023

fanquake deleted the branch on Nov 22, 2023

Frank-GER referenced this in commit 434ceaef54 on Nov 28, 2023

fanquake referenced this in commit 44d8b13c81 on Dec 4, 2023

Mita23456 commented at 6:50 am on December 15, 2023: none

So not doing at

[26.x] Changes for rc3 #28872

Code Coverage

Reviews